Josh Karlin (Gerrit)

unread,

Jul 29, 2026, 10:00:08 AM (3 days ago) Jul 29

to Josh Karlin, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Yao Xiao

Josh Karlin voted and added 1 comment

Votes added by Josh Karlin

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Josh Karlin . resolved

Yao, PTAL at the ads related code, thanks!

Open in Gerrit

Related details

Attention is currently required from:

Yao Xiao

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Josh Karlin (Gerrit)

unread,

Jul 29, 2026, 10:05:35 AM (3 days ago) Jul 29

to Josh Karlin, Philip Rogers, Dale Curtis, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Dale Curtis, Philip Rogers and Yao Xiao

Josh Karlin added 1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Josh Karlin . resolved

Philip PTAL at layout/
Dale PTAL at media
Thanks!

Open in Gerrit

Related details

Attention is currently required from:

Dale Curtis
Philip Rogers
Yao Xiao

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Yao Xiao (Gerrit)

unread,

Jul 29, 2026, 11:55:09 AM (3 days ago) Jul 29

to Josh Karlin, Philip Rogers, Dale Curtis, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Dale Curtis, Josh Karlin and Philip Rogers

Yao Xiao voted and added 1 comment

Votes added by Yao Xiao

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Yao Xiao . resolved

ads related code LGTM

Open in Gerrit

Related details

Attention is currently required from:

Dale Curtis
Josh Karlin
Philip Rogers

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Dale Curtis (Gerrit)

unread,

Jul 29, 2026, 12:40:35 PM (3 days ago) Jul 29

to Josh Karlin, Yao Xiao, Philip Rogers, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin and Philip Rogers

Dale Curtis added 1 comment

File third_party/blink/renderer/platform/media/web_media_player_impl.cc

Line 1351, Patchset 10 (Latest): return pipeline_metadata_.video_decoder_config.color_space_info().IsHDR();

Dale Curtis . unresolved

The config should generally indicate the video is hdr, but it is possible for the HDR signaling to only be present in the frame. It's also possible to transition later than first frame. Is that okay for your metrics?

Open in Gerrit

Related details

Attention is currently required from:

Josh Karlin
Philip Rogers

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Josh Karlin (Gerrit)

unread,

Jul 29, 2026, 1:14:11 PM (3 days ago) Jul 29

to Josh Karlin, Yao Xiao, Philip Rogers, Dale Curtis, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Dale Curtis and Philip Rogers

Josh Karlin added 1 comment

File third_party/blink/renderer/platform/media/web_media_player_impl.cc

Line 1351, Patchset 10 (Latest): return pipeline_metadata_.video_decoder_config.color_space_info().IsHDR();

Dale Curtis . unresolved

The config should generally indicate the video is hdr, but it is possible for the HDR signaling to only be present in the frame. It's also possible to transition later than first frame. Is that okay for your metrics?

Josh Karlin

Thanks for calling that out. I think first frame is fine for measurement. If we intervene we should revisit and consider per-frame enforcement. WDYT?

Open in Gerrit

Related details

Attention is currently required from:

Dale Curtis
Philip Rogers

satisfied_requirement

unsatisfied_requirement

open

diffy

Dale Curtis (Gerrit)

unread,

Jul 29, 2026, 1:33:55 PM (3 days ago) Jul 29

to Josh Karlin, Yao Xiao, Philip Rogers, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin and Philip Rogers

Dale Curtis voted and added 3 comments

Votes added by Dale Curtis

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Dale Curtis . resolved

media lgtm

File third_party/blink/public/platform/web_media_player.h

Line 267, Patchset 10 (Latest): virtual bool IsHDR() const { return false; }

Dale Curtis . unresolved

Document what specifically this is checking.

File third_party/blink/renderer/platform/media/web_media_player_impl.cc

Line 1351, Patchset 10 (Latest): return pipeline_metadata_.video_decoder_config.color_space_info().IsHDR();

