Closed Bug 1140797 Opened 5 years ago Closed 5 years ago

[EME] Make gmp-clearkey buildable outside of mozilla-central

Categories

(Core :: Audio/Video, defect)

37 Branch
x86_64
Windows 8.1
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox37 --- fixed
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

I've overwritten my non-spec-compliant ClearKey GMP with the one we have in mozilla-central. I required a few small changes in order to make the code build outside of m-c.
Basic include/char encoding fixes to make building gmp-clearkey outside of m-c possible.
Attachment #8574355 - Flags: review?(edwin)
The bsae64decode of keyIds and keys in the gmp still isn't quite right. When build outside of m-c, DecodeBase64() asserts when we do the "*(++out) = aEncoded[i] << (shift + 2);" line on the last 6bit-octet, as the iterator is pointing to aOutDecoded[aEncoded.length() * 6 / 8], whereas the size is one less than that. Note that we call:
reserve aOutDecoded.reserve(aEncoded.length() * 6 / 8 + 1)

so we won't actually be writing into unallocated memory, so what we're doing now isn't fatal. But when we try to build and run this code out side of m-c, the STL asserts that we're writing outside of the bounds of the size of the vector (which we are).

We only use DecodeBase64() for decoding 128 bit keyIds and keys, so 128/6=21.3~, so this last byte we're writing is padding. We don't need it. So we can just check for this last bit and not write it, since it's padding.
Attachment #8574356 - Flags: review?(edwin)
Blocks: 1140549
https://hg.mozilla.org/mozilla-central/rev/58d2dbe695c4
https://hg.mozilla.org/mozilla-central/rev/cd14abe25846
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Will need this so that we can take Bug 1141386 cleanly.
Flags: needinfo?(cpearce)
Comment on attachment 8574355 [details] [diff] [review]
Patch: Basic compile fixes

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Without this, it's harder to uplift Bug 1141386, which is required for EME to work on some EME sites
[Describe test coverage new/current, TreeHerder]: The changes here only affect the build. The build works. ;)
[Risks and why]: Low, the changes here only affect the build.
[String/UUID change made/needed]: None
Attachment #8574355 - Flags: approval-mozilla-aurora?
Comment on attachment 8574356 [details] [diff] [review]
Patch: prevent assert in base64 decode of keyIds

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Without this, it's harder to uplift Bug 1141386, which is required for EME to work on some EME sites
[Describe test coverage new/current, TreeHerder]: Local testing, and there's a test in Bug 1141386 to cover the code changes made here, which I shall also request approval for uplift on.
[Risks and why]: Low, isolated to EME.
[String/UUID change made/needed]: None
Attachment #8574356 - Flags: approval-mozilla-aurora?
Attachment #8574355 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8574356 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This needs to get backported to mozilla-beta (Firefox 37) pretty soon (the final beta goes to build next Thursday), otherwise we won't be able to provide the XULRunner SDK to add-on developers.
Comment on attachment 8574355 [details] [diff] [review]
Patch: Basic compile fixes

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Nil. This code is not enabled on Firefox 38. Some developers will not be able to build the xulrunner SDK without these patches however.
[Describe test coverage new/current, TreeHerder]: Mochitests
[Risks and why]: Low
[String/UUID change made/needed]: None
Flags: needinfo?(cpearce)
Attachment #8574355 - Flags: approval-mozilla-beta?
Comment on attachment 8574356 [details] [diff] [review]
Patch: prevent assert in base64 decode of keyIds

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: Nil. This code is not enabled on Firefox 38. Some developers will not be able to build the xulrunner SDK without these patches however.
[Describe test coverage new/current, TreeHerder]: Mochitests
[Risks and why]: Low
[String/UUID change made/needed]: None
Attachment #8574356 - Flags: approval-mozilla-beta?
Flags: needinfo?(cpearce)
Comment on attachment 8574355 [details] [diff] [review]
Patch: Basic compile fixes

As stated in the approval request, this code will not be executed in the release but is required for the xulrunner SDK build. Beta+
Attachment #8574355 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8574356 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: needinfo?(cpearce)
Duplicate of this bug: 1140549
You need to log in before you can comment on or make changes to this bug.