[webkit-reviews] review granted: [Bug 190684] [GStreamer][WebRTC] Implement black frame generation : [Attachment 353884] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Nov 5 22:48:40 PST 2018
Xabier RodrÃguez Calvar <calvaris at igalia.com> has granted Thibault Saunier
<tsaunier at gnome.org>'s request for review:
Bug 190684: [GStreamer][WebRTC] Implement black frame generation
https://bugs.webkit.org/show_bug.cgi?id=190684
Attachment 353884: Patch
https://bugs.webkit.org/attachment.cgi?id=353884&action=review
--- Comment #4 from Xabier RodrÃguez Calvar <calvaris at igalia.com> ---
Comment on attachment 353884
--> https://bugs.webkit.org/attachment.cgi?id=353884
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=353884&action=review
>
Source/WebCore/platform/mediastream/gstreamer/RealtimeOutgoingVideoSourceLibWeb
RTC.cpp:90
> + GRefPtr<GstCaps> caps = gst_video_info_to_caps(&info);
I think we are leaking the caps here. They should be adopted.
>
Source/WebCore/platform/mediastream/gstreamer/RealtimeOutgoingVideoSourceLibWeb
RTC.cpp:92
> + GstMappedBuffer map(buffer.get(), GST_MAP_WRITE);
Bonus points!
Could you please write this helper in GStreamerCommon.h?
GstMappedBuffer(const GRefPtr<GstBuffer>& buffer, GstMapFlags flags)
: GstMappedBuffer(buffer.get(), flags) { }
Then you don't have to pass buffer.get().
More bonus points!
I see it would be interesting to also write an ASSERT(GST_IS_BUFFER(m_buffer));
at:
~GstMappedBuffer()
{
if (m_isValid) {
ASSERT(GST_IS_BUFFER(m_buffer));
gst_buffer_unmap(m_buffer, &m_info);
}
m_isValid = false;
}
More information about the webkit-reviews
mailing list