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

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Sebastian Dröge <slomo at circular-chaos.org>  2012-01-30 02:40:49 PST ---
(From update of attachment 124093)
View in context: https://bugs.webkit.org/attachment.cgi?id=124093&action=review

> Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCG.mm:30
> +    return adoptRef(new ImageGStreamer(buffer, size));

Why don't you use the format at all here?

> Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCG.mm:51
>          kCGBitmapByteOrder32Little | kCGImageAlphaFirst, provider.get(), 0, false, kCGRenderingIntentDefault);

I think it makes sense to use the format here

> Source/WebCore/platform/graphics/gstreamer/ImageGStreamerCairo.cpp:36
>          cairoFormat = CAIRO_FORMAT_RGB24;

This is not true, GST_ARGB is CAIRO_ARGB32 on big endian systems and GST_BGRA is CAIRO_ARGB32 on little endian systems. Same for the one without alpha channel.

> 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

> Source/WebCore/platform/graphics/gstreamer/ImageGStreamerQt.cpp:35
>          imageFormat = QImage::Format_RGB32;

Same about endianness here too probably

> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer1.cpp:100
> +#if GLIB_CHECK_VERSION(2, 31, 0)

We depend on that version in GStreamer 0.11 anyway

> 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

> 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.

> 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

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