Sangbaek Park (Gerrit)

unread,

Jun 24, 2026, 8:30:53 PM (9 days ago) Jun 24

to Chromium LUCI CQ, Piet Schouten, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Sangbaek Park added 1 comment

File media/mojo/services/media_foundation_service.cc

Line 467, Patchset 1 (Latest): base::UmaHistogramBoolean(

Sangbaek Park . resolved

Test results when `kHardwareSecureDecryptionDolbyVisionWithHdrCheck` is enabled.

a) With **HDR primary display** + external display (**non-HDR**),

Histogram report:
```
Histogram: Media.EME.MediaFoundationService.DolbyVisionSupport.WithHdrCheck recorded 1 samples, mean = 1.0 (flags = 0x41) [#]
0 O (0 = 0.0%)
1 -O (1 = 100.0%) {0.0%}
2 O (0 = 0.0%) {100.0%}

- Histogram: Media.EME.MediaFoundationService.DolbyVisionSupport.WithoutHdrCheck recorded 1 samples, mean = 1.0 (flags = 0x41) [#]
0  O                                                                         (0 = 0.0%)
1  -O                                                                        (1 = 100.0%) {0.0%}
2  O                                                                         (0 = 0.0%) {100.0%}
```
- Capability report:
```
Capabilities	
{
  "Audio Codecs": [
    "aac"
  ],
  "Encryption Schemes": [
    "CENC",
    "CBCS"
  ],
  "Session Types": [
    "temporary"
  ],
  "Video Codecs": {
    "av1": [],
    "dolbyvision": [
      "dolby vision profile 5",
      "dolby vision profile 8"
    ],
    "h264": [],
    "hevc": []
  }
}
```

b) With external display only (**non-HDR**),

Histogram report:
```
Histogram: Media.EME.MediaFoundationService.DolbyVisionSupport.WithHdrCheck recorded 1 samples, mean = 0.0 (flags = 0x41) [#]
0 -O (1 = 100.0%)
1 ...

- Histogram: Media.EME.MediaFoundationService.DolbyVisionSupport.WithoutHdrCheck recorded 1 samples, mean = 1.0 (flags = 0x41) [#]
0  O                                                                         (0 = 0.0%)
1  -O                                                                        (1 = 100.0%) {0.0%}
2  O                                                                         (0 = 0.0%) {100.0%}
```
- Capability report:
```
Capabilities	
{
  "Audio Codecs": [
    "aac"
  ],
  "Encryption Schemes": [
    "CENC"
  ],
  "Session Types": [
    "temporary"
  ],
  "Video Codecs": {
    "h264": [],
    "hevc": []
  }
}
```

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

unsatisfied_requirement

open

diffy

Sangbaek Park (Gerrit)

unread,

Jun 26, 2026, 12:42:46 PM (8 days ago) Jun 26

to Chromium LUCI CQ, Piet Schouten, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Sangbaek Park and Xiaohan Wang

Sangbaek Park voted and added 1 comment

Votes added by Sangbaek Park

Commit-Queue

+1

1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Sangbaek Park . resolved

PTAL, thanks! Detailed test results: https://chromium-review.git.corp.google.com/c/chromium/src/+/7998779/comment/e9d3b1dd_501f8402/

Open in Gerrit

Related details

Attention is currently required from:

Sangbaek Park
Xiaohan Wang

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

Sangbaek Park (Gerrit)

unread,

Jun 26, 2026, 2:13:46 PM (8 days ago) Jun 26

to Vikram Pasupathy, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Evan Liu, Piet Schouten and Vikram Pasupathy

Sangbaek Park added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Sangbaek Park . resolved

@Piet.S...@microsoft.com and @vpasu...@chromium.org, PTAL since xhwang is OOO untile July 6. We want to add this before M151 branch cut (June 29).

Open in Gerrit

Related details

Attention is currently required from:

Evan Liu
Piet Schouten
Vikram Pasupathy

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

Evan Liu (Gerrit)

unread,

Jun 26, 2026, 2:19:42 PM (8 days ago) Jun 26

to Sangbaek Park, Vikram Pasupathy, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Piet Schouten, Sangbaek Park and Vikram Pasupathy

Evan Liu voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Piet Schouten
Sangbaek Park
Vikram Pasupathy

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

Vikram Pasupathy (Gerrit)

unread,

Jun 26, 2026, 3:17:37 PM (8 days ago) Jun 26

