Hi, On 04/27/2012 09:02 PM, Hans Verkuil wrote: > On Friday, April 27, 2012 19:54:50 Sylwester Nawrocki wrote: > > On 04/27/2012 12:12 PM, Hans Verkuil wrote: > > > 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. > > BTW, reading this again I think "ARBITRARY_CBCR" is a confusing name. > Perhaps "REPLACE_CBCR" or "SET_CBCR" is better? This is how it was named in the datasheets ;-) I like V4L2_COLORFX_REPLACE_CBCR better. What about V4L2_COLORFX_CONSTANT_CBCR, V4L2_COLORFX_PATTERN_CBCR or V4L2_COLORFX_FILL_IN_CBCR ? ... > Maybe it would be better to add a V4L2_COLORFX_COLOR menu entry and > > V4L2_CID_COLORFX_CB, V4L2_CID_COLORFX_CR controls ? > > That would work, yes. Although I am not convinced splitting it up is > worthwhile. > The colorspace can change on the fly, so you would have to handle > out-of-range > values anyway. Personally I would stick with CID_COLORFX_CBCR (and > clearly document in which byte the CB and the CR go). That's going to be sufficient I guess. Applications will most likely be just pre-configured with some fixed CB/CR values, mapped to some meaningful names. It should be fine, as far as those coefficient registers are accessible from user space. > That's just my opinion, though. > > Regards, > Hans > > PS: I'll review the "camera control enhancements" patch series in the > next few days. Great, thanks a lot! I was thinking about a guide on which control groups should be used for each devices, to make the driver and application writers' life easier, but its not an easy task in light of high diversity of camera devices... -- Regards, 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
