From 8848ee775befd10cd404f70812b95b0bd3e959d0 Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Wed, 19 Jun 2024 22:36:27 -0500 Subject: [PATCH] LibMedia: Flush the video decoder when seeking This allows H.264 videos to seek correctly. --- .../Color/CodingIndependentCodePoints.h | 10 +-- .../Libraries/LibMedia/PlaybackManager.cpp | 70 ++++++++++--------- Userland/Libraries/LibMedia/PlaybackManager.h | 2 +- Userland/Libraries/LibMedia/VideoDecoder.h | 2 +- 4 files changed, 46 insertions(+), 38 deletions(-) diff --git a/Userland/Libraries/LibMedia/Color/CodingIndependentCodePoints.h b/Userland/Libraries/LibMedia/Color/CodingIndependentCodePoints.h index 00bc8beff06..f0833f70ef6 100644 --- a/Userland/Libraries/LibMedia/Color/CodingIndependentCodePoints.h +++ b/Userland/Libraries/LibMedia/Color/CodingIndependentCodePoints.h @@ -82,6 +82,8 @@ enum class VideoFullRangeFlag : u8 { // https://en.wikipedia.org/wiki/Coding-independent_code_points struct CodingIndependentCodePoints { public: + constexpr CodingIndependentCodePoints() = default; + constexpr CodingIndependentCodePoints(ColorPrimaries color_primaries, TransferCharacteristics transfer_characteristics, MatrixCoefficients matrix_coefficients, VideoFullRangeFlag video_full_range_flag) : m_color_primaries(color_primaries) , m_transfer_characteristics(transfer_characteristics) @@ -124,10 +126,10 @@ public: } private: - ColorPrimaries m_color_primaries; - TransferCharacteristics m_transfer_characteristics; - MatrixCoefficients m_matrix_coefficients; - VideoFullRangeFlag m_video_full_range_flag; + ColorPrimaries m_color_primaries = ColorPrimaries::BT709; + TransferCharacteristics m_transfer_characteristics = TransferCharacteristics::BT709; + MatrixCoefficients m_matrix_coefficients = MatrixCoefficients::BT709; + VideoFullRangeFlag m_video_full_range_flag = VideoFullRangeFlag::Full; }; constexpr StringView color_primaries_to_string(ColorPrimaries color_primaries) diff --git a/Userland/Libraries/LibMedia/PlaybackManager.cpp b/Userland/Libraries/LibMedia/PlaybackManager.cpp index cfb2996cdb8..fd9afdce96f 100644 --- a/Userland/Libraries/LibMedia/PlaybackManager.cpp +++ b/Userland/Libraries/LibMedia/PlaybackManager.cpp @@ -92,7 +92,7 @@ Duration PlaybackManager::current_playback_time() Duration PlaybackManager::duration() { auto duration_result = ({ - auto demuxer_locker = Threading::MutexLocker(m_demuxer_mutex); + auto demuxer_locker = Threading::MutexLocker(m_decoder_mutex); m_demuxer->duration(); }); if (duration_result.is_error()) { @@ -168,7 +168,10 @@ void PlaybackManager::seek_to_timestamp(Duration target_timestamp, SeekMode seek DecoderErrorOr> PlaybackManager::seek_demuxer_to_most_recent_keyframe(Duration timestamp, Optional earliest_available_sample) { - return m_demuxer->seek_to_most_recent_keyframe(m_selected_video_track, timestamp, move(earliest_available_sample)); + auto seeked_timestamp = TRY(m_demuxer->seek_to_most_recent_keyframe(m_selected_video_track, timestamp, move(earliest_available_sample))); + if (seeked_timestamp.has_value()) + m_decoder->flush(); + return seeked_timestamp; } Optional PlaybackManager::dequeue_one_frame() @@ -202,46 +205,49 @@ void PlaybackManager::decode_and_queue_one_sample() FrameQueueItem item_to_enqueue; while (item_to_enqueue.is_empty()) { - // Get a sample to decode. - auto sample_result = [&]() { - // FIXME: Implement and use a class to enforce that this field is accessed through a mutex (like Kernel::MutexProtected). - Threading::MutexLocker demuxer_locker(m_demuxer_mutex); - return m_demuxer->get_next_sample_for_track(m_selected_video_track); - }(); - if (sample_result.is_error()) { - item_to_enqueue = FrameQueueItem::error_marker(sample_result.release_error(), FrameQueueItem::no_timestamp); - break; - } - auto sample = sample_result.release_value(); - - // Submit the sample to the decoder. - auto decode_result = m_decoder->receive_sample(sample.timestamp(), sample.data()); - if (decode_result.is_error()) { - item_to_enqueue = FrameQueueItem::error_marker(decode_result.release_error(), sample.timestamp()); - break; - } - - // Retrieve the last available frame to present. OwnPtr decoded_frame = nullptr; - while (true) { - auto frame_result = m_decoder->get_decoded_frame(); + CodingIndependentCodePoints container_cicp; - if (frame_result.is_error()) { - if (frame_result.error().category() == DecoderErrorCategory::NeedsMoreInput) { - break; - } + { + Threading::MutexLocker decoder_locker(m_decoder_mutex); - item_to_enqueue = FrameQueueItem::error_marker(frame_result.release_error(), sample.timestamp()); + // Get a sample to decode. + auto sample_result = m_demuxer->get_next_sample_for_track(m_selected_video_track); + if (sample_result.is_error()) { + item_to_enqueue = FrameQueueItem::error_marker(sample_result.release_error(), FrameQueueItem::no_timestamp); + break; + } + auto sample = sample_result.release_value(); + container_cicp = sample.auxiliary_data().get().container_cicp(); + + // Submit the sample to the decoder. + auto decode_result = m_decoder->receive_sample(sample.timestamp(), sample.data()); + if (decode_result.is_error()) { + item_to_enqueue = FrameQueueItem::error_marker(decode_result.release_error(), sample.timestamp()); break; } - decoded_frame = frame_result.release_value(); + // Retrieve the last available frame to present. + while (true) { + auto frame_result = m_decoder->get_decoded_frame(); + + if (frame_result.is_error()) { + if (frame_result.error().category() == DecoderErrorCategory::NeedsMoreInput) { + break; + } + + item_to_enqueue = FrameQueueItem::error_marker(frame_result.release_error(), sample.timestamp()); + break; + } + + decoded_frame = frame_result.release_value(); + } } // Convert the frame for display. if (decoded_frame != nullptr) { auto& cicp = decoded_frame->cicp(); - cicp.adopt_specified_values(sample.auxiliary_data().get().container_cicp()); + cicp.adopt_specified_values(container_cicp); cicp.default_code_points_if_unspecified({ ColorPrimaries::BT709, TransferCharacteristics::BT709, MatrixCoefficients::BT709, VideoFullRangeFlag::Studio }); // BT.601, BT.709 and BT.2020 have a similar transfer function to sRGB, so other applications @@ -570,7 +576,7 @@ private: } { - Threading::MutexLocker demuxer_locker(manager().m_demuxer_mutex); + Threading::MutexLocker demuxer_locker(manager().m_decoder_mutex); auto demuxer_seek_result = manager().seek_demuxer_to_most_recent_keyframe(m_target_timestamp, earliest_available_sample); if (demuxer_seek_result.is_error()) { diff --git a/Userland/Libraries/LibMedia/PlaybackManager.h b/Userland/Libraries/LibMedia/PlaybackManager.h index 1fdfa28ac2c..08a049b079e 100644 --- a/Userland/Libraries/LibMedia/PlaybackManager.h +++ b/Userland/Libraries/LibMedia/PlaybackManager.h @@ -176,7 +176,7 @@ private: Duration m_last_present_in_media_time = Duration::zero(); NonnullOwnPtr m_demuxer; - Threading::Mutex m_demuxer_mutex; + Threading::Mutex m_decoder_mutex; Track m_selected_video_track; VideoFrameQueue m_frame_queue; diff --git a/Userland/Libraries/LibMedia/VideoDecoder.h b/Userland/Libraries/LibMedia/VideoDecoder.h index b82ed804af3..578b9c1e80c 100644 --- a/Userland/Libraries/LibMedia/VideoDecoder.h +++ b/Userland/Libraries/LibMedia/VideoDecoder.h @@ -21,7 +21,7 @@ public: virtual DecoderErrorOr receive_sample(Duration timestamp, ReadonlyBytes sample) = 0; DecoderErrorOr receive_sample(Duration timestamp, ByteBuffer const& sample) { return receive_sample(timestamp, sample.span()); } virtual DecoderErrorOr> get_decoded_frame() = 0; - + virtual void flush() = 0; };