1341497 - Move WidevineBuffer and WidevineDecryptedBlock out into WidevineUtils.h

Status: RESOLVED FIXED

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

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

This will make them easier to reuse in the new Chromium CDM code.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1341497 - Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils.

This makes it easier to reuse in the ChromiumCDM code.

Also make WidevineBuffer::mBuffer public, so the contents of the buffer can be Move()ed by
Gecko code.

Review commit: https://reviewboard.mozilla.org/r/114328/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/114328/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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; }`

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

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 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8839762 [details]
Bug 1341497 - Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114328/diff/1-2/

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5ec123aec8aa
Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils. r=gerald

Comment 6

[Iris Hsiao [:ihsiao]]

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

Comment 7

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d6717b64c82
Backed out changeset 5ec123aec8aa for bustage

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8839762 [details]
Bug 1341497 - Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114328/diff/2-3/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1bc86128b84e
Move WidevineBuffer and WidevineDecryptedBlock into WidevineUtils. r=gerald

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1bc86128b84e