1774306 - Make VideoFrame Transferable

Status: RESOLVED FIXED

(Core :: Audio/Video: Web Codecs, task, P1)

Description

[C.M.Chang[:chunmin]]

After VideoFrame is implemented (bug 1774300), we need to make it Transferable (in .webidl)

Comment 1

[C.M.Chang[:chunmin]]

Attachment: Bug 1774306 - Implement [Transferable] for VideoFrame

This patch implements Custom{Read, Write, Free}TransferHandler for
VideoFrame so VideoFrame can be transferred

Depends on D153686

Comment 2

[C.M.Chang[:chunmin]]

Attachment: Bug 1774306 - Add tests for [Transferable] VideoFrame

Depends on D154677

Comment 3

[C.M.Chang[:chunmin]]

Attachment: Bug 1774306 - Ensure VideoFrame is transferred within same origin

Add a same-origin check to ensure transferring VideoFrame across
different agent cluster has doesn't work. Othewise, transferring
VideoFrame works when e10s is disabled.

Depends on D163194

Comment 4

[C.M.Chang[:chunmin]]

Attachment: Bug 1774306 - Avoid manual JS Read/Writes in VideoFrame serialization

Depends on D163195

Comment 5

[C.M.Chang[:chunmin]]

Attachment: Bug 1774306 - Simplify DS for [Serializable] and [Transferable]

Most of the member types in the data structure for [Serializable],
[Transferable], and FrameData are the same. They should be reused as
much as possible to lower the maintenance effort.

Depends on D163196

Comment 6

[C.M.Chang[:chunmin]]

Attachment: Bug 1774306 - Remove TODO comments

Depends on D163197

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51ee6beb36f8
Implement [Transferable] for VideoFrame r=smaug
https://hg.mozilla.org/integration/autoland/rev/836597b2894b
Add tests for [Transferable] VideoFrame r=smaug
https://hg.mozilla.org/integration/autoland/rev/c2bae45b50fb
Ensure VideoFrame is transferred within same origin r=smaug
https://hg.mozilla.org/integration/autoland/rev/3b5616ad1231
Avoid manual JS Read/Writes in VideoFrame serialization r=padenot
https://hg.mozilla.org/integration/autoland/rev/718fc08ac6a0
Simplify DS for [Serializable] and [Transferable] r=padenot
https://hg.mozilla.org/integration/autoland/rev/aebaccb21fac
Remove TODO comments r=smaug

Comment 8

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/37491 for changes under testing/web-platform/tests

Comment 9

[Noemi Erli[:noemi_erli]]

Backed out 6 changesets (Bug 1774306) for causing leak failures CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=B7Q4MjA_SluJds46ueNcJw.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Casan%2Copt%2Cweb%2Cplatform%2Ctests%2Ctest-linux1804-64-asan-qr%2Fopt-web-platform-tests%2Cwpt2&revision=aebaccb21facc9efe11727d6d0d08db3f3acdf17

Log: https://treeherder.mozilla.org/logviewer?job_id=399590567&repo=autoland&lineNumber=12192

Backout: https://hg.mozilla.org/integration/autoland/rev/677450ba3ba8bc14f69f8d88900b4ec120345f08

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 11

[C.M.Chang[:chunmin]]

Attachment: Bug 1774306 - Fix memory leak

Without this patch, when the transferring-videoframe-across-origin test
(added in D163194) runs, StructuredCloneHolder leaks a VideoFrame in
CustomWriteTransferHandler since *aContent is set to a transferred
VideoFrame but neither CustomReadTransferHandler nor
CustomFreeTransferHandler is called to retrieve and release the
VideoFrame.

To avoid the memory leak, the transferred VideoFrame should be stored in
a member variable of the StructuredCloneHolder so it can be released in
StructuredCloneHolder's dtor even if no one takes the transferred
VideoFrame.

Depends on D163289

Comment 12

[Steve Fink [:sfink] [:s:]]

This turns out to be a bug in JSAutoStructuredCloneBuffer, which is badly mismanaging the ownership of the transferrable information. See bug 1807169.

Comment 13

[C.M.Chang[:chunmin]]

The leak will be addressed in bug 1807169

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e48a20bcf6c
Implement [Transferable] for VideoFrame r=smaug
https://hg.mozilla.org/integration/autoland/rev/667f252800dc
Add tests for [Transferable] VideoFrame r=smaug
https://hg.mozilla.org/integration/autoland/rev/d0c655f96db3
Ensure VideoFrame is transferred within same origin r=smaug
https://hg.mozilla.org/integration/autoland/rev/cad7c78a17ea
Avoid manual JS Read/Writes in VideoFrame serialization r=padenot
https://hg.mozilla.org/integration/autoland/rev/57269c00c3d9
Simplify DS for [Serializable] and [Transferable] r=padenot
https://hg.mozilla.org/integration/autoland/rev/c54aef18375d
Remove TODO comments r=smaug

Comment 15

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/3e48a20bcf6c
https://hg.mozilla.org/mozilla-central/rev/667f252800dc
https://hg.mozilla.org/mozilla-central/rev/d0c655f96db3
https://hg.mozilla.org/mozilla-central/rev/cad7c78a17ea
https://hg.mozilla.org/mozilla-central/rev/57269c00c3d9
https://hg.mozilla.org/mozilla-central/rev/c54aef18375d

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot