fix(linux): read the restart position off the GTK main thread
Every seek on Linux stalled the UI thread: the playback-restart handler read `time-pos` with a synchronous mpv_get_property from the GLib main context, which parks that thread on mpv's playloop for as long as the core takes — and the core's render path can itself be waiting on the main thread to post the next render job. The position is now requested through the existing asynchronous property registry when the restart is dequeued, with the source identity captured at that moment, and the unchanged playback-restart envelope is sent when the reply lands. A source boundary — start-file or end-file — delivers any restart still waiting as a position-less envelope for its captured source and retires the reply, so no restart is ever reported after its source's boundary or attributed to the next source. No new event type or thread.
This commit is contained in:
@@ -24,6 +24,7 @@
|
||||
#include <chrono>
|
||||
#include <cmath>
|
||||
#include <cstdint>
|
||||
#include <cstdlib>
|
||||
#include <cstring>
|
||||
|
||||
#include "sanitize_utf8.h"
|
||||
@@ -55,6 +56,19 @@ constexpr uint64_t kVideoParamsUserdata = UINT64_MAX;
|
||||
// side rather than inferred from an overlay readout.
|
||||
constexpr uint64_t kHwdecCurrentUserdata = UINT64_MAX - 1;
|
||||
|
||||
// Parses the string reply of a `time-pos` read: the whole string must be one
|
||||
// finite number. Anything else — an empty reply, "inf"/"nan", trailing text —
|
||||
// is "no position" rather than a manufactured one. LC_NUMERIC is the C locale
|
||||
// process-wide (EnsureProcessNumericLocale), which is also what mpv printed in.
|
||||
bool ParseSeconds(const std::string& value, double* seconds) {
|
||||
if (value.empty()) return false;
|
||||
char* end = nullptr;
|
||||
const double parsed = std::strtod(value.c_str(), &end);
|
||||
if (end != value.c_str() + value.size() || !std::isfinite(parsed)) return false;
|
||||
*seconds = parsed;
|
||||
return true;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
// Flutter on Linux uses EGL (OpenGL ES) for both X11 and Wayland.
|
||||
@@ -835,6 +849,12 @@ void MpvPlayer::SetSourceMetadataCallback(SourceMetadataCallback callback) {
|
||||
}
|
||||
|
||||
void MpvPlayer::GetPropertyAsync(const std::string& name, GetPropertyCallback callback) {
|
||||
#ifdef PLEZY_MPV_PLAYER_LIFECYCLE_TEST
|
||||
if (test_property_read_) {
|
||||
test_property_read_(name, std::move(callback));
|
||||
return;
|
||||
}
|
||||
#endif
|
||||
if (disposed_ || !mpv_) {
|
||||
if (callback) callback(MPV_ERROR_UNINITIALIZED, "");
|
||||
return;
|
||||
@@ -1129,6 +1149,7 @@ void MpvPlayer::HandleMpvEvent(mpv_event* event) {
|
||||
std::lock_guard<std::mutex> lock(native_mutex_);
|
||||
source_hdr_metadata_ = SourceHdrMetadata();
|
||||
}
|
||||
FlushPendingRestartPositions();
|
||||
auto* end = static_cast<mpv_event_end_file*>(event->data);
|
||||
if (!end) break;
|
||||
FlValue* data = fl_value_new_map();
|
||||
@@ -1146,6 +1167,7 @@ void MpvPlayer::HandleMpvEvent(mpv_event* event) {
|
||||
case MPV_EVENT_START_FILE: {
|
||||
auto* start = static_cast<mpv_event_start_file*>(event->data);
|
||||
if (!start) break;
|
||||
FlushPendingRestartPositions();
|
||||
active_source_id_ = start->playlist_entry_id;
|
||||
has_active_source_id_ = true;
|
||||
SendActiveSourceEvent("start-file");
|
||||
@@ -1158,12 +1180,36 @@ void MpvPlayer::HandleMpvEvent(mpv_event* event) {
|
||||
break;
|
||||
}
|
||||
case MPV_EVENT_PLAYBACK_RESTART: {
|
||||
double position_seconds = 0.0;
|
||||
const double* position = nullptr;
|
||||
if (mpv_ && mpv_get_property(mpv_, "time-pos", MPV_FORMAT_DOUBLE, &position_seconds) >= 0) {
|
||||
position = &position_seconds;
|
||||
// The position is asked for, not read: a synchronous read hands the
|
||||
// request to the core and parks the GTK main thread on the playloop — the
|
||||
// thread the core's render path is itself waiting on (see the video-params
|
||||
// note in Initialize) — so the envelope follows the reply instead. The
|
||||
// source is captured now so a boundary crossed in the meantime cannot
|
||||
// relabel it; the epoch lets that boundary retire the reply after
|
||||
// delivering the restart itself position-less.
|
||||
const bool has_source_id = has_active_source_id_;
|
||||
const int64_t source_id = active_source_id_;
|
||||
const uint64_t epoch = restart_epoch_;
|
||||
auto on_position = [this, has_source_id, source_id, epoch](int error, const std::string& value) {
|
||||
if (disposed_ || epoch != restart_epoch_) return;
|
||||
--pending_restart_positions_;
|
||||
double seconds = 0.0;
|
||||
const bool has_position = error >= 0 && ParseSeconds(value, &seconds);
|
||||
SendPlaybackRestartEvent(has_source_id, source_id, has_position ? &seconds : nullptr);
|
||||
};
|
||||
#ifdef PLEZY_MPV_PLAYER_LIFECYCLE_TEST
|
||||
if (test_property_read_) {
|
||||
++pending_restart_positions_;
|
||||
test_property_read_("time-pos", std::move(on_position));
|
||||
break;
|
||||
}
|
||||
SendPlaybackRestartEvent(position);
|
||||
#endif
|
||||
if (!mpv_) {
|
||||
SendPlaybackRestartEvent(has_source_id, source_id, nullptr);
|
||||
break;
|
||||
}
|
||||
++pending_restart_positions_;
|
||||
plezy::mpv_common::SubmitGetPropertyAsync(mpv_, pending_requests_, "time-pos", std::move(on_position));
|
||||
break;
|
||||
}
|
||||
default:
|
||||
@@ -1245,13 +1291,13 @@ void MpvPlayer::SendActiveSourceEvent(const std::string& name) {
|
||||
if (data) fl_value_unref(data);
|
||||
}
|
||||
|
||||
void MpvPlayer::SendPlaybackRestartEvent(const double* position_seconds) {
|
||||
void MpvPlayer::SendPlaybackRestartEvent(bool has_source_id, int64_t source_id, const double* position_seconds) {
|
||||
const bool has_position = position_seconds && std::isfinite(*position_seconds);
|
||||
FlValue* data = nullptr;
|
||||
if (has_active_source_id_ || has_position) {
|
||||
if (has_source_id || has_position) {
|
||||
data = fl_value_new_map();
|
||||
if (has_active_source_id_) {
|
||||
fl_value_set_string_take(data, "sourceId", fl_value_new_int(active_source_id_));
|
||||
if (has_source_id) {
|
||||
fl_value_set_string_take(data, "sourceId", fl_value_new_int(source_id));
|
||||
}
|
||||
if (has_position) {
|
||||
fl_value_set_string_take(data, "positionSeconds", fl_value_new_float(*position_seconds));
|
||||
@@ -1261,6 +1307,15 @@ void MpvPlayer::SendPlaybackRestartEvent(const double* position_seconds) {
|
||||
if (data) fl_value_unref(data);
|
||||
}
|
||||
|
||||
void MpvPlayer::FlushPendingRestartPositions() {
|
||||
// Every pending restart was dequeued under the source that is now ending, so
|
||||
// the active source is the one each of them captured.
|
||||
for (; pending_restart_positions_ > 0; --pending_restart_positions_) {
|
||||
SendPlaybackRestartEvent(has_active_source_id_, active_source_id_, nullptr);
|
||||
}
|
||||
++restart_epoch_;
|
||||
}
|
||||
|
||||
void MpvPlayer::SendEvent(const std::string& name, FlValue* data) {
|
||||
FlValue* event_map = fl_value_new_map();
|
||||
fl_value_set_string_take(event_map, "type", fl_value_new_string("event"));
|
||||
@@ -1398,6 +1453,10 @@ void MpvPlayer::ConfigurePropertyWritesForTesting(PropertyWriteForTesting writer
|
||||
test_property_write_ = std::move(writer);
|
||||
}
|
||||
|
||||
void MpvPlayer::ConfigurePropertyReadsForTesting(PropertyReadForTesting reader) {
|
||||
test_property_read_ = std::move(reader);
|
||||
}
|
||||
|
||||
MpvPlayer::AppliedOutputColourSpace MpvPlayer::AppliedOutputColourSpaceForTesting() const {
|
||||
return {applied_target_trc_, applied_target_prim_, applied_tone_mapping_, applied_target_peak_};
|
||||
}
|
||||
|
||||
@@ -197,6 +197,14 @@ class MpvPlayer {
|
||||
std::function<void(const std::string& name, const std::string& value, StatusCallback callback)>;
|
||||
void ConfigurePropertyWritesForTesting(PropertyWriteForTesting writer);
|
||||
|
||||
/// The read-side counterpart, for the playback-restart position: "read this
|
||||
/// property, then call back with an mpv error code and the string reply".
|
||||
/// Substituting it lets the deferred restart envelope be observed — nothing
|
||||
/// until the reply, retired at a source boundary — without a core to ask.
|
||||
/// Consulted ahead of the same "no handle" short-circuit as the writer.
|
||||
using PropertyReadForTesting = std::function<void(const std::string& name, GetPropertyCallback callback)>;
|
||||
void ConfigurePropertyReadsForTesting(PropertyReadForTesting reader);
|
||||
|
||||
/// The output colour space mpv last accepted in full — the rollback target,
|
||||
/// and what a caller's committed surface description is measured against.
|
||||
struct AppliedOutputColourSpace {
|
||||
@@ -334,8 +342,16 @@ class MpvPlayer {
|
||||
/// Sends a lifecycle event for the active playlist entry.
|
||||
void SendActiveSourceEvent(const std::string& name);
|
||||
|
||||
/// Sends playback-restart with a finite position when libmpv supplied one.
|
||||
void SendPlaybackRestartEvent(const double* position_seconds);
|
||||
/// Sends playback-restart for the source captured when the restart was
|
||||
/// dequeued, with a finite position when libmpv supplied one.
|
||||
void SendPlaybackRestartEvent(bool has_source_id, int64_t source_id, const double* position_seconds);
|
||||
|
||||
/// Delivers every playback-restart still waiting on its `time-pos` reply as a
|
||||
/// position-less envelope for its captured source, and retires the replies so
|
||||
/// they add nothing when they land. Run at the source boundaries — START_FILE
|
||||
/// and END_FILE — so no restart of a source is reported after that source's
|
||||
/// boundary, whatever the core's reply latency.
|
||||
void FlushPendingRestartPositions();
|
||||
|
||||
/// Reparses the `video-params` payload into source_hdr_metadata_ and tells
|
||||
/// the source-metadata callback that it moved. The parse happens under
|
||||
@@ -441,10 +457,25 @@ class MpvPlayer {
|
||||
bool hdr_sequence_in_flight_ = false;
|
||||
std::deque<HdrOutputRequest> hdr_queue_;
|
||||
#ifdef PLEZY_MPV_PLAYER_LIFECYCLE_TEST
|
||||
// The substituted property-write primitive; empty in every build that has a
|
||||
// real core to write to. See ConfigurePropertyWritesForTesting.
|
||||
// The substituted property-write and property-read primitives; empty in every
|
||||
// build that has a real core to talk to. See ConfigurePropertyWritesForTesting
|
||||
// and ConfigurePropertyReadsForTesting.
|
||||
PropertyWriteForTesting test_property_write_;
|
||||
PropertyReadForTesting test_property_read_;
|
||||
#endif
|
||||
// The playback-restart position is read from the core asynchronously — a
|
||||
// synchronous read here would park the GTK main thread on the playloop, which
|
||||
// can itself be waiting on this thread to render — so the envelope is sent
|
||||
// when the reply lands, not when the restart is dequeued. Ordering caveat:
|
||||
// events the core emitted between the restart and its reply (property
|
||||
// changes, file-loaded) now precede the envelope. Replies come back in
|
||||
// request order, so consecutive restarts keep their order, and the source
|
||||
// boundaries flush whatever is still pending (see FlushPendingRestartPositions).
|
||||
// Both touched only on the GLib main context, like hdr_sequence_in_flight_.
|
||||
int pending_restart_positions_ = 0;
|
||||
// Bumped by every flush; a reply whose captured epoch no longer matches was
|
||||
// already delivered position-less and is dropped.
|
||||
uint64_t restart_epoch_ = 0;
|
||||
// Bits per colour channel of the video plane, told to mpv on every render so
|
||||
// it dithers to the plane's real precision instead of the assumed 8.
|
||||
int surface_depth_bits_ = 8;
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
#include <memory>
|
||||
#include <mutex>
|
||||
#include <stdexcept>
|
||||
#include <string>
|
||||
#include <thread>
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
@@ -65,8 +66,8 @@ class MpvPlayerLifecycleTestPeer {
|
||||
player.observed_properties_.Register(name, "node", id);
|
||||
}
|
||||
static void HandleEvent(MpvPlayer& player, mpv_event* event) { player.HandleMpvEvent(event); }
|
||||
static void SendPlaybackRestart(MpvPlayer& player, const double* position_seconds) {
|
||||
player.SendPlaybackRestartEvent(position_seconds);
|
||||
static void SendPlaybackRestart(MpvPlayer& player, int64_t source_id, const double* position_seconds) {
|
||||
player.SendPlaybackRestartEvent(true, source_id, position_seconds);
|
||||
}
|
||||
|
||||
static void HoldLease(
|
||||
@@ -335,9 +336,9 @@ void TestSourceQualifiedEventPayloads() {
|
||||
playback_restart.event_id = MPV_EVENT_PLAYBACK_RESTART;
|
||||
MpvPlayerLifecycleTestPeer::HandleEvent(player, &playback_restart);
|
||||
const double position_seconds = 17.25;
|
||||
MpvPlayerLifecycleTestPeer::SendPlaybackRestart(player, &position_seconds);
|
||||
MpvPlayerLifecycleTestPeer::SendPlaybackRestart(player, kFirstSourceId, &position_seconds);
|
||||
const double invalid_position = std::numeric_limits<double>::infinity();
|
||||
MpvPlayerLifecycleTestPeer::SendPlaybackRestart(player, &invalid_position);
|
||||
MpvPlayerLifecycleTestPeer::SendPlaybackRestart(player, kFirstSourceId, &invalid_position);
|
||||
|
||||
constexpr int64_t kEndedSourceId = 6000000002LL;
|
||||
mpv_event_end_file end{};
|
||||
@@ -432,6 +433,141 @@ void TestSourceQualifiedEventPayloads() {
|
||||
for (FlValue* event : events) fl_value_unref(event);
|
||||
}
|
||||
|
||||
// The restart handling below has no core, so the substituted reader stands in
|
||||
// for it: it records what was asked and hands the reply back to the test.
|
||||
struct CapturedPositionRead {
|
||||
std::vector<std::string> names;
|
||||
std::vector<MpvPlayer::GetPropertyCallback> replies;
|
||||
|
||||
void Install(MpvPlayer& player) {
|
||||
player.ConfigurePropertyReadsForTesting([this](const std::string& name, MpvPlayer::GetPropertyCallback callback) {
|
||||
names.push_back(name);
|
||||
replies.push_back(std::move(callback));
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
void HandleStartFile(MpvPlayer& player, int64_t source_id) {
|
||||
mpv_event_start_file start{};
|
||||
start.playlist_entry_id = source_id;
|
||||
mpv_event event{};
|
||||
event.event_id = MPV_EVENT_START_FILE;
|
||||
event.data = &start;
|
||||
MpvPlayerLifecycleTestPeer::HandleEvent(player, &event);
|
||||
}
|
||||
|
||||
void HandleEndFile(MpvPlayer& player, int64_t source_id) {
|
||||
mpv_event_end_file end{};
|
||||
end.reason = MPV_END_FILE_REASON_EOF;
|
||||
end.playlist_entry_id = source_id;
|
||||
mpv_event event{};
|
||||
event.event_id = MPV_EVENT_END_FILE;
|
||||
event.data = &end;
|
||||
MpvPlayerLifecycleTestPeer::HandleEvent(player, &event);
|
||||
}
|
||||
|
||||
void HandlePlaybackRestart(MpvPlayer& player) {
|
||||
mpv_event event{};
|
||||
event.event_id = MPV_EVENT_PLAYBACK_RESTART;
|
||||
MpvPlayerLifecycleTestPeer::HandleEvent(player, &event);
|
||||
}
|
||||
|
||||
int64_t RequireSourceId(FlValue* data, const char* message) {
|
||||
return fl_value_get_int(RequireMapField(data, "sourceId", message));
|
||||
}
|
||||
|
||||
void TestPlaybackRestartWaitsForPositionReply() {
|
||||
MpvPlayer player;
|
||||
std::vector<FlValue*> events;
|
||||
player.SetEventCallback([&events](FlValue* event) { events.push_back(fl_value_ref(event)); });
|
||||
CapturedPositionRead read;
|
||||
read.Install(player);
|
||||
|
||||
constexpr int64_t kSourceId = -4000000004LL;
|
||||
HandleStartFile(player, kSourceId);
|
||||
HandlePlaybackRestart(player);
|
||||
|
||||
Check(read.names.size() == 1 && read.names[0] == "time-pos", "playback-restart must ask the core for time-pos");
|
||||
Check(events.size() == 1, "playback-restart must not be reported before its position reply");
|
||||
|
||||
read.replies[0](0, "17.250000");
|
||||
Check(events.size() == 2, "the position reply must deliver exactly one playback-restart");
|
||||
FlValue* restart_data = RequireEventData(events[1], "playback-restart");
|
||||
Check(
|
||||
RequireSourceId(restart_data, "deferred playback-restart source ID is missing") == kSourceId,
|
||||
"deferred playback-restart must carry the source captured at the restart");
|
||||
FlValue* position = RequireMapField(restart_data, "positionSeconds", "replied playback position is missing");
|
||||
Check(
|
||||
fl_value_get_type(position) == FL_VALUE_TYPE_FLOAT && fl_value_get_float(position) == 17.25,
|
||||
"replied playback position was not parsed");
|
||||
|
||||
player.SetEventCallback(nullptr);
|
||||
for (FlValue* event : events) fl_value_unref(event);
|
||||
}
|
||||
|
||||
void TestEndFileFlushesPendingPlaybackRestart() {
|
||||
MpvPlayer player;
|
||||
std::vector<FlValue*> events;
|
||||
player.SetEventCallback([&events](FlValue* event) { events.push_back(fl_value_ref(event)); });
|
||||
CapturedPositionRead read;
|
||||
read.Install(player);
|
||||
|
||||
constexpr int64_t kSourceId = 5000000005LL;
|
||||
HandleStartFile(player, kSourceId);
|
||||
HandlePlaybackRestart(player);
|
||||
HandleEndFile(player, kSourceId);
|
||||
|
||||
Check(events.size() == 3, "end-file must first deliver the restart still waiting on its position");
|
||||
FlValue* restart_data = RequireEventData(events[1], "playback-restart");
|
||||
Check(
|
||||
RequireSourceId(restart_data, "flushed playback-restart source ID is missing") == kSourceId,
|
||||
"flushed playback-restart must name the ended source");
|
||||
Check(
|
||||
fl_value_lookup_string(restart_data, "positionSeconds") == nullptr,
|
||||
"a restart flushed at end-file has no position to report");
|
||||
FlValue* end_data = RequireEventData(events[2], "end-file");
|
||||
Check(RequireSourceId(end_data, "end-file source ID is missing") == kSourceId, "end-file source ID changed");
|
||||
|
||||
read.replies[0](0, "17.250000");
|
||||
Check(events.size() == 3, "a position reply arriving after end-file must add nothing");
|
||||
|
||||
player.SetEventCallback(nullptr);
|
||||
for (FlValue* event : events) fl_value_unref(event);
|
||||
}
|
||||
|
||||
void TestStartFileFlushesPendingPlaybackRestartUnderPreviousSource() {
|
||||
MpvPlayer player;
|
||||
std::vector<FlValue*> events;
|
||||
player.SetEventCallback([&events](FlValue* event) { events.push_back(fl_value_ref(event)); });
|
||||
CapturedPositionRead read;
|
||||
read.Install(player);
|
||||
|
||||
constexpr int64_t kFirstSourceId = 6000000006LL;
|
||||
constexpr int64_t kNextSourceId = 7000000007LL;
|
||||
HandleStartFile(player, kFirstSourceId);
|
||||
HandlePlaybackRestart(player);
|
||||
HandleStartFile(player, kNextSourceId);
|
||||
|
||||
Check(events.size() == 3, "a replacement start-file must first deliver the pending restart");
|
||||
FlValue* restart_data = RequireEventData(events[1], "playback-restart");
|
||||
Check(
|
||||
RequireSourceId(restart_data, "flushed playback-restart source ID is missing") == kFirstSourceId,
|
||||
"a restart pending across start-file must keep the source it was dequeued under");
|
||||
Check(
|
||||
fl_value_lookup_string(restart_data, "positionSeconds") == nullptr,
|
||||
"a restart flushed at start-file has no position to report");
|
||||
FlValue* next_start_data = RequireEventData(events[2], "start-file");
|
||||
Check(
|
||||
RequireSourceId(next_start_data, "replacement start-file source ID is missing") == kNextSourceId,
|
||||
"replacement start-file must follow the flushed restart");
|
||||
|
||||
read.replies[0](0, "17.250000");
|
||||
Check(events.size() == 3, "a position reply for the previous source must add nothing");
|
||||
|
||||
player.SetEventCallback(nullptr);
|
||||
for (FlValue* event : events) fl_value_unref(event);
|
||||
}
|
||||
|
||||
void TestUnavailableCommandFails() {
|
||||
MpvPlayer player;
|
||||
int callback_count = 0;
|
||||
@@ -804,6 +940,9 @@ int main() {
|
||||
mpv::TestRenderTeardownDoesNotDestroyAStillCurrentContext();
|
||||
mpv::TestNullNodePropertyPayloadDecodesAsNull();
|
||||
mpv::TestSourceQualifiedEventPayloads();
|
||||
mpv::TestPlaybackRestartWaitsForPositionReply();
|
||||
mpv::TestEndFileFlushesPendingPlaybackRestart();
|
||||
mpv::TestStartFileFlushesPendingPlaybackRestartUnderPreviousSource();
|
||||
mpv::TestFailedTeardownIsRetriedAndConsumedExactlyOnce();
|
||||
} catch (const std::exception& error) {
|
||||
g_main_context_pop_thread_default(context);
|
||||
|
||||
Reference in New Issue
Block a user