GNOME Bugzilla – Bug 702112
waylandsink: add support for RGBx and RGBA formats
Last modified: 2013-06-18 12:03:32 UTC
Created attachment 246652 [details] [review] waylandsink: support of RGBx and RGBA Wayland interface could offer two buffers pixels formats: WL_SHM_FORMAT_XRGB8888 and WL_SHM_FORMAT_ARGB8888. Update waylandsink to support them and check if the format is really available.
Review of attachment 246652 [details] [review]: ::: ext/wayland/gstwaylandsink.c @@ -64,3 @@ GST_PAD_ALWAYS, - GST_STATIC_CAPS ("video/x-raw, " - "format = (string) BGRA, " You mean BGRA is not supported anymore? Is this maybe endianness dependant? BGRA on little endian, ARGB on big endian? @@ +337,3 @@ + break; + case GST_VIDEO_FORMAT_RGBA: + *wl_format = WL_SHM_FORMAT_ARGB8888; This could be a implemented with a mapping table for easier future extension @@ +367,3 @@ + if (!(sink->display->formats & (1 << sink->format))) { + GST_DEBUG_OBJECT (sink, "%s not available", + (sink->format == WL_SHM_FORMAT_ARGB8888) ? "ARGB8888" : "XRGB8888"); Some kind of gst_wayland_format_to_string() might be useful here
Created attachment 246693 [details] [review] waylandsink: support of RGBx and RGBA add mapping table and gst_wayland_format_to_string function to the previous patch. waylandsink caps were BGRA while the buffers created from wl_shm pool had WL_SHM_FORMAT_XRGB8888 format. For me that was a mismatch. I have test the both cases with videotestsrc and I have the correct colors, my platform is in little endian.
Created attachment 246740 [details] [review] waylandsink: support of RGBx and RGBA try to get the good format for little and big endian platform.
wayland buffer format definition is here: http://cgit.freedesktop.org/wayland/wayland/tree/protocol/wayland.xml#n275
Review of attachment 246740 [details] [review]: ::: ext/wayland/gstwaylandsink.c @@ +63,3 @@ GST_PAD_SINK, GST_PAD_ALWAYS, + GST_STATIC_CAPS (GST_VIDEO_CAPS_MAKE ("{RGBx, RGBA}")) Are you sure this is correct? It seems to be BGRA/BGRx on little-endian system as I understand it... and ARGB/xRGB on big endian systems. @@ +102,3 @@ +} wl_VideoFormat; + +static wl_VideoFormat formats[] = { This should be const
Created attachment 246809 [details] [review] waylandsink: support of RGBx and RGBA try to distinguish big/little endian when convert format from gst format to wayland formats make formats array const.
Review of attachment 246809 [details] [review]: ::: ext/wayland/gstwaylandsink.c @@ +63,3 @@ GST_PAD_SINK, GST_PAD_ALWAYS, + GST_STATIC_CAPS (GST_VIDEO_CAPS_MAKE ("{xRGB, ARGB, BGRx, BGRA}")) #if G_BYTE_ORDER == G_LITTLE_ENDIAN #define CAPS "BGRx, BGRA" #else #define CAPS "xRGB, ARGB" #endif static GstStaticPadTemplate ..... GST_STATIC_CAPS (CAPS)....
Created attachment 246816 [details] [review] waylandsink: support of RGBx and RGBA follow comments and make CAPS fixed at compilation time.
Review of attachment 246816 [details] [review]: ::: ext/wayland/gstwaylandsink.c @@ +135,3 @@ + + for (i = 0; i < G_N_ELEMENTS (formats); i++) + if (formats[i].wl_format == wl_format) You realize that this will always print xRGB or ARGB, never BGRx and BGRA? :)
Created attachment 247131 [details] [review] waylandsink: support of RGBx and RGBA No I haven't realize that. The error has never occur, it was only "in case of". Patch version 5 should fix that.
commit 3fc6f1d9b7110baa3c9e85a9037b3ff4dbdfd330 Author: Benjamin Gaignard <benjamin.gaignard@linaro.org> Date: Tue Jun 18 13:47:54 2013 +0200 wayland: Add support for RGBx and RGBA formats Wayland interface could offer two buffers pixels formats: WL_SHM_FORMAT_XRGB Update waylandsink to support them and check if the format is really availab https://bugzilla.gnome.org/show_bug.cgi?id=702112