1351953 - Use Shmems to send encrypted video frames and samples through PChromiumCDM

Status: RESOLVED FIXED

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

Description

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

The new PChromiumCDM protocol should be using shmems to send video frames and compressed encrypted samples between the content/GMP process.

Comment 1

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

Attachment: Bug 1351953 - Send Data to CDM for decrypt and or decode in shmems.

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

Comment 2

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

Attachment: Bug 1351953 - Make DecryptJob::PostResult take a mozilla::Span instead of nsTArray.

This means we can pass anything that converts implicitly to a Span to
PostResult, including an nsTArray<uint8_t>. We can also pass a Span
that contains the contents of a Shmem's buffer.

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

Comment 3

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

Attachment: Bug 1351953 - Pre-allocate shmems for the CDM process to use for storing decrypted and audio samples.

Makes transfer of samples between the content and CDM processes use shmems.

The Chromium CDM API requires us to implement a synchronous interface to supply
buffers to the CDM for it to write decrypted samples into. We want our buffers
to be backed by shmems, in order to reduce the overhead of transferring decoded
frames. However due to sandboxing restrictions, the CDM process cannot allocate
shmems itself.  We don't want to be doing synchronous IPC to request shmems
from the content process, nor do we want to have to do intr IPC or make async
IPC conform to the sync allocation interface. So instead we have the content
process pre-allocate a set of shmems and give them to the CDM process in
advance of them being needed.

When the CDM needs to allocate a buffer for storing a decrypted sample, the CDM
host gives it one of these shmems' buffers. When this is sent back to the
content process, we copy the result out (uploading to a GPU surface for video
frames), and send the shmem back to the CDM process so it can reuse it.

We predict the size of buffer the CDM will allocate, and prepopulate the CDM's
list of shmems with shmems of at least that size, plus a bit of padding for
safety.

Normally the CDM won't allocate more than one buffer at once, but we've seen
cases where it allocates two buffers, returns one and holds onto the other. So
the minimum number of shmems we give to the CDM must be at least two, and the
default is three for safety.

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

Comment 4

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

Comment on attachment 8854653 [details]
Bug 1351953 - Pre-allocate shmems for the CDM process to use for storing decrypted and audio samples.

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

Comment 5

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd4ff7dc5e53abf1ae7ad75a0a366274fee72f39&filter-searchStr=mda

Comment 6

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

Comment on attachment 8854652 [details]
Bug 1351953 - Make DecryptJob::PostResult take a mozilla::Span instead of nsTArray.

https://reviewboard.mozilla.org/r/126588/#review129354

Comment 7

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

Comment on attachment 8854651 [details]
Bug 1351953 - Send Data to CDM for decrypt and or decode in shmems.

https://reviewboard.mozilla.org/r/126586/#review129360

Comment 8

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

Comment on attachment 8854653 [details]
Bug 1351953 - Pre-allocate shmems for the CDM process to use for storing decrypted and audio samples.

https://reviewboard.mozilla.org/r/126590/#review129362

::: commit-message-bed71:21
(Diff revision 2)
> +list of shmems with shmems of at least that size, plus a bit of padding for
> +safety.

"plus a bit of padding for safety" -- that sounds dangerous!
I see in the code you make sizes be multiple of 16, maybe you could just explain that and why.

Comment 9

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

Comment on attachment 8854651 [details]
Bug 1351953 - Send Data to CDM for decrypt and or decode in shmems.

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

Comment 10

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

Comment on attachment 8854652 [details]
Bug 1351953 - Make DecryptJob::PostResult take a mozilla::Span instead of nsTArray.

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

Comment 11

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

Comment on attachment 8854653 [details]
Bug 1351953 - Pre-allocate shmems for the CDM process to use for storing decrypted and audio samples.

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/135680bdc3be
Send Data to CDM for decrypt and or decode in shmems. r=gerald
https://hg.mozilla.org/integration/autoland/rev/b2674e5e09f9
Make DecryptJob::PostResult take a mozilla::Span instead of nsTArray. r=gerald
https://hg.mozilla.org/integration/autoland/rev/d0d1782c7b2f
Pre-allocate shmems for the CDM process to use for storing decrypted and audio samples. r=gerald

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/135680bdc3be
https://hg.mozilla.org/mozilla-central/rev/b2674e5e09f9
https://hg.mozilla.org/mozilla-central/rev/d0d1782c7b2f