Closed Bug 1341497 Opened 3 years ago Closed 3 years ago

Move WidevineBuffer and WidevineDecryptedBlock out into WidevineUtils.h

Categories

(Core :: Audio/Video: GMP, defect, P3)

defect

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 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+
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.
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5ec123aec8aa
Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils. r=gerald
Backout by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d6717b64c82
Backed out changeset 5ec123aec8aa for bustage
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1bc86128b84e
Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils. r=gerald
https://hg.mozilla.org/mozilla-central/rev/1bc86128b84e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.