Summary: | [MediaStream] Video doesn't render in fullscreen on iOS | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||||
Component: | Media | Assignee: | Eric Carlson <eric.carlson> | ||||||
Status: | ASSIGNED --- | ||||||||
Severity: | Normal | CC: | commit-queue, sam, webkit-bug-importer, youennf | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Eric Carlson
2017-04-03 10:02:38 PDT
Created attachment 306086 [details]
Proposed patch.
Comment on attachment 306086 [details]
Proposed patch.
Can this be tested? If not, is there a bug tracking adding infrastructure to test this kind of thing?
Comment on attachment 306086 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=306086&action=review > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h:-56 > -#if PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE) PLATFORM(COCOA)? > Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:1112 > + scheduleDeferredTask([this] { Should we protect ´this'? If so, should the m_backgroundLayer check be done here? Any chance for some kind of test? (In reply to Sam Weinig from comment #2) > Comment on attachment 306086 [details] > Proposed patch. > > Can this be tested? If not, is there a bug tracking adding infrastructure > to test this kind of thing? It can't be tested now, but I will file a bug and note it in the ChangeLog. Comment on attachment 306086 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=306086&action=review >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h:-56 >> -#if PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE) > > PLATFORM(COCOA)? Yep, good idea. >> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:1112 >> + scheduleDeferredTask([this] { > > Should we protect ´this'? > If so, should the m_backgroundLayer check be done here? scheduleDeferredTask creates a weak pointer, so that is done automatically. Good point about the m_backgroundLayer check, it should be done inside of the lambda. Comment on attachment 306086 [details] Proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=306086&action=review >>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.h:-56 >>> -#if PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE) >> >> PLATFORM(COCOA)? > > Yep, good idea. Actually that won't work because not all versions of macOS support PiP. Created attachment 306298 [details]
Patch for landing.
Comment on attachment 306298 [details] Patch for landing. Clearing flags on attachment: 306298 Committed r214953: <http://trac.webkit.org/changeset/214953> |