diff --git a/fuchsia/engine/browser/frame_impl.h b/fuchsia/engine/browser/frame_impl.h index 38b5f8871464fc..ea50835c2f8631 100644 --- a/fuchsia/engine/browser/frame_impl.h +++ b/fuchsia/engine/browser/frame_impl.h @@ -84,7 +84,9 @@ class FrameImpl : public fuchsia::web::Frame, FrameImpl(const FrameImpl&) = delete; FrameImpl& operator=(const FrameImpl&) = delete; - uint64_t media_session_id() const { return media_session_id_; } + absl::optional media_session_id() const { + return media_session_id_; + } FramePermissionController* permission_controller() { return &permission_controller_; @@ -360,7 +362,7 @@ class FrameImpl : public fuchsia::web::Frame, // Session ID to use for fuchsia.media.AudioConsumer. Set with // SetMediaSessionId(). - uint64_t media_session_id_ = 0; + absl::optional media_session_id_; // Stored settings for web contents in the current Frame. fuchsia::web::ContentAreaSettings content_area_settings_; diff --git a/fuchsia/engine/browser/fuchsia_media_resource_provider_impl.cc b/fuchsia/engine/browser/fuchsia_media_resource_provider_impl.cc index 35db0e5ffe0184..11cd2491337eb3 100644 --- a/fuchsia/engine/browser/fuchsia_media_resource_provider_impl.cc +++ b/fuchsia/engine/browser/fuchsia_media_resource_provider_impl.cc @@ -32,6 +32,13 @@ FuchsiaMediaResourceProviderImpl::FuchsiaMediaResourceProviderImpl( FuchsiaMediaResourceProviderImpl::~FuchsiaMediaResourceProviderImpl() = default; +void FuchsiaMediaResourceProviderImpl::ShouldUseAudioConsumer( + ShouldUseAudioConsumerCallback callback) { + auto* frame_impl = FrameImpl::FromRenderFrameHost(render_frame_host()); + DCHECK(frame_impl); + std::move(callback).Run(frame_impl->media_session_id().has_value()); +} + void FuchsiaMediaResourceProviderImpl::CreateAudioConsumer( fidl::InterfaceRequest request) { if (base::CommandLine::ForCurrentProcess()->HasSwitch( @@ -47,7 +54,14 @@ void FuchsiaMediaResourceProviderImpl::CreateAudioConsumer( ->Connect(); auto* frame_impl = FrameImpl::FromRenderFrameHost(render_frame_host()); DCHECK(frame_impl); - factory->CreateAudioConsumer(frame_impl->media_session_id(), + + if (!frame_impl->media_session_id().has_value()) { + LOG(WARNING) << "Renderer tried creating AudioConsumer for a Frame without " + "media_session_id()."; + return; + } + + factory->CreateAudioConsumer(frame_impl->media_session_id().value(), std::move(request)); } diff --git a/fuchsia/engine/browser/fuchsia_media_resource_provider_impl.h b/fuchsia/engine/browser/fuchsia_media_resource_provider_impl.h index 68459cf1a0b4a3..fb23f17120f799 100644 --- a/fuchsia/engine/browser/fuchsia_media_resource_provider_impl.h +++ b/fuchsia/engine/browser/fuchsia_media_resource_provider_impl.h @@ -41,6 +41,7 @@ class FuchsiaMediaResourceProviderImpl final receiver); // media::mojom::FuchsiaMediaResourceProvider: + void ShouldUseAudioConsumer(ShouldUseAudioConsumerCallback callback) override; void CreateAudioConsumer( fidl::InterfaceRequest request) override; void CreateAudioCapturer( diff --git a/fuchsia/engine/renderer/web_engine_audio_device_factory.cc b/fuchsia/engine/renderer/web_engine_audio_device_factory.cc index c88b59949e25a5..d624e2778a42ff 100644 --- a/fuchsia/engine/renderer/web_engine_audio_device_factory.cc +++ b/fuchsia/engine/renderer/web_engine_audio_device_factory.cc @@ -62,10 +62,6 @@ WebEngineAudioDeviceFactory::CreateAudioRendererSink( // Return nullptr for WebRTC streams. This will cause the caller to // fallback to AudioOutputDevice, which outputs through // AudioOutputStreamFuchsia. - // - // TODO(crbug.com/1066203): Make sure FuchsiaAudioOutputDevice doesn't - // increase latency (or degrade quality otherwise) and then switch to - // using FuchsiaAudioOutputDevice for WebRTC. return nullptr; // kNone is used in AudioDeviceFactory::GetOutputDeviceInfo() to get @@ -90,6 +86,13 @@ WebEngineAudioDeviceFactory::CreateAudioRendererSink( render_frame->GetBrowserInterfaceBroker()->GetInterface( media_resource_provider.BindNewPipeAndPassReceiver()); + // If AudioConsumer is not enabled then fallback to AudioOutputDevice. + bool use_audio_consumer = false; + if (!media_resource_provider->ShouldUseAudioConsumer(&use_audio_consumer) || + !use_audio_consumer) { + return nullptr; + } + // AudioConsumer can be used only to output to the default device. if (!params.device_id.empty()) return nullptr; diff --git a/fuchsia/engine/renderer/web_engine_content_renderer_client.cc b/fuchsia/engine/renderer/web_engine_content_renderer_client.cc index e22dedad66be21..f06f33f91b1768 100644 --- a/fuchsia/engine/renderer/web_engine_content_renderer_client.cc +++ b/fuchsia/engine/renderer/web_engine_content_renderer_client.cc @@ -25,6 +25,7 @@ #include "services/network/public/cpp/features.h" #include "services/service_manager/public/cpp/binder_registry.h" #include "third_party/blink/public/common/associated_interfaces/associated_interface_registry.h" +#include "third_party/blink/public/common/browser_interface_broker_proxy.h" #include "third_party/blink/public/web/web_view.h" #include "third_party/widevine/cdm/widevine_cdm_common.h" @@ -271,9 +272,22 @@ WebEngineContentRendererClient::GetBaseRendererFactory( media::DecoderFactory* decoder_factory, base::RepeatingCallback get_gpu_factories_cb) { + auto* interface_broker = render_frame->GetBrowserInterfaceBroker(); + + mojo::Remote + media_resource_provider; + interface_broker->GetInterface( + media_resource_provider.BindNewPipeAndPassReceiver()); + + bool use_audio_consumer = false; + if (!media_resource_provider->ShouldUseAudioConsumer(&use_audio_consumer) || + !use_audio_consumer) { + return nullptr; + } + return std::make_unique( media_log, decoder_factory, std::move(get_gpu_factories_cb), - render_frame->GetBrowserInterfaceBroker()); + std::move(media_resource_provider)); } bool WebEngineContentRendererClient::RunClosureWhenInForeground( diff --git a/fuchsia/engine/renderer/web_engine_media_renderer_factory.cc b/fuchsia/engine/renderer/web_engine_media_renderer_factory.cc index 2db54fb062e92d..4b3b62c47432ce 100644 --- a/fuchsia/engine/renderer/web_engine_media_renderer_factory.cc +++ b/fuchsia/engine/renderer/web_engine_media_renderer_factory.cc @@ -11,24 +11,21 @@ #include "base/bind.h" #include "fuchsia/engine/renderer/web_engine_audio_renderer.h" #include "media/base/decoder_factory.h" -#include "media/fuchsia/mojom/fuchsia_media_resource_provider.mojom.h" #include "media/renderers/renderer_impl.h" #include "media/renderers/video_renderer_impl.h" #include "media/video/gpu_memory_buffer_video_frame_pool.h" #include "media/video/gpu_video_accelerator_factories.h" -#include "mojo/public/cpp/bindings/pending_remote.h" -#include "mojo/public/cpp/bindings/remote.h" -#include "third_party/blink/public/common/browser_interface_broker_proxy.h" WebEngineMediaRendererFactory::WebEngineMediaRendererFactory( media::MediaLog* media_log, media::DecoderFactory* decoder_factory, GetGpuFactoriesCB get_gpu_factories_cb, - blink::BrowserInterfaceBrokerProxy* interface_broker) + mojo::Remote + media_resource_provider) : media_log_(media_log), decoder_factory_(decoder_factory), get_gpu_factories_cb_(std::move(get_gpu_factories_cb)), - interface_broker_(interface_broker) { + media_resource_provider_(std::move(media_resource_provider)) { DCHECK(decoder_factory_); } @@ -54,16 +51,8 @@ std::unique_ptr WebEngineMediaRendererFactory::CreateRenderer( media::VideoRendererSink* video_renderer_sink, media::RequestOverlayInfoCB request_overlay_info_cb, const gfx::ColorSpace& target_color_space) { - mojo::PendingRemote - media_resource_provider; - interface_broker_->GetInterface( - media_resource_provider.InitWithNewPipeAndPassReceiver()); - - mojo::Remote - remote_media_resource_provider; - remote_media_resource_provider.Bind(std::move(media_resource_provider)); fidl::InterfaceHandle audio_consumer_handle; - remote_media_resource_provider->CreateAudioConsumer( + media_resource_provider_->CreateAudioConsumer( audio_consumer_handle.NewRequest()); auto audio_renderer = std::make_unique( media_log_, std::move(audio_consumer_handle)); diff --git a/fuchsia/engine/renderer/web_engine_media_renderer_factory.h b/fuchsia/engine/renderer/web_engine_media_renderer_factory.h index c94c454fa05ece..886a9f24f570c4 100644 --- a/fuchsia/engine/renderer/web_engine_media_renderer_factory.h +++ b/fuchsia/engine/renderer/web_engine_media_renderer_factory.h @@ -10,10 +10,8 @@ #include "base/callback.h" #include "media/base/renderer_factory.h" - -namespace blink { -class BrowserInterfaceBrokerProxy; -} // namespace blink +#include "media/fuchsia/mojom/fuchsia_media_resource_provider.mojom.h" +#include "mojo/public/cpp/bindings/remote.h" namespace media { class AudioRendererSink; @@ -35,7 +33,8 @@ class WebEngineMediaRendererFactory final : public media::RendererFactory { media::MediaLog* media_log, media::DecoderFactory* decoder_factory, GetGpuFactoriesCB get_gpu_factories_cb, - blink::BrowserInterfaceBrokerProxy* interface_broker); + mojo::Remote + media_resource_provider); ~WebEngineMediaRendererFactory() override; // RendererFactory interface. @@ -63,7 +62,8 @@ class WebEngineMediaRendererFactory final : public media::RendererFactory { // Creates factories for supporting video accelerators. May be null. GetGpuFactoriesCB get_gpu_factories_cb_; - blink::BrowserInterfaceBrokerProxy* const interface_broker_; + mojo::Remote + media_resource_provider_; }; #endif // FUCHSIA_ENGINE_RENDERER_WEB_ENGINE_MEDIA_RENDERER_FACTORY_H_ diff --git a/fuchsia/engine/web_engine_integration_test.cc b/fuchsia/engine/web_engine_integration_test.cc index 91c46809cbdb0a..8a3933f1724050 100644 --- a/fuchsia/engine/web_engine_integration_test.cc +++ b/fuchsia/engine/web_engine_integration_test.cc @@ -347,7 +347,7 @@ TEST_F(WebEngineIntegrationTest, ContentDirectoryProvider) { TEST_F(WebEngineIntegrationMediaTest, PlayAudio) { CreateContextAndFrame(ContextParamsWithAudioAndTestData()); - static uint16_t kTestMediaSessionId = 43; + static const uint16_t kTestMediaSessionId = 43; frame_->SetMediaSessionId(kTestMediaSessionId); ASSERT_NO_FATAL_FAILURE(LoadUrlAndExpectResponse( @@ -388,6 +388,9 @@ TEST_F(WebEngineIntegrationMediaTest, PlayAudio_NoFlag) { [&is_requested](auto request) { is_requested = true; })); ZX_CHECK(status == ZX_OK, status) << "AddPublicService"; + static const uint16_t kTestMediaSessionId = 1; + frame_->SetMediaSessionId(kTestMediaSessionId); + ASSERT_NO_FATAL_FAILURE(LoadUrlAndExpectResponse( "fuchsia-dir://testdata/play_audio.html", cr_fuchsia::CreateLoadUrlParamsWithUserActivation())); @@ -399,6 +402,9 @@ TEST_F(WebEngineIntegrationMediaTest, PlayAudio_NoFlag) { TEST_F(WebEngineIntegrationMediaTest, PlayVideo) { CreateContextAndFrame(ContextParamsWithAudioAndTestData()); + static const uint16_t kTestMediaSessionId = 1; + frame_->SetMediaSessionId(kTestMediaSessionId); + ASSERT_NO_FATAL_FAILURE(LoadUrlAndExpectResponse( kAutoplayVp9OpusToEndUrl, cr_fuchsia::CreateLoadUrlParamsWithUserActivation())); @@ -609,6 +615,9 @@ TEST_F(MAYBE_VulkanWebEngineIntegrationTest, fuchsia::web::ContextFeatureFlags::AUDIO); CreateContextAndFrame(std::move(create_params)); + static const uint16_t kTestMediaSessionId = 1; + frame_->SetMediaSessionId(kTestMediaSessionId); + ASSERT_NO_FATAL_FAILURE(LoadUrlAndExpectResponse( kAutoplayVp9OpusToEndUrl, cr_fuchsia::CreateLoadUrlParamsWithUserActivation())); @@ -630,6 +639,9 @@ TEST_F(WebEngineIntegrationMediaTest, HardwareVideoDecoderFlag_NotProvided) { ContextParamsWithAudioAndTestData(); CreateContextAndFrame(std::move(create_params)); + static const uint16_t kTestMediaSessionId = 1; + frame_->SetMediaSessionId(kTestMediaSessionId); + ASSERT_NO_FATAL_FAILURE(LoadUrlAndExpectResponse( kAutoplayVp9OpusToEndUrl, cr_fuchsia::CreateLoadUrlParamsWithUserActivation())); diff --git a/media/fuchsia/mojom/fuchsia_media_resource_provider.mojom b/media/fuchsia/mojom/fuchsia_media_resource_provider.mojom index 596727bf9016e1..95449dd3d6b2ad 100644 --- a/media/fuchsia/mojom/fuchsia_media_resource_provider.mojom +++ b/media/fuchsia/mojom/fuchsia_media_resource_provider.mojom @@ -17,6 +17,12 @@ struct AudioCapturerRequest { // Interface used by the render to create media resources. Instances are // document-scoped. interface FuchsiaMediaResourceProvider { + // Returns true if the frame should use AudioConsumer to render audio + // streams. The call needs to be synchronous because `media::Renderer` needs + // to be constructed synchronously. + [Sync] + ShouldUseAudioConsumer() => (bool result); + // Creates a fuchsia.media.AudioConsumer for the current frame. CreateAudioConsumer(AudioConsumerRequest request);