[libcamera-devel] [PATCHv2] gstreamer: Add bayer8 support to libcamerasrc element

[Laurent Pinchart]

Hi Pavel,

Thank you for the patch.

On Fri, Dec 23, 2022 at 06:23:12PM +0100, Pavel Machek via libcamera-devel wrote:
> Bayer8 support is useful on hardware such as Librem 5, as GStreamer
> provides easy solution for debayering and display of the camera
> data. Add neccessary glue to libcamerasrc element.

s/neccessary/necessary/

> Signed-off-by: Pavel Machek <pavel at ucw.cz>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> 
> --

If you want to add text that should be dropped by git-am, the correct
marker is ---, not --.

> Thanks for the review!
> 
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index 36b9564c..b2cc7661 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -20,6 +20,12 @@ static struct {
>  	/* Compressed */
>  	{ GST_VIDEO_FORMAT_ENCODED, formats::MJPEG },
>  
> +	/* Bayer formats, gstreamer only supports 8-bit */
> +	{ GST_VIDEO_FORMAT_ENCODED, formats::SGRBG8 },
> +	{ GST_VIDEO_FORMAT_ENCODED, formats::SGBRG8 },
> +	{ GST_VIDEO_FORMAT_ENCODED, formats::SRGGB8 },
> +	{ GST_VIDEO_FORMAT_ENCODED, formats::SBGGR8 },

If you don't mind, I'd like to sort these alphabetically.

> +
>  	/* RGB16 */
>  	{ GST_VIDEO_FORMAT_RGB16, formats::RGB565 },
>  
> @@ -228,6 +234,22 @@ gst_format_to_pixel_format(GstVideoFormat gst_format)
>  	return PixelFormat{};
>  }
>  
> +static const gchar *
> +bayer_format_to_string(int format)
> +{
> +	switch (format) {
> +	case formats::SGRBG8:
> +		return "grbg";
> +	case formats::SGBRG8:
> +		return "gbrg";
> +	case formats::SRGGB8:
> +		return "rggb";
> +	case formats::SBGGR8:
> +		return "bggr";

Same here.

> +	}
> +	return NULL;
> +}
> +
>  static GstStructure *
>  bare_structure_from_format(const PixelFormat &format)
>  {
> @@ -243,6 +265,12 @@ bare_structure_from_format(const PixelFormat &format)
>  	switch (format) {
>  	case formats::MJPEG:
>  		return gst_structure_new_empty("image/jpeg");
> +	case formats::SGRBG8:
> +	case formats::SGBRG8:
> +	case formats::SRGGB8:
> +	case formats::SBGGR8:

And here too.

If you're fine with these small changes, I can apply them locally,
there's no need to send a v3.

Conditionally-Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +		return gst_structure_new("video/x-bayer", "format", G_TYPE_STRING,
> +					 bayer_format_to_string(format), nullptr);
>  	default:
>  		return nullptr;
>  	}
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 8d97d7c2..a10cbd4f 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -161,7 +161,7 @@ G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT,
>  			GST_DEBUG_CATEGORY_INIT(source_debug, "libcamerasrc", 0,
>  						"libcamera Source"))
>  
> -#define TEMPLATE_CAPS GST_STATIC_CAPS("video/x-raw; image/jpeg")
> +#define TEMPLATE_CAPS GST_STATIC_CAPS("video/x-raw; image/jpeg; video/x-bayer")
>  
>  /* For the simple case, we have a src pad that is always present. */
>  GstStaticPadTemplate src_template = {

-- 
Regards,

Laurent Pinchart