Bug 205734 - REGRESSION: [ Mac ] fast/canvas/webgl/texImage2D-video-flipY-false.html is Timing out
Summary: REGRESSION: [ Mac ] fast/canvas/webgl/texImage2D-video-flipY-false.html is Ti...
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Russell
Keywords: InRadar
: 207857 (view as bug list)
Depends on: 200904 205483
Blocks: 212260 207857 209837
  Show dependency treegraph
Reported: 2020-01-03 10:58 PST by Truitt Savell
Modified: 2020-11-17 11:31 PST (History)
13 users (show)

See Also:

Patch (6.58 KB, patch)
2020-03-04 20:15 PST, Kenneth Russell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Truitt Savell 2020-01-03 10:58:58 PST

This test began timing out flakily after https://trac.webkit.org/changeset/253926/webkit

Comment 1 Radar WebKit Bug Importer 2020-01-03 10:59:18 PST
Comment 2 Alexey Proskuryakov 2020-01-03 20:02:18 PST
Comment 3 Kenneth Russell 2020-02-18 14:48:13 PST
I'll investigate.
Comment 4 Kenneth Russell 2020-02-20 11:35:08 PST
Presumably this started timing out again after the recent switch to ANGLE in bug 205483.
Comment 5 Kenneth Russell 2020-02-21 15:56:44 PST
Something's going wrong with the handling of LayoutTests/fast/canvas/webgl/resources/orientation-flipped.mp4 . Playback of that video when ANGLE is the WebGL backend is broken; the frame counter in the video never advances past 0, and there are strange green glitches on the right-hand side of the video. timeupdate events aren't fired past currentTime=3, and the browser isn't responsive - the only way to switch URLs is to close the current window.

This doesn't happen with resources/orientation-normal.mp4.

Continuing to investigate what code paths are taken when this video is displayed.

Note that neither Chrome nor Firefox will load resources/orientation-flipped.mp4 - both browsers' media stacks report that the media resource was not suitable.
Comment 6 Kenneth Russell 2020-02-21 16:08:40 PST
Here's ffmpeg's information for the two videos:

Input #0, mov,mp4,m4a,3gp,3g2,mj2, from 'orientation-normal.mp4':
    major_brand     : mp42
    minor_version   : 512
    compatible_brands: isomiso2avc1mp41
    creation_time   : 2017-08-28T20:46:43.000000Z
    encoder         : HandBrake 1.0.7 2017040900
  Duration: 00:00:10.01, start: 0.000000, bitrate: 110 kb/s
    Stream #0:0(und): Video: h264 (Main) (avc1 / 0x31637661), yuv420p(tv, smpte170m/smpte170m/bt709), 300x300 [SAR 1:1 DAR 1:1], 106 kb/s, 29.97 fps, 29.97 tbr, 90k tbn, 180k tbc (default)
      creation_time   : 2017-08-28T20:46:43.000000Z
      handler_name    : VideoHandler

Input #0, mov,mp4,m4a,3gp,3g2,mj2, from 'orientation-flipped.mp4':
    major_brand     : mp42
    minor_version   : 1
    compatible_brands: mp42mp41
    creation_time   : 2017-09-05T23:25:39.000000Z
  Duration: 00:00:10.01, start: 0.000000, bitrate: 152 kb/s
    Stream #0:0(eng): Video: mpeg4 (Simple Profile) (mp4v / 0x7634706D), yuv420p(pc, smpte170m/unknown/smpte170m), 300x300 [SAR 1:1 DAR 1:1], 149 kb/s, 29.97 fps, 29.97 tbr, 1k tbn, 1k tbc (default)
      creation_time   : 2017-09-05T23:25:39.000000Z
      handler_name    : Apple Video Media Handler
Comment 7 Kenneth Russell 2020-02-21 17:29:34 PST
It looks like the video's currentTime never advances past 3 when the test fails. Still debugging what is going on.

One interesting thing to note is that this test takes the software uploading path:

    frame #0: 0x000000010ff55697 WebCore`WebCore::VideoTextureCopierCV::copyImageToPlatformTexture(this=0x000000013ad9d4c8, image=0x000000012ffe1650, width=300, height=300, outputTexture=2, outputTarget=3553, level=0, internalFormat=6408, format=6408, type=5121, premultiplyAlpha=false, flipY=false) at VideoTextureCopierCV.cpp:810:9
   807 	    OSType pixelFormat = CVPixelBufferGetPixelFormatType(image);
   808 	    if (pixelFormat != kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange && pixelFormat != kCVPixelFormatType_420YpCbCr8BiPlanarFullRange) {
   809 	        LOG(WebGL, "VideoTextureCopierCV::copyVideoTextureToPlatformTexture(%p) - Asked to copy an unsupported pixel format ('%s').", this, FourCC(pixelFormat).toString().utf8().data());
-> 810 	        return false;
   811 	    }
   813 	    IOSurfaceRef surface = CVPixelBufferGetIOSurface(image);
