FFmpegDemuxer: Clear |extra_data| when enabling bitstream conversion.

FFmpegDemuxer: Clear |extra_data| when enabling bitstream conversion.

After this change, fallback from GpuVideoDecoder to FFmpegVideoDecoder
should work. The exact mechanism is perhaps surprising:

  - After successfully initializing GpuVideoDecoder, bitstream
    conversion is enabled. We can't easily turn it off again when we
    fall back.

  - FFmpegVideoDecoder can decode a converted bitstream, but only if it
    knows the conversion has occurred. It checks for the presence of an
    AVCC record in |extra_data| to deduce the bitstream format.

  - Clearing |extra_data| is reasonable, because the AVCC record serves
    no purpose for an Annex B stream. (Note: the MSE path doesn't
    provide |exta_data| at all.)

  - Converting |extra_data| into a different format (eg. SPS + PPS
    concatenated in Annex B format) would be logical, but currently
    would serve no purpose (in particular because the conversion would
    happen after Initialize()).

  - The result then is that |extra_data| is cleared after initializing
    GpuVideoDecoder. Upon fallback, FFmpegVideoDecoder will interpret
    the alread-converted bitstream correctly. GpuVideoDecoder still
    receives |extra_data|, and on Android actually makes uses of it.

  - Note that if GpuVideoDecoder is not initialized successfully, then
    bitstream conversion is never enabled and FFmpegVideoDecoder works
    exactly as it did before (with |extra_data| passed in the codec
    context).

BUG=605790
Committed: https://crrev.com/e4041392035ece38d99818ea1bbe8f912a27a19e
Cr-Commit-Position: refs/heads/master@{#427781}

[sandersd (OOO until July 31), 2016-10-25 00:09:44]

sandersd@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2016-10-25 00:18:47]

This is hairy enough to need a test. Is it possible to add one?

https://codereview.chromium.org/2448763002/diff/1/media/base/video_decoder_co...
File media/base/video_decoder_config.cc (right):

https://codereview.chromium.org/2448763002/diff/1/media/base/video_decoder_co...
media/base/video_decoder_config.cc:71: void VideoDecoderConfig::set_extra_data(
Must be inlined to use hacker_style(). We have no other setters on this class,
so instead of making it mutable why not just construct a new one in the demuxer?

[sandersd (OOO until July 31), 2016-10-25 20:58:42]

https://codereview.chromium.org/2448763002/diff/1/media/base/video_decoder_co...
File media/base/video_decoder_config.cc (right):

https://codereview.chromium.org/2448763002/diff/1/media/base/video_decoder_co...
media/base/video_decoder_config.cc:71: void VideoDecoderConfig::set_extra_data(
On 2016/10/25 00:18:46, DaleCurtis wrote:
> Must be inlined to use hacker_style(). We have no other setters on this class,
> so instead of making it mutable why not just construct a new one in the
demuxer?

Done.

[DaleCurtis, 2016-10-25 21:28:00]

lgtm as is, but you can probably reduce the places at which you added the new
parameters in favor of some default values.

[sandersd (OOO until July 31), 2016-10-25 22:22:57]

The CQ bit was checked by sandersd@chromium.org

[sandersd (OOO until July 31), 2016-10-25 22:22:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2448763002/#ps60001
(title: "Remove useless default.")

[commit-bot: I haz the power, 2016-10-25 22:23:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-25 22:26:38]

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

[commit-bot: I haz the power, 2016-10-25 22:26:39]

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/bui...)

[sandersd (OOO until July 31), 2016-10-26 00:40:12]

The CQ bit was checked by sandersd@chromium.org

[sandersd (OOO until July 31), 2016-10-26 00:40:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2448763002/#ps80001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-10-26 00:40:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 01:09:30]

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

[commit-bot: I haz the power, 2016-10-26 01:09:31]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sandersd (OOO until July 31), 2016-10-26 18:05:04]

The CQ bit was checked by sandersd@chromium.org

[sandersd (OOO until July 31), 2016-10-26 18:05:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2448763002/#ps100001
(title: "Restore variant used by fuzzertest.")

[commit-bot: I haz the power, 2016-10-26 18:05:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 19:49:48]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-26 20:00:22]

Description was changed from

==========
FFmpegDemuxer: Clear |extra_data| when enabling bitstream conversion.

After this change, fallback from GpuVideoDecoder to FFmpegVideoDecoder
should work. The exact mechanism is perhaps surprising:

  - After successfully initializing GpuVideoDecoder, bitstream
    conversion is enabled. We can't easily turn it off again when we
    fall back.

  - FFmpegVideoDecoder can decode a converted bitstream, but only if it
    knows the conversion has occurred. It checks for the presence of an
    AVCC record in |extra_data| to deduce the bitstream format.

  - Clearing |extra_data| is reasonable, because the AVCC record serves
    no purpose for an Annex B stream. (Note: the MSE path doesn't
    provide |exta_data| at all.)

  - Converting |extra_data| into a different format (eg. SPS + PPS
    concatenated in Annex B format) would be logical, but currently
    would serve no purpose (in particular because the conversion would
    happen after Initialize()).

  - The result then is that |extra_data| is cleared after initializing
    GpuVideoDecoder. Upon fallback, FFmpegVideoDecoder will interpret
    the alread-converted bitstream correctly. GpuVideoDecoder still
    receives |extra_data|, and on Android actually makes uses of it.

  - Note that if GpuVideoDecoder is not initialized successfully, then
    bitstream conversion is never enabled and FFmpegVideoDecoder works
    exactly as it did before (with |extra_data| passed in the codec
    context).

BUG=605790
==========

to

==========
FFmpegDemuxer: Clear |extra_data| when enabling bitstream conversion.

After this change, fallback from GpuVideoDecoder to FFmpegVideoDecoder
should work. The exact mechanism is perhaps surprising:

  - After successfully initializing GpuVideoDecoder, bitstream
    conversion is enabled. We can't easily turn it off again when we
    fall back.

  - FFmpegVideoDecoder can decode a converted bitstream, but only if it
    knows the conversion has occurred. It checks for the presence of an
    AVCC record in |extra_data| to deduce the bitstream format.

  - Clearing |extra_data| is reasonable, because the AVCC record serves
    no purpose for an Annex B stream. (Note: the MSE path doesn't
    provide |exta_data| at all.)

  - Converting |extra_data| into a different format (eg. SPS + PPS
    concatenated in Annex B format) would be logical, but currently
    would serve no purpose (in particular because the conversion would
    happen after Initialize()).

  - The result then is that |extra_data| is cleared after initializing
    GpuVideoDecoder. Upon fallback, FFmpegVideoDecoder will interpret
    the alread-converted bitstream correctly. GpuVideoDecoder still
    receives |extra_data|, and on Android actually makes uses of it.

  - Note that if GpuVideoDecoder is not initialized successfully, then
    bitstream conversion is never enabled and FFmpegVideoDecoder works
    exactly as it did before (with |extra_data| passed in the codec
    context).

BUG=605790

Committed: https://crrev.com/e4041392035ece38d99818ea1bbe8f912a27a19e
Cr-Commit-Position: refs/heads/master@{#427781}
==========

[commit-bot: I haz the power, 2016-10-26 20:00:23]

Patchset 6 (id:??) landed as
https://crrev.com/e4041392035ece38d99818ea1bbe8f912a27a19e
Cr-Commit-Position: refs/heads/master@{#427781}