From a960dd4b16b5fd620b2e50bd95e8fa1ef404be44 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Fri, 30 Oct 2026 19:19:51 +0100 Subject: [PATCH 1/2] fix: start playback after a seek at the position asked for The reader lands on the keyframe before the position. The player now decodes from there, drops the video before the position and cuts the audio frame it falls in. --- docs/guide/media/media-files.md | 2 +- .../src/main/cpp/include/media/AudioDecoder.h | 16 +++++- .../src/main/cpp/include/media/MediaPlayer.h | 11 +++- .../src/main/cpp/src/media/AudioDecoder.cpp | 30 +++++++++- .../src/main/cpp/src/media/MediaPlayer.cpp | 24 ++++++++ .../media/player/HardwareDecodingTest.java | 5 +- .../webrtc/media/player/MediaPlayerTest.java | 56 ++++++++++++++++++- 7 files changed, 135 insertions(+), 9 deletions(-) diff --git a/docs/guide/media/media-files.md b/docs/guide/media/media-files.md index e0cc8e24b..ec6e6fae1 100644 --- a/docs/guide/media/media-files.md +++ b/docs/guide/media/media-files.md @@ -144,7 +144,7 @@ source.getState(); // IDLE, PLAYING, PAUSED, ENDED or CLOSED Looping keeps the timing across the seam, so a looping file works as a stand-in for a camera that never stops. A looping source never reports an end of stream. -A seek lands on the keyframe at or before the position asked for, which is how far back the decoder has to go to produce a picture at all. How close that is to the position asked for depends on how often the media was encoded with keyframes. +A seek starts playback at the position asked for. The decoder has to go back to the keyframe at or before it to produce a picture at all, so the frames from there up to the position are decoded and dropped, and the audio is cut at it. How long a seek takes therefore depends on how often the media was encoded with keyframes. ## Following Playback diff --git a/webrtc-media/src/main/cpp/include/media/AudioDecoder.h b/webrtc-media/src/main/cpp/include/media/AudioDecoder.h index 5ad22660f..ddafbf2f5 100644 --- a/webrtc-media/src/main/cpp/include/media/AudioDecoder.h +++ b/webrtc-media/src/main/cpp/include/media/AudioDecoder.h @@ -72,9 +72,17 @@ namespace ffmpeg void Close(); // Drops the decoder's state and the partial chunk being gathered, - // which is what a seek needs. + // which is what a seek needs. Forgets a position set with + // DiscardBefore. void Flush(); + // Makes the audio after a flush start at the given time on the + // source's timeline. A seek lands on a keyframe before it, and the + // audio decoded from there up to the position asked for is + // dropped: frames that end before it entirely, and the start of + // the frame it falls in, so the first chunk starts at it. + void DiscardBefore(int64_t timestamp_us); + // Hands a packet to the decoder, or null to start draining at the // end of the stream. Returns 0 or a negative AVERROR. int SendPacket(const AVPacket * packet); @@ -142,6 +150,12 @@ namespace ffmpeg int64_t next_timestamp_us_ = 0; bool have_timestamp_ = false; + // Where the audio after a seek is to start. Only acted on while + // there is no timestamp yet, which is up to the first frame that + // reaches it. + int64_t discard_before_us_ = 0; + bool discarding_ = false; + // Set once the resampler has been drained at the end of the // stream, so that it is drained once and not on every call. bool resampler_drained_ = false; diff --git a/webrtc-media/src/main/cpp/include/media/MediaPlayer.h b/webrtc-media/src/main/cpp/include/media/MediaPlayer.h index 14d4c8b6f..b0ed87ec5 100644 --- a/webrtc-media/src/main/cpp/include/media/MediaPlayer.h +++ b/webrtc-media/src/main/cpp/include/media/MediaPlayer.h @@ -90,7 +90,8 @@ namespace ffmpeg void Pause(); // Moves playback to the given position, in microseconds from the - // start. What is already queued is dropped. + // start. What is already queued is dropped, and what is decoded + // between the keyframe the seek lands on and the position as well. void Seek(int64_t position_us); void SetLooping(bool looping); @@ -169,6 +170,14 @@ namespace ffmpeg // asks it to start again. bool ended_ = false; + // Where the video after a seek is to start. A seek lands on a + // keyframe before it, and the frames decoded from there up to it + // are dropped. Cleared by the first frame that reaches it, so + // that a source whose timestamps start over, as a loop's do, + // loses nothing after that. + int64_t discard_video_before_us_ = 0; + bool discarding_video_ = false; + // The largest presentation time seen, used to advance the loop // offset when the container does not say how long it runs. int64_t max_source_pts_us_ = 0; diff --git a/webrtc-media/src/main/cpp/src/media/AudioDecoder.cpp b/webrtc-media/src/main/cpp/src/media/AudioDecoder.cpp index 8b1d5c8af..71ee4fab2 100644 --- a/webrtc-media/src/main/cpp/src/media/AudioDecoder.cpp +++ b/webrtc-media/src/main/cpp/src/media/AudioDecoder.cpp @@ -121,6 +121,7 @@ namespace ffmpeg channels_ = 0; next_timestamp_us_ = 0; have_timestamp_ = false; + discarding_ = false; resampler_drained_ = false; } @@ -141,6 +142,13 @@ namespace ffmpeg // The next frame decoded tells where the audio now starts. have_timestamp_ = false; + discarding_ = false; + } + + void AudioDecoder::DiscardBefore(int64_t timestamp_us) + { + discard_before_us_ = timestamp_us; + discarding_ = true; } int AudioDecoder::SendPacket(const AVPacket * packet) @@ -232,8 +240,28 @@ namespace ffmpeg int skip = 0; if (!have_timestamp_) { - next_timestamp_us_ = pts != AV_NOPTS_VALUE + int64_t start_us = pts != AV_NOPTS_VALUE ? av_rescale_q(pts, time_base_, AV_TIME_BASE_Q) : 0; + + // A frame without a timestamp cannot be placed against the + // position, and is taken to start there. + if (discarding_ && pts != AV_NOPTS_VALUE) { + const int64_t cut = av_rescale(discard_before_us_ - start_us, + in_rate_, AV_TIME_BASE); + + if (cut >= frame->nb_samples) { + // Before the position as a whole. The audio still has no + // timestamp, so the next frame is measured the same way. + return 0; + } + if (cut > 0) { + skip = static_cast(cut); + start_us += av_rescale(cut, AV_TIME_BASE, in_rate_); + } + } + + discarding_ = false; + next_timestamp_us_ = start_us; have_timestamp_ = true; } else if (pts != AV_NOPTS_VALUE) { diff --git a/webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp b/webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp index a44e8765f..243dba344 100644 --- a/webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp +++ b/webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp @@ -331,6 +331,11 @@ namespace ffmpeg video_decoder_.Flush(); audio_decoder_.Flush(); + // A seek past the last frame leaves the video waiting for a + // position this pass never reached, and the next one starts + // below it. + discarding_video_ = false; + // The pacer keeps its queue and its mapping, so the seam // between one pass and the next is not heard or seen. continue; @@ -460,6 +465,17 @@ namespace ffmpeg return result; } + if (discarding_video_) { + if (timestamp_us < discard_video_before_us_) { + // Decoded only because the frames after it depend on it. + av_frame_free(&frame); + + continue; + } + + discarding_video_ = false; + } + if (timestamp_us > max_source_pts_us_) { max_source_pts_us_ = timestamp_us; } @@ -513,6 +529,14 @@ namespace ffmpeg video_decoder_.Flush(); audio_decoder_.Flush(); + // The reader lands on the keyframe at or before the position, which + // is where decoding has to begin. Playback begins at the position + // itself: what comes before it is decoded and dropped, and the audio + // frame it falls in is cut there. + discard_video_before_us_ = position_us; + discarding_video_ = true; + audio_decoder_.DiscardBefore(position_us); + // A seek starts a timeline of its own: the pacer maps the first item // that arrives onto the clock afresh, so the offset that kept looping // monotonic is no longer needed and would only skew the position. diff --git a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/HardwareDecodingTest.java b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/HardwareDecodingTest.java index 670f0d56f..76ec2b8f2 100644 --- a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/HardwareDecodingTest.java +++ b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/HardwareDecodingTest.java @@ -501,8 +501,9 @@ void loopsInHardware() throws Exception { @Test void seekBetweenKeyFrames() throws Exception { - // Half a second in is not a key frame; the player has to start from - // the key frame before it, however the reader gets there. + // Half a second in is not a key frame; the player has to decode from + // the key frame before it, however the reader gets there, and start + // playing at half a second. Result software = play(H264, false, 500_000); Result hardware = play(H264, true, 500_000); diff --git a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java index 9d7ebedfe..7419abf8b 100644 --- a/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java +++ b/webrtc-media/src/test/java/dev/onvoid/webrtc/media/player/MediaPlayerTest.java @@ -267,9 +267,7 @@ void pauseHoldsPlayback() throws Exception { @Test void seekMovesPlayback() throws Exception { try (Playback playback = new Playback()) { - // A seek lands on the keyframe at or before the target, so asking - // for 2.5 s starts playback at the 2 s keyframe, leaving about a - // second to play out. + // Half a second is left to play out from 2.5 s. playback.player.seek(2_500_000); playback.player.play(); @@ -282,6 +280,51 @@ void seekMovesPlayback() throws Exception { } } + @Test + void seekStartsAtThePositionAskedFor() throws Exception { + try (Playback playback = new Playback()) { + // Between the keyframes at 1 s and 2 s. The reader lands on the + // one at 1 s, and what lies between it and 1.5 s is decoded only + // to get there. + playback.player.seek(1_500_000); + playback.player.play(); + + assertTrue(playback.ended.await(15, TimeUnit.SECONDS), "no end of stream"); + + // The first frame at or after 1.5 s is the one at 23 / 15 s. + long first = playback.firstFramePositionUs.get(); + + assertTrue(first >= 1_500_000 && first < 1_500_000 + 1_000_000 / 15, + "first frame at " + first + " us"); + + // 1.533 s to 2.933 s, and 1.5 s to 3.008 s of audio. Starting at + // the keyframe would have played 15 frames and 50 chunks more. + assertTrue(Math.abs(playback.frames.get() - 22) <= 1, + "video frames: " + playback.frames.get()); + assertTrue(Math.abs(playback.chunks.get() - 151) <= 2, + "audio chunks: " + playback.chunks.get()); + } + } + + @Test + void seekCutsTheAudioFrameItFallsIn() throws Exception { + // Two seconds in frames of 100 ms, at the rate WebRTC takes, so that + // nothing is resampled and every sample played is one of the file's. + Path flac = TestMedia.constantFlac(tempDir, 48000, 20, (short) 1000); + + try (Playback playback = new Playback(new MediaReader(flac))) { + // Half way into the frame at 1 s, which the reader lands on. + playback.player.seek(1_050_000); + playback.player.play(); + + assertTrue(playback.ended.await(15, TimeUnit.SECONDS), "no end of stream"); + + // 0.95 s is exactly 95 chunks, none of them padded. + assertEquals(95, playback.chunks.get()); + assertEquals(95 * 480, playback.nonZeroSamples.get()); + } + } + @Test void seekNearTheEndIsNotLost() throws Exception { // Two seconds of video only. All of it is decoded and queued within @@ -781,12 +824,19 @@ private final class Playback implements AutoCloseable { final AtomicInteger framesPerChunk = new AtomicInteger(); final AtomicReference error = new AtomicReference<>(); final List frameTimes = Collections.synchronizedList(new ArrayList<>()); + final AtomicLong firstFramePositionUs = new AtomicLong(-1); final AtomicLong firstChunkNs = new AtomicLong(); final AtomicInteger unevenChunks = new AtomicInteger(); final AtomicLong nonZeroSamples = new AtomicLong(); final List silentChunks = Collections.synchronizedList(new ArrayList<>()); private final VideoTrackSink videoSink = frame -> { + // The pacer delivers on its own thread, having just moved the + // position to the frame's presentation time. + if (firstFramePositionUs.get() < 0) { + firstFramePositionUs.set(Playback.this.player.getPositionUs()); + } + frameTimes.add(System.nanoTime()); frames.incrementAndGet(); width.set(frame.buffer.getWidth()); From d2de5b63f5978151b9672911881a0a960de3bd98 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Sat, 31 Oct 2026 01:41:09 +0100 Subject: [PATCH 2/2] perf: drop the frames before a seek's position before converting them The video decoder drops them as they come out of the codec, so a frame decoded in hardware is not read back and none goes through swscale. While the hardware is on probation they are converted all the same, since only a full picture ends it. --- .../src/main/cpp/include/media/MediaPlayer.h | 8 --- .../src/main/cpp/include/media/VideoDecoder.h | 20 +++++- .../src/main/cpp/src/media/MediaPlayer.cpp | 19 +----- .../src/main/cpp/src/media/VideoDecoder.cpp | 65 ++++++++++++++++++- 4 files changed, 82 insertions(+), 30 deletions(-) diff --git a/webrtc-media/src/main/cpp/include/media/MediaPlayer.h b/webrtc-media/src/main/cpp/include/media/MediaPlayer.h index b0ed87ec5..3c6d29e14 100644 --- a/webrtc-media/src/main/cpp/include/media/MediaPlayer.h +++ b/webrtc-media/src/main/cpp/include/media/MediaPlayer.h @@ -170,14 +170,6 @@ namespace ffmpeg // asks it to start again. bool ended_ = false; - // Where the video after a seek is to start. A seek lands on a - // keyframe before it, and the frames decoded from there up to it - // are dropped. Cleared by the first frame that reaches it, so - // that a source whose timestamps start over, as a loop's do, - // loses nothing after that. - int64_t discard_video_before_us_ = 0; - bool discarding_video_ = false; - // The largest presentation time seen, used to advance the loop // offset when the container does not say how long it runs. int64_t max_source_pts_us_ = 0; diff --git a/webrtc-media/src/main/cpp/include/media/VideoDecoder.h b/webrtc-media/src/main/cpp/include/media/VideoDecoder.h index 606f2dc9b..45288f6fa 100644 --- a/webrtc-media/src/main/cpp/include/media/VideoDecoder.h +++ b/webrtc-media/src/main/cpp/include/media/VideoDecoder.h @@ -70,8 +70,16 @@ namespace ffmpeg // Drops everything the decoder holds, which is what a seek needs: // the frames in flight belong to the position that was left. + // Forgets a position set with DiscardBefore. void Flush(); + // Makes the frames after a flush start at the given time on the + // source's timeline. A seek lands on a keyframe before it, and + // the frames from there up to it are decoded and dropped, before + // they are read back from the hardware and converted. Ends with + // the first frame that reaches it. + void DiscardBefore(int64_t timestamp_us); + // Hands a packet to the decoder, or null to start draining at the // end of the stream. Returns 0 or a negative AVERROR. int SendPacket(const AVPacket * packet); @@ -114,9 +122,15 @@ namespace ffmpeg static AVPixelFormat ChooseFormat(AVCodecContext * context, const AVPixelFormat * formats); // The next picture of the codec, in I420, hardware frames brought - // into system memory first. + // into system memory first, passing over those before the + // position set with DiscardBefore. int ReceiveFromCodec(AVFrame ** frame, int64_t * timestamp_us); + // One picture of the codec, as ReceiveFromCodec takes them. Leaves + // the frame null, and returns 0, for one it dropped before reading + // it back and converting it. + int ReceivePicture(AVFrame ** frame, int64_t * timestamp_us); + // Whether a result of the hardware decoder means that it failed. // A stream that cannot be decoded, which is how corrupt data shows, // fails a software decoder as well and says nothing of the @@ -194,6 +208,10 @@ namespace ffmpeg bool awaiting_key_frame_ = false; size_t skipped_ = 0; std::deque pending_; + + // Where the frames after a seek are to start. + int64_t discard_before_us_ = 0; + bool discarding_ = false; }; } diff --git a/webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp b/webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp index 243dba344..b90be950a 100644 --- a/webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp +++ b/webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp @@ -331,11 +331,6 @@ namespace ffmpeg video_decoder_.Flush(); audio_decoder_.Flush(); - // A seek past the last frame leaves the video waiting for a - // position this pass never reached, and the next one starts - // below it. - discarding_video_ = false; - // The pacer keeps its queue and its mapping, so the seam // between one pass and the next is not heard or seen. continue; @@ -465,17 +460,6 @@ namespace ffmpeg return result; } - if (discarding_video_) { - if (timestamp_us < discard_video_before_us_) { - // Decoded only because the frames after it depend on it. - av_frame_free(&frame); - - continue; - } - - discarding_video_ = false; - } - if (timestamp_us > max_source_pts_us_) { max_source_pts_us_ = timestamp_us; } @@ -533,8 +517,7 @@ namespace ffmpeg // is where decoding has to begin. Playback begins at the position // itself: what comes before it is decoded and dropped, and the audio // frame it falls in is cut there. - discard_video_before_us_ = position_us; - discarding_video_ = true; + video_decoder_.DiscardBefore(position_us); audio_decoder_.DiscardBefore(position_us); // A seek starts a timeline of its own: the pacer maps the first item diff --git a/webrtc-media/src/main/cpp/src/media/VideoDecoder.cpp b/webrtc-media/src/main/cpp/src/media/VideoDecoder.cpp index cbf47d30d..1608218de 100644 --- a/webrtc-media/src/main/cpp/src/media/VideoDecoder.cpp +++ b/webrtc-media/src/main/cpp/src/media/VideoDecoder.cpp @@ -336,6 +336,7 @@ namespace ffmpeg draining_ = false; awaiting_key_frame_ = false; skipped_ = 0; + discarding_ = false; time_base_ = { 0, 1 }; } @@ -353,6 +354,13 @@ namespace ffmpeg draining_ = false; awaiting_key_frame_ = false; skipped_ = 0; + discarding_ = false; + } + + void VideoDecoder::DiscardBefore(int64_t timestamp_us) + { + discard_before_us_ = timestamp_us; + discarding_ = true; } bool VideoDecoder::IsHardwareFailure(int result) const @@ -441,6 +449,35 @@ namespace ffmpeg } int VideoDecoder::ReceiveFromCodec(AVFrame ** frame, int64_t * timestamp_us) + { + for (;;) { + *frame = nullptr; + + int result = ReceivePicture(frame, timestamp_us); + + if (result < 0) { + return result; + } + if (*frame == nullptr) { + // Dropped before it was read back and converted. + continue; + } + + if (discarding_ && *timestamp_us < discard_before_us_) { + // Converted all the same, as the hardware was still on + // probation, which only a picture made in full can end. + av_frame_free(frame); + + continue; + } + + discarding_ = false; + + return 0; + } + } + + int VideoDecoder::ReceivePicture(AVFrame ** frame, int64_t * timestamp_us) { if (hardware_ && HardwareFault::HoldsPictures()) { while (avcodec_receive_frame(codec_context_, decoded_) >= 0) { @@ -477,6 +514,19 @@ namespace ffmpeg const bool hardware_frame = hardware_format_ != AV_PIX_FMT_NONE && decoded_->format == hardware_format_; + if (discarding_ && !probation_ && *timestamp_us < discard_before_us_) { + // Before the position a seek asked for, and decoded only because + // the pictures after it depend on it. Reading it back from the + // hardware and converting it would be most of what it costs. + if (!hardware_frame) { + hardware_ = false; + } + + av_frame_unref(decoded_); + + return 0; + } + if (hardware_frame) { // The picture is in the memory of the hardware decoder. Bring it // into system memory, as NV12, and convert it like any other @@ -638,13 +688,22 @@ namespace ffmpeg AVFrame * frame = nullptr; int64_t timestamp_us = 0; - if (ReceiveFromCodec(&frame, ×tamp_us) < 0) { + // Out of pictures when all it held lay before the position of a + // seek, and then it takes the packet. + int received = ReceiveFromCodec(&frame, ×tamp_us); + + if (received < 0 && received != AVERROR(EAGAIN)) { break; } - - pending_.push_back({ frame, timestamp_us }); + if (received == 0) { + pending_.push_back({ frame, timestamp_us }); + } sent = avcodec_send_packet(codec_context_, packet); + + if (received == AVERROR(EAGAIN)) { + break; + } } }