Hi Hans, Please find my comments inline. Regards, Hardik Shah > -----Original Message----- > From: Hans Verkuil [mailto:hverkuil@xxxxxxxxx] > Sent: Monday, March 30, 2009 5:29 PM > To: Shah, Hardik > Cc: linux-media@xxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; Jadav, Brijesh R; > Hiremath, Vaibhav > Subject: Re: [PATCH 2/3] New V4L2 CIDs for OMAP class of Devices. > > On Friday 20 March 2009 06:19:57 Hardik Shah wrote: > > Added V4L2_CID_BG_COLOR for background color setting. > > Added V4L2_CID_ROTATION for rotation setting. > > Above two ioclts are indepth discussed. Posting > > again with the driver usage. > > > > --- > > linux/drivers/media/video/v4l2-common.c | 7 +++++++ > > linux/include/linux/videodev2.h | 6 +++++- > > 2 files changed, 12 insertions(+), 1 deletions(-) > > > > diff --git a/linux/drivers/media/video/v4l2-common.c > b/linux/drivers/media/video/v4l2-common.c > > index 3c42316..fa408f0 100644 > > --- a/linux/drivers/media/video/v4l2-common.c > > +++ b/linux/drivers/media/video/v4l2-common.c > > @@ -422,6 +422,8 @@ const char *v4l2_ctrl_get_name(u32 id) > > case V4L2_CID_CHROMA_AGC: return "Chroma AGC"; > > case V4L2_CID_COLOR_KILLER: return "Color Killer"; > > case V4L2_CID_COLORFX: return "Color Effects"; > > + case V4L2_CID_ROTATION: return "Rotation"; > > I'm having second thoughts about this name. I think V4L2_CID_ROTATE is better, > since it is an action ('you rotate an image') rather than a status ('what is > the rotation of an image'). > > What do you think? [Shah, Hardik] Yes, it should be V4L2_CID_ROTATE as V4L2_CID_HFLIP. > > > + case V4L2_CID_BG_COLOR: return "Background color"; > > > > /* MPEG controls */ > > case V4L2_CID_MPEG_CLASS: return "MPEG Encoder Controls"; > > @@ -547,6 +549,10 @@ int v4l2_ctrl_query_fill(struct v4l2_queryctrl *qctrl, > s32 min, s32 max, s32 ste > > qctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; > > min = max = step = def = 0; > > break; > > + case V4L2_CID_BG_COLOR: > > + qctrl->type = V4L2_CTRL_TYPE_INTEGER; > > + step = 1; > > + break; > > Set the min to 0 and max to 0xffffff. [Shah, Hardik] Added > > > default: > > qctrl->type = V4L2_CTRL_TYPE_INTEGER; > > break; > > @@ -571,6 +577,7 @@ int v4l2_ctrl_query_fill(struct v4l2_queryctrl *qctrl, > s32 min, s32 max, s32 ste > > case V4L2_CID_BLUE_BALANCE: > > case V4L2_CID_GAMMA: > > case V4L2_CID_SHARPNESS: > > + case V4L2_CID_BG_COLOR: > > This definitely isn't a slider control. [Shah, Hardik] Removed > > > -- > > 1.5.6 > > Regards, > > Hans > > -- > Hans Verkuil - video4linux developer - sponsored by TANDBERG -- 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
