Move WidevineBuffer and WidevineDecryptedBlock out into WidevineUtils.h

RESOLVED FIXED in Firefox 54

Status

()

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

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

()

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

Attachments

(1 attachment)

(Assignee)

Description

10 months ago
This will make them easier to reuse in the new Chromium CDM code.
Comment hidden (mozreview-request)

Comment 2

10 months ago
mozreview-review
Comment on attachment 8839762 [details]
Bug 1341497 - Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils.

https://reviewboard.mozilla.org/r/114328/#review115764

r+ with suggestion:

::: dom/media/gmp/widevine-adapter/WidevineUtils.h:75
(Diff revision 1)
> +  uint32_t Capacity() const override;
> +  uint8_t* Data() override;
> +  void SetSize(uint32_t aSize) override;
> +  uint32_t Size() const override;
> +
> +  nsTArray<uint8_t> mBuffer;

I see that the commit description says "make WidevineBuffer::mBuffer public, so the contents of the buffer can be Move()ed by Gecko code."

I would suggest you keep the member private, and instead provide a move-accessor, e.g.:
`nsTArray<uint8_t>&& MoveBuffer() { return move(mBuffer); }`
Or if you want to guarantee that mBuffer will be empty afterward:
`nsTArray<uint8_t> ExtractBuffer() { nsTArray out; out.SwapElements(mBuffer); return out; }`
Attachment #8839762 - Flags: review?(gsquelart) → review+
(Assignee)

Comment 3

10 months ago
mozreview-review-reply
Comment on attachment 8839762 [details]
Bug 1341497 - Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils.

https://reviewboard.mozilla.org/r/114328/#review115764

> I see that the commit description says "make WidevineBuffer::mBuffer public, so the contents of the buffer can be Move()ed by Gecko code."
> 
> I would suggest you keep the member private, and instead provide a move-accessor, e.g.:
> `nsTArray<uint8_t>&& MoveBuffer() { return move(mBuffer); }`
> Or if you want to guarantee that mBuffer will be empty afterward:
> `nsTArray<uint8_t> ExtractBuffer() { nsTArray out; out.SwapElements(mBuffer); return out; }`

Thanks, that's a good idea. I'll add an ExtractBuffer() function.
Comment hidden (mozreview-request)

Comment 5

10 months ago
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5ec123aec8aa
Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils. r=gerald

Comment 6

10 months ago
sorry had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=79226858&repo=autoland&lineNumber=12839

Comment 7

10 months ago
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d6717b64c82
Backed out changeset 5ec123aec8aa for bustage
Comment hidden (mozreview-request)

Comment 9

10 months ago
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1bc86128b84e
Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils. r=gerald

Comment 10

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