[Webkit-unassigned] [Bug 77087] [GStreamer] 0.11 video-sink

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=77087





--- Comment #5 from Philippe Normand <pnormand at igalia.com>  2012-02-06 09:18:48 PST ---
(From update of attachment 124093)
View in context: https://bugs.webkit.org/attachment.cgi?id=124093&action=review

>> Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp:55
>>      m_image = BitmapImage::create(surface);
> 
> Is it necessary to keep the buffer around until the object is destroyed? Same question for the other implementations

I don't think so because the ImageGStreamer ctors copy the buffer data to platform-dependant representations of the Image.

>> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer1.cpp:182
>> +        gst_buffer_copy_into(newBuffer, buffer, (GstBufferCopyFlags) GST_BUFFER_COPY_ALL, 0, bufferSize);
> 
> You copy twice here. Just create and allocate a new buffer and fill the new buffer in the loop below

Oh hum I meant to copy only the metadata here, so I'll use  GST_BUFFER_COPY_METADATA instead of _ALL :)

>> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer1.cpp:248
>> +#endif
> 
> I'd like to see these abstracted into a helper method.

Well I'm not sure it's really needed. As Sebastian points out GStreamer 0.11 requires a quite recent glib anyway and these g_cond_ calls are not used in lots of places.

>> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer1.cpp:351
>> +        gst_buffer_pool_config_add_option(config, GST_BUFFER_POOL_OPTION_VIDEO_META);
> 
> Do you handle the video meta in the request-repaint signal handlers? There might be any possible stride used if you add this meta instead of the old GST_ROUND_UP_4(width) strides.

Hum I don't use it in the signal handler. I guess it's best to remove this option from the pool then?

>> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer1.cpp:356
>> +    gst_query_add_allocation_meta(query, GST_VIDEO_META_API);
> 
> You might want to add support for the crop meta too here

Is it really needed?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.