WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
177119
Creating a second AVPlayerItemVideoOutput causes flakey failures
https://bugs.webkit.org/show_bug.cgi?id=177119
Summary
Creating a second AVPlayerItemVideoOutput causes flakey failures
Dean Jackson
Reported
2017-09-18 19:39:05 PDT
Since landing the flipY patch (176491), the WebGL conformance test that exercises video -> texture is flakey. This is strange because the test never actually exercises the GPU path: hasNewPixelBufferForItemTime always returns false. This happens in Safari too, as if it takes a little while before there is output to actually read. LayoutTests/webgl/1.0.2/conformance/textures/tex-image-and-sub-image-2d-with-video.html As far as I can tell, the first time through we create the AVPlayerItemVideoOutput, but do not have hasNewPixelBufferForItemTime. The second time through we have the AVPlayerItemVideoOutput, but still don't have hasNewPixelBufferForItemTime. The third time through we get true from hasNewPixelBufferForItemTime, and the fast path produces the texture. We never hit this before because the previous code returned immediately if flipY was set, so it instantly fell back to the software path and never tried to create the AVPlayerItemVideoOutput. The failure is that the software path fails to produce a texture, simply uploading black. It doesn't produce an error, so we think it works. Can you think of a reason why creating the AVPlayerItemVideoOutput and adding it to the AVPlayerItem would cause another output to stop working, but only sometimes? Note that even returning early from MediaPlayerPrivateAVFoundationObjC::copyVideoTextureToPlatformTexture, before we ask the output if there is a frame ready, causes the failure. It's just whether or not we created the AVPlayerItemVideoOutput. The other thing to understand is why the AVPlayerItemVideoOutput doesn't have a pixel buffer available right away. I guess that's because it's waiting for a frame to decode?
Attachments
Patch
(10.21 KB, patch)
2017-09-19 10:18 PDT
,
Jer Noble
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-elcapitan
(1.37 MB, application/zip)
2017-09-19 11:27 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
(1.53 MB, application/zip)
2017-09-19 11:30 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews112 for mac-elcapitan
(2.21 MB, application/zip)
2017-09-19 11:48 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews125 for ios-simulator-wk2
(1.43 MB, application/zip)
2017-09-19 11:54 PDT
,
Build Bot
no flags
Details
Patch
(31.45 KB, patch)
2017-11-17 14:00 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-elcapitan
(2.25 MB, application/zip)
2017-11-17 14:51 PST
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
(3.05 MB, application/zip)
2017-11-17 15:07 PST
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews115 for mac-elcapitan
(3.34 MB, application/zip)
2017-11-17 16:03 PST
,
EWS Watchlist
no flags
Details
Patch
(33.09 KB, patch)
2017-12-01 23:13 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-elcapitan
(2.42 MB, application/zip)
2017-12-01 23:59 PST
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews105 for mac-elcapitan-wk2
(2.64 MB, application/zip)
2017-12-02 00:27 PST
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews116 for mac-elcapitan
(2.87 MB, application/zip)
2017-12-02 00:37 PST
,
EWS Watchlist
no flags
Details
Archive of layout-test-results from ews126 for ios-simulator-wk2
(2.11 MB, application/zip)
2017-12-02 00:38 PST
,
EWS Watchlist
no flags
Details
Patch
(353.67 KB, patch)
2017-12-02 08:13 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews125 for ios-simulator-wk2
(2.14 MB, application/zip)
2017-12-02 09:44 PST
,
EWS Watchlist
no flags
Details
Patch
(360.78 KB, patch)
2017-12-05 08:33 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(360.88 KB, patch)
2017-12-07 11:11 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Show Obsolete
(16)
View All
Add attachment
proposed patch, testcase, etc.
Dean Jackson
Comment 1
2017-09-18 19:39:21 PDT
<
rdar://problem/34507977
>
Jer Noble
Comment 2
2017-09-19 10:18:43 PDT
Created
attachment 321207
[details]
Patch
Build Bot
Comment 3
2017-09-19 11:27:32 PDT
Comment hidden (obsolete)
Comment on
attachment 321207
[details]
Patch
Attachment 321207
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/4595252
New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
Build Bot
Comment 4
2017-09-19 11:27:34 PDT
Comment hidden (obsolete)
Created
attachment 321219
[details]
Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 5
2017-09-19 11:30:07 PDT
Comment hidden (obsolete)
Comment on
attachment 321207
[details]
Patch
Attachment 321207
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/4595250
New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
Build Bot
Comment 6
2017-09-19 11:30:08 PDT
Comment hidden (obsolete)
Created
attachment 321220
[details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Build Bot
Comment 7
2017-09-19 11:48:46 PDT
Comment hidden (obsolete)
Comment on
attachment 321207
[details]
Patch
Attachment 321207
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/4595312
New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
Build Bot
Comment 8
2017-09-19 11:48:48 PDT
Comment hidden (obsolete)
Created
attachment 321225
[details]
Archive of layout-test-results from ews112 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Build Bot
Comment 9
2017-09-19 11:54:26 PDT
Comment hidden (obsolete)
Comment on
attachment 321207
[details]
Patch
Attachment 321207
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
http://webkit-queues.webkit.org/results/4595324
New failing tests: webgl/1.0.2/conformance/textures/tex-image-and-sub-image-2d-with-video.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
Build Bot
Comment 10
2017-09-19 11:54:27 PDT
Comment hidden (obsolete)
Created
attachment 321228
[details]
Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Jer Noble
Comment 11
2017-11-17 14:00:35 PST
Comment hidden (obsolete)
Created
attachment 327233
[details]
Patch
EWS Watchlist
Comment 12
2017-11-17 14:50:59 PST
Comment hidden (obsolete)
Comment on
attachment 327233
[details]
Patch
Attachment 327233
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/5280900
New failing tests: fast/canvas/webgl/texImage2D-video-flipY-true.html fast/canvas/webgl/texImage2D-mse-flipY-true.html
EWS Watchlist
Comment 13
2017-11-17 14:51:00 PST
Comment hidden (obsolete)
Created
attachment 327247
[details]
Archive of layout-test-results from ews103 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
EWS Watchlist
Comment 14
2017-11-17 15:07:40 PST
Comment hidden (obsolete)
Comment on
attachment 327233
[details]
Patch
Attachment 327233
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/5281052
New failing tests: fast/canvas/webgl/texImage2D-video-flipY-true.html fast/canvas/webgl/texImage2D-mse-flipY-true.html fast/canvas/webgl/texImage2D-mse-flipY-false.html
EWS Watchlist
Comment 15
2017-11-17 15:07:42 PST
Comment hidden (obsolete)
Created
attachment 327251
[details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
EWS Watchlist
Comment 16
2017-11-17 16:03:55 PST
Comment hidden (obsolete)
Comment on
attachment 327233
[details]
Patch
Attachment 327233
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/5281647
New failing tests: fast/canvas/webgl/texImage2D-mse-flipY-true.html fast/canvas/webgl/texImage2D-mse-flipY-false.html fast/canvas/webgl/texImage2D-video-flipY-true.html fast/canvas/webgl/texImage2D-video-flipY-false.html platform/mac/media/media-source/videoplaybackquality-decompressionsession.html fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
EWS Watchlist
Comment 17
2017-11-17 16:03:56 PST
Comment hidden (obsolete)
Created
attachment 327262
[details]
Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Jer Noble
Comment 18
2017-11-17 16:44:04 PST
(In reply to Build Bot from
comment #14
)
> Comment on
attachment 327233
[details]
> Patch > >
Attachment 327233
[details]
did not pass mac-wk2-ews (mac-wk2): > Output:
http://webkit-queues.webkit.org/results/5281052
> > New failing tests: > fast/canvas/webgl/texImage2D-video-flipY-true.html > fast/canvas/webgl/texImage2D-mse-flipY-true.html > fast/canvas/webgl/texImage2D-mse-flipY-false.html
Looks like these tests are failing because the blue is not quite blue enough and the red is not quite red enough (i.e., the red channel in the blue test is 20/255 when it's testing for <20/255).
Jer Noble
Comment 19
2017-12-01 23:13:41 PST
Created
attachment 328217
[details]
Patch
EWS Watchlist
Comment 20
2017-12-01 23:59:42 PST
Comment hidden (obsolete)
Comment on
attachment 328217
[details]
Patch
Attachment 328217
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/5464414
New failing tests: fast/canvas/webgl/texImage2D-mse-flipY-false.html
EWS Watchlist
Comment 21
2017-12-01 23:59:43 PST
Comment hidden (obsolete)
Created
attachment 328219
[details]
Archive of layout-test-results from ews103 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
EWS Watchlist
Comment 22
2017-12-02 00:27:06 PST
Comment hidden (obsolete)
Comment on
attachment 328217
[details]
Patch
Attachment 328217
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/5464518
New failing tests: fast/canvas/webgl/texImage2D-mse-flipY-false.html
EWS Watchlist
Comment 23
2017-12-02 00:27:08 PST
Comment hidden (obsolete)
Created
attachment 328222
[details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
EWS Watchlist
Comment 24
2017-12-02 00:37:44 PST
Comment hidden (obsolete)
Comment on
attachment 328217
[details]
Patch
Attachment 328217
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/5464507
New failing tests: fast/canvas/webgl/texImage2D-mse-flipY-false.html
EWS Watchlist
Comment 25
2017-12-02 00:37:45 PST
Comment hidden (obsolete)
Created
attachment 328223
[details]
Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
EWS Watchlist
Comment 26
2017-12-02 00:38:17 PST
Comment on
attachment 328217
[details]
Patch
Attachment 328217
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
http://webkit-queues.webkit.org/results/5464516
New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
EWS Watchlist
Comment 27
2017-12-02 00:38:18 PST
Created
attachment 328224
[details]
Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Jer Noble
Comment 28
2017-12-02 08:13:07 PST
Created
attachment 328231
[details]
Patch
EWS Watchlist
Comment 29
2017-12-02 09:44:23 PST
Comment on
attachment 328231
[details]
Patch
Attachment 328231
[details]
did not pass ios-sim-ews (ios-simulator-wk2): Output:
http://webkit-queues.webkit.org/results/5467367
New failing tests: fast/canvas/webgl/tex-image-and-sub-image-2d-with-video.html
EWS Watchlist
Comment 30
2017-12-02 09:44:25 PST
Created
attachment 328236
[details]
Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Jer Noble
Comment 31
2017-12-02 16:29:55 PST
And now it looks like iOS is getting the color channels reversed
Jer Noble
Comment 32
2017-12-05 08:33:12 PST
Created
attachment 328452
[details]
Patch
Dean Jackson
Comment 33
2017-12-06 16:29:08 PST
Comment on
attachment 328452
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=328452&action=review
> Source/WebCore/ChangeLog:39 > + uploading BGRA pixel formatted data to a texture; work aronud this bug for now.
Typo: around
> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2333 > - if (!pixelBuffer) > + if (!updateLastPixelBuffer() && (m_lastImage || !m_lastPixelBuffer))
I don't quite understand the logic change here. The !pixelBuffer and !updateLastPixelBuffer() are effectively the same thing... but why did you add && (m_lastImage || !m_lastPixelBuffer)?
> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:329 > + fragmentShaderSource.appendLiteral("uniform int u_swapColors;\n");
naming: flipColorChannels? swapColorChannels?
> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:452 > - " yuv.gb = TEXTUREFUNC(u_uvTexture, v_uvTextureCoordinate).rg - vec2(0.5, 0.5);\n" > - " gl_FragColor = vec4(yuv * u_colorMatrix, 1);\n" > + " yuv.gb = TEXTUREFUNC(u_uvTexture, v_uvTextureCoordinate).rg;\n"
Why aren't you subtracting 0.5, 0.5 any more?
> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:-723 > -VideoTextureCopierCV::GC3DStateSaver::GC3DStateSaver(GraphicsContext3D& context) > - : m_context(context)
Booohooooo.... this had so much promise :)
Jer Noble
Comment 34
2017-12-06 16:36:53 PST
Comment on
attachment 328452
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=328452&action=review
>> Source/WebCore/ChangeLog:39 >> + uploading BGRA pixel formatted data to a texture; work aronud this bug for now. > > Typo: around
Will fix.
>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2333 >> + if (!updateLastPixelBuffer() && (m_lastImage || !m_lastPixelBuffer)) > > I don't quite understand the logic change here. The !pixelBuffer and !updateLastPixelBuffer() are effectively the same thing... but why did you add && (m_lastImage || !m_lastPixelBuffer)?
The logic is: "If updateLastPixelBuffer() didn't end up with anything new (and returned false), and we either already converted the last valid m_lastPixelBuffer to an m_lastImage, or there's no m_lastPixelBuffer in the first place, then bail out early." If we didn't get a new m_lastPixelBuffer, and we also don't have a m_lastImage, then we should _not_ do an early return and do the YUV->RGB conversion in the conformer below. The key (I think) to understanding why we do this check are the lines above in updateLastPixelBuffer():
> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2310 > + m_lastPixelBuffer = adoptCF([m_videoOutput.get() copyPixelBufferForItemTime:currentTime itemTimeForDisplay:nil]); > + m_lastImage = nullptr;
Whenever we update m_lastPixelBuffer with a new value, we clear out m_lastImage. So a call to paint into a WebGL canvas context will result in a non-null m_lastPixelBuffer, and a subsequent call to paint into a 2d canvas context will conform that existing m_lastPixelBuffer into a m_lastImage.
>> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:452 >> + " yuv.gb = TEXTUREFUNC(u_uvTexture, v_uvTextureCoordinate).rg;\n" > > Why aren't you subtracting 0.5, 0.5 any more?
The subtraction is now part of the u_colorMatrix in the "w" column.
>> Source/WebCore/platform/graphics/cv/VideoTextureCopierCV.cpp:-723 >> - : m_context(context) > > Booohooooo.... this had so much promise :)
It did!
Jer Noble
Comment 35
2017-12-07 11:11:56 PST
Created
attachment 328709
[details]
Patch for landing
WebKit Commit Bot
Comment 36
2017-12-07 11:55:50 PST
The commit-queue encountered the following flaky tests while processing
attachment 328709
[details]
: http/tests/xmlhttprequest/redirect-cross-origin-tripmine.html
bug 179840
(authors:
ap@webkit.org
and
rniwa@webkit.org
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 37
2017-12-07 11:56:31 PST
Comment on
attachment 328709
[details]
Patch for landing Clearing flags on attachment: 328709 Committed
r225638
: <
https://trac.webkit.org/changeset/225638
>
WebKit Commit Bot
Comment 38
2017-12-07 11:56:33 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug