Re: [PATCH 1/1] v4l uapi: Make "Vertical Colour Bars" menu item a little more generic

2018-12-04 Thread Mauro Carvalho Chehab 
Em Tue, 4 Dec 2018 12:32:32 -0200
Mauro Carvalho Chehab  escreveu:

> Em Tue,  4 Dec 2018 15:45:06 +0200
> Sakari Ailus  escreveu:
> 
> > The test pattern could contain a different number of colour bars than
> > eight, make the entry more useful by removing "Eight " from the name.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  include/uapi/linux/v4l2-controls.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/uapi/linux/v4l2-controls.h 
> > b/include/uapi/linux/v4l2-controls.h
> > index acb2a57fa5d6..88f2759c2ce4 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -1016,7 +1016,7 @@ enum v4l2_jpeg_chroma_subsampling {
> >  #define V4L2_CID_TEST_PATTERN  
> > (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
> >  #define V4L2_TEST_PATTERN_DISABLED "Disabled"
> >  #define V4L2_TEST_PATTERN_SOLID_COLOUR "Solid Colour"
> > -#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
> > +#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Vertical Colour Bars"
> 
> No, we can't do that. This is on an uAPI file.
> 
> We should, instead, create another #define for non-eight vertical
> color bars.
> 
> Before you say, yeah, I agree that we messed this one, as the defined
> name doesn't mention 8 bars...
> 
> I would, instead, do something like:
> 
> -#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS   "Eight Vertical Colour Bars"
> +#define V4L2_TEST_PATTERN_VERT_8_COLOUR_BARS "Eight Vertical Colour Bars"
> +#define V4L2_TEST_PATTERN_VERT_N_COLOUR_BARS "Vertical Colour Bars"
> +
> + /* Kept for backward-compatibility */
> +#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS   
> V4L2_TEST_PATTERN_VERT_8_COLOUR_BARS
> 
> And, of course, update the documentation accordingly.

Please ignore this comment. I didn't realize that those definitions
don't exist yet at the uAPI file, and that this is a follow up for
another patch:

Subject: [PATCH v2 1/1] media: Use common test pattern menu entries

Next time, please put them into a patch series, as it makes easier for
reviewers.

Thanks,
Mauro

Re: [PATCH 1/1] v4l uapi: Make "Vertical Colour Bars" menu item a little more generic

2018-12-04 Thread Mauro Carvalho Chehab 
Em Tue,  4 Dec 2018 15:45:06 +0200
Sakari Ailus  escreveu:

> The test pattern could contain a different number of colour bars than
> eight, make the entry more useful by removing "Eight " from the name.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  include/uapi/linux/v4l2-controls.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/v4l2-controls.h 
> b/include/uapi/linux/v4l2-controls.h
> index acb2a57fa5d6..88f2759c2ce4 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1016,7 +1016,7 @@ enum v4l2_jpeg_chroma_subsampling {
>  #define V4L2_CID_TEST_PATTERN
> (V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
>  #define V4L2_TEST_PATTERN_DISABLED   "Disabled"
>  #define V4L2_TEST_PATTERN_SOLID_COLOUR   "Solid Colour"
> -#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS   "Eight Vertical Colour Bars"
> +#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS   "Vertical Colour Bars"

No, we can't do that. This is on an uAPI file.

We should, instead, create another #define for non-eight vertical
color bars.

Before you say, yeah, I agree that we messed this one, as the defined
name doesn't mention 8 bars...

I would, instead, do something like:

-#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_VERT_8_COLOUR_BARS   "Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_VERT_N_COLOUR_BARS   "Vertical Colour Bars"
+
+ /* Kept for backward-compatibility */
+#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS 
V4L2_TEST_PATTERN_VERT_8_COLOUR_BARS

And, of course, update the documentation accordingly.

>  #define V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars With 
> Fade to Grey"
>  #define V4L2_TEST_PATTERN_PN9"Pseudorandom Sequence 
> (PN9)"
>  #define V4L2_CID_DEINTERLACING_MODE  (V4L2_CID_IMAGE_PROC_CLASS_BASE 
> + 4)



Thanks,
Mauro

[PATCH 1/1] v4l uapi: Make "Vertical Colour Bars" menu item a little more generic

2018-12-04 Thread Sakari Ailus 
The test pattern could contain a different number of colour bars than
eight, make the entry more useful by removing "Eight " from the name.

Signed-off-by: Sakari Ailus 
---
 include/uapi/linux/v4l2-controls.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/v4l2-controls.h 
b/include/uapi/linux/v4l2-controls.h
index acb2a57fa5d6..88f2759c2ce4 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1016,7 +1016,7 @@ enum v4l2_jpeg_chroma_subsampling {
 #define V4L2_CID_TEST_PATTERN  (V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 3)
 #define V4L2_TEST_PATTERN_DISABLED "Disabled"
 #define V4L2_TEST_PATTERN_SOLID_COLOUR "Solid Colour"
-#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Eight Vertical Colour Bars"
+#define V4L2_TEST_PATTERN_VERT_COLOUR_BARS "Vertical Colour Bars"
 #define V4L2_TEST_PATTERN_VERT_COLOUR_BARS_FADE_TO_GREY "Colour Bars With Fade 
to Grey"
 #define V4L2_TEST_PATTERN_PN9  "Pseudorandom Sequence (PN9)"
 #define V4L2_CID_DEINTERLACING_MODE(V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 4)
-- 
2.11.0