From f0ab344a4e8db7acd6c8259f786e3aea094c752c Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Fri, 5 Jun 2026 00:38:40 -0500 Subject: [PATCH] LibMedia: Handle outputting silence after EOS in the audio sink This allows nodes downstream of the mixer to know the exact frame at which EOS is reached. More concepts have been introduced into PipelineStatus.h to make each condition in the pipeline clearer about its intent. --- Libraries/LibMedia/AudioBlockTimingRing.h | 12 ++++ Libraries/LibMedia/PipelineStatus.h | 30 +++++++--- .../PlaybackStates/SeekingStateHandler.h | 4 +- Libraries/LibMedia/Processors/AudioMixer.cpp | 29 ++++++---- Libraries/LibMedia/Processors/AudioMixer.h | 2 +- .../LibMedia/Sinks/AudioPlaybackSink.cpp | 58 ++++++++++++++++--- Libraries/LibMedia/Sinks/AudioPlaybackSink.h | 1 - .../LibMedia/Sinks/DisplayingVideoSink.cpp | 4 +- 8 files changed, 108 insertions(+), 32 deletions(-) diff --git a/Libraries/LibMedia/AudioBlockTimingRing.h b/Libraries/LibMedia/AudioBlockTimingRing.h index aba0742eb5..d399894eb2 100644 --- a/Libraries/LibMedia/AudioBlockTimingRing.h +++ b/Libraries/LibMedia/AudioBlockTimingRing.h @@ -36,6 +36,18 @@ public: m_latest_sequence.store(sequence); } + Optional latest_timing() const + { + auto latest_sequence = m_latest_sequence.load(); + if (latest_sequence < m_first_valid_sequence.load()) + return {}; + + auto record = read_record(latest_sequence); + if (!record.has_value()) + return {}; + return record->timing; + } + Optional find_timing_for_frame_index(i64 frame_index) const { auto latest_sequence = m_latest_sequence.load(); diff --git a/Libraries/LibMedia/PipelineStatus.h b/Libraries/LibMedia/PipelineStatus.h index 19df9fc6be..2296e00e91 100644 --- a/Libraries/LibMedia/PipelineStatus.h +++ b/Libraries/LibMedia/PipelineStatus.h @@ -23,20 +23,36 @@ enum class PipelineStatus : u8 { constexpr bool is_waiting_for_data(PipelineStatus status) { - if (status == PipelineStatus::Pending) + return status != PipelineStatus::HaveData; +} + +constexpr bool is_terminal(PipelineStatus status) +{ + if (status == PipelineStatus::Error) return true; - if (status == PipelineStatus::MovedPosition) - return true; - if (status == PipelineStatus::Blocked) + if (status == PipelineStatus::EndOfStream) return true; return false; } -constexpr bool can_carry_data(PipelineStatus status) +constexpr bool resolves_seek(PipelineStatus status) { - if (status == PipelineStatus::HaveData) + if (!is_waiting_for_data(status)) return true; - if (status == PipelineStatus::EndOfStream) + if (is_terminal(status)) + return true; + return false; +} + +constexpr bool status_change_should_wake(PipelineStatus previous, PipelineStatus current) +{ + if (previous == current) + return false; + if (!resolves_seek(current)) + return false; + if (is_waiting_for_data(previous)) + return true; + if (previous == PipelineStatus::EndOfStream) return true; return false; } diff --git a/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h b/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h index e7cec39a7e..bbcb5cb442 100644 --- a/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h +++ b/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h @@ -63,7 +63,7 @@ public: virtual void on_audio_sink_state_changed(PipelineStatus status) override { - if (is_waiting_for_data(status)) + if (!resolves_seek(status)) return; m_audio_seek_pending = false; possibly_complete_seek(); @@ -71,7 +71,7 @@ public: virtual void on_video_sink_state_changed(Track const& track, PipelineStatus status) override { - if (is_waiting_for_data(status)) + if (!resolves_seek(status)) return; m_video_seeks_pending.remove(track); possibly_complete_seek(); diff --git a/Libraries/LibMedia/Processors/AudioMixer.cpp b/Libraries/LibMedia/Processors/AudioMixer.cpp index 10646dda4d..eb147f7ded 100644 --- a/Libraries/LibMedia/Processors/AudioMixer.cpp +++ b/Libraries/LibMedia/Processors/AudioMixer.cpp @@ -37,7 +37,7 @@ ErrorOr AudioMixer::connect_input(NonnullRefPtr const& inpu { Sync::MutexLocker locker { m_mutex }; m_status = combined_input_status(); - should_wake_downstream = m_downstream_needs_wake && !is_waiting_for_data(m_status); + should_wake_downstream = status_change_should_wake(m_last_returned_status, m_status); if (m_moved_position_pending) m_status = PipelineStatus::MovedPosition; } @@ -69,7 +69,7 @@ void AudioMixer::disconnect_input_while_locked(NonnullRefPtr cons input->set_wake_handler(nullptr); m_inputs.remove(input); m_status = combined_input_status(); - if (m_downstream_needs_wake && !is_waiting_for_data(m_status)) { + if (status_change_should_wake(m_last_returned_status, m_status)) { Core::deferred_invoke([self = NonnullRefPtr(*this)] { self->dispatch_wake(); }); @@ -151,7 +151,7 @@ void AudioMixer::seek(AK::Duration timestamp) } m_status = PipelineStatus::Pending; - m_downstream_needs_wake = true; + m_last_returned_status = PipelineStatus::Pending; } if (m_inputs.is_empty()) { @@ -175,7 +175,7 @@ PipelineStatus AudioMixer::status() const m_status = combined_input_status(); if (m_moved_position_pending) m_status = PipelineStatus::MovedPosition; - m_downstream_needs_wake = is_waiting_for_data(m_status); + m_last_returned_status = m_status; return m_status; } @@ -186,10 +186,6 @@ void AudioMixer::set_wake_handler(PipelineWakeHandler handler) void AudioMixer::dispatch_wake() { - { - Sync::MutexLocker locker { m_mutex }; - m_downstream_needs_wake = false; - } if (m_wake_handler) m_wake_handler(); } @@ -205,7 +201,7 @@ PipelineStatus AudioMixer::combined_input_status() const continue; } - if (!can_carry_data(input_data.last_status)) { + if (is_waiting_for_data(input_data.last_status) && !is_terminal(input_data.last_status)) { while ((input_data.last_status = input->status()) == PipelineStatus::MovedPosition) { input->pull(input_data.current_block); VERIFY(input_data.current_block.is_empty()); @@ -243,6 +239,7 @@ void AudioMixer::pull(AudioBlock& into) auto combined_status_after_mix = PipelineStatus::EndOfStream; i64 latest_mixed_frame = frames_end_cap; + i64 latest_frame_containing_data = buffer_start_frame; for (auto& [input, input_data] : m_inputs) input_data.next_frame = buffer_start_frame; @@ -331,6 +328,7 @@ void AudioMixer::pull(AudioBlock& into) } input_data.next_frame = next_frame + static_cast(frames_to_write); + latest_frame_containing_data = max(latest_frame_containing_data, input_data.next_frame); } for (auto& [input, input_data] : m_inputs) { @@ -340,14 +338,21 @@ void AudioMixer::pull(AudioBlock& into) } VERIFY(latest_mixed_frame >= buffer_start_frame); - auto frame_count = static_cast(latest_mixed_frame - buffer_start_frame); - if (combined_status_after_mix == PipelineStatus::EndOfStream) { - m_next_frame_to_write = frames_end_cap; + if (latest_frame_containing_data > buffer_start_frame) { + auto frame_count = static_cast(latest_frame_containing_data - buffer_start_frame); + into.trim(frame_count); + m_next_frame_to_write += static_cast(frame_count); + m_status = PipelineStatus::HaveData; + return; + } + into.clear(); m_status = PipelineStatus::EndOfStream; return; } + auto frame_count = static_cast(latest_mixed_frame - buffer_start_frame); + if (frame_count == 0) { into.clear(); if (combined_status_after_mix == PipelineStatus::HaveData) diff --git a/Libraries/LibMedia/Processors/AudioMixer.h b/Libraries/LibMedia/Processors/AudioMixer.h index 2505279c0f..f639eb1131 100644 --- a/Libraries/LibMedia/Processors/AudioMixer.h +++ b/Libraries/LibMedia/Processors/AudioMixer.h @@ -65,10 +65,10 @@ private: float m_playback_rate { 1.0f }; bool m_started { false }; bool m_moved_position_pending { false }; - mutable bool m_downstream_needs_wake { true }; PipelineWakeHandler m_wake_handler; mutable PipelineStatus m_status { PipelineStatus::Pending }; + mutable PipelineStatus m_last_returned_status { PipelineStatus::Pending }; }; } diff --git a/Libraries/LibMedia/Sinks/AudioPlaybackSink.cpp b/Libraries/LibMedia/Sinks/AudioPlaybackSink.cpp index e83a61f1b8..0f8334b635 100644 --- a/Libraries/LibMedia/Sinks/AudioPlaybackSink.cpp +++ b/Libraries/LibMedia/Sinks/AudioPlaybackSink.cpp @@ -25,6 +25,13 @@ namespace Media { static constexpr size_t OUTPUT_BLOCK_QUEUE_CAPACITY = 4; +static bool audio_processor_will_enqueue(PipelineStatus status) +{ + if (status == PipelineStatus::EndOfStream) + return true; + return !is_waiting_for_data(status); +} + class AudioPlaybackSink::OutputThreadData : public AtomicRefCounted { public: OutputThreadData(PipelineStateChangeHandler on_state_changed) @@ -51,6 +58,7 @@ public: i64 m_next_frame_to_play { 0 }; AudioBlockTimingRing m_block_timings; float m_playback_rate { 1.0f }; + float m_eos_media_frame_remainder { 0.0f }; PipelineStateChangeHandler m_on_state_changed; PipelineStatus m_last_pull_status { PipelineStatus::Pending }; @@ -89,11 +97,12 @@ ErrorOr> AudioPlaybackSink::try_create(Pipeline output_thread_data->m_block_count = 0; output_thread_data->m_block_timings.clear(); output_thread_data->m_last_real_data_end_in_frames = output_thread_data->m_next_frame_to_play; + output_thread_data->m_eos_media_frame_remainder = 0.0f; output_thread_data->m_waiting_for_upstream_data = false; output_thread_data->m_output_condition.broadcast(); continue; } - if (!is_waiting_for_data(status)) { + if (audio_processor_will_enqueue(status)) { Sync::MutexLocker locker { output_thread_data->m_output_mutex }; output_thread_data->m_last_pull_status = status; output_thread_data->m_waiting_for_upstream_data = false; @@ -131,15 +140,42 @@ ErrorOr> AudioPlaybackSink::try_create(Pipeline auto status = input->status(); if (status == PipelineStatus::MovedPosition) continue; - input->pull(output_block); + if (status == PipelineStatus::EndOfStream) + output_block.clear(); + else + input->pull(output_block); { Sync::MutexLocker locker { output_thread_data->m_output_mutex }; if (output_thread_data->m_seek_id != seek_id_at_pull) continue; output_thread_data->m_last_pull_status = status; + if (status == PipelineStatus::EndOfStream) { + VERIFY(output_block.is_empty()); + VERIFY(output_thread_data->m_sample_specification.is_valid()); + auto channel_count = output_thread_data->m_sample_specification.channel_count(); + size_t frame_count = 1024 / channel_count; + VERIFY(frame_count > 0); + auto maybe_previous_timing = output_thread_data->m_block_timings.latest_timing(); + auto first_frame_index = max(output_thread_data->m_last_real_data_end_in_frames, output_thread_data->m_next_frame_to_play); + if (maybe_previous_timing.has_value()) + first_frame_index = max(first_frame_index, maybe_previous_timing->end_frame_index()); + output_block.initialize(output_thread_data->m_sample_specification, first_frame_index, frame_count); + for (size_t channel = 0; channel < output_block.channel_count(); channel++) + output_block.channel_data(channel).fill(0.0f); + + auto sample_rate = output_thread_data->m_sample_specification.sample_rate(); + auto media_frame_count_with_remainder = (frame_count * output_thread_data->m_playback_rate) + output_thread_data->m_eos_media_frame_remainder; + auto media_frame_count = static_cast(media_frame_count_with_remainder); + output_thread_data->m_eos_media_frame_remainder = media_frame_count_with_remainder - media_frame_count; + auto media_time_start = AK::Duration::from_time_units(first_frame_index, 1, sample_rate); + if (maybe_previous_timing.has_value()) + media_time_start = maybe_previous_timing->media_time_at_frame_index(first_frame_index); + output_block.set_media_time_start(media_time_start); + output_block.set_media_time_duration(AK::Duration::from_time_units(media_frame_count, 1, sample_rate)); + } if (!output_block.is_empty()) { - VERIFY(can_carry_data(status)); + VERIFY(audio_processor_will_enqueue(status)); output_thread_data->m_block_tail = (output_thread_data->m_block_tail + 1) % OUTPUT_BLOCK_QUEUE_CAPACITY; output_thread_data->m_block_count++; output_thread_data->m_block_timings.enqueue(output_block.timing()); @@ -147,14 +183,20 @@ ErrorOr> AudioPlaybackSink::try_create(Pipeline if (output_thread_data->m_playback_stream) output_thread_data->m_playback_stream->notify_data_available(); - if (status == PipelineStatus::HaveData) + if (status == PipelineStatus::HaveData) { output_thread_data->m_last_real_data_end_in_frames = output_block.end_frame_index(); + output_thread_data->m_eos_media_frame_remainder = 0.0f; + } } - output_thread_data->m_waiting_for_upstream_data = !can_carry_data(status); + output_thread_data->m_waiting_for_upstream_data = !audio_processor_will_enqueue(status); - if (!can_carry_data(output_thread_data->m_last_dispatched_status) && can_carry_data(status)) - output_thread_data->dispatch_state_if_changed(status, seek_id_at_pull); + if (!status_change_should_wake(output_thread_data->m_last_dispatched_status, status)) + continue; + if (is_waiting_for_data(status) && output_thread_data->m_next_frame_to_play < output_thread_data->m_last_real_data_end_in_frames) + continue; + + output_thread_data->dispatch_state_if_changed(status, seek_id_at_pull); } } @@ -436,6 +478,8 @@ void AudioPlaybackSink::seek(AK::Duration time) m_output_thread_data->m_last_pull_status = PipelineStatus::Pending; m_output_thread_data->m_last_dispatched_status = PipelineStatus::Pending; + m_output_thread_data->m_last_real_data_end_in_frames = seek_target_in_frames; + m_output_thread_data->m_eos_media_frame_remainder = 0.0f; m_output_thread_data->m_waiting_for_upstream_data = true; m_output_thread_data->m_block_timings.clear(); diff --git a/Libraries/LibMedia/Sinks/AudioPlaybackSink.h b/Libraries/LibMedia/Sinks/AudioPlaybackSink.h index b575b46b57..66ccd16e80 100644 --- a/Libraries/LibMedia/Sinks/AudioPlaybackSink.h +++ b/Libraries/LibMedia/Sinks/AudioPlaybackSink.h @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include diff --git a/Libraries/LibMedia/Sinks/DisplayingVideoSink.cpp b/Libraries/LibMedia/Sinks/DisplayingVideoSink.cpp index 83e9fa5121..2249ce4c09 100644 --- a/Libraries/LibMedia/Sinks/DisplayingVideoSink.cpp +++ b/Libraries/LibMedia/Sinks/DisplayingVideoSink.cpp @@ -55,7 +55,7 @@ ErrorOr DisplayingVideoSink::connect_input(NonnullRefPtr co input->set_wake_handler([this, input] { auto status = PipelineStatus::Pending; consume_moved_position_signals(status); - if (is_waiting_for_data(status)) + if (!resolves_seek(status)) return; dispatch_state_if_changed(status); }); @@ -143,7 +143,7 @@ DisplayingVideoSinkUpdateResult DisplayingVideoSink::update() if (m_seek_status == SeekStatus::FrameInvalidated) m_current_frame.clear(); } - if (!is_waiting_for_data(last_status)) + if (resolves_seek(last_status)) m_seek_status = SeekStatus::None; if (m_seek_status != SeekStatus::None) break;