[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 New
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
> > >

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 *