Re: [PATCH 01/13] V4L: Extend V4L2_CID_COLORFX with more image effects

[Sylwester Nawrocki <snjw23@gmail.com>]

Hi Hans,

thanks for the review!

On 04/27/2012 12:12 PM, Hans Verkuil wrote:
> Hi Sylwester!
> 
> On Friday, April 27, 2012 11:52:54 Sylwester Nawrocki wrote:
>> This patch adds definition of additional color effects:
>>   - V4L2_COLORFX_AQUA,
>>   - V4L2_COLORFX_ART_FREEZE,
>>   - V4L2_COLORFX_SILHOUETTE,
>>   - V4L2_COLORFX_SOLARIZATION,
>>   - V4L2_COLORFX_ANTIQUE,
>>   - V4L2_COLORFX_ARBITRARY_CBCR.
>>
>> The control's type in the documentation is changed from 'enum' to 'menu'
>> - V4L2_CID_COLORFX has always been a menu, not an integer type control.
>>
>> The V4L2_COLORFX_ARBITRARY_CBCR option enables custom color effects,
>> which are impossible or impractical to define as menu items. The
>> V4L2_CID_BLUE_BALANCE and V4L2_CID_RED_BALANCE controls allow in this
>> case to configure the Cb, Cr coefficients.
> 
> So this just hijacks the RED/BLUE_BALANCE controls for a different purpose?

Uh, the meaning is indeed a bit different. Probably not a good idea to reuse
the controls like this in the standard API.

> If I understand this 'effect' correctly it just replaces the Cb and Cr
> coefficients with fixed values, basically giving you a B&W picture (the Y
> coefficient), except that it is really a 'Black&FixedColor' picture.

Yes, this is also my understanding. The TRMs are not very verbose about it,
but I think it is exactly how it works. The effect is similar to looking
through a coloured glass, where colour changes from green through red to violet
when changing the (CR, CB) coefficients gradually from (0, 0) -> (0, 255) -> 
(255, 255).
 
> I think you should add a new control for setting this. V4L2_CID_COLORFX_COLOR
> or something.

Do you mean something similar to V4L2_CID_BG_COLOR ? When a different colour 
space is used then the range for those Cb, Cr components changes. It can be 
0...255 or 16...240. So best would be to have 2 controls, for reporting min/max
to the user.

Maybe it would be better to add a V4L2_COLORFX_COLOR menu entry and
V4L2_CID_COLORFX_CB, V4L2_CID_COLORFX_CR controls ?


--

Regards,
Sylwester