diff --git a/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc b/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc index 2a387ac76f4b7..2a2ecb55ba816 100644 --- a/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc +++ b/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.cc @@ -59,6 +59,10 @@ namespace { // Any reasonable size, will be overridden by the decoder anyway. constexpr gfx::Size kDefaultSize(640, 480); +// How long InitializeSync() will block waiting for the underlying decoder to +// finish initializing on the media thread before giving up. +base::TimeDelta g_initialization_timeout = base::Seconds(10); + // Maximum number of buffers that we will queue in |pending_buffers_|. constexpr int32_t kMaxPendingBuffers = 8; @@ -231,6 +235,9 @@ class RTCVideoDecoderAdapter::Impl { std::optional NeedSoftwareFallback( media::VideoCodec codec, const media::DecoderBuffer& buffer) const; + void OnInitializeDone(base::OnceCallback init_cb, + media::VideoDecoderType* decoder_type, + media::DecoderStatus status); void DecodePendingBuffers(); void OnDecodeDone(media::DecoderStatus status); void OnOutput(scoped_refptr frame); @@ -291,22 +298,31 @@ void RTCVideoDecoderAdapter::Impl::Initialize( media::VideoDecoder::OutputCB output_cb = ConvertToBaseRepeatingCallback(CrossThreadBindRepeating( &RTCVideoDecoderAdapter::Impl::OnOutput, weak_decoder_this_)); + // Some media::VideoDecoder implementations post the init callback back to + // this sequence rather than running it synchronously, so it may be invoked + // after Release() has destroyed this Impl. Bind to |weak_decoder_this_| so + // the callback becomes a no-op in that case; while the Impl is alive, + // Release() is still blocked, so |decoder_type| and the bound |init_cb| + // remain valid as well. video_decoder_->Initialize( config, /*low_delay=*/true, /*cdm_context=*/nullptr, - base::BindOnce( - [](base::OnceCallback cb, - media::VideoDecoderType* decoder_type, - media::VideoDecoder* video_decoder, media::DecoderStatus status) { - *decoder_type = video_decoder->GetDecoderType(); - std::move(cb).Run(status.is_ok()); - }, - ConvertToBaseOnceCallback(std::move(init_cb)), - CrossThreadUnretained(decoder_type), - CrossThreadUnretained(video_decoder_.get())), + base::BindOnce(&RTCVideoDecoderAdapter::Impl::OnInitializeDone, + weak_decoder_this_, + ConvertToBaseOnceCallback(std::move(init_cb)), + CrossThreadUnretained(decoder_type)), output_cb, base::DoNothing()); } +void RTCVideoDecoderAdapter::Impl::OnInitializeDone( + base::OnceCallback init_cb, + media::VideoDecoderType* decoder_type, + media::DecoderStatus status) { + DCHECK_CALLED_ON_VALID_SEQUENCE(media_sequence_checker_); + *decoder_type = video_decoder_->GetDecoderType(); + std::move(init_cb).Run(status.is_ok()); +} + void RTCVideoDecoderAdapter::Impl::Decode( scoped_refptr buffer, base::WaitableEvent* waiter, @@ -629,7 +645,7 @@ bool RTCVideoDecoderAdapter::InitializeSync( start_time, CrossThreadUnretained(&decoder_type_)))) { // TODO(crbug.com/1076817) Remove if a root cause is found. - if (!async_init_waiter_->TimedWait(base::Seconds(10))) { + if (!async_init_waiter_->TimedWait(g_initialization_timeout)) { RecordInitializationLatency(base::TimeTicks::Now() - start_time); return false; } @@ -946,4 +962,9 @@ void RTCVideoDecoderAdapter::DecrementCurrentDecoderCountForTesting() { g_num_decoders_--; } +void RTCVideoDecoderAdapter::SetInitializationTimeoutForTesting( + base::TimeDelta timeout) { + g_initialization_timeout = timeout; +} + } // namespace blink diff --git a/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.h b/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.h index f2ddb9888a62d..3a4f192c7d4da 100644 --- a/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.h +++ b/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter.h @@ -13,6 +13,7 @@ #include "base/memory/weak_ptr.h" #include "base/sequence_checker.h" #include "base/synchronization/waitable_event.h" +#include "base/time/time.h" #include "build/build_config.h" #include "media/base/decoder.h" #include "media/base/video_decoder_config.h" @@ -91,6 +92,7 @@ class PLATFORM_EXPORT RTCVideoDecoderAdapter : public webrtc::VideoDecoder { static int GetCurrentDecoderCountForTesting(); static void IncrementCurrentDecoderCountForTesting(); static void DecrementCurrentDecoderCountForTesting(); + static void SetInitializationTimeoutForTesting(base::TimeDelta timeout); static std::atomic g_num_decoders_; diff --git a/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter_test.cc b/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter_test.cc index 5bc83ddd88164..cec811513edf7 100644 --- a/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter_test.cc +++ b/third_party/blink/renderer/platform/peerconnection/rtc_video_decoder_adapter_test.cc @@ -21,6 +21,7 @@ #include "base/test/mock_callback.h" #include "base/test/scoped_feature_list.h" #include "base/test/task_environment.h" +#include "base/threading/platform_thread.h" #include "base/threading/thread.h" #include "base/time/time.h" #include "build/build_config.h" @@ -544,6 +545,39 @@ TEST_F(RTCVideoDecoderAdapterTest, InitializationFailure) { ASSERT_FALSE(CreateAndInitialize(false)); } +TEST_F(RTCVideoDecoderAdapterTest, CreateFailsCleanlyOnInitializeTimeout) { + // Some media::VideoDecoder implementations post their init callback back to + // the media task runner rather than invoking it synchronously. If the media + // thread is backlogged enough for InitializeSync() to time out, that posted + // callback can run after the adapter has already been torn down via + // Release(). Verify the adapter handles this without touching freed state. + RTCVideoDecoderAdapter::SetInitializationTimeoutForTesting( + base::Milliseconds(100)); + + EXPECT_CALL(*video_decoder_, Initialize_(_, _, _, _, _, _)) + .WillOnce([](const media::VideoDecoderConfig&, bool, media::CdmContext*, + media::VideoDecoder::InitCB& init_cb, + const media::VideoDecoder::OutputCB&, + const media::WaitingCB&) { + base::SequencedTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, base::BindOnce(std::move(init_cb), + media::DecoderStatus::Codes::kFailed)); + }); + + // Stall the media thread so InitializeSync()'s timed wait expires before the + // posted Initialize task runs. + media_thread_.task_runner()->PostTask( + FROM_HERE, base::BindOnce(&base::PlatformThread::Sleep, + base::Milliseconds(500))); + + EXPECT_FALSE(RTCVideoDecoderAdapterWrapper::Create( + &gpu_factories_, webrtc::SdpVideoFormat("VP9"), + /*pass_resolution_monitor=*/true)); + media_thread_.FlushForTesting(); + + RTCVideoDecoderAdapter::SetInitializationTimeoutForTesting(base::Seconds(10)); +} + TEST_F(RTCVideoDecoderAdapterTest, Decode) { ASSERT_TRUE(BasicSetup());