From e6d8eefdc90fef61dcc2cfd41d6b9e041d9bc771 Mon Sep 17 00:00:00 2001 From: Austin Osagie Date: Mon, 9 Dec 2024 15:08:54 -0800 Subject: [PATCH 1/6] Add BASE_FEATURE flag for progressive playback Progressive playback support can be disabled at runtime by passing the argument "--disable-features=CobaltProgressivePlayback". Progressive playback support remains enabled by default. This change only affects Cobalt Starboard Media builds. b/382791540 --- media/base/media_switches.cc | 7 +++ media/base/media_switches.h | 3 ++ media/filters/demuxer_manager.cc | 11 ++++- .../core/html/media/html_media_element.cc | 49 ++++++++++++++----- 4 files changed, 56 insertions(+), 14 deletions(-) diff --git a/media/base/media_switches.cc b/media/base/media_switches.cc index b8f748b18365..8541027f992e 100644 --- a/media/base/media_switches.cc +++ b/media/base/media_switches.cc @@ -481,6 +481,13 @@ const base::FeatureParam kDecreaseProcessingAudioFifoSizeValue{ #endif +#if BUILDFLAG(USE_STARBOARD_MEDIA) +// When set, Cobalt rejects progressive video formats. +BASE_FEATURE(kCobaltProgressivePlayback, + "CobaltProgressivePlayback", + base::FEATURE_ENABLED_BY_DEFAULT); +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + #if BUILDFLAG(IS_CHROMEOS) // To control running audio communication effect on Chrome OS Audio Server. BASE_FEATURE(kCrOSSystemAEC, diff --git a/media/base/media_switches.h b/media/base/media_switches.h index 43265d6c016d..376f1fd3e46c 100644 --- a/media/base/media_switches.h +++ b/media/base/media_switches.h @@ -176,6 +176,9 @@ MEDIA_EXPORT BASE_DECLARE_FEATURE(kDecreaseProcessingAudioFifoSize); MEDIA_EXPORT extern const base::FeatureParam kDecreaseProcessingAudioFifoSizeValue; #endif +#if BUILDFLAG(USE_STARBOARD_MEDIA) +MEDIA_EXPORT BASE_DECLARE_FEATURE(kCobaltProgressivePlayback); +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) #if BUILDFLAG(IS_CHROMEOS) MEDIA_EXPORT BASE_DECLARE_FEATURE(kCrOSSystemAEC); MEDIA_EXPORT BASE_DECLARE_FEATURE(kCrOSSystemAECDeactivatedGroups); diff --git a/media/filters/demuxer_manager.cc b/media/filters/demuxer_manager.cc index b5659f604991..eef2ee1e4ebb 100644 --- a/media/filters/demuxer_manager.cc +++ b/media/filters/demuxer_manager.cc @@ -26,7 +26,10 @@ #endif // BUILDFLAG(ENABLE_HLS_DEMUXER) #if BUILDFLAG(ENABLE_FFMPEG) -// ProgressiveDemuxer is enabled when use_starboard_media=true and media_use_ffmpeg=false. +// The ProgressiveDemuxer is enabled when use_starboard_media=true and +// media_use_ffmpeg=false. This can be overridden by the BASE_FEATURE +// CobaltProgressivePlayback, which will disable the ProgressiveDemuxer +// when false. #elif BUILDFLAG(USE_STARBOARD_MEDIA) #include "media/starboard/progressive/demuxer_extension_wrapper.h" // nogncheck #include "media/starboard/progressive/progressive_demuxer.h" // nogncheck @@ -417,7 +420,11 @@ PipelineStatus DemuxerManager::CreateDemuxer( #if BUILDFLAG(ENABLE_FFMPEG) SetDemuxer(CreateFFmpegDemuxer()); #elif BUILDFLAG(USE_STARBOARD_MEDIA) - SetDemuxer(CreateProgressiveDemuxer()); + if (base::FeatureList::IsEnabled(media::kCobaltProgressivePlayback)) { + SetDemuxer(CreateProgressiveDemuxer()); + } else { + LOG(INFO) << "Cobalt progressive playback is disabled via base features."; + } #else return DEMUXER_ERROR_COULD_NOT_OPEN; #endif diff --git a/third_party/blink/renderer/core/html/media/html_media_element.cc b/third_party/blink/renderer/core/html/media/html_media_element.cc index 45de3858c816..d97540d14245 100644 --- a/third_party/blink/renderer/core/html/media/html_media_element.cc +++ b/third_party/blink/renderer/core/html/media/html_media_element.cc @@ -405,6 +405,23 @@ std::ostream& operator<<(std::ostream& stream, return stream << static_cast(&media_element); } +#if BUILDFLAG(USE_STARBOARD_MEDIA) +// Checks for progressive formats served by the YouTube H5 player. +// These formats have a mime type of "video/mp4", and lists both audio and +// video formats under the "codecs" parameter. +bool IsProgressiveFormat(const ContentType& content_type) { + String type = content_type.GetType(); + String codecs = content_type.Parameter("codecs"); + if (!type.empty() && !codecs.empty()) { + Vector split_codecs; + String separator(", "); + codecs.Split(separator, split_codecs); + return type.Utf8() == "video/mp4" && split_codecs.size() == 2; + } + return false; +} +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + } // anonymous namespace // static @@ -412,19 +429,27 @@ MIMETypeRegistry::SupportsType HTMLMediaElement::GetSupportsType( const ContentType& content_type) { #if BUILDFLAG(USE_STARBOARD_MEDIA) // Interupt Chromium's IsTypeSupported() from here for better performance. - SbMediaSupportType support_type = - SbMediaCanPlayMimeAndKeySystem(content_type.Raw().Ascii().c_str(), ""); MIMETypeRegistry::SupportsType result; - switch (support_type) { - case kSbMediaSupportTypeNotSupported: - result = MIMETypeRegistry::kNotSupported; - break; - case kSbMediaSupportTypeMaybe: - result = MIMETypeRegistry::kMaybeSupported; - break; - case kSbMediaSupportTypeProbably: - result = MIMETypeRegistry::kSupported; - break; + if (!base::FeatureList::IsEnabled(media::kCobaltProgressivePlayback) && + IsProgressiveFormat(content_type)) { + LOG(INFO) << "Content type \'" << content_type.Raw() + << "\' is unsupported as Cobalt progressive playback is disabled " + "via base features."; + result = MIMETypeRegistry::kNotSupported; + } else { + SbMediaSupportType support_type = + SbMediaCanPlayMimeAndKeySystem(content_type.Raw().Ascii().c_str(), ""); + switch (support_type) { + case kSbMediaSupportTypeNotSupported: + result = MIMETypeRegistry::kNotSupported; + break; + case kSbMediaSupportTypeMaybe: + result = MIMETypeRegistry::kMaybeSupported; + break; + case kSbMediaSupportTypeProbably: + result = MIMETypeRegistry::kSupported; + break; + } } LOG(INFO) << __func__ << "(" << content_type.Raw() << ") -> " << result; return result; From dc96748aaf9feaa11bbd04542b79446ba8a11ecf Mon Sep 17 00:00:00 2001 From: Austin Osagie Date: Tue, 10 Dec 2024 14:58:53 -0800 Subject: [PATCH 2/6] Add media element test --- media/base/media_switches.cc | 2 +- media/filters/demuxer_manager.cc | 6 ++- .../core/html/media/html_media_element.cc | 18 ++++---- .../html/media/html_media_element_test.cc | 43 +++++++++++++++++++ 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/media/base/media_switches.cc b/media/base/media_switches.cc index 8541027f992e..5f8373060239 100644 --- a/media/base/media_switches.cc +++ b/media/base/media_switches.cc @@ -482,7 +482,7 @@ const base::FeatureParam kDecreaseProcessingAudioFifoSizeValue{ #endif #if BUILDFLAG(USE_STARBOARD_MEDIA) -// When set, Cobalt rejects progressive video formats. +// When disabled, Cobalt rejects progressive video formats. BASE_FEATURE(kCobaltProgressivePlayback, "CobaltProgressivePlayback", base::FEATURE_ENABLED_BY_DEFAULT); diff --git a/media/filters/demuxer_manager.cc b/media/filters/demuxer_manager.cc index eef2ee1e4ebb..c5aaa2fba3f7 100644 --- a/media/filters/demuxer_manager.cc +++ b/media/filters/demuxer_manager.cc @@ -27,9 +27,10 @@ #if BUILDFLAG(ENABLE_FFMPEG) // The ProgressiveDemuxer is enabled when use_starboard_media=true and -// media_use_ffmpeg=false. This can be overridden by the BASE_FEATURE +// media_use_ffmpeg=false. Cobalt enables the ProgressiveDemuxer by +// default. This can disabled by the BASE_FEATURE // CobaltProgressivePlayback, which will disable the ProgressiveDemuxer -// when false. +// when disabled. #elif BUILDFLAG(USE_STARBOARD_MEDIA) #include "media/starboard/progressive/demuxer_extension_wrapper.h" // nogncheck #include "media/starboard/progressive/progressive_demuxer.h" // nogncheck @@ -424,6 +425,7 @@ PipelineStatus DemuxerManager::CreateDemuxer( SetDemuxer(CreateProgressiveDemuxer()); } else { LOG(INFO) << "Cobalt progressive playback is disabled via base features."; + return DEMUXER_ERROR_COULD_NOT_OPEN; } #else return DEMUXER_ERROR_COULD_NOT_OPEN; diff --git a/third_party/blink/renderer/core/html/media/html_media_element.cc b/third_party/blink/renderer/core/html/media/html_media_element.cc index d97540d14245..8bff6454ede0 100644 --- a/third_party/blink/renderer/core/html/media/html_media_element.cc +++ b/third_party/blink/renderer/core/html/media/html_media_element.cc @@ -410,15 +410,17 @@ std::ostream& operator<<(std::ostream& stream, // These formats have a mime type of "video/mp4", and lists both audio and // video formats under the "codecs" parameter. bool IsProgressiveFormat(const ContentType& content_type) { - String type = content_type.GetType(); - String codecs = content_type.Parameter("codecs"); - if (!type.empty() && !codecs.empty()) { - Vector split_codecs; - String separator(", "); - codecs.Split(separator, split_codecs); - return type.Utf8() == "video/mp4" && split_codecs.size() == 2; + const String type = content_type.GetType(); + const String codecs = content_type.Parameter("codecs"); + + if (type.empty() && codecs.empty()) { + return false; } - return false; + + Vector split_codecs; + const String separator(", "); + codecs.Split(separator, split_codecs); + return type.Utf8() == "video/mp4" && split_codecs.size() == 2; } #endif // BUILDFLAG(USE_STARBOARD_MEDIA) diff --git a/third_party/blink/renderer/core/html/media/html_media_element_test.cc b/third_party/blink/renderer/core/html/media/html_media_element_test.cc index 6881349d0588..11cfd43ddb89 100644 --- a/third_party/blink/renderer/core/html/media/html_media_element_test.cc +++ b/third_party/blink/renderer/core/html/media/html_media_element_test.cc @@ -35,6 +35,12 @@ #include "third_party/blink/renderer/platform/wtf/vector.h" #include "ui/gfx/geometry/size.h" +#if BUILDFLAG(USE_STARBOARD_MEDIA) +#include "base/test/scoped_feature_list.h" +#include "third_party/blink/renderer/platform/network/mime/content_type.h" +#include "third_party/blink/renderer/platform/network/mime/mime_type_registry.h" +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + using ::testing::_; using ::testing::AnyNumber; using ::testing::NanSensitiveDoubleEq; @@ -1568,4 +1574,41 @@ TEST_P(HTMLMediaElementTest, CanFreezeWithMediaPlayerAttached) { EXPECT_FALSE(MediaIsPlaying()); } +#if BUILDFLAG(USE_STARBOARD_MEDIA) +TEST(HTMLMediaElementTest, CanHandleCobaltProgressiveSupportQueries) { + ContentType progressive_type("video/mp4; codecs=\"avc1.42001E, mp4a.40.2\""); + ContentType adaptive_video_type( + "video/mp4; codecs=\"avc1.4d4015\"; framerate=30"); + ContentType adaptive_audio_type( + "audio/mp4; codecs=\"mp4a.40.2\"; channels=2"); + + { + base::test::ScopedFeatureList scoped_list; + scoped_list.InitAndDisableFeature(media::kCobaltProgressivePlayback); + // Reject progressive content types when CobaltProgressivePlayback is + // disabled. + EXPECT_EQ(HTMLMediaElement::GetSupportsType(progressive_type), + MIMETypeRegistry::kNotSupported); + // Continue to support adaptive content types. + EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_video_type), + MIMETypeRegistry::kNotSupported); + EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_audio_type), + MIMETypeRegistry::kNotSupported); + } + { + base::test::ScopedFeatureList scoped_list( + media::kCobaltProgressivePlayback); + // Accept progressive content types when CobaltProgressivePlayback is + // enabled. + EXPECT_NE(HTMLMediaElement::GetSupportsType(progressive_type), + MIMETypeRegistry::kNotSupported); + // Continue to support adaptive content types. + EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_video_type), + MIMETypeRegistry::kNotSupported); + EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_audio_type), + MIMETypeRegistry::kNotSupported); + } +} +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + } // namespace blink From 90e71eaa581fcff1df8c51e09d0543773d0881bf Mon Sep 17 00:00:00 2001 From: Austin Osagie Date: Tue, 10 Dec 2024 15:25:13 -0800 Subject: [PATCH 3/6] Change comment --- media/filters/demuxer_manager.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/media/filters/demuxer_manager.cc b/media/filters/demuxer_manager.cc index c5aaa2fba3f7..5178bf755546 100644 --- a/media/filters/demuxer_manager.cc +++ b/media/filters/demuxer_manager.cc @@ -29,8 +29,7 @@ // The ProgressiveDemuxer is enabled when use_starboard_media=true and // media_use_ffmpeg=false. Cobalt enables the ProgressiveDemuxer by // default. This can disabled by the BASE_FEATURE -// CobaltProgressivePlayback, which will disable the ProgressiveDemuxer -// when disabled. +// CobaltProgressivePlayback at runtime. #elif BUILDFLAG(USE_STARBOARD_MEDIA) #include "media/starboard/progressive/demuxer_extension_wrapper.h" // nogncheck #include "media/starboard/progressive/progressive_demuxer.h" // nogncheck From 51d4d66ecc45474cb25d9b2d69a54caacc498661 Mon Sep 17 00:00:00 2001 From: Austin Osagie Date: Wed, 11 Dec 2024 09:21:21 -0800 Subject: [PATCH 4/6] Remove test --- .../html/media/html_media_element_test.cc | 43 ------------------- 1 file changed, 43 deletions(-) diff --git a/third_party/blink/renderer/core/html/media/html_media_element_test.cc b/third_party/blink/renderer/core/html/media/html_media_element_test.cc index 11cfd43ddb89..6881349d0588 100644 --- a/third_party/blink/renderer/core/html/media/html_media_element_test.cc +++ b/third_party/blink/renderer/core/html/media/html_media_element_test.cc @@ -35,12 +35,6 @@ #include "third_party/blink/renderer/platform/wtf/vector.h" #include "ui/gfx/geometry/size.h" -#if BUILDFLAG(USE_STARBOARD_MEDIA) -#include "base/test/scoped_feature_list.h" -#include "third_party/blink/renderer/platform/network/mime/content_type.h" -#include "third_party/blink/renderer/platform/network/mime/mime_type_registry.h" -#endif // BUILDFLAG(USE_STARBOARD_MEDIA) - using ::testing::_; using ::testing::AnyNumber; using ::testing::NanSensitiveDoubleEq; @@ -1574,41 +1568,4 @@ TEST_P(HTMLMediaElementTest, CanFreezeWithMediaPlayerAttached) { EXPECT_FALSE(MediaIsPlaying()); } -#if BUILDFLAG(USE_STARBOARD_MEDIA) -TEST(HTMLMediaElementTest, CanHandleCobaltProgressiveSupportQueries) { - ContentType progressive_type("video/mp4; codecs=\"avc1.42001E, mp4a.40.2\""); - ContentType adaptive_video_type( - "video/mp4; codecs=\"avc1.4d4015\"; framerate=30"); - ContentType adaptive_audio_type( - "audio/mp4; codecs=\"mp4a.40.2\"; channels=2"); - - { - base::test::ScopedFeatureList scoped_list; - scoped_list.InitAndDisableFeature(media::kCobaltProgressivePlayback); - // Reject progressive content types when CobaltProgressivePlayback is - // disabled. - EXPECT_EQ(HTMLMediaElement::GetSupportsType(progressive_type), - MIMETypeRegistry::kNotSupported); - // Continue to support adaptive content types. - EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_video_type), - MIMETypeRegistry::kNotSupported); - EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_audio_type), - MIMETypeRegistry::kNotSupported); - } - { - base::test::ScopedFeatureList scoped_list( - media::kCobaltProgressivePlayback); - // Accept progressive content types when CobaltProgressivePlayback is - // enabled. - EXPECT_NE(HTMLMediaElement::GetSupportsType(progressive_type), - MIMETypeRegistry::kNotSupported); - // Continue to support adaptive content types. - EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_video_type), - MIMETypeRegistry::kNotSupported); - EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_audio_type), - MIMETypeRegistry::kNotSupported); - } -} -#endif // BUILDFLAG(USE_STARBOARD_MEDIA) - } // namespace blink From dc3f57dc1978d77dcbe9e8274ca2144a2ba7065f Mon Sep 17 00:00:00 2001 From: Austin Osagie Date: Thu, 12 Dec 2024 11:36:33 -0800 Subject: [PATCH 5/6] Re add unit test --- .../core/html/media/html_media_element.cc | 4 +- .../html/media/html_media_element_test.cc | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/third_party/blink/renderer/core/html/media/html_media_element.cc b/third_party/blink/renderer/core/html/media/html_media_element.cc index 8bff6454ede0..0ecba68e421f 100644 --- a/third_party/blink/renderer/core/html/media/html_media_element.cc +++ b/third_party/blink/renderer/core/html/media/html_media_element.cc @@ -408,7 +408,7 @@ std::ostream& operator<<(std::ostream& stream, #if BUILDFLAG(USE_STARBOARD_MEDIA) // Checks for progressive formats served by the YouTube H5 player. // These formats have a mime type of "video/mp4", and lists both audio and -// video formats under the "codecs" parameter. +// video codecs under the "codecs" parameter. bool IsProgressiveFormat(const ContentType& content_type) { const String type = content_type.GetType(); const String codecs = content_type.Parameter("codecs"); @@ -439,7 +439,7 @@ MIMETypeRegistry::SupportsType HTMLMediaElement::GetSupportsType( "via base features."; result = MIMETypeRegistry::kNotSupported; } else { - SbMediaSupportType support_type = + const SbMediaSupportType support_type = SbMediaCanPlayMimeAndKeySystem(content_type.Raw().Ascii().c_str(), ""); switch (support_type) { case kSbMediaSupportTypeNotSupported: diff --git a/third_party/blink/renderer/core/html/media/html_media_element_test.cc b/third_party/blink/renderer/core/html/media/html_media_element_test.cc index 6881349d0588..81841cfae1d3 100644 --- a/third_party/blink/renderer/core/html/media/html_media_element_test.cc +++ b/third_party/blink/renderer/core/html/media/html_media_element_test.cc @@ -35,6 +35,12 @@ #include "third_party/blink/renderer/platform/wtf/vector.h" #include "ui/gfx/geometry/size.h" +#if BUILDFLAG(USE_STARBOARD_MEDIA) +#include "base/test/scoped_feature_list.h" +#include "third_party/blink/renderer/platform/network/mime/content_type.h" +#include "third_party/blink/renderer/platform/network/mime/mime_type_registry.h" +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + using ::testing::_; using ::testing::AnyNumber; using ::testing::NanSensitiveDoubleEq; @@ -1568,4 +1574,42 @@ TEST_P(HTMLMediaElementTest, CanFreezeWithMediaPlayerAttached) { EXPECT_FALSE(MediaIsPlaying()); } +#if BUILDFLAG(USE_STARBOARD_MEDIA) +TEST(HTMLMediaElementTest, CanHandleCobaltProgressiveSupportQueries) { + const ContentType progressive_type( + "video/mp4; codecs=\"avc1.42001E, mp4a.40.2\""); + const ContentType adaptive_video_type( + "video/mp4; codecs=\"avc1.4d4015\"; framerate=30"); + const ContentType adaptive_audio_type( + "audio/mp4; codecs=\"mp4a.40.2\"; channels=2"); + + { + base::test::ScopedFeatureList scoped_list; + scoped_list.InitAndDisableFeature(media::kCobaltProgressivePlayback); + // Reject progressive content types when CobaltProgressivePlayback is + // disabled. + EXPECT_EQ(HTMLMediaElement::GetSupportsType(progressive_type), + MIMETypeRegistry::kNotSupported); + // Continue to support adaptive content types. + EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_video_type), + MIMETypeRegistry::kNotSupported); + EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_audio_type), + MIMETypeRegistry::kNotSupported); + } + { + base::test::ScopedFeatureList scoped_list( + media::kCobaltProgressivePlayback); + // Accept progressive content types when CobaltProgressivePlayback is + // enabled. + EXPECT_NE(HTMLMediaElement::GetSupportsType(progressive_type), + MIMETypeRegistry::kNotSupported); + // Continue to support adaptive content types. + EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_video_type), + MIMETypeRegistry::kNotSupported); + EXPECT_NE(HTMLMediaElement::GetSupportsType(adaptive_audio_type), + MIMETypeRegistry::kNotSupported); + } +} +#endif // BUILDFLAG(USE_STARBOARD_MEDIA) + } // namespace blink From 573c11cab148290b21e9be0d96ed31045773e05c Mon Sep 17 00:00:00 2001 From: Austin Osagie Date: Wed, 18 Dec 2024 15:07:05 -0800 Subject: [PATCH 6/6] Disable progressive support by default --- media/base/media_switches.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/media/base/media_switches.cc b/media/base/media_switches.cc index 5f8373060239..16a035ca7921 100644 --- a/media/base/media_switches.cc +++ b/media/base/media_switches.cc @@ -485,7 +485,7 @@ const base::FeatureParam kDecreaseProcessingAudioFifoSizeValue{ // When disabled, Cobalt rejects progressive video formats. BASE_FEATURE(kCobaltProgressivePlayback, "CobaltProgressivePlayback", - base::FEATURE_ENABLED_BY_DEFAULT); + base::FEATURE_DISABLED_BY_DEFAULT); #endif // BUILDFLAG(USE_STARBOARD_MEDIA) #if BUILDFLAG(IS_CHROMEOS)