[libcamera-devel,PATCHv2] gstreamer: Add bayer8 support to libcamerasrc element
diff mbox series

Message ID Y6XkAEC7SOKsKMBk@duo.ucw.cz
State Accepted
Headers show
Series
  • [libcamera-devel,PATCHv2] gstreamer: Add bayer8 support to libcamerasrc element
Related show

Commit Message

Pavel Machek Dec. 23, 2022, 5:23 p.m. UTC 
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.
    
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

--

Thanks for the review!

Comments

Rafael Diniz Dec. 24, 2022, 10:12 a.m. UTC | #1 
Hi Pavel and all,

Do you have a sample gst-lauch line which forces bayer format in order I 
can test this also in the PinePhone 1?

Rafael

On 12/23/22 20:23, 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.
>      
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> --
> 
> 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 },
> +
>   	/* 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";
> +	}
> +	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:
> +		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 = {
>
Nicolas Dufresne Dec. 24, 2022, 1:29 p.m. UTC | #2 
Le sam. 24 déc. 2022, 05 h 12, Rafael Diniz <rafael@riseup.net> a écrit :

> Hi Pavel and all,
>
> Do you have a sample gst-lauch line which forces bayer format in order I
> can test this also in the PinePhone 1?
>

I can't test, but this should do:

gst-launch-1.0 libcamerasrc ! bayer2rgb ! glimagesink

The bayer2rgb template caps will serve as a filter, like a jpeg decoder
would force jpeg. But if needed, video/x-bayer is the filter you need.

best wishes,
Nicolas


> Rafael
>
> On 12/23/22 20:23, 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.
> >
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> >
> > --
> >
> > 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 },
> > +
> >       /* 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";
> > +     }
> > +     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:
> > +             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 = {
> >
>
Pavel Machek Dec. 24, 2022, 2:31 p.m. UTC | #3 
Hi!

> Do you have a sample gst-lauch line which forces bayer format in order I can
> test this also in the PinePhone 1?

I'm using:

gst-launch-1.0 libcamerasrc camera-name="/base/soc@0/bus@30800000/i2c@30a50000/camera@2d" !  bayer2rgb ! videoscale ! video/x-raw,width=640,height=480 ! videoflip method=clockwise ! ximagesink

You'll need to tweak at least the name.

BR,
								Pavel
Rafael Diniz Dec. 24, 2022, 2:41 p.m. UTC | #4 
Tks all!
: )


On 12/24/22 17:31, Pavel Machek wrote:
> Hi!
> 
>> Do you have a sample gst-lauch line which forces bayer format in order I can
>> test this also in the PinePhone 1?
> 
> I'm using:
> 
> gst-launch-1.0 libcamerasrc camera-name="/base/soc@0/bus@30800000/i2c@30a50000/camera@2d" !  bayer2rgb ! videoscale ! video/x-raw,width=640,height=480 ! videoflip method=clockwise ! ximagesink
> 
> You'll need to tweak at least the name.
> 
> BR,
> 								Pavel
>
Laurent Pinchart Dec. 24, 2022, 3:18 p.m. UTC | #5 
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@ucw.cz>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@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@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 = {
Pavel Machek Dec. 24, 2022, 3:43 p.m. UTC | #6 
Hi!

> 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@ideasonboard.com>

Fine with them, thank you!

Best regards,
								Pavel

Patch
diff mbox series 

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 },
+
 	/* 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";
+	}
+	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:
+		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 = {