On 04/17/2012 12:51 PM, Rémi Denis-Courmont wrote: > On Tue, 17 Apr 2012 12:09:42 +0200, Sylwester Nawrocki > <s.nawrocki@xxxxxxxxxxx> 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, > > There starts to be a lot of different color effects with no obvious way to > determine which ones the current device actually suppots. Should this not > be a menu control instead? Fortunately this has been a menu control, since it was introduced. Only the DocBook erroneously defined it as an enum. This patch also fixes that, please see the DocBook part. >> - V4L2_COLORFX_ARBITRARY. >> >> 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 option enables custom color effects, which > are >> impossible or impractical to define as the menu items. For example, the >> devices may provide coefficients for Cb and Cr manipulation, which yield >> many permutations, e.g. many slightly different color tints. Such > devices >> are better exporting their own API for precise control of non-standard >> color effects. > > I don't understand why you need a number for this, if it's going to use > another control anyway... ? In my use case, the hardware has 3 registers: one of them selects the colour effect and two others determine Cr, Cb coefficients (probably I could use V4L2_CID_RED_BALANCE, V4L2_CID_BLUE_BALANCE for that, but so far these are just private controls). If I would have removed the V4L2_COLORFX_ARBITRARY item, another control would have to be added (let's say boolean V4L2_PRIV_IMG_EFFECT). Just to enable the "arbitrary" effect. Then, to enable the arbitrary effect V4L2_CID_COLORFX would have to be set to V4L2_COLORFX_NONE, nnd V4L2_PRIV_IMG_EFFECT to true. The CB, CR coefficients are meaningful only when the arbitrary effect is selected. So having another option in the menu, which drivers can just mask if they don't support it, appeared better to me. It's a bit similar to gain/autogain scenario, where gain is active only when autogain is off. Maybe I should just add another private control (V4L2_PRIV_IMG_EFFECT) and remove V4L2_COLORFX_ARBITRARY item. --- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
