Skip to content

Commit

Permalink
Introduce new logic for HTMLMediaElement.buffered (#3159)
Browse files Browse the repository at this point in the history
This new logic is aligned with upstream Chromium, and with the [HTML
spec](https://html.spec.whatwg.org/#dom-media-buffered):

*
[HTMLMediaElement::BufferedInternal](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/media/html_media_element.cc;l=3769-3772;drc=eb7475582501e654b3c0bb94be9fdbadfd6b83a0;bpv=0;bpt=0)
*
[MediaSource::SeekableInternal](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/mediasource/media_source.cc;l=882-883;drc=d911cdd7a76cdf8895915eae6b88e3bff02a3072;bpv=1;bpt=0)

Because this is a potentially disruptive change, the code is gated
behind a new H5VCC flag. Use
`"MediaElement.EnableUsingMediaSourceBufferedRange"` to enable this
change.

b/338452286

Change-Id: I52052b2fc5a2c0a1a1c92552434cf53f3bf407e2
  • Loading branch information
at-ninja authored May 7, 2024
1 parent 5327e5f commit a348ae2
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 1 deletion.
32 changes: 32 additions & 0 deletions cobalt/dom/html_media_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,27 @@ bool OriginIsSafe(loader::RequestMode request_mode, const GURL& resource_url,
return false;
}

const MediaSettings& GetMediaSettings(
const web::EnvironmentSettings* settings) {
DCHECK(settings);
DCHECK(settings->context());
DCHECK(settings->context()->web_settings());

const auto& web_settings = settings->context()->web_settings();
return web_settings->media_settings();
}

// If this function returns true, HTMLMediaElement::buffered() will attempt to
// call MediaSource::GetBufferedRange() if available, and fallback to
// WebMediaPlayer::UpdateBufferedTimeRanges().
// The default value is false.
bool IsMediaElementUsingMediaSourceBufferedRangeEnabled(
const web::EnvironmentSettings* settings) {
return GetMediaSettings(settings)
.IsMediaElementUsingMediaSourceBufferedRangeEnabled()
.value_or(false);
}

} // namespace

HTMLMediaElement::HTMLMediaElement(Document* document,
Expand Down Expand Up @@ -218,6 +239,17 @@ uint16_t HTMLMediaElement::network_state() const {
scoped_refptr<TimeRanges> HTMLMediaElement::buffered() const {
scoped_refptr<TimeRanges> buffered = new TimeRanges;

DCHECK(node_document());
DCHECK(node_document()->html_element_context());
DCHECK(node_document()->html_element_context()->environment_settings());
const auto* settings =
node_document()->html_element_context()->environment_settings();
if (IsMediaElementUsingMediaSourceBufferedRangeEnabled(settings)) {
if (media_source_) {
return media_source_->GetBufferedRange();
}
}

if (!player_) {
LOG(INFO) << "(empty)";
return buffered;
Expand Down
6 changes: 6 additions & 0 deletions cobalt/dom/media_settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ bool MediaSettingsImpl::Set(const std::string& name, int value) {
LOG(INFO) << name << ": set to " << value;
return true;
}
} else if (name == "MediaElement.EnableUsingMediaSourceBufferedRange") {
if (value == 0 || value == 1) {
is_media_element_using_media_source_buffered_range_enabled_ = value != 0;
LOG(INFO) << name << ": set to " << value;
return true;
}
} else {
LOG(WARNING) << "Ignore unknown setting with name \"" << name << "\"";
return false;
Expand Down
9 changes: 9 additions & 0 deletions cobalt/dom/media_settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ class MediaSettings {
virtual base::Optional<int>
GetMediaElementTimeupdateEventIntervalInMilliseconds() const = 0;
virtual base::Optional<bool> IsPaintingVideoBackgroundToBlack() const = 0;
virtual base::Optional<bool>
IsMediaElementUsingMediaSourceBufferedRangeEnabled() const = 0;

protected:
MediaSettings() = default;
Expand Down Expand Up @@ -93,6 +95,11 @@ class MediaSettingsImpl : public MediaSettings {
base::Optional<bool> IsPaintingVideoBackgroundToBlack() const override {
return is_painting_video_background_to_black_;
}
base::Optional<bool> IsMediaElementUsingMediaSourceBufferedRangeEnabled()
const override {
base::AutoLock auto_lock(lock_);
return is_media_element_using_media_source_buffered_range_enabled_;
}

// Returns true when the setting associated with `name` is set to `value`.
// Returns false when `name` is not associated with any settings, or if
Expand All @@ -110,6 +117,8 @@ class MediaSettingsImpl : public MediaSettings {
base::Optional<int> max_source_buffer_append_size_in_bytes_;

base::Optional<int> media_element_timeupdate_event_interval_in_milliseconds_;
base::Optional<bool>
is_media_element_using_media_source_buffered_range_enabled_;

base::Optional<bool> is_painting_video_background_to_black_;
};
Expand Down
13 changes: 13 additions & 0 deletions cobalt/dom/media_settings_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ TEST(MediaSettingsImplTest, SunnyDay) {
ASSERT_TRUE(
impl.Set("MediaElement.TimeupdateEventIntervalInMilliseconds", 100001));
ASSERT_TRUE(impl.Set("MediaElement.PaintingVideoBackgroundToBlack", 1));
ASSERT_TRUE(impl.Set("MediaElement.EnableUsingMediaSourceBufferedRange", 1));

EXPECT_EQ(impl.GetSourceBufferEvictExtraInBytes().value(), 100);
EXPECT_EQ(impl.GetMinimumProcessorCountToOffloadAlgorithm().value(), 101);
Expand All @@ -59,6 +60,8 @@ TEST(MediaSettingsImplTest, SunnyDay) {
EXPECT_EQ(impl.GetMediaElementTimeupdateEventIntervalInMilliseconds().value(),
100001);
EXPECT_TRUE(impl.IsPaintingVideoBackgroundToBlack().value());
EXPECT_TRUE(
impl.IsMediaElementUsingMediaSourceBufferedRangeEnabled().value());
}

TEST(MediaSettingsImplTest, RainyDay) {
Expand All @@ -75,6 +78,8 @@ TEST(MediaSettingsImplTest, RainyDay) {
ASSERT_FALSE(
impl.Set("MediaElement.TimeupdateEventIntervalInMilliseconds", 0));
ASSERT_FALSE(impl.Set("MediaElement.PaintingVideoBackgroundToBlack", 2));
ASSERT_FALSE(
impl.Set("MediaElement.EnableUsingMediaSourceBufferedRange", -101));

EXPECT_FALSE(impl.GetSourceBufferEvictExtraInBytes());
EXPECT_FALSE(impl.GetMinimumProcessorCountToOffloadAlgorithm());
Expand All @@ -85,6 +90,7 @@ TEST(MediaSettingsImplTest, RainyDay) {
EXPECT_FALSE(impl.GetMaxSourceBufferAppendSizeInBytes());
EXPECT_FALSE(impl.GetMediaElementTimeupdateEventIntervalInMilliseconds());
EXPECT_FALSE(impl.IsPaintingVideoBackgroundToBlack());
EXPECT_FALSE(impl.IsMediaElementUsingMediaSourceBufferedRangeEnabled());
}

TEST(MediaSettingsImplTest, ZeroValuesWork) {
Expand All @@ -101,6 +107,7 @@ TEST(MediaSettingsImplTest, ZeroValuesWork) {
// O is an invalid value for
// "MediaElement.TimeupdateEventIntervalInMilliseconds".
ASSERT_TRUE(impl.Set("MediaElement.PaintingVideoBackgroundToBlack", 0));
ASSERT_TRUE(impl.Set("MediaElement.EnableUsingMediaSourceBufferedRange", 0));

EXPECT_EQ(impl.GetSourceBufferEvictExtraInBytes().value(), 0);
EXPECT_EQ(impl.GetMinimumProcessorCountToOffloadAlgorithm().value(), 0);
Expand All @@ -109,6 +116,8 @@ TEST(MediaSettingsImplTest, ZeroValuesWork) {
EXPECT_FALSE(impl.IsCallingEndedWhenClosedEnabled().value());
EXPECT_EQ(impl.GetMaxSizeForImmediateJob().value(), 0);
EXPECT_FALSE(impl.IsPaintingVideoBackgroundToBlack().value());
EXPECT_FALSE(
impl.IsMediaElementUsingMediaSourceBufferedRangeEnabled().value());
}

TEST(MediaSettingsImplTest, Updatable) {
Expand All @@ -125,6 +134,7 @@ TEST(MediaSettingsImplTest, Updatable) {
ASSERT_TRUE(
impl.Set("MediaElement.TimeupdateEventIntervalInMilliseconds", 1));
ASSERT_TRUE(impl.Set("MediaElement.PaintingVideoBackgroundToBlack", 0));
ASSERT_TRUE(impl.Set("MediaElement.EnableUsingMediaSourceBufferedRange", 0));

ASSERT_TRUE(impl.Set("MediaSource.SourceBufferEvictExtraInBytes", 1));
ASSERT_TRUE(
Expand All @@ -137,6 +147,7 @@ TEST(MediaSettingsImplTest, Updatable) {
ASSERT_TRUE(
impl.Set("MediaElement.TimeupdateEventIntervalInMilliseconds", 2));
ASSERT_TRUE(impl.Set("MediaElement.PaintingVideoBackgroundToBlack", 1));
ASSERT_TRUE(impl.Set("MediaElement.EnableUsingMediaSourceBufferedRange", 1));

EXPECT_EQ(impl.GetSourceBufferEvictExtraInBytes().value(), 1);
EXPECT_EQ(impl.GetMinimumProcessorCountToOffloadAlgorithm().value(), 1);
Expand All @@ -148,6 +159,8 @@ TEST(MediaSettingsImplTest, Updatable) {
EXPECT_EQ(impl.GetMediaElementTimeupdateEventIntervalInMilliseconds().value(),
2);
EXPECT_TRUE(impl.IsPaintingVideoBackgroundToBlack().value());
EXPECT_TRUE(
impl.IsMediaElementUsingMediaSourceBufferedRangeEnabled().value());
}

TEST(MediaSettingsImplTest, InvalidSettingNames) {
Expand Down
20 changes: 19 additions & 1 deletion cobalt/dom/media_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,18 @@ int GetMaxSizeForImmediateJob(web::EnvironmentSettings* settings) {
return max_size;
}

// If this function returns true, MediaSource::GetSeekable() will short-circuit
// getting the buffered range from HTMLMediaElement by directly calling to
// MediaSource::GetBufferedRange(). This reduces potential cross-object,
// cross-thread calls between MediaSource and HTMLMediaElement.
// The default value is false.
bool IsMediaElementUsingMediaSourceBufferedRangeEnabled(
web::EnvironmentSettings* settings) {
return GetMediaSettings(settings)
.IsMediaElementUsingMediaSourceBufferedRangeEnabled()
.value_or(false);
}

} // namespace

MediaSource::MediaSource(script::EnvironmentSettings* settings)
Expand Down Expand Up @@ -498,7 +510,13 @@ scoped_refptr<TimeRanges> MediaSource::GetSeekable() const {
}

if (source_duration == std::numeric_limits<double>::infinity()) {
scoped_refptr<TimeRanges> buffered = attached_element_->buffered();
scoped_refptr<TimeRanges> buffered = nullptr;
if (IsMediaElementUsingMediaSourceBufferedRangeEnabled(
environment_settings())) {
buffered = GetBufferedRange();
} else {
buffered = attached_element_->buffered();
}

if (live_seekable_range_->length() != 0) {
if (buffered->length() == 0) {
Expand Down

0 comments on commit a348ae2

Please sign in to comment.