Summary: | HTMLVideoElement with MediaStream src shows paused image when all video tracks are disabled | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | George Ruan <gruan> | ||||||||
Component: | Media | Assignee: | 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
George Ruan
2016-07-26 16:15:18 PDT
Created attachment 284725 [details]
Patch
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); Created attachment 284734 [details]
Patch
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 on attachment 284734 [details] Patch Clearing flags on attachment: 284734 Committed r203826: <http://trac.webkit.org/changeset/203826> All reviewed patches have been landed. Closing bug. 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 Re-opened since this is blocked by bug 160314 Created attachment 284839 [details]
Patch
Comment on attachment 284839 [details] Patch Clearing flags on attachment: 284839 Committed r203901: <http://trac.webkit.org/changeset/203901> All reviewed patches have been landed. Closing bug. |