[webkit-reviews] review granted: [Bug 192629] [MediaStream] A stream's first video frame should be rendered : [Attachment 357355] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 192629: [MediaStream] A stream's first video frame should be rendered
https://bugs.webkit.org/show_bug.cgi?id=192629

Attachment 357355: Patch

https://bugs.webkit.org/attachment.cgi?id=357355&action=review




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 357355
  --> https://bugs.webkit.org/attachment.cgi?id=357355
Patch

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

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStrea
mAVFObjC.h:283
> +    int m_waitingForFirstImage { 0 };

I would keep m_shouldDisplayFirstVideoFrame as a bool.

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStrea
mAVFObjC.mm:487
> +    m_backgroundLayer.get().hidden = hidden;

one liner.

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStrea
mAVFObjC.mm:1003
> +		   m_waitingForFirstImage = 6;

Since the issue is with AVVideoCaptureSource, I would for now throw away if
needed at AVVideoCaptureSource level.
This would not penalize other sources like canvas capture or remote tracks.

> LayoutTests/fast/mediastream/media-stream-renders-first-frame.html:36
> +		   }, 500);

Do we need this setTimeout?