Closed
Bug 1341497
Opened 7 years ago
Closed 7 years ago
Move WidevineBuffer and WidevineDecryptedBlock out into WidevineUtils.h
Categories
(Core :: Audio/Video: GMP, defect, P3)
Core
Audio/Video: GMP
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: cpearce, Assigned: cpearce)
References
Details
Attachments
(1 file)
This will make them easier to reuse in the new Chromium CDM code.
Comment hidden (mozreview-request) |
Comment 2•7 years 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•7 years 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) |
Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5ec123aec8aa Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils. r=gerald
Comment 6•7 years ago
|
||
sorry had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=79226858&repo=autoland&lineNumber=12839
Backout by ihsiao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7d6717b64c82 Backed out changeset 5ec123aec8aa for bustage
Comment hidden (mozreview-request) |
Pushed by cpearce@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1bc86128b84e Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils. r=gerald
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1bc86128b84e
Status: NEW → RESOLVED
Closed: 7 years 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.
Description
•