2011/9/6 Sakari Ailus <sakari.ailus@xxxxxx>: > On Tue, Sep 06, 2011 at 07:56:40AM +0000, Bastian Hecht wrote: >> Hello Sakari! > > Hi Bastian, > >> 2011/9/6 Sakari Ailus <sakari.ailus@xxxxxx>: >> > Hi Bastian, >> > >> > On Mon, Sep 05, 2011 at 09:32:55AM +0000, Bastian Hecht wrote: >> >> 2011/9/1 Sakari Ailus <sakari.ailus@xxxxxx>: >> >> > On Thu, Sep 01, 2011 at 11:14:08AM +0200, Sylwester Nawrocki wrote: >> >> >> Hi Sakari, >> >> >> >> >> >> On 09/01/2011 10:47 AM, Sakari Ailus wrote: >> >> >> > On Thu, Sep 01, 2011 at 09:15:20AM +0200, Guennadi Liakhovetski wrote: >> >> >> >> On Thu, 1 Sep 2011, Sakari Ailus wrote: >> >> >> >> >> >> >> >>> On Wed, Aug 31, 2011 at 03:27:49PM +0000, Bastian Hecht wrote: >> >> >> >>>> 2011/8/28 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>: >> >> >> >>> [clip] >> >> >> >>>>> If I'm not mistaken V4L2_CID_PRIVATE_BASE is deprecated. >> >> >> >>>> >> >> >> >>>> I checked at http://v4l2spec.bytesex.org/spec/x542.htm, googled >> >> >> >>>> "V4L2_CID_PRIVATE_BASE deprecated" and read >> >> >> >>>> Documentation/feature-removal-schedule.txt. I couldn't find anything. >> >> >> >>> >> >> >> >>> Hmm. Did you happen to check when that has been written? :) >> >> >> >>> >> >> >> >>> Please use this one instead: >> >> >> >>> >> >> >> >>> <URL:http://hverkuil.home.xs4all.nl/spec/media.html> >> >> >> >> >> >> >> >> "Drivers can also implement their own custom controls using >> >> >> >> V4L2_CID_PRIVATE_BASE and higher values." >> >> >> >> >> >> >> >> Which specific location describes V4L2_CID_PRIVATE_BASE differently there? >> >> >> > >> >> >> > That was a general comment, not related to the private base. There's no >> >> >> > use for a three-year-old spec as a reference! >> >> >> > >> >> >> > The control framework does not support private controls, for example. The >> >> >> > controls should be put to their own class in videodev2.h nowadays, that's my >> >> >> > understanding. Cc Hans. >> >> >> >> >> >> Is this really the case that we close the door for private controls in >> >> >> the mainline kernel ? Or am I misunderstanding something ? >> >> >> How about v4l2_ctrl_new_custom() ? >> >> >> >> >> >> What if there are controls applicable to single driver only ? >> >> >> Do we really want to have plenty of such in videodev2.h ? >> >> > >> >> > We have some of those already in videodev2.h. I'm not certain if I'm happy >> >> > with this myself, considering e.g. that we could get a few truckloads of >> >> > only camera lens hardware specific controls in the near future. >> >> >> >> So in my case (as these are controls that might be used by others too) >> >> I should add something like >> >> >> >> #define V4L2_CID_BLUE_SATURATION (V4L2_CID_CAMERA_CLASS_BASE+19) >> >> #define V4L2_CID_RED_SATURATION (V4L2_CID_CAMERA_CLASS_BASE+20) >> > >> > What do these two controls do? Do they control gain or something else? >> >> Hmm. Maybe I named them a bit unsharp. It is the U Saturation and V >> Saturation. To me it looks like turning up the saturation in HSV >> space, but only for either the blue or the red channel. This would >> correspond to V4L2_CID_{RED,BLUE}_BALANCE when I read the docs. They >> say it is "{Red,Blue} chroma balance". >> >> I have other controls for that I used V4L2_CID_{RED,BLUE}_BALANCE. >> These are gains. So in fact I should swap them in my code and the >> remaining question is, how to name the red and blue gain controls. > > I think Laurent had a similar issue in his Aptina sensor driver. In my > opinion we need a class for low level controls such as the gain ones. Do I > understand correctly they control the red and blue pixel gain in the sensor > pixel matrix? Do you also have gain controls for the two greens? Yes, I assume that this is done there. Either in the analog circuit by decreasing the preload or digitally then. Don't know exactly. There are registers for the green pixels as well. As I used the V4L2_CID_{RED,BLUE}_BALANCE controls and there was no V4L2_CID_GREEN_BALANCE, I just skipped green as one can increase/decrease the global gain and get an arbitrary mix as well. So for these gain settings we should add these? V4L2_CID_RED_GAIN V4L2_CID_BLUE_GAIN V4L2_CID_GREEN_GAIN >> >> #define V4L2_CID_GRAY_SCALE_IMAGE (V4L2_CID_CAMERA_CLASS_BASE+21) >> > >> > V4L2_CID_COLOR_KILLER looks like something which would fit for the purpose. >> >> Oh great! So I just take this. >> >> >> #define V4L2_CID_SOLARIZE_EFFECT (V4L2_CID_CAMERA_CLASS_BASE+22) >> > >> > Sounds interesting for a sensor. I wonder if this would fall under a menu >> > control, V4L2_CID_COLORFX. >> >> When I read the the possible enums for V4L2_CID_COLORFX, it indeed >> sounds very much like my solarize effect should be added there too. I >> found V4L2_COLORFX_BW there, too. Isn't that a duplicate of the color >> killer control then? > > In my opinion V4L2_CID_COLORFX should never be implemented in drivers for > which the hardware doesn't implement these effects in a non-parametrisable > way. This control was originally added for the OMAP 3 ISP driver but the > driver never implemented it. Your triple negation (never, doesn't, non-) is quite tricky xD If I get it right, you say that one should not use V4L2_CID_COLORFX for hardware with parametrisable effects. My BW and Solarize effects are non-parametrisable and they can be turned on together (which makes not so much sense though - but these fun-effects like "solarize" aren't here to make sense, I guess :-) ). > I think you have a valid case using this control. I think the main > difference between the two is that V4L2_COLORFX_BW is something that you > can't use with other effects while V4L2_CID_COLOR_KILLER can be used with > any of the effects. > Based on your original proposal the black/white should stay as a separate > control but the solarise should be configurable through V4L2_CID_COLORFX > menu control. So it boils down to the question whether you can use them at > the same time. I can - so it is still working to enable V4L2_COLORFX_BW and V4L2_CID_COLORFX with a new enum value, right? Is that the way to go now? > -- > Sakari Ailus > e-mail: sakari.ailus@xxxxxx jabber/XMPP/Gmail: sailus@xxxxxxxxxxxxxx > Thanks, Bastian -- 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
