Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1690353002:
media: Remove Vorbis from secure decoder list for Widevine. (Closed)
Descriptionmedia: Remove Vorbis from secure decoder list for Widevine.
We are not advertising audio decoders (including Vorbis) as secure decoders
[1], hence there's no need to put any audio codec in the CDM supported codec
list.
Also Vorbis support will be dropped from the newer version CDM. See BUG for
details. With that change, Vorbis will also be removed from the component
manifest, which is used to populate the CDM supported codec list for component
updated CDM.
[1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/media/chrome_key_systems.cc&l=185
TBR=thestig@chromium.org
BUG=586354
Committed: https://crrev.com/8b1c48db7b3af2b4493b62290a3d636b416b3b94
Cr-Commit-Position: refs/heads/master@{#375257}
Patch Set 1 #
Total comments: 2
Patch Set 2 : comments addressed #
Total comments: 4
Messages
Total messages: 19 (7 generated)
xhwang@chromium.org changed reviewers: + ddorwin@chromium.org
Thinking about it again. Might be easier to just land this CL now... PTAL
lgtm https://codereview.chromium.org/1690353002/diff/1/chrome/common/chrome_conten... File chrome/common/chrome_content_client.cc (right): https://codereview.chromium.org/1690353002/diff/1/chrome/common/chrome_conten... chrome/common/chrome_content_client.cc:175: std::vector<std::string> codecs; // This list must match the CDM that is being bundled with Chrome.
comments addressed
https://chromiumcodereview.appspot.com/1690353002/diff/1/chrome/common/chrome... File chrome/common/chrome_content_client.cc (right): https://chromiumcodereview.appspot.com/1690353002/diff/1/chrome/common/chrome... chrome/common/chrome_content_client.cc:175: std::vector<std::string> codecs; On 2016/02/12 18:47:35, ddorwin wrote: > // This list must match the CDM that is being bundled with Chrome. Done.
Description was changed from ========== media: Remove Vorbis from secure decoder list for Widevine. We are not advertising audio decoders (including Vorbis) as secure decoders [1], hence there's no need to put any audio codec in the CDM supported codec list. Also Vorbis support will be dropped from the newer version CDM. See BUG for details. With that change, Vorbis will also be removed from the component manifest, which is used to populate the CDM supported codec list for component updated CDM. [1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/me... BUG=586354 ========== to ========== media: Remove Vorbis from secure decoder list for Widevine. We are not advertising audio decoders (including Vorbis) as secure decoders [1], hence there's no need to put any audio codec in the CDM supported codec list. Also Vorbis support will be dropped from the newer version CDM. See BUG for details. With that change, Vorbis will also be removed from the component manifest, which is used to populate the CDM supported codec list for component updated CDM. [1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/me... TBR=thestig@chromium.org BUG=586354 ==========
xhwang@chromium.org changed reviewers: + thestig@chromium.org
TBRing thestig@ again for the trivial change in chrome/common.
The CQ bit was checked by xhwang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ddorwin@chromium.org Link to the patchset: https://chromiumcodereview.appspot.com/1690353002/#ps20001 (title: "comments addressed")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1690353002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1690353002/20001
Message was sent while issue was closed.
Description was changed from ========== media: Remove Vorbis from secure decoder list for Widevine. We are not advertising audio decoders (including Vorbis) as secure decoders [1], hence there's no need to put any audio codec in the CDM supported codec list. Also Vorbis support will be dropped from the newer version CDM. See BUG for details. With that change, Vorbis will also be removed from the component manifest, which is used to populate the CDM supported codec list for component updated CDM. [1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/me... TBR=thestig@chromium.org BUG=586354 ========== to ========== media: Remove Vorbis from secure decoder list for Widevine. We are not advertising audio decoders (including Vorbis) as secure decoders [1], hence there's no need to put any audio codec in the CDM supported codec list. Also Vorbis support will be dropped from the newer version CDM. See BUG for details. With that change, Vorbis will also be removed from the component manifest, which is used to populate the CDM supported codec list for component updated CDM. [1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/me... TBR=thestig@chromium.org BUG=586354 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
lgtm https://chromiumcodereview.appspot.com/1690353002/diff/20001/chrome/common/ch... File chrome/common/chrome_content_client.cc (right): https://chromiumcodereview.appspot.com/1690353002/diff/20001/chrome/common/ch... chrome/common/chrome_content_client.cc:175: // This list must match the CDM that is being bundled with Chrome. Is there any way to test and assert this is true?
Message was sent while issue was closed.
https://chromiumcodereview.appspot.com/1690353002/diff/20001/chrome/common/ch... File chrome/common/chrome_content_client.cc (right): https://chromiumcodereview.appspot.com/1690353002/diff/20001/chrome/common/ch... chrome/common/chrome_content_client.cc:175: // This list must match the CDM that is being bundled with Chrome. On 2016/02/12 21:34:17, Lei Zhang wrote: > Is there any way to test and assert this is true? We can bundle the manifest file with the CDM. Then only populate these codec info from the manifest file. ddorwin: WDYT?
Message was sent while issue was closed.
https://chromiumcodereview.appspot.com/1690353002/diff/20001/chrome/common/ch... File chrome/common/chrome_content_client.cc (right): https://chromiumcodereview.appspot.com/1690353002/diff/20001/chrome/common/ch... chrome/common/chrome_content_client.cc:175: // This list must match the CDM that is being bundled with Chrome. On 2016/02/12 21:39:15, xhwang wrote: > On 2016/02/12 21:34:17, Lei Zhang wrote: > > Is there any way to test and assert this is true? > > We can bundle the manifest file with the CDM. Then only populate these codec > info from the manifest file. > > ddorwin: WDYT? This should be part of a larger effort to unify the component and non-component paths.
Message was sent while issue was closed.
https://chromiumcodereview.appspot.com/1690353002/diff/20001/chrome/common/ch... File chrome/common/chrome_content_client.cc (right): https://chromiumcodereview.appspot.com/1690353002/diff/20001/chrome/common/ch... chrome/common/chrome_content_client.cc:175: // This list must match the CDM that is being bundled with Chrome. On 2016/02/12 21:43:32, ddorwin wrote: > On 2016/02/12 21:39:15, xhwang wrote: > > On 2016/02/12 21:34:17, Lei Zhang wrote: > > > Is there any way to test and assert this is true? > > > > We can bundle the manifest file with the CDM. Then only populate these codec > > info from the manifest file. > > > > ddorwin: WDYT? > > This should be part of a larger effort to unify the component and non-component > paths. Agreed. I filed a bug to track this work: https://code.google.com/p/chromium/issues/detail?id=586634
Message was sent while issue was closed.
Description was changed from ========== media: Remove Vorbis from secure decoder list for Widevine. We are not advertising audio decoders (including Vorbis) as secure decoders [1], hence there's no need to put any audio codec in the CDM supported codec list. Also Vorbis support will be dropped from the newer version CDM. See BUG for details. With that change, Vorbis will also be removed from the component manifest, which is used to populate the CDM supported codec list for component updated CDM. [1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/me... TBR=thestig@chromium.org BUG=586354 ========== to ========== media: Remove Vorbis from secure decoder list for Widevine. We are not advertising audio decoders (including Vorbis) as secure decoders [1], hence there's no need to put any audio codec in the CDM supported codec list. Also Vorbis support will be dropped from the newer version CDM. See BUG for details. With that change, Vorbis will also be removed from the component manifest, which is used to populate the CDM supported codec list for component updated CDM. [1] https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/me... TBR=thestig@chromium.org BUG=586354 Committed: https://crrev.com/8b1c48db7b3af2b4493b62290a3d636b416b3b94 Cr-Commit-Position: refs/heads/master@{#375257} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/8b1c48db7b3af2b4493b62290a3d636b416b3b94 Cr-Commit-Position: refs/heads/master@{#375257} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
