From d7940eeeb59cac7eb60fa6a4c044d27ce7530803 Mon Sep 17 00:00:00 2001 From: Karl Tomlinson Date: Tue, 27 Jun 2023 08:53:50 +0000 Subject: [PATCH] Bug 1829068 retain the same AudioSinkWrapper when switching output devices r=padenot This allows video playback to continue uninterrupted. The AudioSinkWrapper now maintains the correct playback position in the media data. Previously the replacement AudioSinkWrapper started its clock at the "current playback position" from the MediaDecoderStateMachine, which did not include additional time from looping, so silence would be played until the missing time had passed. The AudioSinkWrapper now continues to periodically attempt to open a new output device if not initially available. Previously such attempts were only performed on state changes such as seeking or unmuting. The promise returned from MediaDecoderStateMachine::InvokeSetSink() now resolves regardless of whether the new device can be opened successfully. Previously the promise could be rejected if the device was necessary for audio output at the time of the call but no check for device availability (beyond that of device enumeration) was performed if playback was suspended or muted or if a subsequent device change had already been queued. Even previously the underlying device was changed even when the promise was rejected and could be used if a state change occurred. Differential Revision: https://phabricator.services.mozilla.com/D181975 --- dom/html/HTMLMediaElement.cpp | 11 +- dom/media/MediaDecoderStateMachine.cpp | 35 +---- dom/media/MediaDecoderStateMachine.h | 10 +- dom/media/MediaDecoderStateMachineBase.h | 9 ++ dom/media/mediasink/AudioSinkWrapper.cpp | 178 +++++++++++++---------- dom/media/mediasink/AudioSinkWrapper.h | 15 +- dom/media/mediasink/DecodedStream.cpp | 9 +- dom/media/mediasink/DecodedStream.h | 9 +- dom/media/mediasink/MediaSink.h | 15 +- dom/media/mediasink/VideoSink.cpp | 10 +- dom/media/mediasink/VideoSink.h | 5 +- 11 files changed, 153 insertions(+), 153 deletions(-) diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index 3451aa7eb5ce..9503db5f249b 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -5053,16 +5053,7 @@ nsresult HTMLMediaElement::FinishDecoderSetup(MediaDecoder* aDecoder) { // Set sink device if we have one. Otherwise the default is used. if (mSink.second) { - mDecoder - ->SetSink(mSink.second) -#ifdef DEBUG - ->Then(mAbstractMainThread, __func__, - [](const GenericPromise::ResolveOrRejectValue& aValue) { - MOZ_ASSERT(aValue.IsResolve() && !aValue.ResolveValue()); - }); -#else - ; -#endif + mDecoder->SetSink(mSink.second); } if (mMediaKeys) { diff --git a/dom/media/MediaDecoderStateMachine.cpp b/dom/media/MediaDecoderStateMachine.cpp index 18715b90c62e..9aa40e441740 100644 --- a/dom/media/MediaDecoderStateMachine.cpp +++ b/dom/media/MediaDecoderStateMachine.cpp @@ -4430,43 +4430,14 @@ RefPtr MediaDecoderStateMachine::InvokeSetSink( } RefPtr MediaDecoderStateMachine::SetSink( - const RefPtr& aDevice) { + RefPtr aDevice) { MOZ_ASSERT(OnTaskQueue()); if (mIsMediaSinkSuspended) { // Don't create a new media sink when suspended. - return GenericPromise::CreateAndResolve(false, __func__); + return GenericPromise::CreateAndResolve(true, __func__); } - if (mOutputCaptureState != MediaDecoder::OutputCaptureState::None) { - // Nothing to do. - return GenericPromise::CreateAndResolve(IsPlaying(), __func__); - } - - if (mSinkDevice.Ref() != aDevice) { - // A new sink was set before this ran. - return GenericPromise::CreateAndResolve(IsPlaying(), __func__); - } - - if (mMediaSink->AudioDevice() == aDevice) { - // The sink has not changed. - return GenericPromise::CreateAndResolve(IsPlaying(), __func__); - } - - const bool wasPlaying = IsPlaying(); - - // Stop and shutdown the existing sink. - StopMediaSink(); - mMediaSink->Shutdown(); - // Create a new sink according to whether audio is captured. - mMediaSink = CreateMediaSink(); - // Start the new sink - if (wasPlaying) { - nsresult rv = StartMediaSink(); - if (NS_FAILED(rv)) { - return GenericPromise::CreateAndReject(NS_ERROR_ABORT, __func__); - } - } - return GenericPromise::CreateAndResolve(wasPlaying, __func__); + return mMediaSink->SetAudioDevice(std::move(aDevice)); } void MediaDecoderStateMachine::InvokeSuspendMediaSink() { diff --git a/dom/media/MediaDecoderStateMachine.h b/dom/media/MediaDecoderStateMachine.h index d9a5254e0b21..bcedf1790aca 100644 --- a/dom/media/MediaDecoderStateMachine.h +++ b/dom/media/MediaDecoderStateMachine.h @@ -226,15 +226,7 @@ class MediaDecoderStateMachine void SetVideoDecodeModeInternal(VideoDecodeMode aMode); - // Set new sink device and restart MediaSink if playback is started. - // Returned promise will be resolved with true if the playback is - // started and false if playback is stopped after setting the new sink. - // Returned promise will be rejected with value NS_ERROR_ABORT - // if the action fails or it is not supported. - // If there are multiple pending requests only the last one will be - // executed, for all previous requests the promise will be resolved - // with true or false similar to above. - RefPtr SetSink(const RefPtr& aDevice); + RefPtr SetSink(RefPtr aDevice); // Shutdown MediaSink on suspend to clean up resources. void SuspendMediaSink(); diff --git a/dom/media/MediaDecoderStateMachineBase.h b/dom/media/MediaDecoderStateMachineBase.h index 5836b6f2f51a..b4950746e814 100644 --- a/dom/media/MediaDecoderStateMachineBase.h +++ b/dom/media/MediaDecoderStateMachineBase.h @@ -93,6 +93,15 @@ class MediaDecoderStateMachineBase { // Sets the video decode mode. Used by the suspend-video-decoder feature. virtual void SetVideoDecodeMode(VideoDecodeMode aMode) = 0; + // Set new sink device. ExternalEngineStateMachine will reject the returned + // promise with NS_ERROR_ABORT to indicate that the action is not supported. + // MediaDecoderStateMachine will resolve the promise when the previous + // device is no longer in use and an attempt to open the new device + // completes (successfully or not) or is deemed unnecessary because the + // device is not required for output at this time. MediaDecoderStateMachine + // will always consider the switch in underlying output device successful + // and continue attempting to open the new device even if opening initially + // fails. virtual RefPtr InvokeSetSink( const RefPtr& aSink) = 0; virtual void InvokeSuspendMediaSink() = 0; diff --git a/dom/media/mediasink/AudioSinkWrapper.cpp b/dom/media/mediasink/AudioSinkWrapper.cpp index 3692ad880b37..7b632b48f331 100644 --- a/dom/media/mediasink/AudioSinkWrapper.cpp +++ b/dom/media/mediasink/AudioSinkWrapper.cpp @@ -128,8 +128,9 @@ TimeUnit AudioSinkWrapper::GetPosition(TimeStamp* aTimeStamp) { } mLastClockSource = ClockSource::SystemClock; - if (!mAudioSink && mAsyncCreateCount == 0 && t > mRetrySinkTime) { - MaybeAsyncCreateAudioSink(); + if (!mAudioSink && mAsyncCreateCount == 0 && NeedAudioSink() && + t > mRetrySinkTime) { + MaybeAsyncCreateAudioSink(mAudioDevice); } } else { // Return how long we've played if we are not playing. @@ -195,7 +196,7 @@ void AudioSinkWrapper::OnMuted(bool aMuted) { } else { LOG("%p: AudioSinkWrapper unmuted, maybe re-creating an AudioStream.", this); - MaybeAsyncCreateAudioSink(); + MaybeAsyncCreateAudioSink(mAudioDevice); } } @@ -283,6 +284,11 @@ void AudioSinkWrapper::SetPlaying(bool aPlaying) { } } +RefPtr AudioSinkWrapper::SetAudioDevice( + RefPtr aDevice) { + return MaybeAsyncCreateAudioSink(std::move(aDevice)); +} + double AudioSinkWrapper::PlaybackRate() const { AssertOwnerThread(); return mParams.mPlaybackRate; @@ -342,89 +348,105 @@ void AudioSinkWrapper::ShutDownAudioSink() { mAudioSink = nullptr; } -void AudioSinkWrapper::MaybeAsyncCreateAudioSink() { - MOZ_ASSERT(!mAudioSink); - MOZ_ASSERT(!mAudioSinkEndedRequest.Exists()); - - if (!NeedAudioSink()) { - LOG("%p: AudioSinkWrapper::MaybeAsyncCreateAudioSink: AudioSink not needed", +RefPtr AudioSinkWrapper::MaybeAsyncCreateAudioSink( + RefPtr aDevice) { + UniquePtr audioSink; + if (NeedAudioSink() && (!mAudioSink || aDevice != mAudioDevice)) { + LOG("%p: AudioSinkWrapper::MaybeAsyncCreateAudioSink: AudioSink needed", this); - return; + if (mAudioSink) { + ShutDownAudioSink(); + } + audioSink = mSinkCreator(); + } else { + LOG("%p: AudioSinkWrapper::MaybeAsyncCreateAudioSink: no AudioSink change", + this); + // Bounce off the background thread to keep promise resolution in order. } - - LOG("%p: AudioSinkWrapper::MaybeAsyncCreateAudioSink: AudioSink needed", - this); + mAudioDevice = std::move(aDevice); ++mAsyncCreateCount; - UniquePtr audioSink = mSinkCreator(); using Promise = MozPromise, nsresult, /* IsExclusive = */ true>; - InvokeAsync( - mAsyncInitTaskQueue, - "MaybeAsyncCreateAudioSink (Async part: initialization)", - [self = RefPtr(this), audioSink{std::move(audioSink)}, - this]() mutable { - LOG("AudioSink initialization on background thread"); - // This can take about 200ms, e.g. on Windows, we don't want to do - // it on the MDSM thread, because it would make the clock not update - // for that amount of time, and the video would therefore not - // update. The Start() call is very cheap on the other hand, we can - // do it from the MDSM thread. - nsresult rv = audioSink->InitializeAudioStream( - mParams, mAudioDevice, AudioSink::InitializationType::UNMUTING); - if (NS_FAILED(rv)) { - LOG("Async AudioSink initialization failed"); - return Promise::CreateAndReject(rv, __func__); - } - return Promise::CreateAndResolve(std::move(audioSink), __func__); - }) - ->Then(mOwnerThread, - "MaybeAsyncCreateAudioSink (Async part: start from MDSM thread)", - [self = RefPtr(this), - this](Promise::ResolveOrRejectValue&& aValue) mutable { - LOG("AudioSink async init done, back on MDSM thread"); - ScopeExit decr([&] { --mAsyncCreateCount; }); + return InvokeAsync(mAsyncInitTaskQueue, + "MaybeAsyncCreateAudioSink (Async part: initialization)", + [self = RefPtr(this), + audioSink{std::move(audioSink)}, + audioDevice = mAudioDevice, this]() mutable { + if (!audioSink) { + return Promise::CreateAndResolve(nullptr, __func__); + } - if (aValue.IsReject()) { - if (mAudioDevice) { - // Device will be started when available again. - ScheduleRetrySink(); - return; - } - // Default device not available. Report error. - mEndedPromiseHolder.RejectIfExists(aValue.RejectValue(), - __func__); - return; - } + LOG("AudioSink initialization on background thread"); + // This can take about 200ms, e.g. on Windows, we don't + // want to do it on the MDSM thread, because it would + // make the clock not update for that amount of time, and + // the video would therefore not update. The Start() call + // is very cheap on the other hand, we can do it from the + // MDSM thread. + nsresult rv = audioSink->InitializeAudioStream( + mParams, audioDevice, + AudioSink::InitializationType::UNMUTING); + if (NS_FAILED(rv)) { + LOG("Async AudioSink initialization failed"); + return Promise::CreateAndReject(rv, __func__); + } + return Promise::CreateAndResolve(std::move(audioSink), + __func__); + }) + ->Then( + mOwnerThread, + "MaybeAsyncCreateAudioSink (Async part: start from MDSM thread)", + [self = RefPtr(this), audioDevice = mAudioDevice, + this](Promise::ResolveOrRejectValue&& aValue) mutable { + LOG("AudioSink async init done, back on MDSM thread"); + ScopeExit decr([&] { --mAsyncCreateCount; }); - UniquePtr audioSink = std::move(aValue.ResolveValue()); - // It's possible that the newly created isn't needed at this - // point, in some cases: - // 1. An AudioSink was created synchronously while this - // AudioSink was initialized asynchronously, bail out here. This - // happens when seeking (which does a synchronous - // initialization) right after unmuting. - // 2. The media element was muted while the async initialization - // was happening. - // 3. The AudioSinkWrapper was paused or stopped during - // asynchronous initialization. - // 4. The audio has ended during asynchronous initialization. - if (mAudioSink || !NeedAudioSink()) { - LOG("AudioSink initialized async isn't needed, shutting " - "it down."); - audioSink->ShutDown(); - return; - } + if (aValue.IsReject()) { + if (audioDevice) { + // Device will be started when available again. + ScheduleRetrySink(); + return GenericPromise::CreateAndResolve(true, __func__); + } + // Default device not available. Report error. + mEndedPromiseHolder.RejectIfExists(aValue.RejectValue(), + __func__); + return GenericPromise::CreateAndResolve(true, __func__); + } - MOZ_ASSERT(!mAudioSink); - TimeUnit switchTime = GetPosition(); - DropAudioPacketsIfNeeded(switchTime); - if (mTreatUnderrunAsSilence) { - audioSink->EnableTreatAudioUnderrunAsSilence( - mTreatUnderrunAsSilence); - } - LOG("AudioSink async, start"); - StartAudioSink(std::move(audioSink), switchTime); - }); + UniquePtr audioSink = std::move(aValue.ResolveValue()); + if (!audioSink) { + return GenericPromise::CreateAndResolve(true, __func__); + } + // It's possible that the newly created isn't needed at this + // point, in some cases: + // 1. An AudioSink was created synchronously while this + // AudioSink was initialized asynchronously, bail out here. This + // happens when seeking (which does a synchronous + // initialization) right after unmuting. + // 2. The media element was muted while the async initialization + // was happening. + // 3. The AudioSinkWrapper was paused or stopped during + // asynchronous initialization. + // 4. The audio has ended during asynchronous initialization. + // 5. A change to a potentially different sink device is pending. + if (mAudioSink || !NeedAudioSink() || audioDevice != mAudioDevice) { + LOG("AudioSink initialized async isn't needed, shutting " + "it down."); + audioSink->ShutDown(); + return GenericPromise::CreateAndResolve(true, __func__); + } + + MOZ_ASSERT(!mAudioSink); + TimeUnit switchTime = GetPosition(); + DropAudioPacketsIfNeeded(switchTime); + if (mTreatUnderrunAsSilence) { + audioSink->EnableTreatAudioUnderrunAsSilence( + mTreatUnderrunAsSilence); + } + LOG("AudioSink async, start"); + StartAudioSink(std::move(audioSink), switchTime); + return GenericPromise::CreateAndResolve(true, __func__); + }); } nsresult AudioSinkWrapper::SyncCreateAudioSink(const TimeUnit& aStartTime) { diff --git a/dom/media/mediasink/AudioSinkWrapper.h b/dom/media/mediasink/AudioSinkWrapper.h index fe8e59ac5c2f..035f03573505 100644 --- a/dom/media/mediasink/AudioSinkWrapper.h +++ b/dom/media/mediasink/AudioSinkWrapper.h @@ -54,6 +54,8 @@ class AudioSinkWrapper : public MediaSink { void SetPlaybackRate(double aPlaybackRate) override; void SetPreservesPitch(bool aPreservesPitch) override; void SetPlaying(bool aPlaying) override; + RefPtr SetAudioDevice( + RefPtr aDevice) override; double PlaybackRate() const override; @@ -63,8 +65,6 @@ class AudioSinkWrapper : public MediaSink { bool IsStarted() const override; bool IsPlaying() const override; - const AudioDeviceInfo* AudioDevice() const override { return mAudioDevice; } - void Shutdown() override; void GetDebugInfo(dom::MediaSinkDebugInfo& aInfo) override; @@ -103,9 +103,14 @@ class AudioSinkWrapper : public MediaSink { // and when seeking. // In asynchronous mode, the clock will keep going forward (using the system // clock) until the AudioSink is started, at which point the clock will use - // the AudioSink clock. This is used when unmuting a media element. + // the AudioSink clock. This is used when unmuting a media element or + // switching audio output devices. The promise is resolved when the + // previous device is no longer in use and an attempt to open the new device + // completes (successfully or not) or is deemed unnecessary because the + // device is not required for output at this time. nsresult SyncCreateAudioSink(const media::TimeUnit& aStartTime); - void MaybeAsyncCreateAudioSink(); + RefPtr MaybeAsyncCreateAudioSink( + RefPtr aDevice); void ScheduleRetrySink(); // Get the current media position using the system clock. This is used when @@ -124,7 +129,7 @@ class AudioSinkWrapper : public MediaSink { UniquePtr mAudioSink; // The output device this AudioSink is playing data to. The system's default // device is used if this is null. - const RefPtr mAudioDevice; + RefPtr mAudioDevice; // Will only exist when media has an audio track. RefPtr mEndedPromise; MozPromiseHolder mEndedPromiseHolder; diff --git a/dom/media/mediasink/DecodedStream.cpp b/dom/media/mediasink/DecodedStream.cpp index b8520829035a..86254493ee68 100644 --- a/dom/media/mediasink/DecodedStream.cpp +++ b/dom/media/mediasink/DecodedStream.cpp @@ -465,8 +465,7 @@ DecodedStream::DecodedStream( mPlaybackRate(aPlaybackRate), mPreservesPitch(aPreservesPitch), mAudioQueue(aAudioQueue), - mVideoQueue(aVideoQueue), - mAudioDevice(std::move(aAudioDevice)) {} + mVideoQueue(aVideoQueue) {} DecodedStream::~DecodedStream() { MOZ_ASSERT(mStartTime.isNothing(), "playback should've ended."); @@ -722,6 +721,12 @@ void DecodedStream::SetPreservesPitch(bool aPreservesPitch) { } } +RefPtr DecodedStream::SetAudioDevice( + RefPtr aDevice) { + // All audio is captured, so nothing is actually played out, so nothing to do. + return GenericPromise::CreateAndResolve(true, __func__); +} + double DecodedStream::PlaybackRate() const { AssertOwnerThread(); return mPlaybackRate; diff --git a/dom/media/mediasink/DecodedStream.h b/dom/media/mediasink/DecodedStream.h index 4709ffeda674..2ef4086a132c 100644 --- a/dom/media/mediasink/DecodedStream.h +++ b/dom/media/mediasink/DecodedStream.h @@ -60,6 +60,8 @@ class DecodedStream : public MediaSink { void SetPlaybackRate(double aPlaybackRate) override; void SetPreservesPitch(bool aPreservesPitch) override; void SetPlaying(bool aPlaying) override; + RefPtr SetAudioDevice( + RefPtr aDevice) override; double PlaybackRate() const override; @@ -70,7 +72,6 @@ class DecodedStream : public MediaSink { bool IsPlaying() const override; void Shutdown() override; void GetDebugInfo(dom::MediaSinkDebugInfo& aInfo) override; - const AudioDeviceInfo* AudioDevice() const override { return mAudioDevice; } MediaEventSource& AudibleEvent() { return mAudibleEvent; } @@ -136,12 +137,6 @@ class DecodedStream : public MediaSink { MediaQueue& mAudioQueue; MediaQueue& mVideoQueue; - // This is the audio device we were told to play out to. - // All audio is captured, so nothing is actually played out -- but we report - // this upwards as it could save us from being recreated when the sink - // changes. - const RefPtr mAudioDevice; - MediaEventListener mAudioPushListener; MediaEventListener mVideoPushListener; MediaEventListener mAudioFinishListener; diff --git a/dom/media/mediasink/MediaSink.h b/dom/media/mediasink/MediaSink.h index f6170df72891..e9d4db8cbfd1 100644 --- a/dom/media/mediasink/MediaSink.h +++ b/dom/media/mediasink/MediaSink.h @@ -93,6 +93,17 @@ class MediaSink { // Pause/resume the playback. Only work after playback starts. virtual void SetPlaying(bool aPlaying) = 0; + // Set the audio output device. aDevice == nullptr indicates the default + // device. The returned promise is resolved when the previous device is no + // longer in use and an attempt to open the new device completes + // (successfully or not) or is deemed unnecessary because the device is not + // required for output at this time. The promise will be resolved whether + // or not opening the cubeb_stream succeeds because the aDevice is + // considered the new underlying device and will be used when it becomes + // available. + virtual RefPtr SetAudioDevice( + RefPtr aDevice) = 0; + // Get the playback rate. // Can be called in any state. virtual double PlaybackRate() const = 0; @@ -121,10 +132,6 @@ class MediaSink { // Can be called in any state. virtual bool IsPlaying() const = 0; - // The audio output device this MediaSink is playing audio data to. The - // default device is used if this returns null. - virtual const AudioDeviceInfo* AudioDevice() const = 0; - // Called on the state machine thread to shut down the sink. All resources // allocated by this sink should be released. // Must be called after playback stopped. diff --git a/dom/media/mediasink/VideoSink.cpp b/dom/media/mediasink/VideoSink.cpp index 906efdf0db7e..08dd4c727cd3 100644 --- a/dom/media/mediasink/VideoSink.cpp +++ b/dom/media/mediasink/VideoSink.cpp @@ -12,6 +12,7 @@ #include "VideoSink.h" +#include "AudioDeviceInfo.h" #include "MediaQueue.h" #include "VideoUtils.h" @@ -160,6 +161,11 @@ void VideoSink::SetPreservesPitch(bool aPreservesPitch) { mAudioSink->SetPreservesPitch(aPreservesPitch); } +RefPtr VideoSink::SetAudioDevice( + RefPtr aDevice) { + return mAudioSink->SetAudioDevice(std::move(aDevice)); +} + double VideoSink::PlaybackRate() const { AssertOwnerThread(); @@ -297,10 +303,6 @@ bool VideoSink::IsPlaying() const { return mAudioSink->IsPlaying(); } -const AudioDeviceInfo* VideoSink::AudioDevice() const { - return mAudioSink->AudioDevice(); -} - void VideoSink::Shutdown() { AssertOwnerThread(); MOZ_ASSERT(!mAudioSink->IsStarted(), "must be called after playback stops."); diff --git a/dom/media/mediasink/VideoSink.h b/dom/media/mediasink/VideoSink.h index 7f2528d87019..205d3bcfc849 100644 --- a/dom/media/mediasink/VideoSink.h +++ b/dom/media/mediasink/VideoSink.h @@ -51,6 +51,9 @@ class VideoSink : public MediaSink { void SetPlaying(bool aPlaying) override; + RefPtr SetAudioDevice( + RefPtr aDevice) override; + double PlaybackRate() const override; void Redraw(const VideoInfo& aInfo) override; @@ -64,8 +67,6 @@ class VideoSink : public MediaSink { bool IsPlaying() const override; - const AudioDeviceInfo* AudioDevice() const override; - void Shutdown() override; void SetSecondaryVideoContainer(VideoFrameContainer* aSecondary) override;