Commit 0d3ebf3b authored by Xiaohan Wang's avatar Xiaohan Wang Committed by Commit Bot

media: Fix MediaFactory renderer factory selection logic

Currently when |use_media_player| is true, we'll add
FactoryType::kMediaPlayer as the base factory type. But we didn't set
|use_default_renderer_factory| to be false, so later we'll add
FactoryType::kDefault as the base factory type again, which overrides
the FactoryType::kMediaPlayer one we just added.

This CL makes sure we set |use_default_renderer_factory| to be false so
that this bug doesn't happen.

This function is still hard to read and error prone. In the future we
should improve this function further.

Bug: 1040456
Test: Manually tested.
Change-Id: Iade8a45426b3ff9835bb2b6cbc415f4df8a0cf23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2005090Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarThomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732665}
parent 97bd456d
...@@ -466,6 +466,7 @@ MediaFactory::CreateRendererFactorySelector( ...@@ -466,6 +466,7 @@ MediaFactory::CreateRendererFactorySelector(
return nullptr; return nullptr;
auto factory_selector = std::make_unique<media::RendererFactorySelector>(); auto factory_selector = std::make_unique<media::RendererFactorySelector>();
bool use_default_renderer_factory = true;
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
DCHECK(interface_broker_); DCHECK(interface_broker_);
...@@ -483,6 +484,7 @@ MediaFactory::CreateRendererFactorySelector( ...@@ -483,6 +484,7 @@ MediaFactory::CreateRendererFactorySelector(
if (use_media_player) { if (use_media_player) {
factory_selector->AddBaseFactory(FactoryType::kMediaPlayer, factory_selector->AddBaseFactory(FactoryType::kMediaPlayer,
std::move(media_player_factory)); std::move(media_player_factory));
use_default_renderer_factory = false;
} else { } else {
// Always give |factory_selector| a MediaPlayerRendererClient factory. WMPI // Always give |factory_selector| a MediaPlayerRendererClient factory. WMPI
// might fallback to it if the final redirected URL is an HLS url. // might fallback to it if the final redirected URL is an HLS url.
...@@ -509,8 +511,8 @@ MediaFactory::CreateRendererFactorySelector( ...@@ -509,8 +511,8 @@ MediaFactory::CreateRendererFactorySelector(
FactoryType::kFlinging, std::move(flinging_factory), is_flinging_cb); FactoryType::kFlinging, std::move(flinging_factory), is_flinging_cb);
#endif // defined(OS_ANDROID) #endif // defined(OS_ANDROID)
bool use_default_renderer_factory = true;
#if BUILDFLAG(ENABLE_MOJO_RENDERER) #if BUILDFLAG(ENABLE_MOJO_RENDERER)
DCHECK(!use_media_player);
if (enable_mojo_renderer) { if (enable_mojo_renderer) {
use_default_renderer_factory = false; use_default_renderer_factory = false;
#if BUILDFLAG(ENABLE_CAST_RENDERER) #if BUILDFLAG(ENABLE_CAST_RENDERER)
......
...@@ -1113,6 +1113,10 @@ void PipelineImpl::RendererWrapper::ReportMetadata(StartType start_type) { ...@@ -1113,6 +1113,10 @@ void PipelineImpl::RendererWrapper::ReportMetadata(StartType start_type) {
} }
bool PipelineImpl::RendererWrapper::HasEncryptedStream() { bool PipelineImpl::RendererWrapper::HasEncryptedStream() {
// Encrypted streams are only handled explicitly for STREAM type.
if (demuxer_->GetType() != MediaResource::Type::STREAM)
return false;
auto streams = demuxer_->GetAllStreams(); auto streams = demuxer_->GetAllStreams();
for (auto* stream : streams) { for (auto* stream : streams) {
......
...@@ -15,6 +15,7 @@ RendererFactorySelector::~RendererFactorySelector() = default; ...@@ -15,6 +15,7 @@ RendererFactorySelector::~RendererFactorySelector() = default;
void RendererFactorySelector::AddBaseFactory( void RendererFactorySelector::AddBaseFactory(
RendererFactoryType type, RendererFactoryType type,
std::unique_ptr<RendererFactory> factory) { std::unique_ptr<RendererFactory> factory) {
DVLOG(1) << __func__ << ": type=" << static_cast<int>(type);
DCHECK(!base_factory_type_) << "At most one base factory!"; DCHECK(!base_factory_type_) << "At most one base factory!";
AddFactory(type, std::move(factory)); AddFactory(type, std::move(factory));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment