Bug 225651 – [MediaStream][GStreamer] Flaky fast/mediastream/MediaStream-video-element-video-tracks-disabled.html

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alicia Boya García]

Created attachment 428266 [details]
Screenshot from the test runner

fast/mediastream/MediaStream-video-element-video-tracks-disabled.html is flaky image failure. Is this an unitialized read? What I can see in the image doesn't look like an image from my camera.

[Philippe Normand]

(In reply to Alicia Boya García from comment #0)
> Created attachment 428266 [details]
> Screenshot from the test runner
> 
> fast/mediastream/MediaStream-video-element-video-tracks-disabled.html is
> flaky image failure. Is this an unitialized read? What I can see in the
> image doesn't look like an image from my camera.

The tests uses mock audio and video sources.
Anyway, I'll check this, thanks for the report.

[Philippe Normand]

(In reply to Alicia Boya García from comment #0)
> Created attachment 428266 [details]
> Screenshot from the test runner
> 
> fast/mediastream/MediaStream-video-element-video-tracks-disabled.html is
> flaky image failure. Is this an unitialized read? 

I don't think so. I can reproduce the failure, the video element associated with the MediaStream renders the mock video track that is disabled, instead of rendering black frames.

[Philippe Normand]

I have a patch but the test still flakes:

    function canplaythrough()
    {    
        mediaStream.getVideoTracks()[0].enabled = false;
        window.testRunner.notifyDone();
    }


Due to the asynchronous nature of the GStreamer pipeline involved, we can't be sure the video is black right after the track was disabled, so we would need to delay the notifyDone call. Would this be acceptable?

[Philippe Normand]

Created attachment 428843 [details]
Patch

[Alicia Boya García]

Comment on attachment 428843 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428843&action=review

> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:252
> +            m_needsDiscont = true;

If there is no unfortunate technical reason that makes it impossible, we should flush here. That will ensure that the next frame the sink receives is a blank frame.

> LayoutTests/fast/mediastream/MediaStream-video-element-video-tracks-disabled.html:45
> +        setTimeout(() => { window.testRunner.notifyDone(); }, 200);

This is certainly not ideal... Let's see by adding the flush this is no longer necessary, or if it can be reduced to a requestAnimationFrame(() => { window.testRunner.notifyDone(); )

[Philippe Normand]

Comment on attachment 428843 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428843&action=review

>> Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:252
>> +            m_needsDiscont = true;
> 
> If there is no unfortunate technical reason that makes it impossible, we should flush here. That will ensure that the next frame the sink receives is a blank frame.

Yeah I thought about this too but even so, there's no guarantee the test won't flake, unless... see below

>> LayoutTests/fast/mediastream/MediaStream-video-element-video-tracks-disabled.html:45
>> +        setTimeout(() => { window.testRunner.notifyDone(); }, 200);
> 
> This is certainly not ideal... Let's see by adding the flush this is no longer necessary, or if it can be reduced to a requestAnimationFrame(() => { window.testRunner.notifyDone(); )

the rAF idea is interesting, I can try it. I don't have much hope though because I suspect rAF is not hooked into <video>. There is a spec proposal to have some rAF-equivalent for <video> but it's not implemented in WebKit yet AFAIK.

[Philippe Normand]

The rAF idea doesn't work, the video-rAF proposal is https://wicg.github.io/video-rvfc/ BTW.

[Philippe Normand]

Created attachment 428927 [details]
Patch

[Philippe Normand]

Unfortunately flushing doesn't help with the flakiness :(

[Alicia Boya García]

(In reply to Philippe Normand from comment #7)
> The rAF idea doesn't work, the video-rAF proposal is
> https://wicg.github.io/video-rvfc/ BTW.

> Unfortunately flushing doesn't help with the flakiness :(

Sad...(In reply to Philippe Normand from comment #9)

[Alicia Boya García]

Comment on attachment 428927 [details]
Patch

I still think the flush should be done but I leave that decision to you.

[Philippe Normand]

I'll add the flush support, it's harmless even though it's not helping much :)
Thanks for the review!

[Philippe Normand]

Committed r277651 (237857@main): <https://commits.webkit.org/237857@main>