fix: keep a seek near the end of the source and the position after seeking back (#328)

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.

Author: devopvoid

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;

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<std::mutex> 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;
 

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<std::mutex> 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)

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