[v3,7/9] pipeline: v4l2_subdevice: Add color space to format string representation
diff mbox series

Message ID 20250707085520.39777-8-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Wdr preparations
Related show

Commit Message

Stefan Klug July 7, 2025, 8:55 a.m. UTC
Add the color space to the string representation of V4L2SubdeviceFormat
that is returned by toString() and operator<<().

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

Changes in v3:
- Moved color space output to V4L2SubdeviceFormat
- Dropped tags, as the patch changed a bit more

Changes in v2:
- Collected tag
---
 src/libcamera/v4l2_subdevice.cpp | 2 ++
 1 file changed, 2 insertions(+)

Comments

Kieran Bingham July 7, 2025, 10 a.m. UTC | #1
Quoting Stefan Klug (2025-07-07 09:55:10)
> Add the color space to the string representation of V4L2SubdeviceFormat
> that is returned by toString() and operator<<().
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v3:
> - Moved color space output to V4L2SubdeviceFormat
> - Dropped tags, as the patch changed a bit more
> 
> Changes in v2:
> - Collected tag
> ---
>  src/libcamera/v4l2_subdevice.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 33279654db8c..ce6b0d38cbf1 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -949,6 +949,8 @@ std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f)
>         else
>                 out << it->second.name;
>  
> +       out << "/" << ColorSpace::toString(f.colorSpace);
> +

Looks good to me.

I assume the toString will deal with cases if f.colorSpace is invalid.


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

>         return out;
>  }
>  
> -- 
> 2.48.1
>
Isaac Scott July 7, 2025, 10:37 a.m. UTC | #2
Hi Stefan,

Thank you for the patch!

On Mon, 2025-07-07 at 10:55 +0200, Stefan Klug wrote:
> Add the color space to the string representation of
> V4L2SubdeviceFormat
> that is returned by toString() and operator<<().
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v3:
> - Moved color space output to V4L2SubdeviceFormat
> - Dropped tags, as the patch changed a bit more
> 
> Changes in v2:
> - Collected tag
> ---
>  src/libcamera/v4l2_subdevice.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/libcamera/v4l2_subdevice.cpp
> b/src/libcamera/v4l2_subdevice.cpp
> index 33279654db8c..ce6b0d38cbf1 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -949,6 +949,8 @@ std::ostream &operator<<(std::ostream &out, const
> V4L2SubdeviceFormat &f)
>  	else
>  		out << it->second.name;
>  
> +	out << "/" << ColorSpace::toString(f.colorSpace);
> +

Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>

>  	return out;
>  }
>
Laurent Pinchart July 7, 2025, 1:11 p.m. UTC | #3
Hi Stefan,

Thank you for the patch.

On Mon, Jul 07, 2025 at 10:55:10AM +0200, Stefan Klug wrote:
> Add the color space to the string representation of V4L2SubdeviceFormat
> that is returned by toString() and operator<<().

A *why* would be good here too.

Any reason not to do the same for V4L2DeviceFormat ?

> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v3:
> - Moved color space output to V4L2SubdeviceFormat
> - Dropped tags, as the patch changed a bit more
> 
> Changes in v2:
> - Collected tag
> ---
>  src/libcamera/v4l2_subdevice.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 33279654db8c..ce6b0d38cbf1 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -949,6 +949,8 @@ std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f)
>  	else
>  		out << it->second.name;
>  
> +	out << "/" << ColorSpace::toString(f.colorSpace);
> +
>  	return out;
>  }
>
Stefan Klug July 7, 2025, 1:29 p.m. UTC | #4
Hi Laurent,

Thank you for the comment.

Quoting Laurent Pinchart (2025-07-07 15:11:31)
> Hi Stefan,
> 
> Thank you for the patch.
> 
> On Mon, Jul 07, 2025 at 10:55:10AM +0200, Stefan Klug wrote:
> > Add the color space to the string representation of V4L2SubdeviceFormat
> > that is returned by toString() and operator<<().
> 
> A *why* would be good here too.

I thought about writing all the history about the bug in linux <= 6.13
and why that information is useful to debug that issue. Then I got
inspired by the concise commit message of
https://git.libcamera.org/libcamera/libcamera.git/commit/?id=9b50d3c23dea1bc2882cd3e6566a3d4cb9f7296f
which was proposed as base for the rework of this commit...

I can extend the message.

> 
> Any reason not to do the same for V4L2DeviceFormat ?

No reason other than I didn't need it.

Best regards,
Stefan

> 
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > 
> > ---
> > 
> > Changes in v3:
> > - Moved color space output to V4L2SubdeviceFormat
> > - Dropped tags, as the patch changed a bit more
> > 
> > Changes in v2:
> > - Collected tag
> > ---
> >  src/libcamera/v4l2_subdevice.cpp | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 33279654db8c..ce6b0d38cbf1 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -949,6 +949,8 @@ std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f)
> >       else
> >               out << it->second.name;
> >  
> > +     out << "/" << ColorSpace::toString(f.colorSpace);
> > +
> >       return out;
> >  }
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series

diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
index 33279654db8c..ce6b0d38cbf1 100644
--- a/src/libcamera/v4l2_subdevice.cpp
+++ b/src/libcamera/v4l2_subdevice.cpp
@@ -949,6 +949,8 @@  std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f)
 	else
 		out << it->second.name;
 
+	out << "/" << ColorSpace::toString(f.colorSpace);
+
 	return out;
 }