Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(616)

Issue 2805553004: Wire up MediaCapabilities is_supported to MimeUtil (Closed)

Created:
2 years, 10 months ago by chcunningham
Modified:
2 years, 10 months ago
CC:
blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, dglazkov+blink, feature-media-reviews_chromium.org, haraken
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Wire up MediaCapabilities is_supported to media/ navigator.mediaCapabilities.decodingInfo(...) will now query the media layer to parse the mime type and check platform support. Some work remains to reject ambiguous mime strings. In some cases we are also not deeply checking for platform support. Will get to this soon. BUG=695264 TEST=new browser tests Review-Url: https://codereview.chromium.org/2805553004 Cr-Commit-Position: refs/heads/master@{#467184} Committed: https://chromium.googlesource.com/chromium/src/+/20645b6c0f3fa1fe3d20742ca7fcd77058147ad8

Patch Set 1 #

Total comments: 2

Patch Set 2 : Add browser test. Rebase. #

Patch Set 3 : fix boolean silliness #

Patch Set 4 : Rebase and merge #

Total comments: 3

Patch Set 5 : Fix failing test. Merge #

Total comments: 9

Patch Set 6 : Feedback and test fix #

Patch Set 7 : Remove test for theora - not supported on android #

Unified diffs Side-by-side diffs Delta from patch set Stats (+307 lines, -36 lines) Patch
A content/browser/media/media_capabilities_browsertest.cc View 1 2 3 4 5 6 1 chunk +147 lines, -0 lines 0 comments Download
M content/renderer/media_recorder/media_recorder_handler.cc View 1 2 3 4 1 chunk +11 lines, -25 lines 0 comments Download
M content/test/BUILD.gn View 1 2 3 4 2 chunks +2 lines, -1 line 0 comments Download
M media/base/mime_util_internal.cc View 1 2 3 4 5 1 chunk +3 lines, -1 line 0 comments Download
M media/blink/webmediacapabilitiesclient_impl.cc View 1 2 3 4 5 2 chunks +49 lines, -5 lines 0 comments Download
A media/test/data/decode_capabilities_test.html View 1 1 chunk +72 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/media_capabilities/MediaCapabilities.cpp View 1 2 3 4 3 chunks +19 lines, -2 lines 0 comments Download
M third_party/WebKit/public/platform/modules/media_capabilities/WebAudioConfiguration.h View 1 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/public/platform/modules/media_capabilities/WebVideoConfiguration.h View 1 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 41 (27 generated)
chcunningham
Don't review yet - tests on the way. Just FYI in case you're wondering where ...
2 years, 10 months ago (2017-04-07 00:54:38 UTC) #2
mlamouri (slow - plz ping)
https://codereview.chromium.org/2805553004/diff/1/media/blink/webmediacapabilitiesclient_impl.cc File media/blink/webmediacapabilitiesclient_impl.cc (right): https://codereview.chromium.org/2805553004/diff/1/media/blink/webmediacapabilitiesclient_impl.cc#newcode36 media/blink/webmediacapabilitiesclient_impl.cc:36: DCHECK_EQ(codec_vector.size(), 1U); Maybe we should do this check on ...
2 years, 10 months ago (2017-04-10 12:31:50 UTC) #3
chcunningham
Ready to go :) https://codereview.chromium.org/2805553004/diff/1/media/blink/webmediacapabilitiesclient_impl.cc File media/blink/webmediacapabilitiesclient_impl.cc (right): https://codereview.chromium.org/2805553004/diff/1/media/blink/webmediacapabilitiesclient_impl.cc#newcode36 media/blink/webmediacapabilitiesclient_impl.cc:36: DCHECK_EQ(codec_vector.size(), 1U); On 2017/04/10 12:31:50, ...
2 years, 10 months ago (2017-04-20 21:48:10 UTC) #4
chcunningham
+ jrummel@ for media/
2 years, 10 months ago (2017-04-20 21:53:56 UTC) #7
chcunningham
+ mcasas for media recorder
2 years, 10 months ago (2017-04-21 19:33:54 UTC) #17
mcasas
encoding parts lgtm, thanks! https://codereview.chromium.org/2805553004/diff/60001/third_party/WebKit/Source/modules/media_capabilities/MediaCapabilities.cpp File third_party/WebKit/Source/modules/media_capabilities/MediaCapabilities.cpp (right): https://codereview.chromium.org/2805553004/diff/60001/third_party/WebKit/Source/modules/media_capabilities/MediaCapabilities.cpp#newcode117 third_party/WebKit/Source/modules/media_capabilities/MediaCapabilities.cpp:117: DCHECK(configuration.hasType()); nit: I missed this ...
2 years, 10 months ago (2017-04-21 20:18:33 UTC) #20
jrummell
media LGTM https://codereview.chromium.org/2805553004/diff/60001/content/browser/media/media_capabilities_browsertest.cc File content/browser/media/media_capabilities_browsertest.cc (right): https://codereview.chromium.org/2805553004/diff/60001/content/browser/media/media_capabilities_browsertest.cc#newcode117 content/browser/media/media_capabilities_browsertest.cc:117: EXPECT_EQ(kUnsupported, CanDecodeVideo("'video/mp4; codecs=\"vp9\"'")); How about testing audio ...
2 years, 10 months ago (2017-04-21 21:14:36 UTC) #23
chcunningham
Mounir, gentle ping https://codereview.chromium.org/2805553004/diff/60001/content/browser/media/media_capabilities_browsertest.cc File content/browser/media/media_capabilities_browsertest.cc (right): https://codereview.chromium.org/2805553004/diff/60001/content/browser/media/media_capabilities_browsertest.cc#newcode117 content/browser/media/media_capabilities_browsertest.cc:117: EXPECT_EQ(kUnsupported, CanDecodeVideo("'video/mp4; codecs=\"vp9\"'")); On 2017/04/21 21:14:36, ...
2 years, 10 months ago (2017-04-24 22:47:52 UTC) #26
mlamouri (slow - plz ping)
lgtm. Sorry for the delay. I took Monday off-reviews :) https://codereview.chromium.org/2805553004/diff/80001/content/browser/media/media_capabilities_browsertest.cc File content/browser/media/media_capabilities_browsertest.cc (right): https://codereview.chromium.org/2805553004/diff/80001/content/browser/media/media_capabilities_browsertest.cc#newcode39 ...
2 years, 10 months ago (2017-04-25 12:48:44 UTC) #29
chcunningham
Thanks everyone. https://codereview.chromium.org/2805553004/diff/80001/content/browser/media/media_capabilities_browsertest.cc File content/browser/media/media_capabilities_browsertest.cc (right): https://codereview.chromium.org/2805553004/diff/80001/content/browser/media/media_capabilities_browsertest.cc#newcode39 content/browser/media/media_capabilities_browsertest.cc:39: MediaCapabilitiesTest() {} On 2017/04/25 12:48:43, mlamouri wrote: ...
2 years, 10 months ago (2017-04-25 20:53:51 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2805553004/100001
2 years, 10 months ago (2017-04-25 20:55:27 UTC) #33
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/201536)
2 years, 10 months ago (2017-04-25 21:47:41 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2805553004/120001
2 years, 10 months ago (2017-04-25 22:24:33 UTC) #38
commit-bot: I haz the power
2 years, 10 months ago (2017-04-26 00:53:16 UTC) #41
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/20645b6c0f3fa1fe3d20742ca7fc...

Powered by Google App Engine
This is Rietveld 408576698