Remove deprecated media switches.

Remove deprecated media switches.

These switches are no longer necessary, having shipped for multiple
stable version of Chrome now.

BUG=343976
TEST=compile.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255589

[DaleCurtis, 2014-03-06 21:45:20]

ddorwin: media/
avi: content/
vigneshv: fyi

[vignesh, 2014-03-06 21:54:09]

i was meaning to do this for vp8 alpha and opus. thanks for taking care of this.

[ddorwin, 2014-03-06 21:57:16]

LGTM with question/comment.

https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
File media/filters/vpx_video_decoder.cc (right):

https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
media/filters/vpx_video_decoder.cc:262: if (config.codec() == kCodecVP8 &&
config.format() != VideoFrame::YV12A) {
It appears the old code would not support VP8 entirely if disable-vp8-alpha was
specified. Am I reading that right? Is that because non-alpha VP8 goes through
FFmpeg? This could probably use a comment.

[Avi (use Gerrit), 2014-03-06 22:06:01]

lgtm stampity stamp

[vignesh, 2014-03-06 22:06:53]

https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
File media/filters/vpx_video_decoder.cc (right):

https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
media/filters/vpx_video_decoder.cc:262: if (config.codec() == kCodecVP8 &&
config.format() != VideoFrame::YV12A) {
On 2014/03/06 21:57:17, ddorwin wrote:
> It appears the old code would not support VP8 entirely if disable-vp8-alpha
was
> specified. Am I reading that right? Is that because non-alpha VP8 goes through
> FFmpeg? This could probably use a comment.

the current check is wrong. this will always reject VP8 Alpha because of the
second else if.

the following could be easier to follow.
if (config.codec() != kCodecVP8 && config.codec() != kCodecVP9)
  return false;
// Only VP8 videos with alpha goes through this decoder. Everything else goes
through ffmpeg.
if (config.codec() == kCodecVP8 && config.format() != VideoFrame::YV12A)
  return false;

The pipeline integration tests wouldn't have caught this because the ffmpeg
decoder still won't say no to VP8 videos with alpha, it will decode them
ignoring the alpha channel, so the pipeline won't see any errors.

[DaleCurtis, 2014-03-06 22:13:23]

https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
File media/filters/vpx_video_decoder.cc (right):

https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
media/filters/vpx_video_decoder.cc:262: if (config.codec() == kCodecVP8 &&
config.format() != VideoFrame::YV12A) {
On 2014/03/06 22:06:53, vignesh wrote:
> On 2014/03/06 21:57:17, ddorwin wrote:
> > It appears the old code would not support VP8 entirely if disable-vp8-alpha
> was
> > specified. Am I reading that right? Is that because non-alpha VP8 goes
through
> > FFmpeg? This could probably use a comment.
> 
> the current check is wrong. this will always reject VP8 Alpha because of the
> second else if.
> 
> the following could be easier to follow.
> if (config.codec() != kCodecVP8 && config.codec() != kCodecVP9)
>   return false;
> // Only VP8 videos with alpha goes through this decoder. Everything else goes
> through ffmpeg.
> if (config.codec() == kCodecVP8 && config.format() != VideoFrame::YV12A)
>   return false;
> 
> The pipeline integration tests wouldn't have caught this because the ffmpeg
> decoder still won't say no to VP8 videos with alpha, it will decode them
> ignoring the alpha channel, so the pipeline won't see any errors.

Whoops, fixed.

[ddorwin, 2014-03-06 22:15:41]

lgtm

Thank you.

[vignesh, 2014-03-06 22:16:01]

On 2014/03/06 22:13:23, DaleCurtis wrote:
>
https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
> File media/filters/vpx_video_decoder.cc (right):
> 
>
https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
> media/filters/vpx_video_decoder.cc:262: if (config.codec() == kCodecVP8 &&
> config.format() != VideoFrame::YV12A) {
> On 2014/03/06 22:06:53, vignesh wrote:
> > On 2014/03/06 21:57:17, ddorwin wrote:
> > > It appears the old code would not support VP8 entirely if
disable-vp8-alpha
> > was
> > > specified. Am I reading that right? Is that because non-alpha VP8 goes
> through
> > > FFmpeg? This could probably use a comment.
> > 
> > the current check is wrong. this will always reject VP8 Alpha because of the
> > second else if.
> > 
> > the following could be easier to follow.
> > if (config.codec() != kCodecVP8 && config.codec() != kCodecVP9)
> >   return false;
> > // Only VP8 videos with alpha goes through this decoder. Everything else
goes
> > through ffmpeg.
> > if (config.codec() == kCodecVP8 && config.format() != VideoFrame::YV12A)
> >   return false;
> > 
> > The pipeline integration tests wouldn't have caught this because the ffmpeg
> > decoder still won't say no to VP8 videos with alpha, it will decode them
> > ignoring the alpha channel, so the pipeline won't see any errors.
> 
> Whoops, fixed.

LGTM.

Also turns out, i was wrong about the tests. The tests would have caught it as
it explicitly checks for output color format for VP8 videos with alpha.

[Tom Finegan, 2014-03-06 22:16:33]

On 2014/03/06 22:13:23, DaleCurtis wrote:
>
https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
> File media/filters/vpx_video_decoder.cc (right):
> 
>
https://codereview.chromium.org/189123003/diff/1/media/filters/vpx_video_deco...
> media/filters/vpx_video_decoder.cc:262: if (config.codec() == kCodecVP8 &&
> config.format() != VideoFrame::YV12A) {
> On 2014/03/06 22:06:53, vignesh wrote:
> > On 2014/03/06 21:57:17, ddorwin wrote:
> > > It appears the old code would not support VP8 entirely if
disable-vp8-alpha
> > was
> > > specified. Am I reading that right? Is that because non-alpha VP8 goes
> through
> > > FFmpeg? This could probably use a comment.
> > 
> > the current check is wrong. this will always reject VP8 Alpha because of the
> > second else if.
> > 
> > the following could be easier to follow.
> > if (config.codec() != kCodecVP8 && config.codec() != kCodecVP9)
> >   return false;
> > // Only VP8 videos with alpha goes through this decoder. Everything else
goes
> > through ffmpeg.
> > if (config.codec() == kCodecVP8 && config.format() != VideoFrame::YV12A)
> >   return false;
> > 
> > The pipeline integration tests wouldn't have caught this because the ffmpeg
> > decoder still won't say no to VP8 videos with alpha, it will decode them
> > ignoring the alpha channel, so the pipeline won't see any errors.

Just to clarify this last bit: The test should fail if VP8A goes through FFmpeg.
The test confirms that the output is YV12A.

https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/pipe...

> 
> Whoops, fixed.

[DaleCurtis, 2014-03-06 22:21:39]

Thanks for the review!

[DaleCurtis, 2014-03-06 22:21:43]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2014-03-06 22:34:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/189123003/20001

[commit-bot: I haz the power, 2014-03-07 00:23:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-07 00:23:27]

Try jobs failed on following builders: mac_chromium_rel

[DaleCurtis, 2014-03-07 00:25:48]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2014-03-07 00:35:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/189123003/20001

[commit-bot: I haz the power, 2014-03-07 11:47:06]

Change committed as 255589