Dale Curtis . resolved

The config should generally indicate the video is hdr, but it is possible for the HDR signaling to only be present in the frame. It's also possible to transition later than first frame. Is that okay for your metrics?

Josh Karlin

Thanks for calling that out. I think first frame is fine for measurement. If we intervene we should revisit and consider per-frame enforcement. WDYT?

Dale Curtis

sgtm.

Open in Gerrit

Related details

Attention is currently required from:

Josh Karlin
Philip Rogers

satisfied_requirement

unsatisfied_requirement

open

diffy

Josh Karlin (Gerrit)

unread,

Jul 29, 2026, 1:45:00 PM (3 days ago) Jul 29

to Josh Karlin, Dale Curtis, Yao Xiao, Philip Rogers, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Philip Rogers

Josh Karlin added 1 comment

File third_party/blink/public/platform/web_media_player.h

Line 267, Patchset 10: virtual bool IsHDR() const { return false; }

Dale Curtis . resolved

Document what specifically this is checking.

Josh Karlin

Done

Open in Gerrit

Related details

Attention is currently required from:

Philip Rogers

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Philip Rogers (Gerrit)

unread,

Jul 29, 2026, 2:10:52 PM (3 days ago) Jul 29

to Josh Karlin, Dale Curtis, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin

Philip Rogers voted and added 3 comments

Votes added by Philip Rogers

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Philip Rogers . resolved

LGTM

File third_party/blink/renderer/core/layout/layout_object.cc

Line 4589, Patchset 11 (Latest): (element && element->IsAdRelated()) || (frame && frame->IsAdFrame());

Philip Rogers . unresolved

Should this also check `image->GetAdProvenance().has_value();`? This will catch ads loaded from ad urls but where the element and frame aren't marked as such.

Line 4605, Patchset 11 (Latest): desired_headroom = gfx::HdrMetadataExtendedRange::kDefaultHdrHeadroom;

Philip Rogers . unresolved

WDYT of:
desired_headroom = std::max(desired_headroom, gfx::HdrMetadataExtendedRange::kDefaultHdrHeadroom);

Open in Gerrit

Related details

Attention is currently required from:

Josh Karlin

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Philip Rogers (Gerrit)

unread,

Jul 29, 2026, 2:15:54 PM (3 days ago) Jul 29

to Josh Karlin, Dale Curtis, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin

Philip Rogers added 1 comment

Patchset-level comments

File-level comment, Patchset 11 (Latest):

Philip Rogers . resolved

Can you check that this captures the hdr case that inspired this? There are some cases, such as canvas and video poster images, which are not caught by this patch, and it would be good to confirm this works on the real-world cases, if possible (it may no longer be possible).

satisfied_requirement

unsatisfied_requirement

open

diffy

Josh Karlin (Gerrit)

unread,

Jul 30, 2026, 12:35:38 PM (2 days ago) Jul 30

to Josh Karlin, ccameron chromium, Philip Rogers, Dale Curtis, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin

Josh Karlin voted and added 3 comments

Votes added by Josh Karlin

Commit-Queue

+1

3 comments

Patchset-level comments

File-level comment, Patchset 11:

Philip Rogers . resolved

Can you check that this captures the hdr case that inspired this? There are some cases, such as canvas and video poster images, which are not caught by this patch, and it would be good to confirm this works on the real-world cases, if possible (it may no longer be possible).

Josh Karlin

I haven't been able to reproduce the reported eye-melting ad unfortunately. This use counter will help to figure out if it's happening often, and if so, on which sites (once there is follow-up ukm). I believe that video and images will be the common approach, so am starting with those.

File third_party/blink/renderer/core/layout/layout_object.cc

Line 4589, Patchset 11: (element && element->IsAdRelated()) || (frame && frame->IsAdFrame());

Philip Rogers . resolved

