[v2,3/3] gstreamer: Enable bayer formats with 10/12/14/16 bits
diff mbox series

Message ID 20250722105627.11961-4-jaslo@ziska.de
State Accepted
Headers show
Series
  • gstreamer: Miscellaneous fixes
Related show

Commit Message

Jaslo Ziska July 22, 2025, 10:39 a.m. UTC
GStreamer supports 10/12/14/16-bit bayer formats since version 1.24.

Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
---
 src/gstreamer/gstlibcamera-utils.cpp | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

Comments

Umang Jain July 22, 2025, 11:38 a.m. UTC | #1
Hi Jaslo,

On Tue, Jul 22, 2025 at 12:39:30PM +0200, Jaslo Ziska wrote:
> GStreamer supports 10/12/14/16-bit bayer formats since version 1.24.

Won't this require bumping the `gst_dep_version` ?

Currently:
src/gstreamer/meson.build:gst_dep_version = '>=1.14.0'

is what the minimum is and there might be reasons for keeping at 1.14
probably? 
See 04e7823eb24bb ("gstreamer: Document improvements when updating
		   minimum GStreamer version")
> 
> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> ---
>  src/gstreamer/gstlibcamera-utils.cpp | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index a548b0c1..c55a52d7 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -20,7 +20,7 @@ static struct {
>  	/* Compressed */
>  	{ GST_VIDEO_FORMAT_ENCODED, formats::MJPEG },
>  
> -	/* Bayer formats, gstreamer only supports 8-bit */
> +	/* Bayer formats */
>  	{ GST_VIDEO_FORMAT_ENCODED, formats::SBGGR8 },
>  	{ GST_VIDEO_FORMAT_ENCODED, formats::SGBRG8 },
>  	{ GST_VIDEO_FORMAT_ENCODED, formats::SGRBG8 },
> @@ -317,20 +317,15 @@ bare_structure_from_format(const PixelFormat &format)
>  		return gst_structure_new("video/x-raw", "format", G_TYPE_STRING,
>  					 gst_video_format_to_string(gst_format), nullptr);
>  
> -	switch (format) {
> -	case formats::MJPEG:
> +	if (format == formats::MJPEG)
>  		return gst_structure_new_empty("image/jpeg");
>  
> -	case formats::SBGGR8:
> -	case formats::SGBRG8:
> -	case formats::SGRBG8:
> -	case formats::SRGGB8:
> -		return gst_structure_new("video/x-bayer", "format", G_TYPE_STRING,
> -					 bayer_format_to_string(format), nullptr);
> -
> -	default:
> +	const gchar *s = bayer_format_to_string(format);
> +	if (s)
> +		return gst_structure_new("video/x-bayer", "format",
> +					 G_TYPE_STRING, s, nullptr);
> +	else
>  		return nullptr;
> -	}
>  }
>  
>  GstCaps *
> -- 
> 2.50.0
>
Kieran Bingham July 22, 2025, 12:29 p.m. UTC | #2
Quoting Umang Jain (2025-07-22 12:38:16)
> Hi Jaslo,
> 
> On Tue, Jul 22, 2025 at 12:39:30PM +0200, Jaslo Ziska wrote:
> > GStreamer supports 10/12/14/16-bit bayer formats since version 1.24.
> 
> Won't this require bumping the `gst_dep_version` ?

I assume not - as there is no ABI requirements here?

> 
> Currently:
> src/gstreamer/meson.build:gst_dep_version = '>=1.14.0'
> 
> is what the minimum is and there might be reasons for keeping at 1.14
> probably? 
> See 04e7823eb24bb ("gstreamer: Document improvements when updating
>                    minimum GStreamer version")
> > 
> > Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> > ---
> >  src/gstreamer/gstlibcamera-utils.cpp | 19 +++++++------------
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > index a548b0c1..c55a52d7 100644
> > --- a/src/gstreamer/gstlibcamera-utils.cpp
> > +++ b/src/gstreamer/gstlibcamera-utils.cpp
> > @@ -20,7 +20,7 @@ static struct {
> >       /* Compressed */
> >       { GST_VIDEO_FORMAT_ENCODED, formats::MJPEG },
> >  
> > -     /* Bayer formats, gstreamer only supports 8-bit */
> > +     /* Bayer formats */
> >       { GST_VIDEO_FORMAT_ENCODED, formats::SBGGR8 },
> >       { GST_VIDEO_FORMAT_ENCODED, formats::SGBRG8 },
> >       { GST_VIDEO_FORMAT_ENCODED, formats::SGRBG8 },
> > @@ -317,20 +317,15 @@ bare_structure_from_format(const PixelFormat &format)
> >               return gst_structure_new("video/x-raw", "format", G_TYPE_STRING,
> >                                        gst_video_format_to_string(gst_format), nullptr);
> >  
> > -     switch (format) {
> > -     case formats::MJPEG:
> > +     if (format == formats::MJPEG)
> >               return gst_structure_new_empty("image/jpeg");
> >  

This seems fine as only MJPEG and Bayer formats are
GST_VIDEO_FORMAT_ENCODED and everything else is handled before this
stage as video/x-raw.

> > -     case formats::SBGGR8:
> > -     case formats::SGBRG8:
> > -     case formats::SGRBG8:
> > -     case formats::SRGGB8:
> > -             return gst_structure_new("video/x-bayer", "format", G_TYPE_STRING,
> > -                                      bayer_format_to_string(format), nullptr);
> > -
> > -     default:
> > +     const gchar *s = bayer_format_to_string(format);
> > +     if (s)
> > +             return gst_structure_new("video/x-bayer", "format",
> > +                                      G_TYPE_STRING, s, nullptr);

as far as I can tell here - we convert the bayer format to a string - and
ask gstreamer to use it. If it's an old gstreamer, it may not be
supported, but if it's newer - then it will be successful...

So I don't think this requires increasing a gstreamer minimum version:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> > +     else
> >               return nullptr;
> > -     }
> >  }
> >  
> >  GstCaps *
> > -- 
> > 2.50.0
> >
Umang Jain July 23, 2025, 6:56 a.m. UTC | #3
On Tue, Jul 22, 2025 at 01:29:03PM +0100, Kieran Bingham wrote:
> Quoting Umang Jain (2025-07-22 12:38:16)
> > Hi Jaslo,
> > 
> > On Tue, Jul 22, 2025 at 12:39:30PM +0200, Jaslo Ziska wrote:
> > > GStreamer supports 10/12/14/16-bit bayer formats since version 1.24.
> > 
> > Won't this require bumping the `gst_dep_version` ?
> 
> I assume not - as there is no ABI requirements here?
> 
> > 
> > Currently:
> > src/gstreamer/meson.build:gst_dep_version = '>=1.14.0'
> > 
> > is what the minimum is and there might be reasons for keeping at 1.14
> > probably? 
> > See 04e7823eb24bb ("gstreamer: Document improvements when updating
> >                    minimum GStreamer version")
> > > 
> > > Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> > > ---
> > >  src/gstreamer/gstlibcamera-utils.cpp | 19 +++++++------------
> > >  1 file changed, 7 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > > index a548b0c1..c55a52d7 100644
> > > --- a/src/gstreamer/gstlibcamera-utils.cpp
> > > +++ b/src/gstreamer/gstlibcamera-utils.cpp
> > > @@ -20,7 +20,7 @@ static struct {
> > >       /* Compressed */
> > >       { GST_VIDEO_FORMAT_ENCODED, formats::MJPEG },
> > >  
> > > -     /* Bayer formats, gstreamer only supports 8-bit */
> > > +     /* Bayer formats */
> > >       { GST_VIDEO_FORMAT_ENCODED, formats::SBGGR8 },
> > >       { GST_VIDEO_FORMAT_ENCODED, formats::SGBRG8 },
> > >       { GST_VIDEO_FORMAT_ENCODED, formats::SGRBG8 },
> > > @@ -317,20 +317,15 @@ bare_structure_from_format(const PixelFormat &format)
> > >               return gst_structure_new("video/x-raw", "format", G_TYPE_STRING,
> > >                                        gst_video_format_to_string(gst_format), nullptr);
> > >  
> > > -     switch (format) {
> > > -     case formats::MJPEG:
> > > +     if (format == formats::MJPEG)
> > >               return gst_structure_new_empty("image/jpeg");
> > >  
> 
> This seems fine as only MJPEG and Bayer formats are
> GST_VIDEO_FORMAT_ENCODED and everything else is handled before this
> stage as video/x-raw.
> 
> > > -     case formats::SBGGR8:
> > > -     case formats::SGBRG8:
> > > -     case formats::SGRBG8:
> > > -     case formats::SRGGB8:
> > > -             return gst_structure_new("video/x-bayer", "format", G_TYPE_STRING,
> > > -                                      bayer_format_to_string(format), nullptr);
> > > -
> > > -     default:
> > > +     const gchar *s = bayer_format_to_string(format);
> > > +     if (s)
> > > +             return gst_structure_new("video/x-bayer", "format",
> > > +                                      G_TYPE_STRING, s, nullptr);
> 
> as far as I can tell here - we convert the bayer format to a string - and
> ask gstreamer to use it. If it's an old gstreamer, it may not be
> supported, but if it's newer - then it will be successful...
> 
> So I don't think this requires increasing a gstreamer minimum version:

Oh - I was thinking if we should be supporting *all* and hence my
comment on minimum version bump.

> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 

Reviewed-by: Umang Jain <uajain@igalia.com>
> > > +     else
> > >               return nullptr;
> > > -     }
> > >  }
> > >  
> > >  GstCaps *
> > > -- 
> > > 2.50.0
> > >
Nicolas Dufresne July 28, 2025, 12:49 p.m. UTC | #4
Hi,

Le mardi 22 juillet 2025 à 17:08 +0530, Umang Jain a écrit :
> Hi Jaslo,
> 
> On Tue, Jul 22, 2025 at 12:39:30PM +0200, Jaslo Ziska wrote:
> > GStreamer supports 10/12/14/16-bit bayer formats since version 1.24.
> 
> Won't this require bumping the `gst_dep_version` ?

Bayer formats don't have a matching library with an fixed enums, so the formats
are just strings. In this context, we can enable support for any 1.X version,
the only downside is that bayer2rgb (mostly used to get a visual, since it does
not do anything to improve the image), won't support that.

Nicolas

> 
> Currently:
> src/gstreamer/meson.build:gst_dep_version = '>=1.14.0'
> 
> is what the minimum is and there might be reasons for keeping at 1.14
> probably? 
> See 04e7823eb24bb ("gstreamer: Document improvements when updating
> 		   minimum GStreamer version")
> > 
> > Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> > ---
> >  src/gstreamer/gstlibcamera-utils.cpp | 19 +++++++------------
> >  1 file changed, 7 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > index a548b0c1..c55a52d7 100644
> > --- a/src/gstreamer/gstlibcamera-utils.cpp
> > +++ b/src/gstreamer/gstlibcamera-utils.cpp
> > @@ -20,7 +20,7 @@ static struct {
> >  	/* Compressed */
> >  	{ GST_VIDEO_FORMAT_ENCODED, formats::MJPEG },
> >  
> > -	/* Bayer formats, gstreamer only supports 8-bit */
> > +	/* Bayer formats */
> >  	{ GST_VIDEO_FORMAT_ENCODED, formats::SBGGR8 },
> >  	{ GST_VIDEO_FORMAT_ENCODED, formats::SGBRG8 },
> >  	{ GST_VIDEO_FORMAT_ENCODED, formats::SGRBG8 },
> > @@ -317,20 +317,15 @@ bare_structure_from_format(const PixelFormat &format)
> >  		return gst_structure_new("video/x-raw", "format", G_TYPE_STRING,
> >  					 gst_video_format_to_string(gst_format), nullptr);
> >  
> > -	switch (format) {
> > -	case formats::MJPEG:
> > +	if (format == formats::MJPEG)
> >  		return gst_structure_new_empty("image/jpeg");
> >  
> > -	case formats::SBGGR8:
> > -	case formats::SGBRG8:
> > -	case formats::SGRBG8:
> > -	case formats::SRGGB8:
> > -		return gst_structure_new("video/x-bayer", "format", G_TYPE_STRING,
> > -					 bayer_format_to_string(format), nullptr);
> > -
> > -	default:
> > +	const gchar *s = bayer_format_to_string(format);
> > +	if (s)
> > +		return gst_structure_new("video/x-bayer", "format",
> > +					 G_TYPE_STRING, s, nullptr);
> > +	else
> >  		return nullptr;
> > -	}
> >  }
> >  
> >  GstCaps *
> > -- 
> > 2.50.0
> >
Laurent Pinchart July 30, 2025, 9:03 p.m. UTC | #5
On Tue, Jul 22, 2025 at 01:29:03PM +0100, Kieran Bingham wrote:
> Quoting Umang Jain (2025-07-22 12:38:16)
> > Hi Jaslo,
> > 
> > On Tue, Jul 22, 2025 at 12:39:30PM +0200, Jaslo Ziska wrote:
> > > GStreamer supports 10/12/14/16-bit bayer formats since version 1.24.
> > 
> > Won't this require bumping the `gst_dep_version` ?
> 
> I assume not - as there is no ABI requirements here?
> 
> > 
> > Currently:
> > src/gstreamer/meson.build:gst_dep_version = '>=1.14.0'
> > 
> > is what the minimum is and there might be reasons for keeping at 1.14
> > probably? 
> > See 04e7823eb24bb ("gstreamer: Document improvements when updating
> >                    minimum GStreamer version")
> > > 
> > > Signed-off-by: Jaslo Ziska <jaslo@ziska.de>
> > > ---
> > >  src/gstreamer/gstlibcamera-utils.cpp | 19 +++++++------------
> > >  1 file changed, 7 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > > index a548b0c1..c55a52d7 100644
> > > --- a/src/gstreamer/gstlibcamera-utils.cpp
> > > +++ b/src/gstreamer/gstlibcamera-utils.cpp
> > > @@ -20,7 +20,7 @@ static struct {
> > >       /* Compressed */
> > >       { GST_VIDEO_FORMAT_ENCODED, formats::MJPEG },
> > >  
> > > -     /* Bayer formats, gstreamer only supports 8-bit */
> > > +     /* Bayer formats */
> > >       { GST_VIDEO_FORMAT_ENCODED, formats::SBGGR8 },
> > >       { GST_VIDEO_FORMAT_ENCODED, formats::SGBRG8 },
> > >       { GST_VIDEO_FORMAT_ENCODED, formats::SGRBG8 },
> > > @@ -317,20 +317,15 @@ bare_structure_from_format(const PixelFormat &format)
> > >               return gst_structure_new("video/x-raw", "format", G_TYPE_STRING,
> > >                                        gst_video_format_to_string(gst_format), nullptr);
> > >  
> > > -     switch (format) {
> > > -     case formats::MJPEG:
> > > +     if (format == formats::MJPEG)
> > >               return gst_structure_new_empty("image/jpeg");
> > >  
> 
> This seems fine as only MJPEG and Bayer formats are
> GST_VIDEO_FORMAT_ENCODED and everything else is handled before this
> stage as video/x-raw.
> 
> > > -     case formats::SBGGR8:
> > > -     case formats::SGBRG8:
> > > -     case formats::SGRBG8:
> > > -     case formats::SRGGB8:
> > > -             return gst_structure_new("video/x-bayer", "format", G_TYPE_STRING,
> > > -                                      bayer_format_to_string(format), nullptr);
> > > -
> > > -     default:
> > > +     const gchar *s = bayer_format_to_string(format);
> > > +     if (s)
> > > +             return gst_structure_new("video/x-bayer", "format",
> > > +                                      G_TYPE_STRING, s, nullptr);
> 
> as far as I can tell here - we convert the bayer format to a string - and
> ask gstreamer to use it. If it's an old gstreamer, it may not be
> supported, but if it's newer - then it will be successful...
> 
> So I don't think this requires increasing a gstreamer minimum version:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > > +     else
> > >               return nullptr;

I'll drop the else.

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

> > > -     }
> > >  }
> > >  
> > >  GstCaps *

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
index a548b0c1..c55a52d7 100644
--- a/src/gstreamer/gstlibcamera-utils.cpp
+++ b/src/gstreamer/gstlibcamera-utils.cpp
@@ -20,7 +20,7 @@  static struct {
 	/* Compressed */
 	{ GST_VIDEO_FORMAT_ENCODED, formats::MJPEG },
 
-	/* Bayer formats, gstreamer only supports 8-bit */
+	/* Bayer formats */
 	{ GST_VIDEO_FORMAT_ENCODED, formats::SBGGR8 },
 	{ GST_VIDEO_FORMAT_ENCODED, formats::SGBRG8 },
 	{ GST_VIDEO_FORMAT_ENCODED, formats::SGRBG8 },
@@ -317,20 +317,15 @@  bare_structure_from_format(const PixelFormat &format)
 		return gst_structure_new("video/x-raw", "format", G_TYPE_STRING,
 					 gst_video_format_to_string(gst_format), nullptr);
 
-	switch (format) {
-	case formats::MJPEG:
+	if (format == formats::MJPEG)
 		return gst_structure_new_empty("image/jpeg");
 
-	case formats::SBGGR8:
-	case formats::SGBRG8:
-	case formats::SGRBG8:
-	case formats::SRGGB8:
-		return gst_structure_new("video/x-bayer", "format", G_TYPE_STRING,
-					 bayer_format_to_string(format), nullptr);
-
-	default:
+	const gchar *s = bayer_format_to_string(format);
+	if (s)
+		return gst_structure_new("video/x-bayer", "format",
+					 G_TYPE_STRING, s, nullptr);
+	else
 		return nullptr;
-	}
 }
 
 GstCaps *