Closed Bug 1774300 Opened 3 years ago Closed 3 years ago

Implement basic VideoFrame

Categories

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

task

Tracking

()

RESOLVED FIXED
107 Branch
Tracking Status
firefox107 --- fixed

People

(Reporter: chunmin, Assigned: chunmin)

References

(Blocks 5 open bugs)

Details

Attachments

(19 files, 6 obsolete files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

This bug is a sub-task for bug 1749539. The task is to implement the basic VideoFrame operations for some common formats.

The goal in this bug is to implement the VideoColorSpace interface and VideoFrame interface defined in the WebCodecs API, and make it pass the following tests:

  • testing/web-platform/tests/webcodecs/videoFrame-construction.any.js
  • testing/web-platform/tests/webcodecs/videoFrame-construction.window.js
  • testing/web-platform/tests/webcodecs/videoFrame-copyTo.any.js

For now, we won't support all the Pixel Format since gecko only uses part of its defined formats. The supported formats in the implementation will be:

  • I420
  • I420A
  • NV12
  • RGBA
  • RGBX
  • BGRA
  • BGRX

The I422 and I444 formats will be implemented in bug 1779655 and bug 1779656 respectively.

Aside from that, the Serializable and Transferable of the VideoFrame won't be implemented in this bug. They will be done in Bug 1774302 and Bug 1774306 respectively.

Blocks: 1774302
Severity: -- → S3
Blocks: 1774306

This patch creates a blank class for the VideoColorSpace interface. The
files are generated by running ./mach build-backend && ./mach webidl-example VideoColorSpace with necessary changes to make it
buildable.

The implementations are plain blank now. They will be filled out in the
following patches.

This patch creates a blank class for the VideoFrame interface. The files
are generated by running ./mach build-backend && ./mach webidl-example VideoFrame with necessary changes to make it buildable.

The implementations are plain blank now. They will be filled out in the
following patches.

Depends on D144771

Depends on D144772

Implement the VideoFrame's Constructor for RGBA ArrayBuffer* and the
basic getters.

This patch pass the wpt test Test invalid buffer constructed VideoFrame by luck since constructing a VideoFrame from ArrayBuffer* is
acceptable now. However, we only support RGBA format in this patch so
that passed test doesn't be verified correctly. We will address this
in the following patches.

Depends on D145395

Including mozilla/layers/LayersSurfaces.h makes the next patch
unbuildable.

Depends on D148930

Attachment #9281323 - Attachment description: WIP: Bug 1774300 - Implement basic VideoFrame interfaces → WIP: Bug 1774300 - Implement VideoFrame interfaces for RGBA ArrayBuffer*

Adding VideoFrame into CanvasImageSource so the spec is up to date
[1, 2].

[1] https://html.spec.whatwg.org/multipage/canvas.html#canvasimagesource
[2] https://github.com/w3c/webcodecs/issues/158

Depends on D144772

Depends on D150680

Expose some SVG classes to mozilla/dom so VideoFrame can include
mozilla/dom/SVGImageElement.h and build gecko successfully.

Depends on D150681

Depends on D150837

Depends on D151091

Attachment #9283549 - Attachment is obsolete: true
Attachment #9283774 - Attachment is obsolete: true
Attachment #9284242 - Attachment is obsolete: true
Attachment #9281320 - Attachment description: WIP: Bug 1774300 - Add a blank VideoColorSpace interface → Bug 1774300 - Add a blank VideoColorSpace interface
Attachment #9281321 - Attachment description: WIP: Bug 1774300 - Add a blank VideoFrame interface → Bug 1774300 - Add a blank VideoFrame interface
Attachment #9283547 - Attachment description: WIP: Bug 1774300 - Add VideoFrame into CanvasImageSource → Bug 1774300 - Add VideoFrame into CanvasImageSource
Attachment #9281322 - Attachment description: WIP: Bug 1774300 - Implement VideoColorSpace → Bug 1774300 - Implement VideoColorSpace
Attachment #9281323 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame interfaces for RGBA ArrayBuffer* → Bug 1774300 - Implement VideoFrame interfaces for RGBA ArrayBuffer*
Attachment #9281324 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for I420 ArrayBuffer* → Bug 1774300 - Implement VideoFrame Constructor for I420 ArrayBuffer*
Attachment #9281733 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for NV12 ArrayBuffer* → Bug 1774300 - Implement VideoFrame Constructor for NV12 ArrayBuffer*
Attachment #9282301 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for I420A ArrayBuffer* → Bug 1774300 - Implement VideoFrame Constructor for I420A ArrayBuffer*
Attachment #9281327 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for ImageBitmap → Bug 1774300 - Implement VideoFrame Constructor for ImageBitmap
Attachment #9281325 - Attachment description: WIP: Bug 1774300 - Remove unnecessary include → Bug 1774300 - Remove unnecessary include
Attachment #9281326 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for OffscreenCanvas → Bug 1774300 - Implement VideoFrame Constructor for OffscreenCanvas
Attachment #9282421 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for VideoFrame → Bug 1774300 - Implement VideoFrame Constructor for VideoFrame
Attachment #9283317 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for HTMLVideoElement → Bug 1774300 - Implement VideoFrame Constructor for HTMLVideoElement
Attachment #9283318 - Attachment description: WIP: Bug 1774300 - Add a wpt test: construct a VideoFrame from a <video> → Bug 1774300 - Add a wpt test: construct a VideoFrame from a <video>
Attachment #9283548 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for HTMLImageElement → Bug 1774300 - Implement VideoFrame Constructor for HTMLImageElement
Attachment #9283772 - Attachment description: WIP: Bug 1774300 - Expose some SVG classes → Bug 1774300 - Expose some SVG classes
Attachment #9283773 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for SVGImageElement → Bug 1774300 - Implement VideoFrame Constructor for SVGImageElement
Attachment #9284241 - Attachment description: WIP: Bug 1774300 - Implement VideoFrame Constructor for HTMLCanvasElement → Bug 1774300 - Implement VideoFrame Constructor for HTMLCanvasElement
Attachment #9284243 - Attachment description: WIP: Bug 1774300 - Add a wpt test: construct a VideoFrame with a smaller-than-codesize buffer → Bug 1774300 - Add a wpt test: construct a VideoFrame with a smaller-than-codesize buffer

chunmin, can you write an overall summary of what's changing and why?

Flags: needinfo?(cchang)

Sure. I've updated comment 0 and all the summaries of the patches.

Flags: needinfo?(cchang)
Blocks: 1779655
Blocks: 1779656
Blocks: 1780656

This patch adds the VideoFrame into TexImageSource so the spec of
TexImageSource is up to date [1]

[1] https://registry.khronos.org/webgl/specs/latest/1.0/#5.14

Depends on D150679

Comment on attachment 9287144 [details]
Bug 1774300 - Add VideoFrame into TexImageSource

Revision D152815 was moved to bug 1781526. Setting attachment 9287144 [details] to obsolete.

Attachment #9287144 - Attachment is obsolete: true

Comment on attachment 9283547 [details]
Bug 1774300 - Add VideoFrame into CanvasImageSource

Revision D150679 was moved to bug 1781527. Setting attachment 9283547 [details] to obsolete.

Attachment #9283547 - Attachment is obsolete: true
Blocks: 1782128
Attachment #9284243 - Attachment is obsolete: true
Attachment #9292361 - Attachment description: WIP: Bug 1774300 - Add a wpt test: create a VideoFrame from a cross-origin <img> → Bug 1774300 - Add a wpt test: create a VideoFrame from a cross-origin <img>
Attachment #9292361 - Attachment description: Bug 1774300 - Add a wpt test: create a VideoFrame from a cross-origin <img> → Bug 1774300 - Add a wpt test: create VideoFrames from cross-origin CanvasImageSources
Attachment #9292361 - Attachment description: Bug 1774300 - Add a wpt test: create VideoFrames from cross-origin CanvasImageSources → WIP: Bug 1774300 - Add a wpt test: create VideoFrames from cross-origin CanvasImageSources
Attachment #9292361 - Attachment description: WIP: Bug 1774300 - Add a wpt test: create VideoFrames from cross-origin CanvasImageSources → Bug 1774300 - Add a wpt test: create VideoFrames from cross-origin CanvasImageSources
Pushed by cchang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4a145bdd6b86 Add a blank VideoColorSpace interface r=padenot,jgilbert,emilio https://hg.mozilla.org/integration/autoland/rev/ba9bc5c631b0 Add a blank VideoFrame interface r=padenot,jgilbert,emilio https://hg.mozilla.org/integration/autoland/rev/2bc06d0e8808 Implement VideoColorSpace r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/33b150b4c911 Implement VideoFrame interfaces for RGBA ArrayBuffer* r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/50995e55c546 Implement VideoFrame Constructor for I420 ArrayBuffer* r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/19f750d86a73 Implement VideoFrame Constructor for NV12 ArrayBuffer* r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/825ae6e7be9b Implement VideoFrame Constructor for I420A ArrayBuffer* r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/e56596b91a86 Implement VideoFrame Constructor for ImageBitmap r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/4a7dca31703e Remove unnecessary include r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/c5f67f22597e Implement VideoFrame Constructor for OffscreenCanvas r=padenot https://hg.mozilla.org/integration/autoland/rev/6ce76e0d6fd2 Implement VideoFrame Constructor for VideoFrame r=padenot https://hg.mozilla.org/integration/autoland/rev/52fdd5a613f4 Implement VideoFrame Constructor for HTMLVideoElement r=padenot https://hg.mozilla.org/integration/autoland/rev/d5d263cedbc9 Add a wpt test: construct a VideoFrame from a <video> r=padenot https://hg.mozilla.org/integration/autoland/rev/db37fffc8519 Implement VideoFrame Constructor for HTMLImageElement r=padenot https://hg.mozilla.org/integration/autoland/rev/56bfa6480cb5 Expose some SVG classes r=padenot https://hg.mozilla.org/integration/autoland/rev/5f837d7e621b Implement VideoFrame Constructor for SVGImageElement r=padenot https://hg.mozilla.org/integration/autoland/rev/3ab7aead37b5 Implement VideoFrame Constructor for HTMLCanvasElement r=padenot https://hg.mozilla.org/integration/autoland/rev/5af8771526a1 Add a wpt test: create VideoFrames from cross-origin CanvasImageSources r=padenot
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36273 for changes under testing/web-platform/tests
Upstream PR was closed without merging

VideoColorSpace and VideoFrame are defined behind a pref. Disable
the no-undef for now until the pref is enabled by default

Depends on D156022

D158611 should be able to solve the lint issue.

Flags: needinfo?(cchang)
Pushed by cchang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c0cce5ee36e6 Add a blank VideoColorSpace interface r=padenot,jgilbert,emilio https://hg.mozilla.org/integration/autoland/rev/543ebbcb3e78 Add a blank VideoFrame interface r=padenot,jgilbert,emilio https://hg.mozilla.org/integration/autoland/rev/9301467e47ce Implement VideoColorSpace r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/c77682ecaa83 Implement VideoFrame interfaces for RGBA ArrayBuffer* r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/6ff9c84b02fc Ignore no-undef for WebCodecs interfaces r=Standard8 https://hg.mozilla.org/integration/autoland/rev/1e65a9844adc Implement VideoFrame Constructor for I420 ArrayBuffer* r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/f347779c8963 Implement VideoFrame Constructor for NV12 ArrayBuffer* r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/a1b280bfa6c1 Implement VideoFrame Constructor for I420A ArrayBuffer* r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/219fd6678030 Implement VideoFrame Constructor for ImageBitmap r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/bde8924b0c97 Remove unnecessary include r=padenot,jgilbert https://hg.mozilla.org/integration/autoland/rev/cf5e4d9c1b91 Implement VideoFrame Constructor for OffscreenCanvas r=padenot https://hg.mozilla.org/integration/autoland/rev/a692c13589cd Implement VideoFrame Constructor for VideoFrame r=padenot https://hg.mozilla.org/integration/autoland/rev/74a429c4333b Implement VideoFrame Constructor for HTMLVideoElement r=padenot https://hg.mozilla.org/integration/autoland/rev/847b30b6f5bf Add a wpt test: construct a VideoFrame from a <video> r=padenot https://hg.mozilla.org/integration/autoland/rev/66761011e4d3 Implement VideoFrame Constructor for HTMLImageElement r=padenot https://hg.mozilla.org/integration/autoland/rev/61e59ea7d415 Expose some SVG classes r=padenot https://hg.mozilla.org/integration/autoland/rev/adcf2d82738f Implement VideoFrame Constructor for SVGImageElement r=padenot https://hg.mozilla.org/integration/autoland/rev/fa4c07395979 Implement VideoFrame Constructor for HTMLCanvasElement r=padenot https://hg.mozilla.org/integration/autoland/rev/63d5939f41d3 Add a wpt test: create VideoFrames from cross-origin CanvasImageSources r=padenot
Upstream PR merged by moz-wptsync-bot

(In reply to Atila Butkovits from comment #31)

Backed out for causing lint failure.

Backout link: https://hg.mozilla.org/integration/autoland/rev/fd4addd084c6dff9b50fe4da66d7262858b5d323

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=f5460317d2440c225f4a6942453749168765f089&selectedTaskRun=fqk3yORcRFWxAc44SIRfAQ.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=392240661&repo=autoland&lineNumber=112

== Change summary for alert #35640 (as of Fri, 07 Oct 2022 23:13:55 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
0.11% installer size osx-shippable instrumented 121,817,623.25 -> 121,951,031.58

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=35640

Hi chunmin! Is it possible that the backout of this defect or 1793497 may have caused this slight increase in installer size ?

Flags: needinfo?(cchang)

Hi Alex, How did you tell the increased size is caused by that "backout"? Is there a way to verify it? I don't understand how "backout" can increase the installer size exactly. That "backout" removed all the code implemented in bug 1774300 and bug 1793497 in our codebase. If the "backout", which removes the codes from our codebase, increase the installer size, then it means putting those code in our codebase can have a smaller installer size. If "backout" was the cause of increased size, then it's not anymore. Those "backout" code has been landed in our code base again (comment 36), so it shouldn't be an issue now.

Flags: needinfo?(cchang) → needinfo?(afinder)
Blocks: 1795783
See Also: → 1794237
No longer regressions: 1795236

(In reply to C.M.Chang[:chunmin] from comment #39)

Hi Alex, How did you tell the increased size is caused by that "backout"? Is there a way to verify it? I don't understand how "backout" can increase the installer size exactly. That "backout" removed all the code implemented in bug 1774300 and bug 1793497 in our codebase. If the "backout", which removes the codes from our codebase, increase the installer size, then it means putting those code in our codebase can have a smaller installer size. If "backout" was the cause of increased size, then it's not anymore. Those "backout" code has been landed in our code base again (comment 36), so it shouldn't be an issue now.

After performing additional retriggers around that revision, it seems the spike was actually noise on the graph. Sorry for the confusion and the late reply, I'll close the regression and the alert as invalid.

Flags: needinfo?(afinder)
Blocks: 1798959
Blocks: 1798975
Component: Audio/Video → Audio/Video: Web Codecs
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: