Re: [PATCH] media: v4l2-ctrls: add control for test pattern

2012-09-03 Thread Prabhakar Lad
Hi Sakari,

Thanks for the review.

On Tuesday 04 September 2012 01:09 AM, Sakari Ailus wrote:
> Hi Prabhakar,
> 
> Thanks for the patch.
> 
> On Mon, Sep 03, 2012 at 02:46:17PM +0530, Prabhakar Lad wrote:
>> From: Lad, Prabhakar 
>>
>> add V4L2_CID_TEST_PATTERN of type menu, which determines
>> the internal test pattern selected by the device.
>>
>> Signed-off-by: Lad, Prabhakar 
>> Signed-off-by: Manjunath Hadli 
>> Cc: Sakari Ailus 
>> Cc: Hans Verkuil 
>> Cc: Laurent Pinchart 
>> Cc: Mauro Carvalho Chehab 
>> Cc: Sylwester Nawrocki 
>> Cc: Hans de Goede 
>> Cc: Kyungmin Park 
>> Cc: Rob Landley 
>> Cc: HeungJun Kim 
>> Cc: Rob Landley 
>> ---
>>  This patches has one checkpatch warning for line over
>>  80 characters altough it can be avoided I have kept it
>>  for consistency.
>>
>>  Documentation/DocBook/media/v4l/controls.xml |   52 
>> ++
>>  drivers/media/v4l2-core/v4l2-ctrls.c |   16 
>>  include/linux/videodev2.h|   12 ++
>>  3 files changed, 80 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
>> b/Documentation/DocBook/media/v4l/controls.xml
>> index f704218..06f16e7 100644
>> --- a/Documentation/DocBook/media/v4l/controls.xml
>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> @@ -4313,6 +4313,58 @@ interface and may change in the future.
>>
>>  
>>
>> +  
>> +> spanname="id">V4L2_CID_TEST_PATTERN
>> +menu
>> +  
>> +  
>> + The capture devices/sensors have the 
>> capability to
>> +generate internal test patterns. This test patterns are used to 
>> test a device
>> +is properly working and can generate the desired waveforms that it 
>> supports.
>> +
>> +  
>> +  
>> +
>> +  
>> +
>> + V4L2_TEST_PATTERN_DISABLED
>> +  Test pattern generation is disabled
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_VERTICAL_LINES
>> +  Generate vertical lines as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_HORIZONTAL_LINES
>> +  Generate horizontal lines as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_DIAGONAL_LINES
>> +  Generate diagonal lines as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_SOLID_BLACK
>> +  Generate solid black color as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_SOLID_WHITE
>> +  Generate solid white color as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_SOLID_BLUE
>> +  Generate solid blue color as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_SOLID_RED
>> +  Generate solid red color as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_CHECKER_BOARD
>> +  Generate a checker board as test pattern
>> +
> 
> You're defining 8 different test patterns based on a single device, I guess? 
> 
No.

> As the test patterns are not standardised, I'd suppose that if another
> driver implements the same control, it would require another n menu items
> added to the same standard menu. That way we'd run quickly out of menu items
> as the maximum is 32.
> 
Agreed the test patterns are not standardized and are hardware
dependent, but this entries which have been added are generally common
across capture/display/sensors.

> For this reason I'd leave the items in the menu up to the driver that
> implements the control, until we have more information on the test patterns
> different devices implement --- as discussed earlier.
> 
Assuming that I only added disable test pattern and if a driver wants to
implement this control and wants add  an item to this menu, on what
basis an entry to this menu will be qualified then ?

Thanks and Regards,
--Prabhakar Lad

> Kind regards,
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] media: v4l2-ctrls: add control for test pattern

2012-09-03 Thread Sakari Ailus
Hi Prabhakar,

Thanks for the patch.

On Mon, Sep 03, 2012 at 02:46:17PM +0530, Prabhakar Lad wrote:
> From: Lad, Prabhakar 
> 
> add V4L2_CID_TEST_PATTERN of type menu, which determines
> the internal test pattern selected by the device.
> 
> Signed-off-by: Lad, Prabhakar 
> Signed-off-by: Manjunath Hadli 
> Cc: Sakari Ailus 
> Cc: Hans Verkuil 
> Cc: Laurent Pinchart 
> Cc: Mauro Carvalho Chehab 
> Cc: Sylwester Nawrocki 
> Cc: Hans de Goede 
> Cc: Kyungmin Park 
> Cc: Rob Landley 
> Cc: HeungJun Kim 
> Cc: Rob Landley 
> ---
>  This patches has one checkpatch warning for line over
>  80 characters altough it can be avoided I have kept it
>  for consistency.
> 
>  Documentation/DocBook/media/v4l/controls.xml |   52 
> ++
>  drivers/media/v4l2-core/v4l2-ctrls.c |   16 
>  include/linux/videodev2.h|   12 ++
>  3 files changed, 80 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
> b/Documentation/DocBook/media/v4l/controls.xml
> index f704218..06f16e7 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -4313,6 +4313,58 @@ interface and may change in the future.
> 
>   
> 
> +   
> +  spanname="id">V4L2_CID_TEST_PATTERN
> + menu
> +   
> +   
> +  The capture devices/sensors have the 
> capability to
> + generate internal test patterns. This test patterns are used to 
> test a device
> + is properly working and can generate the desired waveforms that it 
> supports.
> + 
> +   
> +   
> + 
> +   
> + 
> +  V4L2_TEST_PATTERN_DISABLED
> +   Test pattern generation is disabled
> + 
> + 
> +   
> V4L2_TEST_PATTERN_VERTICAL_LINES
> +   Generate vertical lines as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_HORIZONTAL_LINES
> +   Generate horizontal lines as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_DIAGONAL_LINES
> +   Generate diagonal lines as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_SOLID_BLACK
> +   Generate solid black color as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_SOLID_WHITE
> +   Generate solid white color as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_SOLID_BLUE
> +   Generate solid blue color as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_SOLID_RED
> +   Generate solid red color as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_CHECKER_BOARD
> +   Generate a checker board as test pattern
> + 

You're defining 8 different test patterns based on a single device, I guess? 

As the test patterns are not standardised, I'd suppose that if another
driver implements the same control, it would require another n menu items
added to the same standard menu. That way we'd run quickly out of menu items
as the maximum is 32.

For this reason I'd leave the items in the menu up to the driver that
implements the control, until we have more information on the test patterns
different devices implement --- as discussed earlier.

Kind regards,

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] media: v4l2-ctrls: add control for test pattern

2012-09-03 Thread Prabhakar Lad
Hi Sylwester,

Thanks for the review.

On Monday 03 September 2012 02:55 PM, Sylwester Nawrocki wrote:
> On 09/03/2012 11:16 AM, Prabhakar Lad wrote:
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 2d7bc15..ae709d1 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -430,6 +430,18 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>  "Advanced Predictor",
>>  NULL,
>>  };
>> +static const char * const test_pattern[] = {
>> +"Test Pattern Disabled",
> 
> How about just "Disabled" ?
> 
Ok.

Thanks and Regards,
--Prabhakar Lad

>> +"Vertical Lines",
>> +"Horizontal Lines",
>> +"Diagonal Lines",
>> +"Solid Black",
>> +"Solid White",
>> +"Solid Blue",
>> +"Solid Red",
>> +"Checker Board",
>> +NULL,
>> +};
> 
> --
> 
> Regards,
> Sylwester
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] media: v4l2-ctrls: add control for test pattern

2012-09-03 Thread Prabhakar Lad
Hi Hans,

Thanks for the review.

On Monday 03 September 2012 02:52 PM, Hans Verkuil wrote:
> On Mon September 3 2012 11:16:17 Prabhakar Lad wrote:
>> From: Lad, Prabhakar 
>>
>> add V4L2_CID_TEST_PATTERN of type menu, which determines
>> the internal test pattern selected by the device.
>>
>> Signed-off-by: Lad, Prabhakar 
>> Signed-off-by: Manjunath Hadli 
>> Cc: Sakari Ailus 
>> Cc: Hans Verkuil 
>> Cc: Laurent Pinchart 
>> Cc: Mauro Carvalho Chehab 
>> Cc: Sylwester Nawrocki 
>> Cc: Hans de Goede 
>> Cc: Kyungmin Park 
>> Cc: Rob Landley 
>> Cc: HeungJun Kim 
>> Cc: Rob Landley 
>> ---
>>  This patches has one checkpatch warning for line over
>>  80 characters altough it can be avoided I have kept it
>>  for consistency.
>>
>>  Documentation/DocBook/media/v4l/controls.xml |   52 
>> ++
>>  drivers/media/v4l2-core/v4l2-ctrls.c |   16 
>>  include/linux/videodev2.h|   12 ++
>>  3 files changed, 80 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
>> b/Documentation/DocBook/media/v4l/controls.xml
>> index f704218..06f16e7 100644
>> --- a/Documentation/DocBook/media/v4l/controls.xml
>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> @@ -4313,6 +4313,58 @@ interface and may change in the future.
>>
>>  
>>
>> +  
>> +> spanname="id">V4L2_CID_TEST_PATTERN
>> +menu
>> +  
>> +  
>> + The capture devices/sensors have the 
>> capability to
> 
> Test patterns are also applicable to output devices, not just capture and 
> sensor devices.
> 
Agreed. I'll make it 'capture/display/sensors'.

>> +generate internal test patterns. This test patterns are used to 
>> test a device
>> +is properly working and can generate the desired waveforms that it 
>> supports.
>> +
>> +  
>> +  
>> +
>> +  
>> +
>> + V4L2_TEST_PATTERN_DISABLED
>> +  Test pattern generation is disabled
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_VERTICAL_LINES
>> +  Generate vertical lines as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_HORIZONTAL_LINES
>> +  Generate horizontal lines as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_DIAGONAL_LINES
>> +  Generate diagonal lines as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_SOLID_BLACK
>> +  Generate solid black color as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_SOLID_WHITE
>> +  Generate solid white color as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_SOLID_BLUE
>> +  Generate solid blue color as test pattern
>> +
>> +
>> +  
>> V4L2_TEST_PATTERN_SOLID_RED
>> +  Generate solid red color as test pattern
>> +
> 
> Just wondering: is there no SOLID_GREEN available with this sensor?
> 
Not sure I guess it should be.

Thanks and Regards,
--Prabhakar Lad

> Regards,
> 
>   Hans
> 
>> +
>> +  
>> V4L2_TEST_PATTERN_CHECKER_BOARD
>> +  Generate a checker board as test pattern
>> +
>> +  
>> +
>> +  
>>  
>>  
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
>> b/drivers/media/v4l2-core/v4l2-ctrls.c
>> index 2d7bc15..ae709d1 100644
>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -430,6 +430,18 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>  "Advanced Predictor",
>>  NULL,
>>  };
>> +static const char * const test_pattern[] = {
>> +"Test Pattern Disabled",
>> +"Vertical Lines",
>> +"Horizontal Lines",
>> +"Diagonal Lines",
>> +"Solid Black",
>> +"Solid White",
>> +"Solid Blue",
>> +"Solid Red",
>> +"Checker Board",
>> +NULL,
>> +};
>>  
>>  switch (id) {
>>  case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
>> @@ -509,6 +521,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>  return jpeg_chroma_subsampling;
>>  case V4L2_CID_DPCM_PREDICTOR:
>>  return dpcm_predictor;
>> +case V4L2_CID_TEST_PATTERN:
>> +return test_pattern;
>>  
>>  default:
>>  return NULL;
>> @@ -740,6 +754,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>  case V4L2_CID_LINK_FREQ:return "Link Frequency";
>>  case V4L2_CID_PIXEL_RATE:   return "Pixel Rate";
>>  case V4L2_CID_DPCM_PREDICTOR:   return "DPCM Predictor";
>> +case V4L2_CID_TEST_PATTERN: return "Test Pattern";
>>  
>>  default:
>>

Re: [PATCH] media: v4l2-ctrls: add control for test pattern

2012-09-03 Thread Sylwester Nawrocki
On 09/03/2012 11:16 AM, Prabhakar Lad wrote:
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 2d7bc15..ae709d1 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -430,6 +430,18 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   "Advanced Predictor",
>   NULL,
>   };
> + static const char * const test_pattern[] = {
> + "Test Pattern Disabled",

How about just "Disabled" ?

> + "Vertical Lines",
> + "Horizontal Lines",
> + "Diagonal Lines",
> + "Solid Black",
> + "Solid White",
> + "Solid Blue",
> + "Solid Red",
> + "Checker Board",
> + NULL,
> + };

--

Regards,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] media: v4l2-ctrls: add control for test pattern

2012-09-03 Thread Hans Verkuil
On Mon September 3 2012 11:16:17 Prabhakar Lad wrote:
> From: Lad, Prabhakar 
> 
> add V4L2_CID_TEST_PATTERN of type menu, which determines
> the internal test pattern selected by the device.
> 
> Signed-off-by: Lad, Prabhakar 
> Signed-off-by: Manjunath Hadli 
> Cc: Sakari Ailus 
> Cc: Hans Verkuil 
> Cc: Laurent Pinchart 
> Cc: Mauro Carvalho Chehab 
> Cc: Sylwester Nawrocki 
> Cc: Hans de Goede 
> Cc: Kyungmin Park 
> Cc: Rob Landley 
> Cc: HeungJun Kim 
> Cc: Rob Landley 
> ---
>  This patches has one checkpatch warning for line over
>  80 characters altough it can be avoided I have kept it
>  for consistency.
> 
>  Documentation/DocBook/media/v4l/controls.xml |   52 
> ++
>  drivers/media/v4l2-core/v4l2-ctrls.c |   16 
>  include/linux/videodev2.h|   12 ++
>  3 files changed, 80 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml 
> b/Documentation/DocBook/media/v4l/controls.xml
> index f704218..06f16e7 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -4313,6 +4313,58 @@ interface and may change in the future.
> 
>   
> 
> +   
> +  spanname="id">V4L2_CID_TEST_PATTERN
> + menu
> +   
> +   
> +  The capture devices/sensors have the 
> capability to

Test patterns are also applicable to output devices, not just capture and 
sensor devices.

> + generate internal test patterns. This test patterns are used to 
> test a device
> + is properly working and can generate the desired waveforms that it 
> supports.
> + 
> +   
> +   
> + 
> +   
> + 
> +  V4L2_TEST_PATTERN_DISABLED
> +   Test pattern generation is disabled
> + 
> + 
> +   
> V4L2_TEST_PATTERN_VERTICAL_LINES
> +   Generate vertical lines as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_HORIZONTAL_LINES
> +   Generate horizontal lines as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_DIAGONAL_LINES
> +   Generate diagonal lines as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_SOLID_BLACK
> +   Generate solid black color as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_SOLID_WHITE
> +   Generate solid white color as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_SOLID_BLUE
> +   Generate solid blue color as test pattern
> + 
> + 
> +   
> V4L2_TEST_PATTERN_SOLID_RED
> +   Generate solid red color as test pattern
> + 

Just wondering: is there no SOLID_GREEN available with this sensor?

Regards,

Hans

> + 
> +   
> V4L2_TEST_PATTERN_CHECKER_BOARD
> +   Generate a checker board as test pattern
> + 
> +   
> + 
> +   
>   
>   
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
> b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 2d7bc15..ae709d1 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -430,6 +430,18 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   "Advanced Predictor",
>   NULL,
>   };
> + static const char * const test_pattern[] = {
> + "Test Pattern Disabled",
> + "Vertical Lines",
> + "Horizontal Lines",
> + "Diagonal Lines",
> + "Solid Black",
> + "Solid White",
> + "Solid Blue",
> + "Solid Red",
> + "Checker Board",
> + NULL,
> + };
>  
>   switch (id) {
>   case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
> @@ -509,6 +521,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>   return jpeg_chroma_subsampling;
>   case V4L2_CID_DPCM_PREDICTOR:
>   return dpcm_predictor;
> + case V4L2_CID_TEST_PATTERN:
> + return test_pattern;
>  
>   default:
>   return NULL;
> @@ -740,6 +754,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>   case V4L2_CID_LINK_FREQ:return "Link Frequency";
>   case V4L2_CID_PIXEL_RATE:   return "Pixel Rate";
>   case V4L2_CID_DPCM_PREDICTOR:   return "DPCM Predictor";
> + case V4L2_CID_TEST_PATTERN: return "Test Pattern";
>  
>   default:
>   return NULL;
> @@ -841,6 +856,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
> v4l2_ctrl_type *type,
>   case V4L2_CID_EXPOSURE_METERING:
>   case V4L2_CID_SCENE_MODE:
>   case V4L2_CID_DPCM_PREDICTOR:
> + case V4L2_CID_TEST_PATTERN:
>