Should this also check `image->GetAdProvenance().has_value();`? This will catch ads loaded from ad urls but where the element and frame aren't marked as such.

Josh Karlin

Yes, done, and added a test.

Line 4605, Patchset 11: desired_headroom = gfx::HdrMetadataExtendedRange::kDefaultHdrHeadroom;

Philip Rogers . resolved

WDYT of:
desired_headroom = std::max(desired_headroom, gfx::HdrMetadataExtendedRange::kDefaultHdrHeadroom);

Josh Karlin

The default is high, like nearly 5x. Keeping the code as-is allows the image to use < 1.5.

Open in Gerrit

Related details

Attention is currently required from:

Josh Karlin

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Josh Karlin (Gerrit)

unread,

Jul 30, 2026, 2:07:20 PM (2 days ago) Jul 30

to Josh Karlin, ccameron chromium, Philip Rogers, Dale Curtis, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Josh Karlin voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

ccameron chromium (Gerrit)

unread,

Jul 30, 2026, 3:02:00 PM (2 days ago) Jul 30

to Josh Karlin, ccameron chromium, Philip Rogers, Dale Curtis, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin

ccameron chromium voted and added 4 comments

Votes added by ccameron chromium

Code-Review

+1

4 comments

Patchset-level comments

File-level comment, Patchset 13 (Latest):

ccameron chromium . resolved

lgtm with couple of comments

File third_party/blink/renderer/core/layout/layout_object.cc

Line 4587, Patchset 13 (Latest): PaintImage paint_image = img->PaintImageForCurrentFrame();

ccameron chromium . unresolved

Can you add a function `cc::PaintImage::GetMaximumRenderedHdrHeadroom`, and move the logic there

(that function will be useful in other contexts, and there are a few cases that are missed here).

Line 4602, Patchset 13 (Latest): if (desired_headroom >= 1.5f) {

ccameron chromium . unresolved

headroom is on a log2 scale, so 1.5 means "goes up to exp2(1.5)=Xx brighter than white".

File third_party/blink/renderer/platform/media/web_media_player_impl.cc

Line 1352, Patchset 13 (Latest):}

ccameron chromium . unresolved

This is fine for getting data, but beware that with ST 2094-50, one can use SDR as the "base video", and then make it bright dynamically (frame-by-frame) using metadata.

(Nothing is doing that today, but it will be common in ~1 year).

Open in Gerrit

Related details

Attention is currently required from:

Josh Karlin

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Dale Curtis (Gerrit)

unread,

Jul 30, 2026, 4:43:41 PM (2 days ago) Jul 30

to Josh Karlin, ccameron chromium, Philip Rogers, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, cc-...@chromium.org, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin, Philip Rogers, Yao Xiao and ccameron chromium

Dale Curtis voted and added 1 comment

Votes added by Dale Curtis

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 14 (Latest):

Dale Curtis . resolved

WMP still lgtm

Open in Gerrit

Related details

Attention is currently required from:

Josh Karlin
Philip Rogers
Yao Xiao
ccameron chromium

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Josh Karlin (Gerrit)

unread,

Jul 30, 2026, 4:43:51 PM (2 days ago) Jul 30

to Josh Karlin, Dale Curtis, ccameron chromium, Philip Rogers, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, cc-...@chromium.org, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Philip Rogers, Yao Xiao and ccameron chromium

Josh Karlin added 4 comments

Patchset-level comments

File-level comment, Patchset 14 (Latest):

Josh Karlin . resolved

Done, ccameron@ PTAL again, thanks!

File third_party/blink/renderer/core/layout/layout_object.cc

Line 4587, Patchset 13: PaintImage paint_image = img->PaintImageForCurrentFrame();

ccameron chromium . resolved

Can you add a function `cc::PaintImage::GetMaximumRenderedHdrHeadroom`, and move the logic there
(that function will be useful in other contexts, and there are a few cases that are missed here).

