Bug 237885 – Migrate use of MediaSampleGStreamer to VideoFrame in WebRTC pipelines

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Migrate use of MediaSampleGStreamer to VideoFrame in WebRTC pipelines

[Philippe Normand]

I started a patch.

[Philippe Normand]

Created attachment 454819 [details]
Patch

[youenn fablet]

Comment on attachment 454819 [details]
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?

[Philippe Normand]

Comment on attachment 454819 [details]
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?

Ah yes, I forgot to remove those.

[Xabier Rodríguez Calvar]

Comment on attachment 454819 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.cpp:84
> +Ref<MediaSampleGStreamer> MediaSampleGStreamer::createFakeSample(GstCaps*, MediaTime pts, MediaTime dts, MediaTime duration, const FloatSize& presentationSize, const AtomString& trackId)

Can we make the MediaTimes const &?

[Philippe Normand]

Created attachment 454842 [details]
[fast-cq] Patch

[EWS]

Committed r291357 (248490@main): <https://commits.webkit.org/248490@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454842 [details].

[Radar WebKit Bug Importer]

<rdar://problem/90378933>