Bug 237086

Summary: webrtc/video-replace-muted-track.html is unnecessary long to run
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson, ews-watchlist, glenn, jer.noble, peng.liu6, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

youenn fablet
Reported 2022-02-23 07:47:52 PST
webrtc/video-replace-muted-track.html is unnecessary long to run
Attachments
Patch (1.63 KB, patch)
2022-02-23 07:51 PST, youenn fablet
no flags
Patch for landing (2.85 KB, patch)
2022-02-24 01:28 PST, youenn fablet
no flags
youenn fablet
Comment 1 2022-02-23 07:51:17 PST
Peng Liu
Comment 2 2022-02-23 10:28:58 PST
Comment on attachment 452980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452980&action=review > LayoutTests/webrtc/video-replace-muted-track.html:47 > + return checkVideoBlack(true, canvas, video, "Check we do not receive any black frame", 360).then(assert_unreached, () => { }); To be honest, I was confused by the change at the beginning. Because you change 40 -> 360 here, which is not consistent with the change log. But I got your point after reading the source code. :-)
youenn fablet
Comment 3 2022-02-23 10:37:09 PST
I should probably use a decrementing counter to make it easier to understand. I'll update the patch tomorrow if it is not changing a lot of tests.
Darin Adler
Comment 4 2022-02-23 13:03:33 PST
Comment on attachment 452980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452980&action=review > LayoutTests/ChangeLog:11 > + We were waiting for 360 iterations to validate no black frame is found. > + This makes it last around 20 seconds. > + Decrease the number of iterations to 40, around 2 seconds. I see Peng’s comment but I do not understand: Why does this change log comment say the opposite of what the change is?
Peng Liu
Comment 5 2022-02-23 14:22:21 PST
Comment on attachment 452980 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452980&action=review >> LayoutTests/ChangeLog:11 >> + Decrease the number of iterations to 40, around 2 seconds. > > I see Peng’s comment but I do not understand: Why does this change log comment say the opposite of what the change is? The checkVideoBlack() function is below: ``` async function checkVideoBlack(expected, canvas, video, errorMessage, counter) { if (isVideoBlack(canvas, video) === expected) return Promise.resolve(); if (counter === undefined) counter = 0; if (counter > 400) { if (!errorMessage) errorMessage = "checkVideoBlack timed out expecting " + expected; return Promise.reject(errorMessage); } await waitFor(50); return checkVideoBlack(expected, canvas, video, errorMessage, ++counter); } ``` So the iteration number is changed from (400-40) -> (400 - 360).
youenn fablet
Comment 6 2022-02-24 01:28:40 PST
Created attachment 453084 [details] Patch for landing
EWS
Comment 7 2022-02-24 04:30:41 PST
Committed r290425 (247733@main): <https://commits.webkit.org/247733@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453084 [details].
Radar WebKit Bug Importer
Comment 8 2022-02-24 04:31:23 PST
Note You need to log in before you can comment on or make changes to this bug.