Josh Karlin

Done

Line 4602, Patchset 13: if (desired_headroom >= 1.5f) {

ccameron chromium . resolved

headroom is on a log2 scale, so 1.5 means "goes up to exp2(1.5)=Xx brighter than white".

Josh Karlin

Ack, noted in a new comment.

File third_party/blink/renderer/platform/media/web_media_player_impl.cc

Line 1352, Patchset 13:}

ccameron chromium . resolved

This is fine for getting data, but beware that with ST 2094-50, one can use SDR as the "base video", and then make it bright dynamically (frame-by-frame) using metadata.
(Nothing is doing that today, but it will be common in ~1 year).

Josh Karlin

Thanks for potinting it out! Philipi also mentioned this. Seems OK for now, but is something to consider if we later intervene.

Open in Gerrit

Related details

Attention is currently required from:

Philip Rogers
Yao Xiao
ccameron chromium

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

ccameron chromium (Gerrit)

unread,

Jul 30, 2026, 4:47:51 PM (2 days ago) Jul 30

to Josh Karlin, Dale Curtis, ccameron chromium, Philip Rogers, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, cc-...@chromium.org, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin, Philip Rogers and Yao Xiao

ccameron chromium added 3 comments

Patchset-level comments

File-level comment, Patchset 14 (Latest):

ccameron chromium . resolved

sgtm if log space is fixed

File cc/paint/paint_image.h

Line 408, Patchset 14 (Latest): // color space. Returns 1.0f if the image is SDR.

ccameron chromium . unresolved

It's in log space, so 0 is SDR.

File cc/paint/paint_image.cc

Line 524, Patchset 14 (Latest): return 1.0f;

ccameron chromium . unresolved

return 0 for SDR

Open in Gerrit

Related details

Attention is currently required from:

Josh Karlin
Philip Rogers
Yao Xiao

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

ccameron chromium (Gerrit)

unread,

Jul 30, 2026, 5:30:28 PM (2 days ago) Jul 30

to Josh Karlin, ccameron chromium, Dale Curtis, Philip Rogers, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, cc-...@chromium.org, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Josh Karlin, Philip Rogers and Yao Xiao

ccameron chromium voted and added 1 comment

Votes added by ccameron chromium

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 14 (Latest):

ccameron chromium . resolved

lgtm (with nits)

Open in Gerrit

Related details

Attention is currently required from:

Josh Karlin
Philip Rogers
Yao Xiao

Submit Requirements:

Code-Coverage
Code-Owners

satisfied_requirement

unsatisfied_requirement

open

diffy

Josh Karlin (Gerrit)

unread,

Jul 30, 2026, 5:36:51 PM (2 days ago) Jul 30

to Josh Karlin, ccameron chromium, Dale Curtis, Philip Rogers, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, cc-...@chromium.org, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Philip Rogers and Yao Xiao

Josh Karlin added 2 comments

File cc/paint/paint_image.h

Line 408, Patchset 14: // color space. Returns 1.0f if the image is SDR.

ccameron chromium . resolved

It's in log space, so 0 is SDR.

Josh Karlin

Done

File cc/paint/paint_image.cc

Line 524, Patchset 14: return 1.0f;

ccameron chromium . resolved

return 0 for SDR

Josh Karlin

Updated the function to return log2 space.

Open in Gerrit

Related details

Attention is currently required from:

Philip Rogers
Yao Xiao

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Josh Karlin (Gerrit)

unread,

Jul 30, 2026, 5:37:01 PM (2 days ago) Jul 30

to Josh Karlin, ccameron chromium, Dale Curtis, Philip Rogers, Yao Xiao, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, cc-...@chromium.org, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Attention needed from Philip Rogers and Yao Xiao

Josh Karlin voted Commit-Queue+2

Commit-Queue

+2

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jul 30, 2026, 7:33:45 PM (2 days ago) Jul 30

