Closed Bug 1314858 Opened 3 years ago Closed 3 years ago

Make GMPDecoderModule::SupportsMimeType call GMPService::HasPluginForAPI() directly

Categories

(Core :: Audio/Video: Playback, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

Details

Attachments

(6 files)

The GMPDecoderModule doesn't need to maintain a table of the GMP's capabilities; in bug 1314445 we made the GMPService maintain its own table, we don't need to maintain two.
Comment on attachment 8807062 [details]
Bug 1314858 - Add MP4Decoder::IsAAC.

https://reviewboard.mozilla.org/r/90340/#review90040

r+ with nit:

::: dom/media/fmp4/MP4Decoder.h:1
(Diff revision 1)
>  /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

In the commit description: "This is used in the next patch."
Lies, I tell you!
It's not used in *the* next patch, but the one after. You could be more vague or just remove it.
Attachment #8807062 - Flags: review?(gsquelart) → review+
Comment on attachment 8807063 [details]
Bug 1314858 - Make content process cache of GMP capabilities thread safe.

https://reviewboard.mozilla.org/r/90342/#review90052
Attachment #8807063 - Flags: review?(gsquelart) → review+
Comment on attachment 8807064 [details]
Bug 1314858 - Make GMPDecoderModule::SupportsMimeType call GMPService::HasPluginForAPI() directly.

https://reviewboard.mozilla.org/r/90344/#review90056
Attachment #8807064 - Flags: review?(gsquelart) → review+
Comment on attachment 8807065 [details]
Bug 1314858 - Remove GMPDecoderModule::UpdateUsableCodecs.

https://reviewboard.mozilla.org/r/90346/#review90058
Attachment #8807065 - Flags: review?(gsquelart) → review+
Comment on attachment 8807066 [details]
Bug 1314858 - Remove PContent.NotifyGMPsChanged.

https://reviewboard.mozilla.org/r/90348/#review90062
Attachment #8807066 - Flags: review?(gsquelart) → review+
Comment on attachment 8807067 [details]
Bug 1314858 - Refactor multiple callers of HasPluginForAPI into a helper.

https://reviewboard.mozilla.org/r/90350/#review90064

r+, but finally found something to complain about:

::: dom/media/gmp/GMPUtils.h:85
(Diff revision 1)
>                 nsCString& aOutDst,
>                 size_t aMaxLength);
>  
> +bool
> +HaveGMPFor(const nsCString& aAPI,
> +           nsTArray<nsCString> && aTags);

Remove space before '&&'.
(and same in the cpp)
Attachment #8807067 - Flags: review?(gsquelart) → review+
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/018ce7c8c415
Add MP4Decoder::IsAAC. r=gerald
https://hg.mozilla.org/integration/autoland/rev/86b73731ba98
Make content process cache of GMP capabilities thread safe. r=gerald
https://hg.mozilla.org/integration/autoland/rev/a37b70e85582
Make GMPDecoderModule::SupportsMimeType call GMPService::HasPluginForAPI() directly. r=gerald
https://hg.mozilla.org/integration/autoland/rev/d0325b2bd63c
Remove GMPDecoderModule::UpdateUsableCodecs. r=gerald
https://hg.mozilla.org/integration/autoland/rev/bc420ca09b6c
Remove PContent.NotifyGMPsChanged. r=gerald
https://hg.mozilla.org/integration/autoland/rev/0e9928989e9c
Refactor multiple callers of HasPluginForAPI into a helper. r=gerald
I see a perf win here when this landed:
== Change summary for alert #3987 (as of November 03 2016 16:42 UTC) ==

Improvements:

  3%  tresize windows8-64 opt e10s     10.7 -> 10.33

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=3987

thanks for the patches!
Thank you Joel. I was wondering if this would show up. :)
You need to log in before you can comment on or make changes to this bug.