Use encryption scheme enum from content_decryption_module.h rather than re-implementing as a GMP type
Categories
(Core :: Audio/Video: GMP, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox81 | --- | fixed |
People
(Reporter: bryce, Assigned: bryce)
References
Details
Attachments
(1 file)
The CDM headers have a definition of encryption scheme[0] we can use rather than re-implementing our own type[1]. Historically we've used our own types, but in this case we're just adding an extra conversion step.
If we ever need more flexibility we can add back in our own type, but for now this allows us to cut some code.
[0] https://searchfox.org/mozilla-central/rev/3b6958c26049c1e27b2790a43154caaba9f6dd4a/dom/media/gmp/widevine-adapter/content_decryption_module.h#101
[1] https://searchfox.org/mozilla-central/rev/3b6958c26049c1e27b2790a43154caaba9f6dd4a/dom/media/gmp/gmp-api/gmp-video-codec.h#222
Assignee | ||
Comment 1•4 years ago
|
||
This removes the need to do some conversions and simplifies the code a little.
Drive by remove an assert which is already covered by a switch statement
containing a MOZ_ASSERT_UNREACHABLE in ChromiumCDMParent.
Comment 3•4 years ago
|
||
Backed out for bustages on GMPTypes.h
Backout link: https://hg.mozilla.org/integration/autoland/rev/65bce61606aad1a5c472c5275e9502c77f5dc878
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311176641&repo=autoland&lineNumber=18211
Assignee | ||
Comment 4•4 years ago
|
||
Ah, good ole X11 defines [0]. Looks like the problem is we get X11 headers which #define Status int
and then when we compile the ipdl files we include the CDM headers which have an unscoped enum named Status that gets turned into enum int : uint32_t
which breaks building the ipdl protocols[1].
We're able to use types from the CDM headers in other ipc[2]. I'm not 100% clear on what exactly is going on with the build. Chromium have updated their headers so the enum causing us problems here no longer exists. Re-pulling the headers may be an easier way to fix this than messing with the build. It certainly is from my perspective.
[0] https://searchfox.org/mozilla-central/source/gfx/src/X11UndefineNone.h
[1] https://searchfox.org/mozilla-central/rev/c6676771df58c6e0098574bc6b11517acbf264cf/python/mozbuild/mozbuild/frontend/data.py#377
[2] https://searchfox.org/mozilla-central/rev/c6676771df58c6e0098574bc6b11517acbf264cf/dom/media/gmp/PChromiumCDM.ipdl#9
Assignee | ||
Comment 5•4 years ago
|
||
Re-pulling headers may not work, as I see there exist at least one other Status
enum that continues to exist after the update. I wonder if the #define trampling here but not in the other files is a product of the unified build boundaries. Going to be on PTO shortly, holding NI for when I'm back.
Assignee | ||
Comment 6•4 years ago
|
||
Suggestion to self when I return to this is to create a header that contains the CDM header, but ensures Status is undefined. We already do this in GMPUtils [0], we can just shift that into a unified build friendly header so that we can use the CDM symbols without breaking the build in different contexts. Gfx does similarly for X11 [1].
Alternatively we could find what's dragging X11 headers into the unified build and fix that. In some ways that seems more ideal, but I don't know how to quickly do that, and the above seems like a more pragmatic use of time.
[0] https://searchfox.org/mozilla-central/rev/c6676771df58c6e0098574bc6b11517acbf264cf/dom/media/gmp/GMPMessageUtils.h#13
[1] https://searchfox.org/mozilla-central/rev/c6676771df58c6e0098574bc6b11517acbf264cf/gfx/src/X11Util.h#15
Comment 7•4 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:bryce, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 8•4 years ago
•
|
||
Thanks bot. Been on pto, but acking the need to action this.
Comment 10•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Description
•