| Summary: | WPT test MediaStream-MediaElement-srcObject.https.html times out | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||||||||||||
| Component: | WebRTC | Assignee: | Eric Carlson <eric.carlson> | ||||||||||||||
| Status: | REOPENED --- | ||||||||||||||||
| Severity: | Normal | CC: | ap, calvaris, cdumez, commit-queue, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, hta, jer.noble, philipj, sergio, tommyw, tsavell, webkit-bug-importer, youennf | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | Other | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Eric Carlson
2019-12-02 12:56:21 PST
Created attachment 384722 [details]
Patch
Comment on attachment 384722 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384722&action=review > Source/WebCore/html/HTMLMediaElement.cpp:4858 > + if (loop() && !m_mediaController && playbackRate > 0 && !hasMediaStreamSrcObject()) { Should loop() (or a hasLoop() variant) always return false if there is a media stream src object? > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:830 > + m_player->durationChanged(); Should we introduce a player routine to do all these things together (if player knows it is ended)? > LayoutTests/TestExpectations:3880 > +imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-MediaElement-srcObject.https.html [ Failure ] We could update the test to do assert_true( ===). instead of assert_equal(, ). That would be good to ensure we do not regress this test. Either as part of this patch or as a follow-up that we would upstream to WPT. Created attachment 384740 [details]
Patch
Created attachment 384745 [details]
Patch for landing
Created attachment 384756 [details]
Patch for landing
Comment on attachment 384756 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=384756&action=review > LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-MediaElement-srcObject.https-expected.txt:16 > +FAIL Tests that a media element with an assigned MediaStream reports the played attribute as expected assert_equals: A MediaStream's end MUST return the last known currentTime expected 0.252313446 but got 0.252291624 This doesn't look like it can possibly be a stable result. Or can it? (In reply to Alexey Proskuryakov from comment #7) > Comment on attachment 384756 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384756&action=review > > > LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-MediaElement-srcObject.https-expected.txt:16 > > +FAIL Tests that a media element with an assigned MediaStream reports the played attribute as expected assert_equals: A MediaStream's end MUST return the last known currentTime expected 0.252313446 but got 0.252291624 > > This doesn't look like it can possibly be a stable result. Or can it? The test is marked as Failed for now. Plan is to either fix the implementation quickly to pass this assert or change the test to do assert_true instead of assert_equals to have a stable assertion failure message. Created attachment 384831 [details]
Patch for landing
Created attachment 384835 [details]
Patch for landing
Comment on attachment 384835 [details] Patch for landing Clearing flags on attachment: 384835 Committed r253148: <https://trac.webkit.org/changeset/253148> All reviewed patches have been landed. Closing bug. It looks like the changes in https://trac.webkit.org/changeset/253148/webkit has caused fast/mediastream/stream-switch.html to timeout on Mac. History: https://results.webkit.org/?suite=layout-tests&test=fast%2Fmediastream%2Fstream-switch.html I was able to reproduce this locally by just running the test in iterations. This was rolled out in https://trac.webkit.org/changeset/253215/webkit Reopening |