Bug 170404 – [MediaStream] Video doesn't render in fullscreen on iOS

• Status: ASSIGNED
• Component: Media

[Eric Carlson]

Video doesn't render in fullscreen on iOS

[Eric Carlson]

Created attachment 306086 [details]
Proposed patch.

[Sam Weinig]

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?

[Radar WebKit Bug Importer]

<rdar://problem/31407253>

[youenn fablet]

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?

[youenn fablet]

Any chance for some kind of test?

[Eric Carlson]

(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.

[Eric Carlson]

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.

[Eric Carlson]

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.

[Eric Carlson]

Created attachment 306298 [details]
Patch for landing.

[WebKit Commit Bot]

Comment on attachment 306298 [details]
Patch for landing.

Clearing flags on attachment: 306298

Committed r214953: <http://trac.webkit.org/changeset/214953>