to Sangbaek Park, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Piet Schouten and Sangbaek Park

Vikram Pasupathy voted and added 3 comments

Votes added by Vikram Pasupathy

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Vikram Pasupathy . resolved

lgtm % one question / optimization.

File media/base/media_switches.cc

Line 1026, Patchset 1 (Latest): base::FEATURE_DISABLED_BY_DEFAULT);

Vikram Pasupathy . resolved

In another CL, do you want to move this flag and the two above to under the IS_WIN buildflag?

File media/mojo/services/media_foundation_service.cc

Line 458, Patchset 1 (Latest): bool dv_support_with_hdr = is_type_supported_cb.Run(

Vikram Pasupathy . unresolved

Do you want to guard all of (#2 + the Histogram reporting for WithHdrCheck) with the `if(base::FeatureList::IsEnabled(kHardwareSecureDecryptionDolbyVisionWithHdrCheck))`?.
              
Because otherwise, we spend time checking with HDR with an extra IsTypeSupported query, but we don't use it (apart from reporting to histogram).

Maybe you want to report both but only determine the final support based on feature flag, so the code is correct the way it is. Feel free to resolve if that was your intention.

Open in Gerrit

Related details

Attention is currently required from:

Piet Schouten
Sangbaek Park

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

Sangbaek Park (Gerrit)

unread,

Jun 26, 2026, 5:27:19 PM (7 days ago) Jun 26

to Vikram Pasupathy, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Piet Schouten

Sangbaek Park added 1 comment

File media/mojo/services/media_foundation_service.cc

Line 458, Patchset 1 (Latest): bool dv_support_with_hdr = is_type_supported_cb.Run(

Vikram Pasupathy . resolved

Do you want to guard all of (#2 + the Histogram reporting for WithHdrCheck) with the `if(base::FeatureList::IsEnabled(kHardwareSecureDecryptionDolbyVisionWithHdrCheck))`?.
              
Because otherwise, we spend time checking with HDR with an extra IsTypeSupported query, but we don't use it (apart from reporting to histogram).
Maybe you want to report both but only determine the final support based on feature flag, so the code is correct the way it is. Feel free to resolve if that was your intention.

Sangbaek Park

Good point but we still want to report both before/after enabling the feature since things can get changed. Once everything is working well, we can remove both metrics and query only one DV check with HDR.

Open in Gerrit

Related details

Attention is currently required from:

Piet Schouten

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

Sangbaek Park (Gerrit)

unread,

Jun 26, 2026, 6:18:45 PM (7 days ago) Jun 26

to Eugene Zemtsov, Vikram Pasupathy, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Eugene Zemtsov and Piet Schouten

Sangbaek Park added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Sangbaek Park . resolved

@eug...@chromium.org, could you please take a look at this for media_switches/media_foundation_service since Xiaohan and Dale both are OOO? Thanks!

Open in Gerrit

Related details

Attention is currently required from:

Eugene Zemtsov
Piet Schouten

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

Vikram Pasupathy (Gerrit)

unread,

Jun 26, 2026, 6:20:31 PM (7 days ago) Jun 26

to Sangbaek Park, Eugene Zemtsov, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Eugene Zemtsov and Piet Schouten

Vikram Pasupathy added 1 comment

File media/mojo/services/media_foundation_service.cc

Line 458, Patchset 1 (Latest): bool dv_support_with_hdr = is_type_supported_cb.Run(

Vikram Pasupathy . resolved

Do you want to guard all of (#2 + the Histogram reporting for WithHdrCheck) with the `if(base::FeatureList::IsEnabled(kHardwareSecureDecryptionDolbyVisionWithHdrCheck))`?.
              
Because otherwise, we spend time checking with HDR with an extra IsTypeSupported query, but we don't use it (apart from reporting to histogram).
Maybe you want to report both but only determine the final support based on feature flag, so the code is correct the way it is. Feel free to resolve if that was your intention.

Sangbaek Park

Good point but we still want to report both before/after enabling the feature since things can get changed. Once everything is working well, we can remove both metrics and query only one DV check with HDR.

Vikram Pasupathy

SGTM, thank you!

satisfied_requirement

unsatisfied_requirement

open

diffy

Eugene Zemtsov (Gerrit)

unread,

Jun 26, 2026, 8:30:39 PM (7 days ago) Jun 26

to Sangbaek Park, Eugene Zemtsov, Vikram Pasupathy, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Piet Schouten and Sangbaek Park

Eugene Zemtsov voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Piet Schouten
Sangbaek Park

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

Eugene Zemtsov (Gerrit)

unread,

Jun 26, 2026, 8:30:49 PM (7 days ago) Jun 26

to Sangbaek Park, Eugene Zemtsov, Vikram Pasupathy, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Piet Schouten and Sangbaek Park

Eugene Zemtsov added 2 comments

File media/mojo/services/media_foundation_service.cc

Line 64, Patchset 1 (Latest):#if BUILDFLAG(ENABLE_PLATFORM_DOLBY_VISION)

Eugene Zemtsov . unresolved

There is a buildflag mismatch that will cause compilation failures in certain configurations. The constants `kHdrQueryName` and `kDolbyVisionSupportUmaPrefix` are defined under the preprocessor guard `#if BUILDFLAG(ENABLE_PLATFORM_DOLBY_VISION)`.

However, they are used inside `GetCdmCapability()` under a block guarded by `#if BUILDFLAG(ENABLE_PLATFORM_ENCRYPTED_DOLBY_VISION)`.

Line 458, Patchset 1 (Latest): bool dv_support_with_hdr = is_type_supported_cb.Run(

Eugene Zemtsov . unresolved

This code unconditionally executes the second Media Foundation query (`dv_support_with_hdr`) even if the first query (`dv_support_without_hdr`) returns `false`

Open in Gerrit

Related details

Attention is currently required from:

Piet Schouten
Sangbaek Park

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

Sangbaek Park (Gerrit)

unread,

Jun 29, 2026, 1:44:40 PM (5 days ago) Jun 29

to Eugene Zemtsov, Vikram Pasupathy, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Piet Schouten

Sangbaek Park added 2 comments

File media/mojo/services/media_foundation_service.cc

Line 64, Patchset 1:#if BUILDFLAG(ENABLE_PLATFORM_DOLBY_VISION)

Eugene Zemtsov . resolved

There is a buildflag mismatch that will cause compilation failures in certain configurations. The constants `kHdrQueryName` and `kDolbyVisionSupportUmaPrefix` are defined under the preprocessor guard `#if BUILDFLAG(ENABLE_PLATFORM_DOLBY_VISION)`.
However, they are used inside `GetCdmCapability()` under a block guarded by `#if BUILDFLAG(ENABLE_PLATFORM_ENCRYPTED_DOLBY_VISION)`.

Sangbaek Park

Thank you for catching this! Corrected `ENABLE_PLATFORM_DOLBY_VISION` to `ENABLE_PLATFORM_ENCRYPTED_DOLBY_VISION`.

Line 458, Patchset 1: bool dv_support_with_hdr = is_type_supported_cb.Run(

Eugene Zemtsov . resolved

This code unconditionally executes the second Media Foundation query (`dv_support_with_hdr`) even if the first query (`dv_support_without_hdr`) returns `false`

Sangbaek Park

That's a great point to save the query time, thanks! Done.

Open in Gerrit

Related details

Attention is currently required from:

Piet Schouten

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

Sangbaek Park (Gerrit)

unread,

Jun 29, 2026, 2:46:28 PM (5 days ago) Jun 29

to Eugene Zemtsov, Vikram Pasupathy, Evan Liu, Piet Schouten, Chromium LUCI CQ, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Attention needed from Piet Schouten

Sangbaek Park voted Commit-Queue+2

Commit-Queue

+2

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Jun 29, 2026, 3:46:52 PM (5 days ago) Jun 29

to Sangbaek Park, Eugene Zemtsov, Vikram Pasupathy, Evan Liu, Piet Schouten, Daoyuan Li, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, eme-r...@chromium.org, feature-me...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

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

```
The name of the file: media/mojo/services/media_foundation_service.cc
Insertions: 14, Deletions: 8.

@@ -61,11 +61,11 @@
 const char kRobustnessQueryName[] = "encryption-robustness";
 const char kEncryptionSchemeQueryName[] = "encryption-type";
 const char kEncryptionIvQueryName[] = "encryption-iv-size";
-#if BUILDFLAG(ENABLE_PLATFORM_DOLBY_VISION)
+#if BUILDFLAG(ENABLE_PLATFORM_ENCRYPTED_DOLBY_VISION)
 const char kHdrQueryName[] = "hdr";
 const char kDolbyVisionSupportUmaPrefix[] =
     "Media.EME.MediaFoundationService.DolbyVisionSupport";
-#endif  // BUILDFLAG(ENABLE_PLATFORM_DOLBY_VISION)
+#endif  // BUILDFLAG(ENABLE_PLATFORM_ENCRYPTED_DOLBY_VISION)
 
 const char kSwSecureRobustness[] = "SW_SECURE_DECODE";
 const char kHwSecureRobustness[] = "HW_SECURE_ALL";
@@ -453,12 +453,18 @@
 
       // 2. Query with HDR support. When multiple displays are connected to the
       // device, the query result is expected to return TRUE if the primary
-      // display (internal) is HDR.
-      extra_features.insert({{kHdrQueryName, "1"}});
-      bool dv_support_with_hdr = is_type_supported_cb.Run(
-          is_hw_secure, GetTypeString(video_codec, /*audio_codec=*/std::nullopt,
-                                      extra_features));
-      extra_features.erase(kHdrQueryName);
+      // display (internal) is HDR. If the query result without HDR check is
+      // FALSE, we know the quer result with HDR check is expected to return
+      // FALSE as well.
+      bool dv_support_with_hdr = false;
+      if (dv_support_without_hdr) {
+        extra_features.insert({{kHdrQueryName, "1"}});
+        dv_support_with_hdr = is_type_supported_cb.Run(
+            is_hw_secure,
+            GetTypeString(video_codec, /*audio_codec=*/std::nullopt,
+                          extra_features));
+        extra_features.erase(kHdrQueryName);
+      }
 
       DVLOG(3) << __func__ << ": Dolby Vision support - dv_support_with_hdr="
                << dv_support_with_hdr
```

Change information

Commit message:

media: Report Dolby Vision capability histograms with/without HDR check

- Report Dolby Vision capability histograms with/without HDR check
```
Media.EME.MediaFoundationService.DolbyVisionSupport.WithHdrCheck
Media.EME.MediaFoundationService.DolbyVisionSupport.WithoutHdrCheck
```
- Add feature flag `kHardwareSecureDecryptionDolbyVisionWithHdrCheck`.
  If this flag is on, we enable hardware secure Dolby Vision decoding
  always with HDR display check. Disabled by default for now.

Bug: b:519288654, b:407123708

Change-Id: Ib3b9394b8c5364dcf961f98abd54a0b1638e5354

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

Commit-Queue: Sangbaek Park <sangba...@chromium.org>

Reviewed-by: Evan Liu <ev...@google.com>

Reviewed-by: Vikram Pasupathy <vpasu...@chromium.org>

Reviewed-by: Eugene Zemtsov <eug...@chromium.org>

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

Files:

M media/base/media_switches.cc
M media/base/media_switches.h
M media/mojo/services/media_foundation_service.cc
M tools/metrics/histograms/metadata/media/histograms.xml

Change size: M

Delta: 4 files changed, 88 insertions(+), 2 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Evan Liu, +1 by Vikram Pasupathy, +1 by Eugene Zemtsov

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

media: Report Dolby Vision capability histograms with/without HDR check [chromium/src : main]

Sangbaek Park (Gerrit)

Sangbaek Park added 1 comment

Related details

Sangbaek Park (Gerrit)

Sangbaek Park voted and added 1 comment

Votes added by Sangbaek Park

1 comment

Related details

Sangbaek Park (Gerrit)

Sangbaek Park added 1 comment

Related details

Evan Liu (Gerrit)

Evan Liu voted Code-Review+1

Related details

Vikram Pasupathy (Gerrit)

Vikram Pasupathy voted and added 3 comments

Votes added by Vikram Pasupathy

3 comments

Related details

Sangbaek Park (Gerrit)

Sangbaek Park added 1 comment

Related details

Sangbaek Park (Gerrit)

Sangbaek Park added 1 comment

Related details

Vikram Pasupathy (Gerrit)

Vikram Pasupathy added 1 comment

Eugene Zemtsov (Gerrit)

Eugene Zemtsov voted Code-Review+1

Related details

Eugene Zemtsov (Gerrit)

Eugene Zemtsov added 2 comments

Related details

Sangbaek Park (Gerrit)

Sangbaek Park added 2 comments

Related details

Sangbaek Park (Gerrit)

Sangbaek Park voted Commit-Queue+2

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information