Message ID | 20250722105627.11961-4-jaslo@ziska.de |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
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 >
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 > >
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 > > >
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 > >
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 *
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 *
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(-)