1910668 - Fail to encode VideoFrame from <canvas>

Status: RESOLVED FIXED

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

Description

[C.M.Chang[:chunmin] | PTO]

Conversion error will raise due to ConvertToI420 fails to get the DataSourceSurface when encoding a VideoFrame that captures the image from <canvas>. On my Ubuntu, this is because SourceSurfaceCanvasRecording::GetDataSurface only works on main thread, while FFmpegeVideoEncoder is calling ConvertToI420 from non main thread.

Comment 1

[C.M.Chang[:chunmin] | PTO]

Attachment: mp4box.all.min.js

Copyright (c) 2012. Telecom ParisTech/TSI/MM/GPAC Cyril Concolato
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above copyright
notice, this list of conditions and the following disclaimer in the
documentation and/or other materials provided with the distribution.
* Neither the name of the copyright holder nor the
names of its contributors may be used to endorse or promote products
derived from this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

Comment 2

[C.M.Chang[:chunmin] | PTO]

Attachment: canvas-encode.html

Note that the encoding works if the VideoFrame is constructed from ImageBitmap instead of <canvas> (the mp4 mux in the demo has some problems, but the seek the video in Firefox should make it play).

      async function encode(index) {
        const img = await createImageBitmap(canvas);
        const frame = new VideoFrame(/*canvas*/ img, {
          timestamp: index * DURATION_PER_FRAME_US,
          duration: DURATION_PER_FRAME_US,
        });
        let keyFrame = index % KEYFRAME_INDEX_INTERVAL == 0;
        encoder.encode(frame, { keyFrame });
        frame.close();
      }

Comment 3

[C.M.Chang[:chunmin] | PTO]

One way to solve this is to dispatch the GetDataSurface() task to main thread like this, but it will block the encoder waiting for the response from main thread for every encode request. Additionally, I am not sure if the SourceSurfaceCanvasRecording's DataSourceSurface is thread-safe object.

Alternatively, since <canvas> is always on the main thread, capturing the image data into a SourceSurfaceRawData by CreateImageFromRawData when VideoFrame is constructed seems a viable option, and encoder won't be blocked with this approach.

Comment 4

[C.M.Chang[:chunmin] | PTO]

Attachment: Bug 1910668 - Relax image type passed to InitializeFrameWithResourceAndSize

Depends on D218228

Comment 5

[C.M.Chang[:chunmin] | PTO]

Attachment: Bug 1910668 - Allow VideoFrame to capture image from <canvas>

This patch enhances VideoFrame to capture an image from the <canvas>
element provided to its constructor, enabling VideoFrame instances
created with <canvas> to be encoded by VideoEncoder.

Previously, the underlying image in a VideoFrame constructed with
<canvas>. was held within SourceSurfaceCanvasRecording. The
GetDataSurface() method, necessary to access the underlying image,
could only be called on the main thread, while encoding occurs on a
non-main thread.

A naive solution would be to dispatch the GetDataSurface() task to the
main thread, but this would block the encoder while waiting for a
response from the main thread for every encode request. Additionally, it
is unclear if SourceSurfaceCanvasRecording's DataSourceSurface is
thread-safe.

This patch adopts an alternative approach by capturing the image from
the given <canvas> upon VideoFrame construction. This ensures that the
image is owned by the VideoFrame itself and can be processed on any
thread.

Depends on D218229

Comment 6

[C.M.Chang[:chunmin] | PTO]

Attachment: Bug 1910668 - Add a test case

Comment 7

[C.M.Chang[:chunmin] | PTO]

Attachment: Bug 1910668 - Extend test to ImageElement

Depends on D218230

Comment 8

[C.M.Chang[:chunmin] | PTO]

Attachment: Bug 1910668 - Extend test to SVGImageElement

Depends on D218424

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb7125f9a984
Add a test case r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/aae4e1cc5e70
Relax image type passed to InitializeFrameWithResourceAndSize r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/0ce48f0eca6a
Allow VideoFrame to capture image from <canvas> r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/8b47777d8edc
Extend test to ImageElement r=media-playback-reviewers,padenot
https://hg.mozilla.org/integration/autoland/rev/e89f3dfb5d8c
Extend test to SVGImageElement r=media-playback-reviewers,padenot

Comment 10

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

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

Comment 11

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/bb7125f9a984
https://hg.mozilla.org/mozilla-central/rev/aae4e1cc5e70
https://hg.mozilla.org/mozilla-central/rev/0ce48f0eca6a
https://hg.mozilla.org/mozilla-central/rev/8b47777d8edc
https://hg.mozilla.org/mozilla-central/rev/e89f3dfb5d8c

Comment 12

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

Upstream PR merged by moz-wptsync-bot

Comment 13

[C.M.Chang[:chunmin] | PTO]

Comment on attachment 9417395 [details]
Bug 1910668 - Add a test case

Beta/Release Uplift Approval Request

• User impact if declined: webcodecs feature would be partially broken.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: Bug 1908572
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch is a small fix for an issue that blocks webcodecs's release, and it's totally separate from other media code. Since webcodec is aimed to be shipped in Firefox 130, so this stack [1] needs to be merged before the uplift request in bug 1908572 comment 6. Additionally, the webcodecs feature is behind a pref. Thus, the feature can be easily disabled if needed.
• String changes made/needed:
• Is Android affected?: No

[1]
https://hg.mozilla.org/mozilla-central/rev/bb7125f9a984
https://hg.mozilla.org/mozilla-central/rev/aae4e1cc5e70
https://hg.mozilla.org/mozilla-central/rev/0ce48f0eca6a
https://hg.mozilla.org/mozilla-central/rev/8b47777d8edc
https://hg.mozilla.org/mozilla-central/rev/e89f3dfb5d8c

Comment 14

[Pascal Chevrel:pascalc]

Comment on attachment 9417395 [details]
Bug 1910668 - Add a test case

Approved for 130 beta 5, thanks.

Comment 15

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/5ab3c722ed0c
https://hg.mozilla.org/releases/mozilla-beta/rev/fb404e026897
https://hg.mozilla.org/releases/mozilla-beta/rev/c153cf3b7ae7
https://hg.mozilla.org/releases/mozilla-beta/rev/e86c58565ebf
https://hg.mozilla.org/releases/mozilla-beta/rev/709518560349