Closed
Bug 1150437
Opened 10 years ago
Closed 10 years ago
[EME] Relicense gmp-clearkey to only depend on Apache2 code
Categories
(Core :: Audio/Video, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla40
People
(Reporter: cpearce, Assigned: cpearce)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
To make it easier for partners to reuse parts of our code in proprietary plugins, gmp-clearkey should only contain and use/#include Apache 2 licensed code.
Assignee | ||
Comment 1•10 years ago
|
||
Relicense MPL to Apache 2 in gmp-clearkey, and remove dependency on MFBT.
Will take the first review I get...
https://treeherder.mozilla.org/#/jobs?repo=try&revision=72ce6f4fee6f
Attachment #8587317 -
Flags: review?(jwwang)
Attachment #8587317 -
Flags: review?(edwin)
Attachment #8587317 -
Flags: review?(ajones)
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Comment 4•10 years ago
|
||
Comment on attachment 8587317 [details] [diff] [review]
Patch
Review of attachment 8587317 [details] [diff] [review]:
-----------------------------------------------------------------
Define NDEBUG for release builds
Attachment #8587317 -
Flags: review?(edwin) → review+
Updated•10 years ago
|
Attachment #8587317 -
Flags: review?(ajones) → review+
Updated•10 years ago
|
Attachment #8587317 -
Flags: review?(jwwang) → review+
Comment 6•10 years ago
|
||
We should fix this soon, but we don't need to uplift the license change.
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Edwin Flores [:eflores] [:edwin] from comment #5)
> Comment on attachment 8587317 [details] [diff] [review]
> Patch
>
> Review of attachment 8587317 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Define NDEBUG for release builds
I tested, and we do define NDEBUG in --disable-debug builds and don't in --enable-debug. So we're good as is.
Assignee | ||
Comment 8•10 years ago
|
||
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8587317 [details] [diff] [review]
Patch
Review of attachment 8587317 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/gmp-clearkey/0.1/RefCounted.h
@@ +57,5 @@
> + T* operator->() const { return mPtr; }
> +
> +private:
> + void Assign(T* aPtr) {
> + if (mPtr) {
There's a foot-gun here. If we assign a RefPtr to itself, we'll we'll deref mPtr before we addref it, so we'll end up destroying the thing pointed to if that's the last ref.
Comment 10•10 years ago
|
||
Comment on attachment 8587317 [details] [diff] [review]
Patch
Review of attachment 8587317 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/gmp-clearkey/0.1/RefCounted.h
@@ +57,5 @@
> + T* operator->() const { return mPtr; }
> +
> +private:
> + void Assign(T* aPtr) {
> + if (mPtr) {
I noticed and concluded it was safe because Assign() is private and there is no assign operator and RefPtr is not convertible to T*. What call path could result in self-assignment?
Comment 11•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•