Target 0: (MiniBrowser) stopped.
(lldb) p pixelFormat
(OSType) $3 = '2vuy' '2vuy'
Comment 8 Kenneth Russell 2020-02-21 17:47:54 PST
When the test times out, the video is staying in the "seeking" state; HTMLMediaElement::m_seeking is repeatedly true. The test case seems to be able to set the video's currentTime multiple times, but changing it to avoid that doesn't address the problem. Couple of things to investigate:

1) Whether the use of ANGLE is somehow causing the video to be decoded in a different format.

2) Why the video and/or MediaPlayer doesn't successfully seek when ANGLE is enabled.
Comment 9 Kenneth Russell 2020-02-25 17:42:17 PST
When ANGLE is disabled for this test case, WebKit goes down the TextureCacheCV code path in VideoTextureCopierCV::copyImageToPlatformTexture, so it retains hardware-accelerated video-to-texture copies. (I thought that CVOpenGLTextureCache was deprecated - in my earlier experiments it looked like CVOpenGLTextureCacheCreateTextureFromImage was failing in WebKit's ANGLE build.)

Will see whether this is the root cause of the test timeout or whether there are more issues.
Comment 10 Kenneth Russell 2020-02-26 17:27:07 PST
The bugs are in the software fallback path for uploading video elements to WebGL textures. This code path is taken because when ANGLE is enabled, TextureCacheCV (which uses CVOpenGLTextureCache / CVOpenGLESTextureCache) is not used; only the IOSurface code path (for GPU-accelerated video-to-texture uploads, as implemented in Bug 200904) and the MediaPlayer::paintCurrentFrameInContext (the software fallback) are implemented.

The basic bug is that when MediaPlayerPrivateAVFoundationObjC::seekToTime calls [AVPlayerItem seekToTime:toleranceBefore:toleranceAfter:completionHandler:], the completion handler is never called.

The reason for this is an apparent bug in AVFoundation. Fetching a CoreGraphics image from the video is done in MediaPlayerPrivateAVFoundationObjC::createImageForTimeInRect using the AVAssetImageGenerator. Calling [AVAssetImageGenerator copyCGImageAtTime:actualTime:error:] causes the completion handler from [AVPlayerItem seekToTime:] to get squelched somehow, and the seek never completes. This is why the test times out with the software video upload path - the seek to 3 seconds never finishes.

There's another bug where the software path always displays only the first frame of the video. This happens because MediaPlayerPrivateAVFoundationObjC must override and implement:
   float currentTime() const;

Still trying to figure out whether there's any workaround for the above AVFoundation issue.
Comment 11 Kenneth Russell 2020-02-26 18:14:56 PST
Tried changing MediaPlayerPrivateAVFoundationObjC::createImageGenerator to stop setting requestedTimeToleranceBefore and requestedTimeToleranceAfter, on the AVAssetImageGenerator, instead using the default values. Thought this might prevent it from interfering with the AVPlayerItem's seeking (though I see that all AVAssetImageGenerator receives is the AVURLAsset) - unfortunately the problem still happens.

jer.noble@ - do you have any ideas or suggestions about this AVFoundation problem? Thanks in advance.
Comment 12 Jer Noble 2020-02-28 10:29:40 PST
We should avoid using AVAssetImageGenerator whenever possible, preferring to use AVPlayerItemVideoOutput. The reason is twofold: 1) AVAssetImageGenerator will fetch the video again, separately, effectively doubling network cost and 2) it will redecode the resource, effectively doubling CPU cost.

If the asset generator is running, it's probably because the AVPlayerItemVideoOutput doesn't never receive a decoded frame, which indicates a deeper problem with the media stream.

I don't remember how I generated the flipped video; it could just be that the stream is corrupt or malformed in a way that used to work in AVFoundation and stopped. One solution to this test may be to generate a "correct" flipped video that decodes in all the major browsers. IIRC, it was a matter of setting the video transform inside the 'mvhd' box.
Comment 13 Kenneth Russell 2020-03-04 20:15:07 PST
Created attachment 392532 [details]
Comment 14 Kenneth Russell 2020-03-04 20:17:13 PST
dino, jer.noble: could you please review? It makes this layout test run reliably on my machine. Ran all four of the texImage2D-video-flipY-* and texImage2D-mse-flipY-* tests repeatedly; all pass reliably.

Comment 15 Kenneth Russell 2020-03-05 09:59:30 PST
FYI, the win layout test failures are unrelated to this patch. Do I need to do anything in response to them?
Comment 16 Dean Jackson 2020-03-05 10:03:12 PST
Comment on attachment 392532 [details]

Thanks Ken!
Comment 17 WebKit Commit Bot 2020-03-05 11:02:17 PST
The commit-queue encountered the following flaky tests while processing attachment 392532 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 18 WebKit Commit Bot 2020-03-05 11:02:58 PST
Comment on attachment 392532 [details]

Clearing flags on attachment: 392532

Committed r257932: <https://trac.webkit.org/changeset/257932>
Comment 19 WebKit Commit Bot 2020-03-05 11:03:00 PST
All reviewed patches have been landed.  Closing bug.
Comment 20 Kenneth Russell 2020-03-05 11:37:00 PST
*** Bug 207857 has been marked as a duplicate of this bug. ***