diff --git a/linux/runner/mpv/mpv_player.cc b/linux/runner/mpv/mpv_player.cc index 00c419e16..a03dd358b 100644 --- a/linux/runner/mpv/mpv_player.cc +++ b/linux/runner/mpv/mpv_player.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include #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 lock(native_mutex_); source_hdr_metadata_ = SourceHdrMetadata(); } + FlushPendingRestartPositions(); auto* end = static_cast(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(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_}; } diff --git a/linux/runner/mpv/mpv_player.h b/linux/runner/mpv/mpv_player.h index 4d3a52d67..82f6432f4 100644 --- a/linux/runner/mpv/mpv_player.h +++ b/linux/runner/mpv/mpv_player.h @@ -197,6 +197,14 @@ class MpvPlayer { std::function; 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 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 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; diff --git a/linux/runner/mpv/mpv_player_lifecycle_test.cc b/linux/runner/mpv/mpv_player_lifecycle_test.cc index 92c63bab8..2eaae9dfe 100644 --- a/linux/runner/mpv/mpv_player_lifecycle_test.cc +++ b/linux/runner/mpv/mpv_player_lifecycle_test.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -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::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 names; + std::vector 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 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 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 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);