From 3f525ecac0973c3bc89542173305fe2903d1452a Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Mon, 5 Oct 2026 15:15:14 +0200 Subject: [PATCH] fix: keep a seek near the end of the source and the position after seeking back A seek made while the decode thread waited for the queue to play out, which is the last second or two of any source and all of a short one, was taken for the end of the source: the seek emptied the queue, the wait saw it drained, and the player reported the end of the stream and stopped, with the seek still pending. The wait now ends when a seek is pending, and the player carries the seek out instead of ending. Audio only ever moves the position forward, so a position left over from before a seek backwards held an audio-only source at where it had been until the audio caught up. Flushing the pacer now sets the position, and a seek passes its target. The tests fail without the fixes: a seek to the start at 0.7 s of a two second clip played no more frames, and 0.4 s after seeking an audio file back from 1.5 s the position still read 1.5 s. --- .../src/main/cpp/include/media/MediaPacer.h | 6 +- .../src/main/cpp/src/media/MediaPacer.cpp | 6 +- .../src/main/cpp/src/media/MediaPlayer.cpp | 16 ++++-- .../webrtc/media/player/MediaPlayerTest.java | 56 +++++++++++++++++++ 4 files changed, 77 insertions(+), 7 deletions(-) diff --git a/webrtc-java-media/src/main/cpp/include/media/MediaPacer.h b/webrtc-java-media/src/main/cpp/include/media/MediaPacer.h index 546bd1ec..5449b456 100644 --- a/webrtc-java-media/src/main/cpp/include/media/MediaPacer.h +++ b/webrtc-java-media/src/main/cpp/include/media/MediaPacer.h @@ -104,8 +104,10 @@ namespace ffmpeg // Drops everything queued and forgets the clock mapping, so that // the next item delivered starts a new one. This is what a seek - // needs: what is queued belongs to the position being left. - void Flush(); + // needs: what is queued belongs to the position being left. The + // position is where playback is taken to be until an item has been + // delivered, in the timestamps the items are pushed with. + void Flush(int64_t position_us); // True once everything handed over has been delivered. bool IsDrained() const; diff --git a/webrtc-java-media/src/main/cpp/src/media/MediaPacer.cpp b/webrtc-java-media/src/main/cpp/src/media/MediaPacer.cpp index fdb560a5..d1793690 100644 --- a/webrtc-java-media/src/main/cpp/src/media/MediaPacer.cpp +++ b/webrtc-java-media/src/main/cpp/src/media/MediaPacer.cpp @@ -219,13 +219,17 @@ namespace ffmpeg work_.notify_all(); } - void MediaPacer::Flush() + void MediaPacer::Flush(int64_t position_us) { std::lock_guard lock(mutex_); video_queue_.clear(); audio_queue_.clear(); + // Audio only ever moves the position forward, so a position left from + // before a seek backwards would hold it there until audio caught up. + position_us_ = position_us; + // The next item delivered starts a new mapping, wherever it comes from. have_base_ = false; diff --git a/webrtc-java-media/src/main/cpp/src/media/MediaPlayer.cpp b/webrtc-java-media/src/main/cpp/src/media/MediaPlayer.cpp index cd0b3b30..948a4104 100644 --- a/webrtc-java-media/src/main/cpp/src/media/MediaPlayer.cpp +++ b/webrtc-java-media/src/main/cpp/src/media/MediaPlayer.cpp @@ -174,7 +174,7 @@ namespace ffmpeg // on the request until it gets out of that. Dropping what is queued // both frees it and throws away what belongs to the old position; the // thread flushes again once the seek has actually happened. - pacer_->Flush(); + pacer_->Flush(position_us + loop_offset_us_.load()); } void MediaPlayer::SetLooping(bool looping) @@ -317,11 +317,13 @@ namespace ffmpeg continue; } - // Everything has been handed over, but not yet played out. + // Everything has been handed over, but not yet played out. A seek + // ends the wait too: it empties the queue, which would otherwise + // look like the source having played out. for (;;) { std::unique_lock lock(mutex_); - if (closing_ || pacer_->IsDrained()) { + if (closing_ || seek_pending_ || pacer_->IsDrained()) { break; } @@ -335,6 +337,12 @@ namespace ffmpeg break; } + if (seek_pending_) { + // Not at the end any more; the seek is carried out at the + // top, and playback goes on from where it lands. + continue; + } + ended_ = true; playing_ = false; } @@ -492,7 +500,7 @@ namespace ffmpeg loop_offset_us_.store(0); max_source_pts_us_ = position_us; - pacer_->Flush(); + pacer_->Flush(position_us); } void MediaPlayer::SetState(int state) diff --git a/webrtc-java-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java b/webrtc-java-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java index 0beb0156..0219e8ac 100644 --- a/webrtc-java-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java +++ b/webrtc-java-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java @@ -279,6 +279,62 @@ void seekMovesPlayback() throws Exception { } } + @Test + void seekNearTheEndIsNotLost() throws Exception { + // Two seconds of video only. All of it is decoded and queued within + // moments, so the decoding thread spends the playback waiting for the + // queue to play out, which is where a seek used to be mistaken for the + // end of the source. + try (Playback playback = new Playback(new MediaReader( + Paths.get(MediaPlayerTest.class.getResource("/media-test-h264.mkv").toURI())))) { + playback.player.play(); + + Thread.sleep(700); + + int before = playback.frames.get(); + + assertTrue(before > 0 && before < 30, "frames before the seek: " + before); + assertEquals(1, playback.ended.getCount(), "ended before the seek"); + + playback.player.seek(0); + + assertTrue(playback.ended.await(15, TimeUnit.SECONDS), "no end of stream"); + + // The seek went back to the start, so the whole source plays again + // on top of what was delivered before it. + assertTrue(playback.frames.get() >= before + 29, + "playback did not go on after the seek: " + playback.frames.get() + + " frames, " + before + " before it"); + assertEquals(MediaPlayerState.ENDED, playback.player.getState()); + } + } + + @Test + void positionFollowsABackwardSeekOfAudio() throws Exception { + // Audio alone, where nothing but the audio sets the position. + Path flac = TestMedia.constantFlac(tempDir, 48000, 40, (short) 1000); + + try (Playback playback = new Playback(new MediaReader(flac))) { + playback.player.play(); + + Thread.sleep(1500); + + long before = playback.player.getPositionUs(); + + assertTrue(before > 1_000_000, "position before the seek: " + before); + + playback.player.seek(0); + + Thread.sleep(400); + + long after = playback.player.getPositionUs(); + + // About 0.4 s into the source again, not held at where it was. + assertTrue(after < before / 2, "position after seeking back: " + after + + " us, " + before + " us before it"); + } + } + @Test void needsAtLeastOneSource() throws Exception { MediaReader reader = new MediaReader(asset());