[libcamera-devel] [PATCH v3 0/5] Add colorimetry support to libcamera gstreamer element.
Umang Jain
umang.jain at ideasonboard.com
Tue Jul 12 08:19:36 CEST 2022
Hi Nicolas,
Thank you for the feedback.
On 7/11/22 23:02, Nicolas Dufresne via libcamera-devel wrote:
> Hi Rishikesh,
>
> I'm still uneasy on the design decision you are taking in this entire series.
> I'm always available on IRC for discussions if you need to.
>
> In general, your approach seems to only take a subset of libcamera and gstreamer
> (the well known colorspace names). I would personally completely ignore these
> user friendly goodies and just map four lower level enums: range, matrix,
> transfer function and primaries. This way, we can map the full range of options
> without ever having to update the code everytime something new is used by some
> hardware. GStreamer will transparently translate this to a "well-known-name" in
> caps.
So, you prefer to breaking down the GST_VIDEO_COLORIMETRY to lower enums
and create a libcamera::Colorspace out of it?
I assumed that the GStreamer would need to work only on standard
colorspaces on both libcamera and GStreamer's side and we don't need to
carry all the lower level enums mapping in GStreamer element.
But your point of being able to use it when something new is used, is
good. So in that context, I agree on breaking it down to lower enums and
creating a libcamera::Colorspace out of it. My only worry in that case
is that the assembled Colorspace might have one or more lower levels
enums as "Invalid" (i.e. not present in libcamera's colorspace.cpp) and
we proceed with such a Colorspace to be applied on the sensor?
Should we allow this? Or should we check if the constructed colorspace
belongs to a list of standard colorspaces (I sent a
isStandardColorSpace() helper patch on the list). Any thoughts?
>
> regards,
> Nicolas
>
> Le lundi 11 juillet 2022 à 21:07 +0530, Rishikesh Donadkar a écrit :
>> This series aims to add colorimetry support to the libcamera gstreamer
>> element.
>>
>> If colorimetry is specified by the user in the gstreamer pipeline, try to
>> apply the libcamera colorspace corresponding to the GStreamer colorimetry
>> to the camera if the colorspace passes the validation then push the
>> colorimetry corresponding to the valid colorspace in the caps.
>> If the colorimetry pushed into the caps is same as requested negotiation
>> will succeed, else negotiation will fail.
>>
>> If no colorimetry is specified by the user in the gstreamer pipeline,
>> push the GStreamer colorimetry corresponding to the current colorspace
>> applied to the camera into the caps and the negotiation will be take
>> care of by the gstreamer. If the colorimetry corresponding to the current
>> libcamera colorspace is not available in gstreamer then remove the
>> colorimetry field if present in the caps.
>>
>> [PATCH 1/5] - Made a verbose commit message. Introduced a new helper function
>> for converting std::string representation of colorimetry into
>> G_STRING_TYPE. Removed unnecessary g_free(). Removed the std::optional
>> wrapper around the colorSpace when converting into colorspace and
>> passed the value of the colorSpace instead to the function as it is
>> checked to have value before passing to the function.
>> Eliminated the code which was checking if the colorimetry is valid
>> colorimetry will be valid always because we are working with predefined
>> colorimetries in gstreamer.
>>
>> [PATCH 2/5] - Simplified the if-else block. Eliminated the code where colorimetry
>> was set to nullptr when it was not known. Insted the field is left
>> unset. If the field is present and is unset then remove the field.
>>
>> [PATCH 3/5] - colorSpace was passed by reference as a function argument in the
>> function colorimetry_form_colorspace(), Changed the function to
>> return the colorspace instead and setting the colorspace in the
>> StreamConfiguration was done after the colorspace is returned from
>> the function. The local functions that were used in the same file
>> were declared static.
>>
>> [PATCH 4/5] - Made the changes required due to the changes made in patch 3
>>
>> [PATCH 5/5] - If the camera cannot deliver the colorspace requested in the
>> gstreamer pipeline then the colorspace is reset to the value
>> that was present before the attempt was made to set the
>> colorspace corresponding to the requested colorimetry.
>>
>> Rishikesh Donadkar (5):
>> gstreamer: convert from libcamera colorspace to GStreamer colorimetry.
>> gstreamer: Update the obtained colorimetry in caps.
>> gstreamer: Convert from GStreamer colorimetry to libcamera ColorSpace
>> gstreamer: Configure the colorSpace in the Stream configuration.
>> gstreamer: Reset the colorSpace if not the same after validation.
>>
>> src/gstreamer/gstlibcamera-utils.cpp | 69 ++++++++++++++++++++++++++++
>> src/gstreamer/gstlibcamerasrc.cpp | 12 +++++
>> 2 files changed, 81 insertions(+)
>>
More information about the libcamera-devel
mailing list