fix(music): report a gaplessly advanced track's first timeline at its own start
When a gapless advance was announced, the new track's tracker sent its initial report from live player state, which still carried the finished track's position and duration - telling Plex the new track was already at ~100%. PMS recorded a play (and a Last.fm scrobble) at track start on top of the one from the real playthrough, and the tracker latched the new track watched locally the moment it began. The music bind now pins the initial report to the track's own start (position zero, metadata duration); timer ticks keep reading live state. close #1849
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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<void> _sendProgress(String state, {Duration? positionOverride}) async {
|
||||
Future<void> _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;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user