to Josh Karlin, ccameron chromium, Dale Curtis, Philip Rogers, Yao Xiao, android-bu...@system.gserviceaccount.com, Chromium Metrics Reviews, chromium...@chromium.org, srirama chandra sekhar, cc-...@chromium.org, subresource-f...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org, zol...@webkit.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

14 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: cc/paint/paint_image.h
Insertions: 4, Deletions: 3.

@@ -403,9 +403,10 @@
 
   const gfx::HDRMetadata& GetHDRMetadata() const { return hdr_metadata_; }
 
-  // Returns the maximum HDR headroom required to render this image at full HDR
-  // brightness, taking into account gainmap ratio max metadata or the image's
-  // color space. Returns 1.0f if the image is SDR.
+  // Returns the maximum HDR headroom in log2 space required to render this
+  // image at full HDR brightness, taking into account gainmap ratio max
+  // metadata or the image's color space. Returns 0.0f if the image is SDR (e.g.
+  // 0.5f corresponds to 2^0.5 ~= 1.41x SDR white).
   float GetMaximumRenderedHdrHeadroom() const;
 
   std::string ToString() const;
```
```
The name of the file: third_party/blink/renderer/core/layout/layout_object.cc
Insertions: 3, Deletions: 5.

@@ -156,9 +156,7 @@
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 #include "third_party/blink/renderer/platform/wtf/wtf_size_t.h"
 #include "third_party/skia/include/docs/SkPDFDocument.h"
-#include "ui/gfx/color_space.h"
 #include "ui/gfx/geometry/rect_conversions.h"
