From da64588e72f8e420f660ed6ff3fb3431b677006d Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Sat, 23 May 2026 08:33:35 -0500 Subject: [PATCH] LibMedia: Remove forced seeks upon errors from Matroska::Reader This shouldn't actually be necessary, since the sample iterator will resume from where it hit the error last anyway. Now that the decoded data producers don't clear their queues when the demuxer doesn't move, this works just fine for EOF. This could easily be triggered by scrubbing aborting reads to restart seeks, which would force us to decode from a prior keyframe instead of continuing to decode from where the last seek left off if it's faster. --- Libraries/LibMedia/Containers/Matroska/Reader.cpp | 11 ----------- Tests/LibMedia/TestMatroskaDemuxer.cpp | 8 +++++--- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/Libraries/LibMedia/Containers/Matroska/Reader.cpp b/Libraries/LibMedia/Containers/Matroska/Reader.cpp index d261f413f7..e2918ca202 100644 --- a/Libraries/LibMedia/Containers/Matroska/Reader.cpp +++ b/Libraries/LibMedia/Containers/Matroska/Reader.cpp @@ -709,12 +709,6 @@ static AK::Duration block_timestamp_to_duration(AK::Duration cluster_timestamp, DecoderErrorOr> SampleIterator::get_frames(Block block) { - ArmedScopeGuard error_guard = [&] { - // Similar to next_block(), we need to clear the last timestamp if we encounter an error, or a subsequent - // seek may not move the position to a keyframe to resume decoding properly. - m_last_timestamp.clear(); - }; - Streamer streamer { m_stream_cursor }; TRY(streamer.seek_to_position(block.data_position())); Vector frames; @@ -785,7 +779,6 @@ DecoderErrorOr> SampleIterator::get_frames(Block block) frames.append(TRY(streamer.read_raw_octets(block.data_size()))); } - error_guard.disarm(); return frames; } @@ -1206,10 +1199,6 @@ DecoderErrorOr SampleIterator::next_block() Streamer streamer { m_stream_cursor }; TRY(streamer.seek_to_position(m_position)); - // Remove the last timestamp from this iterator so that if we encounter an error, especially EOS, - // we will always seek the sample iterator, ensuring that we will decode the last block again. - m_last_timestamp = {}; - Optional block; while (true) { diff --git a/Tests/LibMedia/TestMatroskaDemuxer.cpp b/Tests/LibMedia/TestMatroskaDemuxer.cpp index b5cff7a976..3053331ab5 100644 --- a/Tests/LibMedia/TestMatroskaDemuxer.cpp +++ b/Tests/LibMedia/TestMatroskaDemuxer.cpp @@ -31,7 +31,9 @@ TEST_CASE(seek_past_eos) EXPECT_EQ(last_timestamp, AK::Duration::from_milliseconds(30126)); auto seek_time = AK::Duration::from_milliseconds(31000); - MUST(demuxer->seek_to_most_recent_keyframe(track, seek_time, Media::DemuxerSeekOptions::None)); - auto sample_after_seek = MUST(demuxer->get_next_sample_for_track(track)); - EXPECT_EQ(sample_after_seek.timestamp(), AK::Duration::zero()); + auto seek_result = MUST(demuxer->seek_to_most_recent_keyframe(track, seek_time, Media::DemuxerSeekOptions::None)); + EXPECT_EQ(seek_result, Media::DemuxerSeekResult::KeptCurrentPosition); + auto sample_result_after_seek = demuxer->get_next_sample_for_track(track); + EXPECT(sample_result_after_seek.is_error()); + EXPECT_EQ(sample_result_after_seek.error().category(), Media::DecoderErrorCategory::EndOfStream); }