Bug 160222

Summary: HTMLVideoElement with MediaStream src shows paused image when all video tracks are disabled
Product: WebKit Reporter: George Ruan <gruan>
Component: MediaAssignee: George Ruan <gruan>
Status: RESOLVED FIXED    
Severity: Minor CC: commit-queue, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: Other   
Bug Depends on: 160314    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description George Ruan 2016-07-26 16:15:18 PDT
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.
Comment 1 Radar WebKit Bug Importer 2016-07-26 16:16:14 PDT
<rdar://problem/27557313>
Comment 2 George Ruan 2016-07-27 13:51:35 PDT
Created attachment 284725 [details]
Patch
Comment 3 Eric Carlson 2016-07-27 14:07:10 PDT
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);
Comment 4 George Ruan 2016-07-27 15:15:23 PDT
Created attachment 284734 [details]
Patch
Comment 5 WebKit Commit Bot 2016-07-28 11:19:15 PDT
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
Comment 6 Ryan Haddad 2016-07-28 11:27:35 PDT
Comment on attachment 284734 [details]
Patch

Clearing flags on attachment: 284734

Committed r203826: <http://trac.webkit.org/changeset/203826>
Comment 7 Ryan Haddad 2016-07-28 11:27:39 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Ryan Haddad 2016-07-28 13:59:20 PDT
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
Comment 9 WebKit Commit Bot 2016-07-28 14:00:44 PDT
Re-opened since this is blocked by bug 160314
Comment 10 George Ruan 2016-07-28 16:58:53 PDT
Created attachment 284839 [details]
Patch
Comment 11 WebKit Commit Bot 2016-07-29 10:01:15 PDT
Comment on attachment 284839 [details]
Patch

Clearing flags on attachment: 284839

Committed r203901: <http://trac.webkit.org/changeset/203901>
Comment 12 WebKit Commit Bot 2016-07-29 10:01:19 PDT
All reviewed patches have been landed.  Closing bug.