Re: [PULL] http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-vpfe-cap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em Tue, 30 Jun 2009 14:39:55 -0500
"Karicheri, Muralidharan" <m-karicheri2@xxxxxx> escreveu:

> 
> Mauro,
> 
> Thanks for responding...
> 
> >You should note that I'm not asking you to remove that code, but just to
> >use
> >the already existing color balance ioctls, for the existing features, or
> >otherwise to discuss the need of extra controls.
> 
> 
> Ok. 
> >
> >The case of image color controlling, we already have several controls:
> >
> >#define V4L2_CID_SATURATION             (V4L2_CID_BASE+2)
> >#define V4L2_CID_RED_BALANCE            (V4L2_CID_BASE+14)
> >#define V4L2_CID_BLUE_BALANCE           (V4L2_CID_BASE+15)
> >#define V4L2_CID_GAMMA                  (V4L2_CID_BASE+16)
> >#define V4L2_CID_GAIN                   (V4L2_CID_BASE+19)
> >
> >Also, some got deprecated, since they were just duplicating existing
> >controls,
> >using a different way, as discussed at ML:
> >
> 
> Ok. I need to investigate this when I support control IOCTLs in vpfe
> capture. As of now control IOCTLs are not supported in vpfe capture.
> 
> >#define V4L2_CID_BLACK_LEVEL            (V4L2_CID_BASE+11) /* Deprecated */
> >#define V4L2_CID_WHITENESS              (V4L2_CID_GAMMA) /* Deprecated */
> >
> >So, you basically need to rewrite your logic in order to control the device
> >in
> >terms of gain and red/blue balance.
> >
> >
> Ok. I get it.
> 
> Hans had an issue with the way we implemented control IOCTL handling in the driver. So the piece of code you had objected to is redundant. I will clean it up or modify it when I support control IOCTLs handling in vpfe capture or alternately remove it using a separate patch. So please go ahead and merge the patches if everything else looks good. 

I guess you didn't understand me. For me to pull from this pull request, I need
to be sure that no uneeded/duplicated/undocumented userspace controls are added
to V4L2 API.

So, we need to solve all PRIVATE controls:

$ grep PRIVATE /tmp/newpatches/hg_v4l-dvb-vpfe-cap_*
/tmp/newpatches/hg_v4l-dvb-vpfe-cap_02.patch:+#define VPFE_CMD_S_CCDC_RAW_PARAMS _IOW('V', BASE_VIDIOC_PRIVATE + 1, \
/tmp/newpatches/hg_v4l-dvb-vpfe-cap_05.patch:+#define CCDC_CID_R_GAIN          (V4L2_CID_PRIVATE_BASE + 0)
/tmp/newpatches/hg_v4l-dvb-vpfe-cap_05.patch:+#define CCDC_CID_GR_GAIN  (V4L2_CID_PRIVATE_BASE + 1)
/tmp/newpatches/hg_v4l-dvb-vpfe-cap_05.patch:+#define CCDC_CID_GB_GAIN  (V4L2_CID_PRIVATE_BASE + 2)
/tmp/newpatches/hg_v4l-dvb-vpfe-cap_05.patch:+#define CCDC_CID_B_GAIN   (V4L2_CID_PRIVATE_BASE + 3)
/tmp/newpatches/hg_v4l-dvb-vpfe-cap_05.patch:+#define CCDC_CID_OFFSET   (V4L2_CID_PRIVATE_BASE + 4)
/tmp/newpatches/hg_v4l-dvb-vpfe-cap_05.patch:+#define CCDC_CID_MAX             (V4L2_CID_PRIVATE_BASE + 5)

(btw, the grep above also showed another of such control at the second patch)

Most of the above controls are duplicated, in the sense that the current color
controls (eventually with some math) are capable of controlling the color gains.

The CCDC_CID_MAX is not even implemented.

The VPFE_CMD_S_CCDC_RAW_PARAMS and CCDC_CID_OFFSET are not properly documented,
so, I have no idea about what you want to control with them.

One quick alternative for them, while they are being under discussions, would
be to put them into #if 0/#endif blocks, but you need to provide a patch to
solve it for me to merge VPFE



Cheers,
Mauro
--
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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux