fix: start playback after a seek at the position asked for

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
 

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;

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

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> pending_;
+
+			// Where the frames after a seek are to start.
+			int64_t discard_before_us_ = 0;
+			bool discarding_ = false;
 	};
 }
 

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<int>(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) {

webrtc-media/src/main/cpp/src/media/MediaPlayer.cpp

@@ -513,6 +513,13 @@ 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.
+		video_decoder_.DiscardBefore(position_us);
+		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.

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, &timestamp_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, &timestamp_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;
+			}
 		}
 	}
 

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

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<String> error = new AtomicReference<>();
 		final List<Long> 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<Integer> 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());