1319197 - Remove audio decoding from gmp-clearkey

Status: RESOLVED FIXED

(Core :: Audio/Video: GMP, defect, P3)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

We should remove the audio decoding code from gmp-clearkey.

The Widevine CDM doesn't contain an audio decoder, and one of the key goals of gmp-clearkey is to provide test coverage for the things that Widevine does, so we should ensure we're testing the audio decryption (rather than decoding) code path  in gmp-clearkey on Windows.

We'll also be removing audio decode capability when we port gmp-clearkey to use the Chromium CDM API. So we may as well remove it before we start.

Comment 1

[jay.harris]

Attachment: Bug 1319197 - Remove audio decoding from gmp-clearkey

Review commit: https://reviewboard.mozilla.org/r/94872/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/94872/

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8813464 [details]
Bug 1319197 - Remove audio decoding from gmp-clearkey

https://reviewboard.mozilla.org/r/94872/#review95088

::: media/gmp-clearkey/0.1/gmp-clearkey.cpp:65
(Diff revision 1)
>  #if defined(ENABLE_WMF)
> -  else if (!strcmp(aApiName, GMP_API_AUDIO_DECODER) &&
> + else if (!strcmp(aApiName, GMP_API_VIDEO_DECODER) &&
> -           wmf::EnsureLibs()) {
> -    *aPluginAPI = new AudioDecoder(static_cast<GMPAudioHost*>(aHostAPI));
> -  } else if (!strcmp(aApiName, GMP_API_VIDEO_DECODER) &&
>               wmf::EnsureLibs()) {

You also should change wmf::EnsureLibs() to not check for the AAC decoder DLL/module, and WMFDecoderDllNameFor() too. That's in media/gmp-clearkey/0.1/WMFUtils.cpp.

Comment 3

[jay.harris]

Comment on attachment 8813464 [details]
Bug 1319197 - Remove audio decoding from gmp-clearkey

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/94872/diff/1-2/

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8813464 [details]
Bug 1319197 - Remove audio decoding from gmp-clearkey

https://reviewboard.mozilla.org/r/94872/#review95092

::: media/gmp-clearkey/0.1/WMFUtils.cpp:72
(Diff revision 2)
>    }
>    return sInitOk;
>  }
>  
>  const char*
>  WMFDecoderDllNameFor(CodecType aCodec)

WMFDecoderDllNameFor only works for H.264, so we may as well rename it to WMFH264DecoderDllName, and remove the CodecType enum as well.

Comment 5

[jay.harris]

Comment on attachment 8813464 [details]
Bug 1319197 - Remove audio decoding from gmp-clearkey

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/94872/diff/2-3/

Comment 6

[jay.harris]

TreeHerder Build for the current commit:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=17fb7d578300842a2d78ce92e173861f2f299996

Comment 7

[jay.harris]

Comment on attachment 8813464 [details]
Bug 1319197 - Remove audio decoding from gmp-clearkey

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/94872/diff/3-4/

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8813464 [details]
Bug 1319197 - Remove audio decoding from gmp-clearkey

r+. For some reason I can't mark this as review-granted inside reviewboard; I keep getting 500 errors.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee43b0af2a45
Remove audio decoding from gmp-clearkey. r=cpearce

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ee43b0af2a45