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());