diff --git a/lib/services/music/music_playback_service_impl.dart b/lib/services/music/music_playback_service_impl.dart index c486557e..41c49013 100644 --- a/lib/services/music/music_playback_service_impl.dart +++ b/lib/services/music/music_playback_service_impl.dart @@ -833,6 +833,14 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO final player = _player; if (player == null) return; + // Every music bind starts its track at the top, but on a gapless advance + // the player's state still carries the *finished* track's position and + // duration when the transition is announced. Reporting that told Plex the + // new track was already at ~100% and double-counted the play (#1849), so + // the initial report is pinned to the track's own start instead of live + // player state. + final initialDuration = track.durationMs != null ? Duration(milliseconds: track.durationMs!) : null; + final client = source.reportingClient; if (client != null) { _tracker = PlaybackProgressTracker( @@ -846,7 +854,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO playMethod: source.playMethod ?? 'DirectPlay', playSessionId: source.playSessionId, mediaInfo: source.mediaInfo, - )..startTracking(); + )..startTracking(initialPosition: Duration.zero, initialDuration: initialDuration); } else if (source.isOffline && _offlineWatchService != null) { _tracker = PlaybackProgressTracker( client: null, @@ -854,7 +862,7 @@ class MusicPlaybackServiceImpl extends MusicPlaybackService with WidgetsBindingO player: player, isOffline: true, offlineWatchService: _offlineWatchService, - )..startTracking(); + )..startTracking(initialPosition: Duration.zero, initialDuration: initialDuration); } final controls = _mediaControls; diff --git a/lib/services/playback_progress_tracker.dart b/lib/services/playback_progress_tracker.dart index ba233c05..35ca387a 100644 --- a/lib/services/playback_progress_tracker.dart +++ b/lib/services/playback_progress_tracker.dart @@ -213,7 +213,17 @@ class PlaybackProgressTracker { ); } - void startTracking() { + /// Starts the periodic report timer and sends the initial report. + /// + /// [initialPosition] and [initialDuration] override the live player state + /// for that initial report only. Music binds a new tracker the instant a + /// gapless advance is announced, when `player.state.position`/`duration` + /// still hold the *outgoing* track's values — reporting those told Plex the + /// new track was already at ~100%, which recorded a play (and a Last.fm + /// scrobble) at track start on top of the real one (#1849). Callers that + /// know where the item truly starts pass it here; timer ticks always read + /// live state. + void startTracking({Duration? initialPosition, Duration? initialDuration}) { if (_progressTimer != null) { appLogger.w('Progress tracking already started'); return; @@ -229,7 +239,7 @@ class PlaybackProgressTracker { // Send initial progress immediately (don't wait for first timer tick) if (player.state.isActive) { - _sendProgress('playing'); + _sendProgress('playing', positionOverride: initialPosition, durationOverride: initialDuration); } _progressTimer = Timer.periodic(updateInterval, (timer) { @@ -290,7 +300,7 @@ class PlaybackProgressTracker { _stoppedProgressServerAcknowledged = false; } - Future _sendProgress(String state, {Duration? positionOverride}) async { + Future _sendProgress(String state, {Duration? positionOverride, Duration? durationOverride}) async { Duration? attemptedPosition; Duration? attemptedDuration; try { @@ -298,7 +308,7 @@ class PlaybackProgressTracker { final hasRenderedOutput = hasRenderedPlayback?.call() ?? canReport; if (state != 'stopped' && !canReport) return; final isSuppressedStop = state == 'stopped' && !canReport; - final duration = player.state.duration; + final duration = durationOverride ?? player.state.duration; final positionSource = isSuppressedStop ? _lastReportablePosition ?? Duration(milliseconds: metadata.viewOffsetMs ?? 0) : positionOverride ?? player.state.position; diff --git a/test/services/music/music_playback_service_test.dart b/test/services/music/music_playback_service_test.dart index 90baa76d..c7d58a05 100644 --- a/test/services/music/music_playback_service_test.dart +++ b/test/services/music/music_playback_service_test.dart @@ -131,12 +131,23 @@ class FakePlayer implements Player { return gate; } + /// Emits a gapless advance. Deliberately leaves `state.position`/`duration` + /// untouched: the real player announces the transition on the event flow, + /// which is not ordered against the property flow — at that instant the live + /// state can still carry the *finished* track's playhead (#1849). Tests + /// model that handover with [setOutgoingPlayhead] before emitting. void emitTransition(String uri) { _armedMedia = null; // the backend advanced into the armed entry - _state = _state.copyWith(completed: false, position: Duration.zero, duration: _trackDuration); + _state = _state.copyWith(completed: false); trackTransitionCtrl.add(uri); } + /// Backdates the live state to the finished track's playhead, as the real + /// player still reads when a gapless transition is announced. + void setOutgoingPlayhead({required Duration position, required Duration duration}) { + _state = _state.copyWith(position: position, duration: duration); + } + void emitCompleted() { _state = _state.copyWith(completed: true, position: _trackDuration); completedCtrl.add(true); @@ -394,11 +405,12 @@ class RecordedReport { final String state; final String itemId; final Duration position; + final Duration? duration; - const RecordedReport(this.state, this.itemId, this.position); + const RecordedReport(this.state, this.itemId, this.position, this.duration); @override - String toString() => '$state($itemId @ ${position.inSeconds}s)'; + String toString() => '$state($itemId @ ${position.inSeconds}s/${duration?.inSeconds}s)'; } /// Records the progress-report surface; everything else is unimplemented @@ -439,7 +451,7 @@ class FakeMediaServerClient extends Fake with PlaybackReportRecorder implements PlaybackReportKind.progress => call.isPaused ? 'paused' : 'progress', PlaybackReportKind.stopped => 'stopped', }; - reports.add(RecordedReport(state, call.itemId, call.position)); + reports.add(RecordedReport(state, call.itemId, call.position, call.duration)); } } @@ -976,6 +988,37 @@ void main() { expect(h.client.reportsFor('started').map((r) => r.itemId), ['t1', 't2']); }); + test('a gaplessly advanced track reports its own start, not the finished track\'s playhead (#1849)', () async { + final long = testMediaItem( + id: 'long', + backend: MediaBackend.plex, + kind: MediaKind.track, + title: 'Long opener', + parentTitle: 'Album', + grandparentTitle: 'Artist', + durationMs: const Duration(minutes: 7).inMilliseconds, + serverId: 'srv', + ); + await h.playTracks([long, t2]); + + // When the advance is announced, the live player state still carries the + // finished track's playhead — the new file has not reported yet. Reporting + // that as the new track's first sample told Plex it was already at ~100%, + // which recorded a play (and a Last.fm scrobble) at track start on top of + // the real one (#1849). + h.player.setOutgoingPlayhead(position: const Duration(minutes: 7), duration: const Duration(minutes: 7)); + h.player.emitTransition(_urlFor(t2)); + await pumpEventQueue(); + + final started = h.client.reportsFor('started').toList(); + expect(started.map((r) => r.itemId), ['long', 't2']); + expect(started.last.position, Duration.zero); + expect(started.last.duration, _trackDuration, reason: 'the initial report carries t2\'s own duration'); + // The finished track is the only one whose watch settles; t2 must not be + // latched watched off the stale ~100% sample. + expect(h.client.markedWatched, ['long']); + }); + test('a track with no metadata duration is reported stopped where the source actually got to', () async { // Nothing supplies a duration to report at, so the outgoing position is the // only truth — and by the time the transition is handled the player's live diff --git a/test/services/playback_progress_tracker_test.dart b/test/services/playback_progress_tracker_test.dart index 564e523d..35993430 100644 --- a/test/services/playback_progress_tracker_test.dart +++ b/test/services/playback_progress_tracker_test.dart @@ -1641,6 +1641,39 @@ void main() { }); }); + test('startTracking overrides only the initial report with the caller-supplied start state (#1849)', () { + fakeAsync((async) { + final client = _FakePlexClient(); + // At bind time the player state can still carry the *previous* item's + // playhead (gapless music advance) — reporting it as this item's first + // sample told the backend playback was already at ~100%. + final player = _FakePlayer(position: const Duration(minutes: 7), duration: const Duration(minutes: 7)); + final tracker = PlaybackProgressTracker( + client: client, + metadata: _meta(), + player: player, + isOffline: false, + updateInterval: const Duration(seconds: 1), + ); + + tracker.startTracking(initialPosition: Duration.zero, initialDuration: const Duration(minutes: 3)); + async.flushMicrotasks(); + expect(client.updateProgressCalls.single.time, 0); + expect(client.updateProgressCalls.single.duration, const Duration(minutes: 3).inMilliseconds); + expect(client.markWatchedCalls, isEmpty); + + // The player has since reported the real source state; ticks read live. + player.position = const Duration(seconds: 30); + player.duration = const Duration(minutes: 3); + async.elapse(const Duration(seconds: 1)); + async.flushMicrotasks(); + expect(client.updateProgressCalls.last.time, 30000); + expect(client.updateProgressCalls.last.duration, const Duration(minutes: 3).inMilliseconds); + + tracker.dispose(); + }); + }); + test('coalesces timer ticks while a progress report is in flight', () { fakeAsync((async) { final client = _DelayedProgressClient();