Remove audio decoding from gmp-clearkey

RESOLVED FIXED in Firefox 53

Status

()

Core
Audio/Video: GMP
P3
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: cpearce, Assigned: jay.harris)

Tracking

unspecified
mozilla53
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
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 hidden (mozreview-request)
(Reporter)

Comment 2

5 months ago
mozreview-review
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.
Attachment #8813464 - Flags: review?(cpearce) → review-
Comment hidden (mozreview-request)
(Reporter)

Comment 4

5 months ago
mozreview-review
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.
Attachment #8813464 - Flags: review?(cpearce) → review-
Comment hidden (mozreview-request)
(Assignee)

Comment 6

5 months ago
TreeHerder Build for the current commit:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=17fb7d578300842a2d78ce92e173861f2f299996
Comment hidden (mozreview-request)
(Reporter)

Comment 8

5 months ago
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.
Attachment #8813464 - Flags: review?(cpearce) → review+
(Assignee)

Updated

5 months ago
Keywords: checkin-needed

Comment 9

5 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee43b0af2a45
Remove audio decoding from gmp-clearkey. r=cpearce
Keywords: checkin-needed

Comment 10

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ee43b0af2a45
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.