-#include "ui/gfx/hdr_metadata.h"
 
 namespace blink {
 
@@ -4583,10 +4581,10 @@
 
     if (is_ad) {
       if (Image* img = image->GetImage()) {
-        // Headroom is on a log2 scale, so 1.5f corresponds to 2^1.5 ~= 2.83x
-        // SDR white.
+        // Headroom is on a log2 scale. 0.5f corresponds to 2^0.5 ~= 1.41x SDR
+        // white.
         if (img->PaintImageForCurrentFrame().GetMaximumRenderedHdrHeadroom() >=
-            1.5f) {
+            0.5f) {
           UseCounter::Count(GetDocument(), WebFeature::kAdImageHDR);
         }
       }
```
```
The name of the file: cc/paint/paint_image.cc
Insertions: 7, Deletions: 5.

@@ -5,6 +5,7 @@
 #include "cc/paint/paint_image.h"
 
 #include <algorithm>
+#include <cmath>
 #include <memory>
 #include <sstream>
 #include <utility>
@@ -514,14 +515,15 @@
 float PaintImage::GetMaximumRenderedHdrHeadroom() const {
   if (HasGainmapInfo()) {
     const SkGainmapInfo& gainmap_info = GetGainmapInfo();
-    return std::max({gainmap_info.fGainmapRatioMax[0],
-                     gainmap_info.fGainmapRatioMax[1],
-                     gainmap_info.fGainmapRatioMax[2]});
+    float max_ratio = std::max({gainmap_info.fGainmapRatioMax[0],
+                                gainmap_info.fGainmapRatioMax[1],
+                                gainmap_info.fGainmapRatioMax[2]});
+    return std::log2(max_ratio);
   }
   if (color_space() && gfx::ColorSpace(*color_space()).IsHDR()) {
-    return gfx::HdrMetadataExtendedRange::kDefaultHdrHeadroom;
+    return std::log2(gfx::HdrMetadataExtendedRange::kDefaultHdrHeadroom);
   }
-  return 1.0f;
+  return 0.0f;
 }
 
 }  // namespace cc
```

Change information

Commit message:

Add UseCounters to measure HDR images and videos in ads

Adds WebFeature UseCounters (kAdImageHDR and kAdVideoHDR) to measure the
prevalence of bright high dynamic range (HDR) images and videos loaded
inside ad elements and ad subframes.

- kAdImageHDR: Triggered in LayoutObject::ImageNotifyFinished when an image loaded within an ad element or ad frame requests high HDR headroom (>= 1.5x, e.g., via UltraHDR gainmap max ratio or an HDR color space profile). Tested with browsertests and copied an hdr image from WPT.

- kAdVideoHDR: Triggered in HTMLVideoElement::OnFirstFrame when a video stream presented within an ad element or ad frame specifies an HDR color profile (PQ / HLG). Unittests only as I can't seem to figure out how to get hdr video in a headless browsertest.

Bug: 539944207

Change-Id: Ic2daa595dd571a04fa9b9992e381b4e94ef79d9d

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8164742

Reviewed-by: Dale Curtis <dalec...@chromium.org>

Commit-Queue: Josh Karlin <jka...@chromium.org>

Reviewed-by: ccameron chromium <ccam...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1671509}

Files:

M cc/paint/paint_image.cc
M cc/paint/paint_image.h
M chrome/browser/subresource_filter/ad_tagging_browsertest.cc
M components/test/data/ad_tagging/ad_script.js
A components/test/data/ad_tagging/gainmap-trattore0.jpg
M third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
M third_party/blink/public/platform/web_media_player.h
M third_party/blink/renderer/core/DEPS
M third_party/blink/renderer/core/html/media/html_video_element.cc
M third_party/blink/renderer/core/html/media/html_video_element_test.cc
M third_party/blink/renderer/core/layout/layout_object.cc
M third_party/blink/renderer/platform/media/web_media_player_impl.cc
M third_party/blink/renderer/platform/media/web_media_player_impl.h
M tools/metrics/histograms/metadata/blink/enums.xml

Change size: L

Delta: 14 files changed, 356 insertions(+), 1 deletion(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Dale Curtis, +1 by ccameron chromium

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Add UseCounters to measure HDR images and videos in ads [chromium/src : main]

Josh Karlin (Gerrit)

Josh Karlin voted and added 1 comment

Votes added by Josh Karlin

1 comment

Related details

Josh Karlin (Gerrit)

Josh Karlin added 1 comment

Related details

Yao Xiao (Gerrit)

Yao Xiao voted and added 1 comment

Votes added by Yao Xiao

1 comment

Related details

Dale Curtis (Gerrit)

Dale Curtis added 1 comment

Related details

Josh Karlin (Gerrit)

Josh Karlin added 1 comment

Related details

Dale Curtis (Gerrit)

Dale Curtis voted and added 3 comments

Votes added by Dale Curtis

3 comments

Related details

Josh Karlin (Gerrit)

Josh Karlin added 1 comment

Related details

Philip Rogers (Gerrit)

Philip Rogers voted and added 3 comments

Votes added by Philip Rogers

3 comments

Related details

Philip Rogers (Gerrit)

Philip Rogers added 1 comment

Josh Karlin (Gerrit)

Josh Karlin voted and added 3 comments

Votes added by Josh Karlin

3 comments

Related details

Josh Karlin (Gerrit)

Josh Karlin voted Commit-Queue+2

Related details

ccameron chromium (Gerrit)

ccameron chromium voted and added 4 comments

Votes added by ccameron chromium

4 comments

Related details

Dale Curtis (Gerrit)

Dale Curtis voted and added 1 comment

Votes added by Dale Curtis

1 comment

Related details

Josh Karlin (Gerrit)

Josh Karlin added 4 comments

Related details

ccameron chromium (Gerrit)

ccameron chromium added 3 comments

Related details

ccameron chromium (Gerrit)

ccameron chromium voted and added 1 comment

Votes added by ccameron chromium

1 comment

Related details

Josh Karlin (Gerrit)

Josh Karlin added 2 comments

Related details

Josh Karlin (Gerrit)

Josh Karlin voted Commit-Queue+2

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information