MSE: Require HEVC codec string in addSourceBuffer for CROS+EME+HEVC usage [chromium/src : master]
Matthew Wolenetz (Gerrit)
Attention is currently required from: Dan Sanders.
Matthew Wolenetz would like Dan Sanders to review this change.
MSE: Require HEVC codec string in addSourceBuffer for CROS+EME+HEVC usage
Partially reverts previous special-case codec-specificity relaxation
logic added in b45c3f1e.
Later (full) codec-specifity relaxation may make part of this update
obsolete.
Specifically, this change:
1) Leaves isTypeSupported() behavior as-is (e.g., incompletely specified
or unsupported types say false, including hevc on chromeos even if
platform hevc and chromeos protected media are enabled).
2) Undoes the allowance for addSourceBuffer(type) and changeType(type)
to have an assumed hevc codec when type is strictly "video/mp4"
without any codecs (iff both platform hevc and chromeos protected
media are enabled).
3) Updates IsTypeSupportedInternal, when used for aSB() or cT() -- but
not for iTS() -- when both platform hevc and chromeos protected media
are enabled: if GetSupportsType() determined lack of support, then
conditionally still proceed iff the type contains precisely 1 HEVC
codec, no other video codec, and optionally 1 audio codec.
4) Updates SourceBufferState to conditionally allow clear HEVC on
CROS+EME+HEVC if cmdline has --enable-clear-hevc-for-testing.
Otherwise, the content must be encrypted on that build configuration
for HEVC to be supported for buffering via MSE.
BUG=535738
Change-Id: I95ac91627cdff9b01d5470488d95e807e9064013
---
M media/filters/chunk_demuxer.cc
M media/filters/source_buffer_state.cc
M media/filters/stream_parser_factory.cc
M third_party/blink/renderer/modules/mediasource/DEPS
M third_party/blink/renderer/modules/mediasource/media_source.cc
M third_party/blink/renderer/modules/mediasource/media_source.h
6 files changed, 95 insertions(+), 78 deletions(-)
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Wolenetz (Gerrit)
Attention is currently required from: Dan Sanders.
Patch set 9:Commit-Queue +1
1 comment:
Patchset:
Dan, please take a look. Or if you prefer, we can await Dale or Chris to review this Monday.
Jeff, please try out some manual tests with this patch applied.
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Wolenetz (Gerrit)
Attention is currently required from: Dan Sanders.
Patch set 9:Code-Review -1
1 comment:
Patchset:
Local manual tests with buildflags simulating CROS+platformhevc fail aSB(hevc-in-mp4). Investigating/adding logging.
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Wolenetz (Gerrit)
Patch set 10:-Code-ReviewCommit-Queue +1
1 comment:
Patchset:
Ok. Please resume review and manual tests using patchset 10. Apologies for premature review request earlier :/
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Jeffrey Kardatzke (Gerrit)
Attention is currently required from: Matthew Wolenetz.
Patch set 10:Code-Review +1
1 comment:
Patchset:
I'm only +1'ing this to indicate it works with Netflix and HEVC. Others should actually review it.
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Wolenetz (Gerrit)
Attention is currently required from: Chrome Cunningham.
Matthew Wolenetz would like Chrome Cunningham to review this change.
6 files changed, 96 insertions(+), 78 deletions(-)
Matthew Wolenetz (Gerrit)
Attention is currently required from: Chrome Cunningham.
Patch set 11:Commit-Queue +1
1 comment:
Patchset:
Jeff, thank you for confirming behavior of this patch.
Switching reviewer to Chris, who has more context on this change.
Chris, please review. Thanks!
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Chrome Cunningham (Gerrit)
Attention is currently required from: Matthew Wolenetz.
Patch set 11:Code-Review +1
1 comment:
Patchset:
LGTM % but consider my alternative proposal (simpler?)
```
#if cros_EMEsplit codecs into codecs_array
filtered_codecs = ""
for (codec : codecs_array) {
if (!found_hevc && ParseVideo(codec,...) && type == hevc) {
found_hevc = true
continue;
}
filtered_codecs += codec
}if (!filtered_codecs.empty()) {
ContentType filtered_type(content_type.GetType() + "; codecs=" + filtered_codecs);
... // check support for whats left
}# else
... // existing support check
#endif
```
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Wolenetz (Gerrit)
Attention is currently required from: Matthew Wolenetz.
Matthew Wolenetz uploaded patch set #12 to this change.
MSE: Require HEVC codec string in addSourceBuffer for CROS+EME+HEVC usage
Partially reverts previous special-case codec-specificity relaxation
logic added in b45c3f1e.
Later (full) codec-specifity relaxation may make part of this update
obsolete.
Specifically, this change:
1) Leaves isTypeSupported() behavior as-is (e.g., incompletely specified
or unsupported types say false, including hevc on chromeos even if
platform hevc and chromeos protected media are enabled).
2) Undoes the allowance for addSourceBuffer(type) and changeType(type)
to have an assumed hevc codec when type is strictly "video/mp4"
without any codecs (iff both platform hevc and chromeos protected
media are enabled).
3) Updates IsTypeSupportedInternal, when used for aSB() or cT() -- but
not for iTS() -- when both platform hevc and chromeos protected media
are enabled: removes any successfully parsed HEVC codec string from
the codecs parameter of the type used to query GetSupportsType().
4) Updates SourceBufferState to conditionally allow clear HEVC on
CROS+EME+HEVC if cmdline has --enable-clear-hevc-for-testing.
Otherwise, the content must be encrypted on that build configuration
for HEVC to be supported for buffering via MSE.
BUG=535738
Change-Id: I95ac91627cdff9b01d5470488d95e807e9064013
---
M media/filters/chunk_demuxer.cc
M media/filters/source_buffer_state.cc
M media/filters/stream_parser_factory.cc
M third_party/blink/renderer/modules/mediasource/DEPS
M third_party/blink/renderer/modules/mediasource/media_source.cc
M third_party/blink/renderer/modules/mediasource/media_source.h
6 files changed, 96 insertions(+), 78 deletions(-)
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Wolenetz (Gerrit)
Patch set 13:Commit-Queue +1
1 comment:
Patchset:
Jeff, please retry the manual tests using patchset 13.
Chris, please take another look.
Thanks!
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Jeffrey Kardatzke (Gerrit)
Attention is currently required from: Matthew Wolenetz.
1 comment:
Patchset:
Jeff, please retry the manual tests using patchset 13. […]
Works for me!
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Chromium LUCI CQ (Gerrit)
Attention is currently required from: Matthew Wolenetz.
CQ is trying the patch.
Note: The patchset #13 "Using simplified approach suggested by chcunningham@" sent to CQ was uploaded after this CL was CR+1-ed.
Reviewer, please verify there is nothing unexpected https://chromium-review.googlesource.com/c/2626112/13
Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/chromium-review.googlesource.com/2626112/13
Bot data: {"action": "start", "triggered_at": "2021-01-26T02:54:00.0Z", "revision": "1be734fa381a27ca8aa32a06bd48163e7c46d09c"}
Chrome Cunningham (Gerrit)
Attention is currently required from: Matthew Wolenetz.
Patch set 13:Commit-Queue +2
1 comment:
Patchset:
LGTM. Thanks for taking my suggestion.
To view, visit change 2626112. To unsubscribe, or for help writing mail filters, visit settings.
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
MSE: Require HEVC codec string in addSourceBuffer for CROS+EME+HEVC usage
Partially reverts previous special-case codec-specificity relaxation
logic added in b45c3f1e.
Later (full) codec-specifity relaxation may make part of this update
obsolete.
Specifically, this change:
1) Leaves isTypeSupported() behavior as-is (e.g., incompletely specified
or unsupported types say false, including hevc on chromeos even if
platform hevc and chromeos protected media are enabled).
2) Undoes the allowance for addSourceBuffer(type) and changeType(type)
to have an assumed hevc codec when type is strictly "video/mp4"
without any codecs (iff both platform hevc and chromeos protected
media are enabled).
3) Updates IsTypeSupportedInternal, when used for aSB() or cT() -- but
not for iTS() -- when both platform hevc and chromeos protected media
are enabled: removes any successfully parsed HEVC codec string from
the codecs parameter of the type used to query GetSupportsType().
4) Updates SourceBufferState to conditionally allow clear HEVC on
CROS+EME+HEVC if cmdline has --enable-clear-hevc-for-testing.
Otherwise, the content must be encrypted on that build configuration
for HEVC to be supported for buffering via MSE.
BUG=535738
Change-Id: I95ac91627cdff9b01d5470488d95e807e9064013
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2626112
Commit-Queue: Matthew Wolenetz <wole...@chromium.org>
Commit-Queue: Chrome Cunningham <chcunn...@chromium.org>
Reviewed-by: Chrome Cunningham <chcunn...@chromium.org>
Reviewed-by: Jeffrey Kardatzke <jkard...@google.com>
Cr-Commit-Position: refs/heads/master@{#847043}
---
M media/filters/chunk_demuxer.cc
M media/filters/source_buffer_state.cc
M media/filters/stream_parser_factory.cc
M third_party/blink/renderer/modules/mediasource/DEPS
M third_party/blink/renderer/modules/mediasource/media_source.cc
M third_party/blink/renderer/modules/mediasource/media_source.h
6 files changed, 75 insertions(+), 85 deletions(-)