Encrypted Media: Enforcing the CDM to decode audio into S16 integers.

Encrypted Media: Enforcing the CDM to decode audio into S16 integers.

After the new FFmpeg roll, FFmpeg decodes into float instead of integers. Therefore, it may be confusing about what the CDMs are returning. In this CL, we limit the DecryptingAudioDecoder to support S16 only to work around this issue. All CDM implementations are expected to return S16.

This is a short term fix for the audio corruption in CDMs. In the future, we need to be able to accept any format returned by the CDMs (see http://crbug.com/169105).

BUG=168862
TEST=media_unittests pass; clearkeycdm audio works well.


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

[xhwang, 2013-01-10 00:49:28]

It's a pain to keep ffmpeg_cdm_audio_decoder.cc in sync with
ffmpeg_audio_decoder.cc. I am making some changes so that these two files share
as much code as possible. But it would be nice to have a better long term
solution for this too :)

PTAL!

[DaleCurtis, 2013-01-10 01:24:35]

You mention you're working on having these classes share code? Is there a bug
tracking this? It makes me sad to see all this duplicated code.

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
File media/filters/decrypting_audio_decoder.cc (right):

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
media/filters/decrypting_audio_decoder.cc:232: SetDecoderConfig();
Is this a bug fix?

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
File media/filters/decrypting_audio_decoder.h (right):

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
media/filters/decrypting_audio_decoder.h:29: // We do not currently have a way
to let the Decrytptor choose the output
s/Decrytptor/Decryptor/

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
media/filters/decrypting_audio_decoder.h:30: // audio sample format and notify
us its choice. Therefore, we require all
s/its/of its/

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
media/filters/decrypting_audio_decoder.h:32: // format with
kSupportedBitsPerChannel.
s/with/designated by/

https://codereview.chromium.org/11778079/diff/1/webkit/media/crypto/ppapi/ffm...
File webkit/media/crypto/ppapi/ffmpeg_cdm_audio_decoder.cc (right):

https://codereview.chromium.org/11778079/diff/1/webkit/media/crypto/ppapi/ffm...
webkit/media/crypto/ppapi/ffmpeg_cdm_audio_decoder.cc:50: LOG(ERROR) <<
"ClearKey CDM bpc: " << config.bits_per_channel;
Remove?

[xhwang, 2013-01-10 18:17:23]

I filed a bug to track the duplication of ffmpeg audio decoder code:
http://crbug.com/169203

Also I split SetDecoderConfig() change into another CL as it addresses different
issues. 

PTAL!

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
File media/filters/decrypting_audio_decoder.cc (right):

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
media/filters/decrypting_audio_decoder.cc:232: SetDecoderConfig();
On 2013/01/10 01:24:35, DaleCurtis wrote:
> Is this a bug fix?

Hmm, yes, filed bug and split this change to
https://codereview.chromium.org/11824066/

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
File media/filters/decrypting_audio_decoder.h (right):

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
media/filters/decrypting_audio_decoder.h:29: // We do not currently have a way
to let the Decrytptor choose the output
On 2013/01/10 01:24:35, DaleCurtis wrote:
> s/Decrytptor/Decryptor/

Done.

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
media/filters/decrypting_audio_decoder.h:30: // audio sample format and notify
us its choice. Therefore, we require all
On 2013/01/10 01:24:35, DaleCurtis wrote:
> s/its/of its/

Done.

https://codereview.chromium.org/11778079/diff/1/media/filters/decrypting_audi...
media/filters/decrypting_audio_decoder.h:32: // format with
kSupportedBitsPerChannel.
On 2013/01/10 01:24:35, DaleCurtis wrote:
> s/with/designated by/

Done.

https://codereview.chromium.org/11778079/diff/1/webkit/media/crypto/ppapi/ffm...
File webkit/media/crypto/ppapi/ffmpeg_cdm_audio_decoder.cc (right):

https://codereview.chromium.org/11778079/diff/1/webkit/media/crypto/ppapi/ffm...
webkit/media/crypto/ppapi/ffmpeg_cdm_audio_decoder.cc:50: LOG(ERROR) <<
"ClearKey CDM bpc: " << config.bits_per_channel;
On 2013/01/10 01:24:35, DaleCurtis wrote:
> Remove?

Done.

[DaleCurtis, 2013-01-10 21:45:16]

lgtm; refactoring to remove duplicated code can't come soon enough though!

[commit-bot: I haz the power, 2013-01-12 17:32:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/11778079/3002

[commit-bot: I haz the power, 2013-01-12 17:53:15]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-13 03:47:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/11778079/17002

[commit-bot: I haz the power, 2013-01-13 04:09:28]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-13 06:44:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/11778079/21002

[commit-bot: I haz the power, 2013-01-13 06:57:49]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-14 23:11:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/11778079/21005

[commit-bot: I haz the power, 2013-01-15 06:25:06]

Change committed as 176805