1757637 - Canvas.drawImage(video, sx, sy, swidth, sheight, dx, dy, dwidth, dheight) races against .videoWidth and .videoHeight

Status: VERIFIED FIXED

(Core :: Audio/Video, defect, P2)

Description

[Anthony Jones]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:97.0) Gecko/20100101 Firefox/97.0

Steps to reproduce:

Create a DASH stream which that paints to canvas using drawImage where sx, sy, swidth or sheight are calculated from videoWidth or videoHeight. For example:

Use a video that changes size from 1280x720 to 640x360

Repeatedly call:
context.drawImage(video, 0, 0, video.videoWidth, video.videoHeight, 0, 0, 1280, 720)

Actual results:

During the transition, an image is drawn half size usually for a single frame until videoWidth and videoHeight are updated to match the image snapshot.

Expected results:

The above example should be equivalent to:

context.drawImage(video, 0, 0, 1280, 720)

The canvas image should be the same size.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Canvas: 2D' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Brad Werth [:bradwerth]]

Thank you for filing. Are you willing to provide a testcase?

Comment 3

[Anthony Jones]

Attachment: drawimage.html

You will see the canvas blink when it changes bitrate.

Comment 4

[Anthony Jones]

A workaround looks something like this:

static drawImage(context, source, sx, sy, sWidth, sHeight, dx, dy, dWidth, dHeight): void {
    context.save();

    const region = new Path2D();
    region.rect(dx, dy, dWidth, dHeight);
    context.clip(region);

    const size = { width: source.videoWidth, height: source.videoHeight };
    if (size.width && size.height) {
      const proportion = {
        x: sx / size.width,
        y: sy / size.height,
        width: sWidth / size.width,
        height: sHeight / size.height
      };

      const paintWidth = dWidth / proportion.width;
      const paintHeight = dHeight / proportion.height;
      const paintX = dx - proportion.x * paintWidth;
      const paintY = dy - proportion.y * paintHeight;

      context.drawImage(source, paintX, paintY, paintWidth, paintHeight);
    }

    context.restore();
  }

Comment 5

[Brad Werth [:bradwerth]]

Thank you for the testcase. I'm not seeing anything wrong with the presentation of the video. If you have specific steps to reproduce, please include them. And it would also be helpful to see a screen recording of the bad behavior on your system.

Comment 6

[Anthony Jones]

Attachment: Slow motion recording of repro

I've attached a video of the issue at 240fps, so I think this is a 4x slowdown. You will see a single frame being copied to canvas incorrectly at time=1:24. It does depend on bitrate adaptation happening on your machine, but I've done my best to make that happen. It may help to disable the cache in devtools.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:lsalzman, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Lee Salzman [:lsalzman]]

I think this is something the media team might be better equipped to look into since the issue at land looks related to HTMLVideoElement and attributes thereof. Recategorizing...

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 10

[Alastor Wu [:alwu]]

Could you try the latest Nightly to see if this issue still exists? This issue might be fixed already by bug 1764425.
Thank you.

Comment 11

[Paul Adenot (:padenot)]

I can repro with the following setup:

• As noted above disable the HTTP cache in the devtools: open the devtools on the repro page (https://bugzilla.mozilla.org/attachment.cgi?id=9266410), F1 to bring the settings, scroll to the bottom, disable HTTP cache when the devtools are opened.
• Enter the responsive mode (ctrl+shift+m here on Linux)
• Enable throttling at the top of the responsive mode. "Good 3G" works ok for me here, it ensures resolution shifts, but ymmv.
• Play the video, observe that there are sometimes glitches on the frame on the right (canvas)

Probably the video attributes are lagging behind or something. In the video example at 1:24, it goes from a small resolution to a higher resolution. Only a quarter of the video is painted. It means that videoHeight and videoWidth are lagging behind a frame or something.

Comment 12

[Anthony Jones]

(In reply to Paul Adenot (:padenot) from comment #11)

Probably the video attributes are lagging behind or something. In the video example at 1:24, it goes from a small resolution to a higher resolution. Only a quarter of the video is painted. It means that videoHeight and videoWidth are lagging behind a frame or something.

The graphics surface has the correct dimensions for itself because it is read directly and videoHeight and videoWidth are out of date because they arrive via messaging from the MediaDecoderStateMachine thread. There are two possible solutions:

• Calculate scaling and clip compensating for out-of-date videoWidth and videoHeight, shown in the Javascript above.
• Snapshot the videoWidth , videoHeight and the video surface toghether

FWIW, I think the first method is the simplest and least likely to cause an issue.

Comment 13

[Paul Adenot (:padenot)]

Attachment: Bug 1757637 - Use the ImageContainer to determine video{Width,Height} instead of passing through the state machine. r?alwu

This means there is significantly less chance of desynchronization between the
actual size of the image and the reported size of the image.

Comment 14

[Paul Adenot (:padenot)]

The attached patch fixes the issue significantly here. There's still a possibility for a issues, because there is a window of time in between fetching, e.g. videoWidth, and the video frame being updated, but it's not very big.

I'm trying to think of a way to fix this in a way that is a bit more solid, but it's a bit hard because everything is async.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/817af8f5f831
Use the ImageContainer to determine video{Width,Height} instead of passing through the state machine. r=alwu

Comment 16

[Norisz Fay [:noriszfay]]

Backed out for causing reftest failures on incorrect_display_in_bytestream_vp8

Backout link

Push with failures

Failure log

Comment 17

[qjfkcovb3]

#1757637 A[Core:Audio/Video]Canvas.drawImage(video, sx, sy, swidth, sheight, dx, dy, dwidth, dheight) races against .videoWidth and .videoHeight

Experiencing extreme performance slowdown since 20220630095519

Noticeable in Youtube, Twitch, and more. (Didn't happen before 20220630095519)

Also, if PiP from tab A is enabled, and if I switch focus to tab B or other program C, it is almost impossible to move around the PiP window.

about:support info provided here: https://drive.proton.me/urls/869F2N139M#WStEsYtcal79

Comment 18

[qjfkcovb3]

Additionally, about:memory provided here: https://drive.proton.me/urls/HX80FRSHJM#J0BAh2EWb4xe

Comment 19

[qjfkcovb3]

Disregard the claims above; problem seems to have resolved itself after new update.
Will leave additional comment if necessary.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/419c7e63a9f4
Use the ImageContainer to determine video{Width,Height} instead of passing through the state machine. r=alwu

Comment 21

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/419c7e63a9f4

Comment 22

[Bogdan Maris, Desktop QA]

Reproduced the glitches using the steps from comment 11 using an old Nightly from 2022-05-02. Verified that using Firefox 104.0b5 on all platforms (Windows 10 64bit, macOS 11.6 and Ubuntu 22.04) the glitches are no longer seen.