[webkit-reviews] review granted: [Bug 237885] Migrate use of MediaSampleGStreamer to VideoFrame in WebRTC pipelines : [Attachment 454819] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 237885: Migrate use of MediaSampleGStreamer to VideoFrame in WebRTC
pipelines
https://bugs.webkit.org/show_bug.cgi?id=237885

Attachment 454819: Patch

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




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

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

> Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.h:69
>      MediaTime m_duration;

Should m_videoRotation and m_videoMirrored be removed as well if they are no
longer used except in implement videoRotation() and videoMirrored() that could
return default values?
Or maybe add a FIXME to say that they should be removed at some point?

> Source/WebCore/platform/graphics/gstreamer/VideoFrameGStreamer.cpp:99
> +    m_sample = sample;

You could do  m_sample(WTFMove(sample)) next to m_presentationSize instead.
Then reuse m_sample instead of sample in the constructor body.

> Source/WebCore/platform/graphics/gstreamer/VideoFrameGStreamer.h:46
> +    RefPtr<JSC::Uint8ClampedArray> getRGBAImageData() const final;

Could be private?

> Source/WebCore/platform/graphics/gstreamer/VideoFrameGStreamer.h:52
> +    VideoFrameGStreamer(const GRefPtr<GstSample>&, const MediaTime&
presentationTime, VideoRotation = VideoRotation::None);

Could be private?