Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(23)

Issue 16830004: media: Remove VP9 flag, and enable VP9 support by default. (Closed)

Created:
7 years, 6 months ago by Tom Finegan
Modified:
7 years, 6 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, feature-media-reviews_chromium.org, pangu
Visibility:
Public.

Description

media: Remove VP9 flag, and enable VP9 support by default. BUG=180280 TEST=VP9 pipeline integration tests pass Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=206530

Patch Set 1 #

Total comments: 4

Patch Set 2 : Fix EAC3 breakage! #

Patch Set 3 : Add canPlayType() support for VP9. #

Total comments: 2

Patch Set 4 : Fix nit in generated_resources.grd #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -42 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 chunk +1 line, -8 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 chunk +0 lines, -1 line 0 comments Download
M media/base/media_switches.h View 1 chunk +0 lines, -2 lines 0 comments Download
M media/base/media_switches.cc View 1 chunk +0 lines, -3 lines 0 comments Download
M media/base/run_all_unittests.cc View 1 chunk +1 line, -4 lines 0 comments Download
M media/filters/stream_parser_factory.cc View 1 1 chunk +4 lines, -10 lines 0 comments Download
M media/filters/vpx_video_decoder.cc View 1 chunk +1 line, -3 lines 0 comments Download
M net/base/mime_util.cc View 1 2 2 chunks +2 lines, -1 line 0 comments Download
M webkit/renderer/media/webmediaplayer_impl.cc View 1 chunk +1 line, -4 lines 0 comments Download

Messages

Total messages: 20 (0 generated)
Tom Finegan
Hello reviewers, This CL, while small, touches a few different places in the tree. acolwell: ...
7 years, 6 months ago (2013-06-12 19:46:13 UTC) #1
scherkus (not reviewing)
https://codereview.chromium.org/16830004/diff/1/webkit/renderer/media/webmediaplayer_impl.cc File webkit/renderer/media/webmediaplayer_impl.cc (right): https://codereview.chromium.org/16830004/diff/1/webkit/renderer/media/webmediaplayer_impl.cc#newcode1081 webkit/renderer/media/webmediaplayer_impl.cc:1081: video_decoders.push_back(new media::VpxVideoDecoder(media_loop_)); OOC this will still prefer FFmpeg's ffvp8 ...
7 years, 6 months ago (2013-06-12 20:06:31 UTC) #2
acolwell GONE FROM CHROMIUM
LGTM % scherkus' comment
7 years, 6 months ago (2013-06-12 20:24:38 UTC) #3
wolenetz
LGTM (stream_parser_factory), though I've added jrummell@ for closer review of stream_parser_factory changes.
7 years, 6 months ago (2013-06-12 20:32:28 UTC) #4
Tom Finegan
Oops, forgot thought that you did the histogram stuff, wolenetz. Thanks! PTAL, jrummel. :) https://codereview.chromium.org/16830004/diff/1/webkit/renderer/media/webmediaplayer_impl.cc ...
7 years, 6 months ago (2013-06-12 20:35:49 UTC) #5
scherkus (not reviewing)
lgtm!
7 years, 6 months ago (2013-06-12 21:48:49 UTC) #6
jrummell
Minor problem with deleting too much code. https://codereview.chromium.org/16830004/diff/1/media/filters/stream_parser_factory.cc File media/filters/stream_parser_factory.cc (left): https://codereview.chromium.org/16830004/diff/1/media/filters/stream_parser_factory.cc#oldcode203 media/filters/stream_parser_factory.cc:203: const CommandLine* ...
7 years, 6 months ago (2013-06-12 22:05:53 UTC) #7
Tom Finegan
https://codereview.chromium.org/16830004/diff/1/media/filters/stream_parser_factory.cc File media/filters/stream_parser_factory.cc (left): https://codereview.chromium.org/16830004/diff/1/media/filters/stream_parser_factory.cc#oldcode203 media/filters/stream_parser_factory.cc:203: const CommandLine* cmd_line = CommandLine::ForCurrentProcess(); On 2013/06/12 22:05:53, jrummell ...
7 years, 6 months ago (2013-06-12 23:17:48 UTC) #8
jochen (gone - plz use gerrit)
lgtm would you mind sending a PSA about this to blink-dev@chromium.org (as it's a web ...
7 years, 6 months ago (2013-06-13 07:49:10 UTC) #9
jrummell
lgtm
7 years, 6 months ago (2013-06-13 16:24:39 UTC) #10
Tom Finegan
On 2013/06/13 07:49:10, jochen wrote: > lgtm > > would you mind sending a PSA ...
7 years, 6 months ago (2013-06-13 17:26:39 UTC) #11
Tom Finegan
Oops! I forgot to add the canPlayType() support. willchan: Please do an OWNERS review of ...
7 years, 6 months ago (2013-06-13 17:27:48 UTC) #12
wolenetz
Sorry, I just spotted this nit. My previous lgtm stands % canPlayType() review and this ...
7 years, 6 months ago (2013-06-13 17:49:02 UTC) #13
Tom Finegan
https://codereview.chromium.org/16830004/diff/19001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/16830004/diff/19001/chrome/app/generated_resources.grd#newcode6747 chrome/app/generated_resources.grd:6747: + <message name="IDS_FLAGS_ENABLE_VP8_ALPHA_PLAYBACK_NAME" desc="Title for the flag to enable ...
7 years, 6 months ago (2013-06-13 17:53:41 UTC) #14
willchan no longer on Chromium
net/ LGTM. I'm still disappointed with the layering violation, but it's no worse than what's ...
7 years, 6 months ago (2013-06-13 20:30:47 UTC) #15
scherkus (not reviewing)
On 2013/06/13 20:30:47, willchan wrote: > net/ LGTM. I'm still disappointed with the layering violation, ...
7 years, 6 months ago (2013-06-13 21:36:34 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tomfinegan@chromium.org/16830004/28001
7 years, 6 months ago (2013-06-14 17:23:49 UTC) #17
commit-bot: I haz the power
Retried try job too often on win7_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&number=50331
7 years, 6 months ago (2013-06-14 22:52:47 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tomfinegan@chromium.org/16830004/28001
7 years, 6 months ago (2013-06-14 22:54:19 UTC) #19
commit-bot: I haz the power
7 years, 6 months ago (2013-06-15 00:50:57 UTC) #20
Message was sent while issue was closed.
Change committed as 206530

Powered by Google App Engine
This is Rietveld 408576698