Bug 160222 – HTMLVideoElement with MediaStream src shows paused image when all video tracks are disabled

• Component: Media
• Status: RESOLVED FIXED

[George Ruan]

According to HTML 5 Spec https://w3c.github.io/mediacapture-main/#life-cycle-and-media-flow, "A muted or disabled MediaStreamTrack renders either silence (audio), black frames (video), or a zero-information-content equivalent."

Currently, when all MediaStreamTracks are disabled, the video renders a paused image from the last moment that the MediaStream had a track that was enabled.

It's preferred that the HTMLVideoElement render black frames for video, since 
- The MSE equivalent of de-selecting all tracks renders black frames.
- If a canvas is painted to from a HTMLVideoElement with all of its tracks disabled, it is painted black.

[Radar WebKit Bug Importer]

<rdar://problem/27557313>

[George Ruan]

Created attachment 284725 [details]
Patch

[Eric Carlson]

Comment on attachment 284725 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:153
> +    if ([m_sampleBufferDisplayLayer isReadyForMoreMediaData]) {
> +        if (m_displayMode == LivePreview)
> +            return true;
> +
> +        if (m_displayMode == PausedImage && !m_isFrameDisplayed)
> +            return true;
> +    }

Nit: I would get rid of this indentation by returning false immediately if the layer is not ready for media data.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:296
> +    RefPtr<Image> image = m_mediaStreamPrivate->currentFrameImage();
> +
> +    ASSERT(image);

If there is a chance that currentFrameImage() can return null, you should add an early return here so we don't crash in a release build.

> LayoutTests/fast/mediastream/MediaStream-video-element-video-tracks-disabled-then-enabled.html:52
> +        setTimeout(function() {
> +            attempt(--numberOfTries, call, callback, successMessage);
> +        }, 50);

Nit: Fat arrow functions are the new hotness:

setTimeout(() => { attempt(--numberOfTries, call, callback, successMessage) }, 50);

[George Ruan]

Created attachment 284734 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 284734 [details]
Patch

Rejecting attachment 284734 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 284734, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
fs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 203818 = d76af4e2980eab559600f17125b3742ef227f3a5
r203819 = 27a911a3631a0d8ca8528f7c68b6b64cbbdea797
r203820 = ee06b56411117e8d1d0a74edf13229b6250c64ce
r203821 = 3a6b2d23ab1bb3e1e7bc7332393d878c130ac67e
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/1768705

[Ryan Haddad]

Comment on attachment 284734 [details]
Patch

Clearing flags on attachment: 284734

Committed r203826: <http://trac.webkit.org/changeset/203826>

[Ryan Haddad]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

The test for this change is failing on El Capitan WK2:
https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r203830%20(8053)/results.html

[WebKit Commit Bot]

Re-opened since this is blocked by bug 160314

[George Ruan]

Created attachment 284839 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 284839 [details]
Patch

Clearing flags on attachment: 284839

Committed r203901: <http://trac.webkit.org/changeset/203901>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.