| Message ID | 1232538820-20307-1-git-send-email-hardik.shah@ti.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
> -----Original Message----- > From: Shah, Hardik > Sent: Wednesday, January 21, 2009 5:24 PM > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, Manjunath; R, > Sivaraj; Hiremath, Vaibhav > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > 1. Control ID added for rotation. Same as HFLIP. > 2. Control ID added for setting background color on > output device. > 3. New ioctl added for setting the color space conversion from > YUV to RGB. > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > Signed-off-by: Manjunath Hadli <mrh@ti.com> > Signed-off-by: R Sivaraj <sivaraj@ti.com> > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > --- > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > linux/include/media/v4l2-ioctl.h | 4 ++++ > 3 files changed, 40 insertions(+), 2 deletions(-) > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > b/linux/drivers/media/video/v4l2-ioctl.c > index 165bc90..7599da8 100644 > --- a/linux/drivers/media/video/v4l2-ioctl.c > +++ b/linux/drivers/media/video/v4l2-ioctl.c > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > #endif > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = "VIDIOC_S_COLOR_SPACE_CONV", > + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] = "VIDIOC_G_COLOR_SPACE_CONV", > }; > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file *file, > } > break; > } > - > + case VIDIOC_S_COLOR_SPACE_CONV: > + { > + struct v4l2_color_space_conversion *p = arg; > + if (!ops->vidioc_s_color_space_conv) > + break; > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > + break; > + } > + case VIDIOC_G_COLOR_SPACE_CONV: > + { > + struct v4l2_color_space_conversion *p = arg; > + if (!ops->vidioc_g_color_space_conv) > + break; > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > + break; > + } > default: > { > if (!ops->vidioc_default) > diff --git a/linux/include/linux/videodev2.h b/linux/include/linux/videodev2.h > index b0c5010..9fbc3b0 100644 > --- a/linux/include/linux/videodev2.h > +++ b/linux/include/linux/videodev2.h > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > /* last CID + 1 */ > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > /* MPEG-class control IDs defined by V4L2 */ > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > }; > > /* > + * Color conversion > + * User needs to pass pointer to color conversion matrix > + * defined by hardware > + */ > +struct v4l2_color_space_conversion { > + __s32 coefficients[3][3]; > + __s32 const_factor; > + __s32 offsets[3]; > +}; > + > +/* > * A U D I O > */ > struct v4l2_audio { > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > #endif > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct v4l2_hw_freq_seek) > + > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > v4l2_color_space_conversion) > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > v4l2_color_space_conversion) > /* Reminder: when adding new ioctls please add support for them to > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > + > #ifdef __OLD_VIDIOC_ > /* for compatibility, will go away some day */ > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > diff --git a/linux/include/media/v4l2-ioctl.h b/linux/include/media/v4l2- > ioctl.h > index b01c044..0c44ecf 100644 > --- a/linux/include/media/v4l2-ioctl.h > +++ b/linux/include/media/v4l2-ioctl.h > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > /* For other private ioctls */ > long (*vidioc_default) (struct file *file, void *fh, > int cmd, void *arg); > + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, > + struct v4l2_color_space_conversion *a); > + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, > + struct v4l2_color_space_conversion *a); > }; > > > -- > 1.5.6 [Shah, Hardik] Hi, Any comments on this patch. Hans/Mauro, If possible can you integrate this onto your development branch. Regards, Hardik Shah -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > -----Original Message----- > > From: Shah, Hardik > > Sent: Wednesday, January 21, 2009 5:24 PM > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, Manjunath; R, > > Sivaraj; Hiremath, Vaibhav > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > 1. Control ID added for rotation. Same as HFLIP. > > 2. Control ID added for setting background color on > > output device. > > 3. New ioctl added for setting the color space conversion from > > YUV to RGB. > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > --- > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > b/linux/drivers/media/video/v4l2-ioctl.c > > index 165bc90..7599da8 100644 > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > #endif > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = "VIDIOC_S_COLOR_SPACE_CONV", > > + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] = "VIDIOC_G_COLOR_SPACE_CONV", > > }; > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file *file, > > } > > break; > > } > > - > > + case VIDIOC_S_COLOR_SPACE_CONV: > > + { > > + struct v4l2_color_space_conversion *p = arg; > > + if (!ops->vidioc_s_color_space_conv) > > + break; > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > + break; > > + } > > + case VIDIOC_G_COLOR_SPACE_CONV: > > + { > > + struct v4l2_color_space_conversion *p = arg; > > + if (!ops->vidioc_g_color_space_conv) > > + break; > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > + break; > > + } > > default: > > { > > if (!ops->vidioc_default) > > diff --git a/linux/include/linux/videodev2.h > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > --- a/linux/include/linux/videodev2.h > > +++ b/linux/include/linux/videodev2.h > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > /* last CID + 1 */ > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > /* MPEG-class control IDs defined by V4L2 */ > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > }; > > > > /* > > + * Color conversion > > + * User needs to pass pointer to color conversion matrix > > + * defined by hardware > > + */ > > +struct v4l2_color_space_conversion { > > + __s32 coefficients[3][3]; > > + __s32 const_factor; > > + __s32 offsets[3]; > > +}; > > + > > +/* > > * A U D I O > > */ > > struct v4l2_audio { > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > #endif > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct v4l2_hw_freq_seek) > > + > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > v4l2_color_space_conversion) > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > v4l2_color_space_conversion) > > /* Reminder: when adding new ioctls please add support for them to > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > + > > #ifdef __OLD_VIDIOC_ > > /* for compatibility, will go away some day */ > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > diff --git a/linux/include/media/v4l2-ioctl.h > > b/linux/include/media/v4l2- ioctl.h > > index b01c044..0c44ecf 100644 > > --- a/linux/include/media/v4l2-ioctl.h > > +++ b/linux/include/media/v4l2-ioctl.h > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > /* For other private ioctls */ > > long (*vidioc_default) (struct file *file, void *fh, > > int cmd, void *arg); > > + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, > > + struct v4l2_color_space_conversion *a); > > + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, > > + struct v4l2_color_space_conversion *a); > > }; > > > > > > -- > > 1.5.6 > > [Shah, Hardik] Hi, > Any comments on this patch. > Hans/Mauro, > If possible can you integrate this onto your development branch. Hi Hardik, I've one question regarding the rotation control: I assume that this is limited to 0, 90, 180 and 270 degrees? I think it might be better to implement this as an enum in that case. Regards, Hans
On Sunday 25 January 2009, Hans Verkuil wrote: > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > -----Original Message----- > > > From: Shah, Hardik > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, Manjunath; R, > > > Sivaraj; Hiremath, Vaibhav > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > 2. Control ID added for setting background color on > > > output device. > > > 3. New ioctl added for setting the color space conversion from > > > YUV to RGB. > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > --- > > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > index 165bc90..7599da8 100644 > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > > #endif > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = "VIDIOC_S_COLOR_SPACE_CONV", > > > + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] = "VIDIOC_G_COLOR_SPACE_CONV", > > > }; > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file *file, > > > } > > > break; > > > } > > > - > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > + { > > > + struct v4l2_color_space_conversion *p = arg; > > > + if (!ops->vidioc_s_color_space_conv) > > > + break; > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > + break; > > > + } > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > + { > > > + struct v4l2_color_space_conversion *p = arg; > > > + if (!ops->vidioc_g_color_space_conv) > > > + break; > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > + break; > > > + } > > > default: > > > { > > > if (!ops->vidioc_default) > > > diff --git a/linux/include/linux/videodev2.h > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > --- a/linux/include/linux/videodev2.h > > > +++ b/linux/include/linux/videodev2.h > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > /* last CID + 1 */ > > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > }; > > > > > > /* > > > + * Color conversion > > > + * User needs to pass pointer to color conversion matrix > > > + * defined by hardware > > > + */ > > > +struct v4l2_color_space_conversion { > > > + __s32 coefficients[3][3]; > > > + __s32 const_factor; > > > + __s32 offsets[3]; > > > +}; > > > + > > > +/* > > > * A U D I O > > > */ > > > struct v4l2_audio { > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > #endif > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct v4l2_hw_freq_seek) > > > + > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > v4l2_color_space_conversion) > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > v4l2_color_space_conversion) > > > /* Reminder: when adding new ioctls please add support for them to > > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > + > > > #ifdef __OLD_VIDIOC_ > > > /* for compatibility, will go away some day */ > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > b/linux/include/media/v4l2- ioctl.h > > > index b01c044..0c44ecf 100644 > > > --- a/linux/include/media/v4l2-ioctl.h > > > +++ b/linux/include/media/v4l2-ioctl.h > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > /* For other private ioctls */ > > > long (*vidioc_default) (struct file *file, void *fh, > > > int cmd, void *arg); > > > + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, > > > + struct v4l2_color_space_conversion *a); > > > + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, > > > + struct v4l2_color_space_conversion *a); > > > }; > > > > > > > > > -- > > > 1.5.6 > > > > [Shah, Hardik] Hi, > > Any comments on this patch. > > Hans/Mauro, > > If possible can you integrate this onto your development branch. > > Hi Hardik, > > I've one question regarding the rotation control: I assume that this is > limited to 0, 90, 180 and 270 degrees? I think it might be better to > implement this as an enum in that case. 90 and 270 degrees would modify the image size. How would that be handled in relationship with VIDIOC_S/G_FMT ? Best regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 25 January 2009, Laurent Pinchart wrote: > On Sunday 25 January 2009, Hans Verkuil wrote: > > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > > -----Original Message----- > > > > From: Shah, Hardik > > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, Manjunath; > > > > R, Sivaraj; Hiremath, Vaibhav > > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > > 2. Control ID added for setting background color on > > > > output device. > > > > 3. New ioctl added for setting the color space conversion from > > > > YUV to RGB. > > > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > > --- > > > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > > index 165bc90..7599da8 100644 > > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > > > #endif > > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = > > > > "VIDIOC_S_COLOR_SPACE_CONV", + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] > > > > = "VIDIOC_G_COLOR_SPACE_CONV", }; > > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file > > > > *file, } > > > > break; > > > > } > > > > - > > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > > + { > > > > + struct v4l2_color_space_conversion *p = arg; > > > > + if (!ops->vidioc_s_color_space_conv) > > > > + break; > > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > > + break; > > > > + } > > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > > + { > > > > + struct v4l2_color_space_conversion *p = arg; > > > > + if (!ops->vidioc_g_color_space_conv) > > > > + break; > > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > > + break; > > > > + } > > > > default: > > > > { > > > > if (!ops->vidioc_default) > > > > diff --git a/linux/include/linux/videodev2.h > > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > > --- a/linux/include/linux/videodev2.h > > > > +++ b/linux/include/linux/videodev2.h > > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > > /* last CID + 1 */ > > > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) > > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > > }; > > > > > > > > /* > > > > + * Color conversion > > > > + * User needs to pass pointer to color conversion matrix > > > > + * defined by hardware > > > > + */ > > > > +struct v4l2_color_space_conversion { > > > > + __s32 coefficients[3][3]; > > > > + __s32 const_factor; > > > > + __s32 offsets[3]; > > > > +}; > > > > + > > > > +/* > > > > * A U D I O > > > > */ > > > > struct v4l2_audio { > > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > > #endif > > > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct > > > > v4l2_hw_freq_seek) + > > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > > v4l2_color_space_conversion) > > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > > v4l2_color_space_conversion) > > > > /* Reminder: when adding new ioctls please add support for them to > > > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > > > + > > > > #ifdef __OLD_VIDIOC_ > > > > /* for compatibility, will go away some day */ > > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > > b/linux/include/media/v4l2- ioctl.h > > > > index b01c044..0c44ecf 100644 > > > > --- a/linux/include/media/v4l2-ioctl.h > > > > +++ b/linux/include/media/v4l2-ioctl.h > > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > > /* For other private ioctls */ > > > > long (*vidioc_default) (struct file *file, void *fh, > > > > int cmd, void *arg); > > > > + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, > > > > + struct v4l2_color_space_conversion *a); > > > > + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, > > > > + struct v4l2_color_space_conversion *a); > > > > }; > > > > > > > > > > > > -- > > > > 1.5.6 > > > > > > [Shah, Hardik] Hi, > > > Any comments on this patch. > > > Hans/Mauro, > > > If possible can you integrate this onto your development branch. > > > > Hi Hardik, > > > > I've one question regarding the rotation control: I assume that this is > > limited to 0, 90, 180 and 270 degrees? I think it might be better to > > implement this as an enum in that case. > > 90 and 270 degrees would modify the image size. How would that be handled > in relationship with VIDIOC_S/G_FMT ? Furthermore, 180 degrees rotation is identical to HFLIP + VFLIP. Best regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 25 January 2009 10:35:49 Hans Verkuil wrote: > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > -----Original Message----- > > > From: Shah, Hardik > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, Manjunath; > > > R, Sivaraj; Hiremath, Vaibhav > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > 2. Control ID added for setting background color on > > > output device. > > > 3. New ioctl added for setting the color space conversion from > > > YUV to RGB. > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > --- > > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > index 165bc90..7599da8 100644 > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > > #endif > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = > > > "VIDIOC_S_COLOR_SPACE_CONV", + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] > > > = "VIDIOC_G_COLOR_SPACE_CONV", }; > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file > > > *file, } > > > break; > > > } > > > - > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > + { > > > + struct v4l2_color_space_conversion *p = arg; > > > + if (!ops->vidioc_s_color_space_conv) > > > + break; > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > + break; > > > + } > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > + { > > > + struct v4l2_color_space_conversion *p = arg; > > > + if (!ops->vidioc_g_color_space_conv) > > > + break; > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > + break; > > > + } > > > default: > > > { > > > if (!ops->vidioc_default) > > > diff --git a/linux/include/linux/videodev2.h > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > --- a/linux/include/linux/videodev2.h > > > +++ b/linux/include/linux/videodev2.h > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > /* last CID + 1 */ > > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > }; > > > > > > /* > > > + * Color conversion > > > + * User needs to pass pointer to color conversion matrix > > > + * defined by hardware > > > + */ > > > +struct v4l2_color_space_conversion { > > > + __s32 coefficients[3][3]; > > > + __s32 const_factor; > > > + __s32 offsets[3]; > > > +}; > > > + > > > +/* > > > * A U D I O > > > */ > > > struct v4l2_audio { > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > #endif > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct > > > v4l2_hw_freq_seek) + > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > v4l2_color_space_conversion) > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > v4l2_color_space_conversion) > > > /* Reminder: when adding new ioctls please add support for them to > > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > + > > > #ifdef __OLD_VIDIOC_ > > > /* for compatibility, will go away some day */ > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > b/linux/include/media/v4l2- ioctl.h > > > index b01c044..0c44ecf 100644 > > > --- a/linux/include/media/v4l2-ioctl.h > > > +++ b/linux/include/media/v4l2-ioctl.h > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > /* For other private ioctls */ > > > long (*vidioc_default) (struct file *file, void *fh, > > > int cmd, void *arg); > > > + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, > > > + struct v4l2_color_space_conversion *a); > > > + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, > > > + struct v4l2_color_space_conversion *a); > > > }; > > > > > > > > > -- > > > 1.5.6 > > > > [Shah, Hardik] Hi, > > Any comments on this patch. > > Hans/Mauro, > > If possible can you integrate this onto your development branch. > > Hi Hardik, > > I've one question regarding the rotation control: I assume that this is > limited to 0, 90, 180 and 270 degrees? I think it might be better to > implement this as an enum in that case. I also would like to see the documentation for these ioctls and controls. I hope my www.linuxtv.org/hg/~hverkuil/v4l-dvb-spec tree will be merged soon with the master, but in the meantime you can mail a diff against the documentation in my tree. I will ensure that the documentation patches will be applied correctly once the spec is merged in the main v4l-dvb repository. Regards, Hans
On Sun, 25 Jan 2009, Hans Verkuil wrote: > I also would like to see the documentation for these ioctls and controls. I > hope my www.linuxtv.org/hg/~hverkuil/v4l-dvb-spec tree will be merged soon > with the master, but in the meantime you can mail a diff against the > documentation in my tree. I will ensure that the documentation patches will > be applied correctly once the spec is merged in the main v4l-dvb > repository. Yes, I would like to see the color conversion documentation. V4L2 has a color space field as part of the pixel format (e.g., V4L2_COLORSPACE_REC709 or V4L2_COLORSPACE_SMPTE170M), how does this new ioctl interact with that? What is the format of the parameters? These are natively real numbers not integers, so to store them in an s32 there must some conversion, i.e. scale and offset. What is the actual equation used for the colorspace conversion? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Hans Verkuil [mailto:hverkuil@xs4all.nl] > Sent: Sunday, January 25, 2009 3:06 PM > To: Shah, Hardik > Cc: video4linux-list@redhat.com; linux-media@vger.kernel.org > Subject: Re: [PATCH] New V4L2 ioctls for OMAP class of Devices > > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > -----Original Message----- > > > From: Shah, Hardik > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, Manjunath; R, > > > Sivaraj; Hiremath, Vaibhav > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > 2. Control ID added for setting background color on > > > output device. > > > 3. New ioctl added for setting the color space conversion from > > > YUV to RGB. > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > --- > > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > index 165bc90..7599da8 100644 > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > > #endif > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = "VIDIOC_S_COLOR_SPACE_CONV", > > > + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] = "VIDIOC_G_COLOR_SPACE_CONV", > > > }; > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file *file, > > > } > > > break; > > > } > > > - > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > + { > > > + struct v4l2_color_space_conversion *p = arg; > > > + if (!ops->vidioc_s_color_space_conv) > > > + break; > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > + break; > > > + } > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > + { > > > + struct v4l2_color_space_conversion *p = arg; > > > + if (!ops->vidioc_g_color_space_conv) > > > + break; > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > + break; > > > + } > > > default: > > > { > > > if (!ops->vidioc_default) > > > diff --git a/linux/include/linux/videodev2.h > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > --- a/linux/include/linux/videodev2.h > > > +++ b/linux/include/linux/videodev2.h > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > /* last CID + 1 */ > > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > }; > > > > > > /* > > > + * Color conversion > > > + * User needs to pass pointer to color conversion matrix > > > + * defined by hardware > > > + */ > > > +struct v4l2_color_space_conversion { > > > + __s32 coefficients[3][3]; > > > + __s32 const_factor; > > > + __s32 offsets[3]; > > > +}; > > > + > > > +/* > > > * A U D I O > > > */ > > > struct v4l2_audio { > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > #endif > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct v4l2_hw_freq_seek) > > > + > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > v4l2_color_space_conversion) > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > v4l2_color_space_conversion) > > > /* Reminder: when adding new ioctls please add support for them to > > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > + > > > #ifdef __OLD_VIDIOC_ > > > /* for compatibility, will go away some day */ > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > b/linux/include/media/v4l2- ioctl.h > > > index b01c044..0c44ecf 100644 > > > --- a/linux/include/media/v4l2-ioctl.h > > > +++ b/linux/include/media/v4l2-ioctl.h > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > /* For other private ioctls */ > > > long (*vidioc_default) (struct file *file, void *fh, > > > int cmd, void *arg); > > > + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, > > > + struct v4l2_color_space_conversion *a); > > > + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, > > > + struct v4l2_color_space_conversion *a); > > > }; > > > > > > > > > -- > > > 1.5.6 > > > > [Shah, Hardik] Hi, > > Any comments on this patch. > > Hans/Mauro, > > If possible can you integrate this onto your development branch. > > Hi Hardik, > > I've one question regarding the rotation control: I assume that this is > limited to 0, 90, 180 and 270 degrees? I think it might be better to > implement this as an enum in that case. Hi Hans, The rotation values are 0, 90, 180 and 270 degree but to disable rotation the value passed should be -1 and this is one more value. I know 0 degree rotation corresponds to rotation disabled but DSS hardware requires 0 degree rotation to be enabled for mirroring. The difference between the 0 degree rotation and no rotation(-1) is that 0 degree rotation will use the rotation engine in OMAP and then do the mirroring while -1 degree rotation will not use rotation engine. There is more bandwidth utilization while using the rotation engine. So people may want to completely disable rotation and people may want 0 degree rotation for mirroring support. That's why I prefer not to use enum. Is that ok for I will update the documentation on the new ioctls soon. you? > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart@skynet.be] > Sent: Sunday, January 25, 2009 3:16 PM > To: video4linux-list@redhat.com > Cc: Hans Verkuil; Shah, Hardik; linux-media@vger.kernel.org > Subject: Re: [PATCH] New V4L2 ioctls for OMAP class of Devices > > On Sunday 25 January 2009, Laurent Pinchart wrote: > > On Sunday 25 January 2009, Hans Verkuil wrote: > > > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > > > -----Original Message----- > > > > > From: Shah, Hardik > > > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, Manjunath; > > > > > R, Sivaraj; Hiremath, Vaibhav > > > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > > > 2. Control ID added for setting background color on > > > > > output device. > > > > > 3. New ioctl added for setting the color space conversion from > > > > > YUV to RGB. > > > > > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > > > --- > > > > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > > > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > > > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > > > index 165bc90..7599da8 100644 > > > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > > > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > > > > #endif > > > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = > > > > > "VIDIOC_S_COLOR_SPACE_CONV", + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] > > > > > = "VIDIOC_G_COLOR_SPACE_CONV", }; > > > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file > > > > > *file, } > > > > > break; > > > > > } > > > > > - > > > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > > > + { > > > > > + struct v4l2_color_space_conversion *p = arg; > > > > > + if (!ops->vidioc_s_color_space_conv) > > > > > + break; > > > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > > > + break; > > > > > + } > > > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > > > + { > > > > > + struct v4l2_color_space_conversion *p = arg; > > > > > + if (!ops->vidioc_g_color_space_conv) > > > > > + break; > > > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > > > + break; > > > > > + } > > > > > default: > > > > > { > > > > > if (!ops->vidioc_default) > > > > > diff --git a/linux/include/linux/videodev2.h > > > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > > > --- a/linux/include/linux/videodev2.h > > > > > +++ b/linux/include/linux/videodev2.h > > > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > > > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > > > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > > > /* last CID + 1 */ > > > > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > > > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | > 0x900) > > > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > > > }; > > > > > > > > > > /* > > > > > + * Color conversion > > > > > + * User needs to pass pointer to color conversion matrix > > > > > + * defined by hardware > > > > > + */ > > > > > +struct v4l2_color_space_conversion { > > > > > + __s32 coefficients[3][3]; > > > > > + __s32 const_factor; > > > > > + __s32 offsets[3]; > > > > > +}; > > > > > + > > > > > +/* > > > > > * A U D I O > > > > > */ > > > > > struct v4l2_audio { > > > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > > > #endif > > > > > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct > > > > > v4l2_hw_freq_seek) + > > > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > > > v4l2_color_space_conversion) > > > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > > > v4l2_color_space_conversion) > > > > > /* Reminder: when adding new ioctls please add support for them to > > > > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > > > > > + > > > > > #ifdef __OLD_VIDIOC_ > > > > > /* for compatibility, will go away some day */ > > > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > > > b/linux/include/media/v4l2- ioctl.h > > > > > index b01c044..0c44ecf 100644 > > > > > --- a/linux/include/media/v4l2-ioctl.h > > > > > +++ b/linux/include/media/v4l2-ioctl.h > > > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > > > /* For other private ioctls */ > > > > > long (*vidioc_default) (struct file *file, void *fh, > > > > > int cmd, void *arg); > > > > > + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, > > > > > + struct v4l2_color_space_conversion *a); > > > > > + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, > > > > > + struct v4l2_color_space_conversion *a); > > > > > }; > > > > > > > > > > > > > > > -- > > > > > 1.5.6 > > > > > > > > [Shah, Hardik] Hi, > > > > Any comments on this patch. > > > > Hans/Mauro, > > > > If possible can you integrate this onto your development branch. > > > > > > Hi Hardik, > > > > > > I've one question regarding the rotation control: I assume that this is > > > limited to 0, 90, 180 and 270 degrees? I think it might be better to > > > implement this as an enum in that case. > > > > 90 and 270 degrees would modify the image size. How would that be handled > > in relationship with VIDIOC_S/G_FMT ? > > Furthermore, 180 degrees rotation is identical to HFLIP + VFLIP. [Shah, Hardik] Hi Laurent, OMAP is also having the hardware support for mirroring. I think HFLIP is essentially same as mirroring so we will be using the HFLIP in providing mirroring support. Regards, Hardik > > Best regards, > > Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Laurent Pinchart [mailto:laurent.pinchart@skynet.be] > Sent: Sunday, January 25, 2009 3:14 PM > To: video4linux-list@redhat.com > Cc: Hans Verkuil; Shah, Hardik; linux-media@vger.kernel.org > Subject: Re: [PATCH] New V4L2 ioctls for OMAP class of Devices > > On Sunday 25 January 2009, Hans Verkuil wrote: > > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > > -----Original Message----- > > > > From: Shah, Hardik > > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, Manjunath; R, > > > > Sivaraj; Hiremath, Vaibhav > > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > > 2. Control ID added for setting background color on > > > > output device. > > > > 3. New ioctl added for setting the color space conversion from > > > > YUV to RGB. > > > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > > --- > > > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > > index 165bc90..7599da8 100644 > > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > > > #endif > > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = > "VIDIOC_S_COLOR_SPACE_CONV", > > > > + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] = > "VIDIOC_G_COLOR_SPACE_CONV", > > > > }; > > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file *file, > > > > } > > > > break; > > > > } > > > > - > > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > > + { > > > > + struct v4l2_color_space_conversion *p = arg; > > > > + if (!ops->vidioc_s_color_space_conv) > > > > + break; > > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > > + break; > > > > + } > > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > > + { > > > > + struct v4l2_color_space_conversion *p = arg; > > > > + if (!ops->vidioc_g_color_space_conv) > > > > + break; > > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > > + break; > > > > + } > > > > default: > > > > { > > > > if (!ops->vidioc_default) > > > > diff --git a/linux/include/linux/videodev2.h > > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > > --- a/linux/include/linux/videodev2.h > > > > +++ b/linux/include/linux/videodev2.h > > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > > /* last CID + 1 */ > > > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) > > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > > }; > > > > > > > > /* > > > > + * Color conversion > > > > + * User needs to pass pointer to color conversion matrix > > > > + * defined by hardware > > > > + */ > > > > +struct v4l2_color_space_conversion { > > > > + __s32 coefficients[3][3]; > > > > + __s32 const_factor; > > > > + __s32 offsets[3]; > > > > +}; > > > > + > > > > +/* > > > > * A U D I O > > > > */ > > > > struct v4l2_audio { > > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > > #endif > > > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct > v4l2_hw_freq_seek) > > > > + > > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > > v4l2_color_space_conversion) > > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > > v4l2_color_space_conversion) > > > > /* Reminder: when adding new ioctls please add support for them to > > > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > > > + > > > > #ifdef __OLD_VIDIOC_ > > > > /* for compatibility, will go away some day */ > > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > > b/linux/include/media/v4l2- ioctl.h > > > > index b01c044..0c44ecf 100644 > > > > --- a/linux/include/media/v4l2-ioctl.h > > > > +++ b/linux/include/media/v4l2-ioctl.h > > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > > /* For other private ioctls */ > > > > long (*vidioc_default) (struct file *file, void *fh, > > > > int cmd, void *arg); > > > > + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, > > > > + struct v4l2_color_space_conversion *a); > > > > + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, > > > > + struct v4l2_color_space_conversion *a); > > > > }; > > > > > > > > > > > > -- > > > > 1.5.6 > > > > > > [Shah, Hardik] Hi, > > > Any comments on this patch. > > > Hans/Mauro, > > > If possible can you integrate this onto your development branch. > > > > Hi Hardik, > > > > I've one question regarding the rotation control: I assume that this is > > limited to 0, 90, 180 and 270 degrees? I think it might be better to > > implement this as an enum in that case. > > 90 and 270 degrees would modify the image size. How would that be handled in > relationship with VIDIOC_S/G_FMT ? [Shah, Hardik] Hi Laurent, After setting the rotation degree user has to once again call the S_FMT with the height and width to allow S_FMT to take care of reversing the height and width of the panel and depending on that your crop window and display window will be set if the rotaion is set to 90 or 270. While the get format will give you same height and width set earlier. Is this answer to your question? Regards, Hardik Shah > > Best regards, > > Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 27 January 2009 05:43:25 Shah, Hardik wrote: > > -----Original Message----- > > From: Hans Verkuil [mailto:hverkuil@xs4all.nl] > > Sent: Sunday, January 25, 2009 3:06 PM > > To: Shah, Hardik > > Cc: video4linux-list@redhat.com; linux-media@vger.kernel.org > > Subject: Re: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > > -----Original Message----- > > > > From: Shah, Hardik > > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, > > > > Manjunath; R, Sivaraj; Hiremath, Vaibhav > > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > > 2. Control ID added for setting background color on > > > > output device. > > > > 3. New ioctl added for setting the color space conversion from > > > > YUV to RGB. > > > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > > --- > > > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > > index 165bc90..7599da8 100644 > > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > > > #endif > > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = > > > > "VIDIOC_S_COLOR_SPACE_CONV", + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] > > > > = "VIDIOC_G_COLOR_SPACE_CONV", }; > > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file > > > > *file, } > > > > break; > > > > } > > > > - > > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > > + { > > > > + struct v4l2_color_space_conversion *p = arg; > > > > + if (!ops->vidioc_s_color_space_conv) > > > > + break; > > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > > + break; > > > > + } > > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > > + { > > > > + struct v4l2_color_space_conversion *p = arg; > > > > + if (!ops->vidioc_g_color_space_conv) > > > > + break; > > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > > + break; > > > > + } > > > > default: > > > > { > > > > if (!ops->vidioc_default) > > > > diff --git a/linux/include/linux/videodev2.h > > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > > --- a/linux/include/linux/videodev2.h > > > > +++ b/linux/include/linux/videodev2.h > > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > > /* last CID + 1 */ > > > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) > > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > > }; > > > > > > > > /* > > > > + * Color conversion > > > > + * User needs to pass pointer to color conversion matrix > > > > + * defined by hardware > > > > + */ > > > > +struct v4l2_color_space_conversion { > > > > + __s32 coefficients[3][3]; > > > > + __s32 const_factor; > > > > + __s32 offsets[3]; > > > > +}; > > > > + > > > > +/* > > > > * A U D I O > > > > */ > > > > struct v4l2_audio { > > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > > #endif > > > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct > > > > v4l2_hw_freq_seek) + > > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > > v4l2_color_space_conversion) > > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > > v4l2_color_space_conversion) > > > > /* Reminder: when adding new ioctls please add support for them to > > > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > > > + > > > > #ifdef __OLD_VIDIOC_ > > > > /* for compatibility, will go away some day */ > > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > > b/linux/include/media/v4l2- ioctl.h > > > > index b01c044..0c44ecf 100644 > > > > --- a/linux/include/media/v4l2-ioctl.h > > > > +++ b/linux/include/media/v4l2-ioctl.h > > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > > /* For other private ioctls */ > > > > long (*vidioc_default) (struct file *file, void *fh, > > > > int cmd, void *arg); > > > > + int (*vidioc_s_color_space_conv) (struct file *file, void > > > > *fh, + struct v4l2_color_space_conversion *a); > > > > + int (*vidioc_g_color_space_conv) (struct file *file, void > > > > *fh, + struct v4l2_color_space_conversion *a); > > > > }; > > > > > > > > > > > > -- > > > > 1.5.6 > > > > > > [Shah, Hardik] Hi, > > > Any comments on this patch. > > > Hans/Mauro, > > > If possible can you integrate this onto your development branch. > > > > Hi Hardik, > > > > I've one question regarding the rotation control: I assume that this is > > limited to 0, 90, 180 and 270 degrees? I think it might be better to > > implement this as an enum in that case. > > Hi Hans, > The rotation values are 0, 90, 180 and 270 degree but to disable rotation > the value passed should be -1 and this is one more value. I know 0 > degree rotation corresponds to rotation disabled but DSS hardware > requires 0 degree rotation to be enabled for mirroring. The difference > between the 0 degree rotation and no rotation(-1) is that 0 degree > rotation will use the rotation engine in OMAP and then do the mirroring > while -1 degree rotation will not use rotation engine. There is more > bandwidth utilization while using the rotation engine. So people may > want to completely disable rotation and people may want 0 degree rotation > for mirroring support. That's why I prefer not to use enum. Is that ok > for Yuck. Why not do this in the driver: if (degrees == 0) { if (mirroring) disable_rotation_engine(); else enable_rotation_engine(); } You are exporting an internal hack to the user API. That's not right. It seems to me that you have all the information needed in the driver. If that is not the case, then the fallback scenario is to implement this as a private control: USE_ROTATION_ENGINE. Regards, Hans > I will update the documentation on the new ioctls soon. > > you? > > > Regards, > > > > Hans > > > > -- > > Hans Verkuil - video4linux developer - sponsored by TANDBERG
> -----Original Message----- > From: Hans Verkuil [mailto:hverkuil@xs4all.nl] > Sent: Tuesday, January 27, 2009 2:40 PM > To: Shah, Hardik > Cc: video4linux-list@redhat.com; linux-media@vger.kernel.org > Subject: Re: [PATCH] New V4L2 ioctls for OMAP class of Devices > > On Tuesday 27 January 2009 05:43:25 Shah, Hardik wrote: > > > -----Original Message----- > > > From: Hans Verkuil [mailto:hverkuil@xs4all.nl] > > > Sent: Sunday, January 25, 2009 3:06 PM > > > To: Shah, Hardik > > > Cc: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > Subject: Re: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > > > -----Original Message----- > > > > > From: Shah, Hardik > > > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, > > > > > Manjunath; R, Sivaraj; Hiremath, Vaibhav > > > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > > > 2. Control ID added for setting background color on > > > > > output device. > > > > > 3. New ioctl added for setting the color space conversion from > > > > > YUV to RGB. > > > > > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > > > --- > > > > > linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- > > > > > linux/include/linux/videodev2.h | 19 ++++++++++++++++++- > > > > > linux/include/media/v4l2-ioctl.h | 4 ++++ > > > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > > > index 165bc90..7599da8 100644 > > > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", > > > > > [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", > > > > > #endif > > > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = > > > > > "VIDIOC_S_COLOR_SPACE_CONV", + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] > > > > > = "VIDIOC_G_COLOR_SPACE_CONV", }; > > > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file > > > > > *file, } > > > > > break; > > > > > } > > > > > - > > > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > > > + { > > > > > + struct v4l2_color_space_conversion *p = arg; > > > > > + if (!ops->vidioc_s_color_space_conv) > > > > > + break; > > > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > > > + break; > > > > > + } > > > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > > > + { > > > > > + struct v4l2_color_space_conversion *p = arg; > > > > > + if (!ops->vidioc_g_color_space_conv) > > > > > + break; > > > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > > > + break; > > > > > + } > > > > > default: > > > > > { > > > > > if (!ops->vidioc_default) > > > > > diff --git a/linux/include/linux/videodev2.h > > > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > > > --- a/linux/include/linux/videodev2.h > > > > > +++ b/linux/include/linux/videodev2.h > > > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > > > #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) > > > > > #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) > > > > > +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > > > /* last CID + 1 */ > > > > > -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) > > > > > +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) > > > > > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | > 0x900) > > > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > > > }; > > > > > > > > > > /* > > > > > + * Color conversion > > > > > + * User needs to pass pointer to color conversion matrix > > > > > + * defined by hardware > > > > > + */ > > > > > +struct v4l2_color_space_conversion { > > > > > + __s32 coefficients[3][3]; > > > > > + __s32 const_factor; > > > > > + __s32 offsets[3]; > > > > > +}; > > > > > + > > > > > +/* > > > > > * A U D I O > > > > > */ > > > > > struct v4l2_audio { > > > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > > > #endif > > > > > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct > > > > > v4l2_hw_freq_seek) + > > > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > > > v4l2_color_space_conversion) > > > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > > > v4l2_color_space_conversion) > > > > > /* Reminder: when adding new ioctls please add support for them to > > > > > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > > > > > + > > > > > #ifdef __OLD_VIDIOC_ > > > > > /* for compatibility, will go away some day */ > > > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > > > b/linux/include/media/v4l2- ioctl.h > > > > > index b01c044..0c44ecf 100644 > > > > > --- a/linux/include/media/v4l2-ioctl.h > > > > > +++ b/linux/include/media/v4l2-ioctl.h > > > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > > > /* For other private ioctls */ > > > > > long (*vidioc_default) (struct file *file, void *fh, > > > > > int cmd, void *arg); > > > > > + int (*vidioc_s_color_space_conv) (struct file *file, void > > > > > *fh, + struct v4l2_color_space_conversion > *a); > > > > > + int (*vidioc_g_color_space_conv) (struct file *file, void > > > > > *fh, + struct v4l2_color_space_conversion > *a); > > > > > }; > > > > > > > > > > > > > > > -- > > > > > 1.5.6 > > > > > > > > [Shah, Hardik] Hi, > > > > Any comments on this patch. > > > > Hans/Mauro, > > > > If possible can you integrate this onto your development branch. > > > > > > Hi Hardik, > > > > > > I've one question regarding the rotation control: I assume that this is > > > limited to 0, 90, 180 and 270 degrees? I think it might be better to > > > implement this as an enum in that case. > > > > Hi Hans, > > The rotation values are 0, 90, 180 and 270 degree but to disable rotation > > the value passed should be -1 and this is one more value. I know 0 > > degree rotation corresponds to rotation disabled but DSS hardware > > requires 0 degree rotation to be enabled for mirroring. The difference > > between the 0 degree rotation and no rotation(-1) is that 0 degree > > rotation will use the rotation engine in OMAP and then do the mirroring > > while -1 degree rotation will not use rotation engine. There is more > > bandwidth utilization while using the rotation engine. So people may > > want to completely disable rotation and people may want 0 degree rotation > > for mirroring support. That's why I prefer not to use enum. Is that ok > > for > > Yuck. Why not do this in the driver: > > if (degrees == 0) { > if (mirroring) > disable_rotation_engine(); > else > enable_rotation_engine(); > } > > You are exporting an internal hack to the user API. That's not right. It > seems to me that you have all the information needed in the driver. > > If that is not the case, then the fallback scenario is to implement this as > a private control: USE_ROTATION_ENGINE. [Shah, Hardik] Hi Hans, I got your above point. Now regarding the enum I am not sure about how to implement it. Are you suggesting me to remove the control ID for rotation and implement in some other way. Please let me know if I am missing something. Currently in driver I have implemented the rotation in below way { .id = V4L2_CID_ROTATION, .name = "Rotation", .minimum = 0, .maximum = 270, .step = 90, .default_value = -1, .flags = 0, .type = V4L2_CTRL_TYPE_INTEGER, You want me to change V4L2_CTRL_TYPE_INTEGER to some enum or something. Regards, Hardik Shah > > Regards, > > Hans > > > I will update the documentation on the new ioctls soon. > > > > you? > > > > > Regards, > > > > > > Hans > > > > > > -- > > > Hans Verkuil - video4linux developer - sponsored by TANDBERG > > > > -- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 27 January 2009 14:02:30 Shah, Hardik wrote: > > -----Original Message----- > > From: Hans Verkuil [mailto:hverkuil@xs4all.nl] > > Sent: Tuesday, January 27, 2009 2:40 PM > > To: Shah, Hardik > > Cc: video4linux-list@redhat.com; linux-media@vger.kernel.org > > Subject: Re: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > On Tuesday 27 January 2009 05:43:25 Shah, Hardik wrote: > > > > -----Original Message----- > > > > From: Hans Verkuil [mailto:hverkuil@xs4all.nl] > > > > Sent: Sunday, January 25, 2009 3:06 PM > > > > To: Shah, Hardik > > > > Cc: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > > Subject: Re: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > > > On Saturday 24 January 2009 18:03:51 Shah, Hardik wrote: > > > > > > -----Original Message----- > > > > > > From: Shah, Hardik > > > > > > Sent: Wednesday, January 21, 2009 5:24 PM > > > > > > To: video4linux-list@redhat.com; linux-media@vger.kernel.org > > > > > > Cc: Shah, Hardik; Jadav, Brijesh R; Nagalla, Hari; Hadli, > > > > > > Manjunath; R, Sivaraj; Hiremath, Vaibhav > > > > > > Subject: [PATCH] New V4L2 ioctls for OMAP class of Devices > > > > > > > > > > > > 1. Control ID added for rotation. Same as HFLIP. > > > > > > 2. Control ID added for setting background color on > > > > > > output device. > > > > > > 3. New ioctl added for setting the color space conversion from > > > > > > YUV to RGB. > > > > > > > > > > > > Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> > > > > > > Signed-off-by: Hari Nagalla <hnagalla@ti.com> > > > > > > Signed-off-by: Hardik Shah <hardik.shah@ti.com> > > > > > > Signed-off-by: Manjunath Hadli <mrh@ti.com> > > > > > > Signed-off-by: R Sivaraj <sivaraj@ti.com> > > > > > > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > > > > > > --- > > > > > > linux/drivers/media/video/v4l2-ioctl.c | 19 > > > > > > ++++++++++++++++++- linux/include/linux/videodev2.h | > > > > > > 19 ++++++++++++++++++- linux/include/media/v4l2-ioctl.h | > > > > > > 4 ++++ > > > > > > 3 files changed, 40 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/linux/drivers/media/video/v4l2-ioctl.c > > > > > > b/linux/drivers/media/video/v4l2-ioctl.c > > > > > > index 165bc90..7599da8 100644 > > > > > > --- a/linux/drivers/media/video/v4l2-ioctl.c > > > > > > +++ b/linux/drivers/media/video/v4l2-ioctl.c > > > > > > @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { > > > > > > [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = > > > > > > "VIDIOC_DBG_G_CHIP_IDENT", [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = > > > > > > "VIDIOC_S_HW_FREQ_SEEK", #endif > > > > > > + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = > > > > > > "VIDIOC_S_COLOR_SPACE_CONV", > > > > > > + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] = > > > > > > "VIDIOC_G_COLOR_SPACE_CONV", }; > > > > > > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > > > > > > > > > > > @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file > > > > > > *file, } > > > > > > break; > > > > > > } > > > > > > - > > > > > > + case VIDIOC_S_COLOR_SPACE_CONV: > > > > > > + { > > > > > > + struct v4l2_color_space_conversion *p = arg; > > > > > > + if (!ops->vidioc_s_color_space_conv) > > > > > > + break; > > > > > > + ret = ops->vidioc_s_color_space_conv(file, fh, p); > > > > > > + break; > > > > > > + } > > > > > > + case VIDIOC_G_COLOR_SPACE_CONV: > > > > > > + { > > > > > > + struct v4l2_color_space_conversion *p = arg; > > > > > > + if (!ops->vidioc_g_color_space_conv) > > > > > > + break; > > > > > > + ret = ops->vidioc_g_color_space_conv(file, fh, p); > > > > > > + break; > > > > > > + } > > > > > > default: > > > > > > { > > > > > > if (!ops->vidioc_default) > > > > > > diff --git a/linux/include/linux/videodev2.h > > > > > > b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 > > > > > > --- a/linux/include/linux/videodev2.h > > > > > > +++ b/linux/include/linux/videodev2.h > > > > > > @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { > > > > > > #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) > > > > > > #define V4L2_CID_CHROMA_AGC > > > > > > (V4L2_CID_BASE+29) #define V4L2_CID_COLOR_KILLER > > > > > > (V4L2_CID_BASE+30) +#define > > > > > > V4L2_CID_ROTATION (V4L2_CID_BASE+31) > > > > > > +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) > > > > > > /* last CID + 1 */ > > > > > > -#define V4L2_CID_LASTP1 > > > > > > (V4L2_CID_BASE+31) +#define V4L2_CID_LASTP1 > > > > > > (V4L2_CID_BASE+33) > > > > > > > > > > > > /* MPEG-class control IDs defined by V4L2 */ > > > > > > #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | > > > > 0x900) > > > > > > > > @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { > > > > > > }; > > > > > > > > > > > > /* > > > > > > + * Color conversion > > > > > > + * User needs to pass pointer to color conversion matrix > > > > > > + * defined by hardware > > > > > > + */ > > > > > > +struct v4l2_color_space_conversion { > > > > > > + __s32 coefficients[3][3]; > > > > > > + __s32 const_factor; > > > > > > + __s32 offsets[3]; > > > > > > +}; > > > > > > + > > > > > > +/* > > > > > > * A U D I O > > > > > > */ > > > > > > struct v4l2_audio { > > > > > > @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { > > > > > > #endif > > > > > > > > > > > > #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct > > > > > > v4l2_hw_freq_seek) + > > > > > > +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct > > > > > > v4l2_color_space_conversion) > > > > > > +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct > > > > > > v4l2_color_space_conversion) > > > > > > /* Reminder: when adding new ioctls please add support for > > > > > > them to drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > > > > > > > > > > > + > > > > > > #ifdef __OLD_VIDIOC_ > > > > > > /* for compatibility, will go away some day */ > > > > > > #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) > > > > > > diff --git a/linux/include/media/v4l2-ioctl.h > > > > > > b/linux/include/media/v4l2- ioctl.h > > > > > > index b01c044..0c44ecf 100644 > > > > > > --- a/linux/include/media/v4l2-ioctl.h > > > > > > +++ b/linux/include/media/v4l2-ioctl.h > > > > > > @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { > > > > > > /* For other private ioctls */ > > > > > > long (*vidioc_default) (struct file *file, void *fh, > > > > > > int cmd, void *arg); > > > > > > + int (*vidioc_s_color_space_conv) (struct file *file, void > > > > > > *fh, + struct v4l2_color_space_conversion > > > > *a); > > > > > > > > + int (*vidioc_g_color_space_conv) (struct file *file, void > > > > > > *fh, + struct v4l2_color_space_conversion > > > > *a); > > > > > > > > }; > > > > > > > > > > > > > > > > > > -- > > > > > > 1.5.6 > > > > > > > > > > [Shah, Hardik] Hi, > > > > > Any comments on this patch. > > > > > Hans/Mauro, > > > > > If possible can you integrate this onto your development branch. > > > > > > > > Hi Hardik, > > > > > > > > I've one question regarding the rotation control: I assume that > > > > this is limited to 0, 90, 180 and 270 degrees? I think it might be > > > > better to implement this as an enum in that case. > > > > > > Hi Hans, > > > The rotation values are 0, 90, 180 and 270 degree but to disable > > > rotation the value passed should be -1 and this is one more value. I > > > know 0 degree rotation corresponds to rotation disabled but DSS > > > hardware requires 0 degree rotation to be enabled for mirroring. The > > > difference between the 0 degree rotation and no rotation(-1) is that > > > 0 degree rotation will use the rotation engine in OMAP and then do > > > the mirroring while -1 degree rotation will not use rotation engine. > > > There is more bandwidth utilization while using the rotation engine. > > > So people may want to completely disable rotation and people may want > > > 0 degree rotation for mirroring support. That's why I prefer not to > > > use enum. Is that ok for > > > > Yuck. Why not do this in the driver: > > > > if (degrees == 0) { > > if (mirroring) > > disable_rotation_engine(); > > else > > enable_rotation_engine(); > > } > > > > You are exporting an internal hack to the user API. That's not right. > > It seems to me that you have all the information needed in the driver. > > > > If that is not the case, then the fallback scenario is to implement > > this as a private control: USE_ROTATION_ENGINE. > > [Shah, Hardik] Hi Hans, > I got your above point. Now regarding the enum I am not sure about how > to implement it. Are you suggesting me to remove the control ID for > rotation and implement in some other way. Please let me know if I am > missing something. Currently in driver I have implemented the rotation in > below way { > .id = V4L2_CID_ROTATION, > .name = "Rotation", > .minimum = 0, > .maximum = 270, > .step = 90, > .default_value = -1, > .flags = 0, > .type = V4L2_CTRL_TYPE_INTEGER, > You want me to change V4L2_CTRL_TYPE_INTEGER to some enum or something. Change it to V4L2_CTRL_TYPE_MENU. See: http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec-single/v4l2.html#VIDIOC-QUERYCTRL Note that support for new controls should also be added to v4l2-common.c: utility functions v4l2_ctrl_get_name(), v4l2_ctrl_get_menu(), v4l2_ctrl_query_fill() and v4l2_ctrl_query_fill_std() need to be updated. It's trivial but it will ensure consistency. Currently the use of these functions is optional and many drivers do not use them. However, I want to integrate control handling in the v4l framework and then these functions will be used heavily. So updating these utility functions will ensure an easy upgrade path. You are aware of the v4l2-ctl test utility included in v4l2-apps/util? It's very handy to verify controls: try v4l2-ctl --list-ctrls-menus. Regards, Hans > > Regards, > Hardik Shah > > > Regards, > > > > Hans > > > > > I will update the documentation on the new ioctls soon. > > > > > > you? > > > > > > > Regards, > > > > > > > > Hans > > > > > > > > -- > > > > Hans Verkuil - video4linux developer - sponsored by TANDBERG > > > > -- > > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 27 Jan 2009, Shah, Hardik wrote: > The rotation values are 0, 90, 180 and 270 degree but to disable rotation > the value passed should be -1 and this is one more value. I know 0 > degree rotation corresponds to rotation disabled but DSS hardware > requires 0 degree rotation to be enabled for mirroring. The difference > between the 0 degree rotation and no rotation(-1) is that 0 degree > rotation will use the rotation engine in OMAP and then do the mirroring > while -1 degree rotation will not use rotation engine. There is more > bandwidth utilization while using the rotation engine. So people may > want to completely disable rotation and people may want 0 degree rotation > for mirroring support. That's why I prefer not to use enum. Is that ok > for That sounds like a hardware quirk that the driver should take care of. Just have the driver turn on the rotation engine if mirroring is used. It seems very hard for an application to make decent use of rotation support if only certain values are supported by the hardware yet anything in degrees can be specified? How is the application supposed to know what values will be acceptable? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 27 Jan 2009, Hans Verkuil wrote: > > [Shah, Hardik] Hi Hans, > > I got your above point. Now regarding the enum I am not sure about how > > to implement it. Are you suggesting me to remove the control ID for > > rotation and implement in some other way. Please let me know if I am > > missing something. Currently in driver I have implemented the rotation in > > below way { > > .id = V4L2_CID_ROTATION, > > .name = "Rotation", > > .minimum = 0, > > .maximum = 270, > > .step = 90, > > .default_value = -1, > > .flags = 0, > > .type = V4L2_CTRL_TYPE_INTEGER, > > You want me to change V4L2_CTRL_TYPE_INTEGER to some enum or something. > > Change it to V4L2_CTRL_TYPE_MENU. See: > http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec-single/v4l2.html#VIDIOC-QUERYCTRL Thinking about it more, I think an integer control like this might make more sense. default_value should be changed to 0 of course. Extracting the real meaning from the control setting is more obvious for the integer control than a menu. And what if some hardware allows for rotations other than 90 degrees? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 27 January 2009 19:08:43 Trent Piepho wrote: > On Tue, 27 Jan 2009, Hans Verkuil wrote: > > > [Shah, Hardik] Hi Hans, > > > I got your above point. Now regarding the enum I am not sure about > > > how to implement it. Are you suggesting me to remove the control ID > > > for rotation and implement in some other way. Please let me know if > > > I am missing something. Currently in driver I have implemented the > > > rotation in below way { > > > .id = V4L2_CID_ROTATION, > > > .name = "Rotation", > > > .minimum = 0, > > > .maximum = 270, > > > .step = 90, > > > .default_value = -1, > > > .flags = 0, > > > .type = V4L2_CTRL_TYPE_INTEGER, > > > You want me to change V4L2_CTRL_TYPE_INTEGER to some enum or > > > something. > > > > Change it to V4L2_CTRL_TYPE_MENU. See: > > http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec-single/v > >4l2.html#VIDIOC-QUERYCTRL > > Thinking about it more, I think an integer control like this might make > more sense. default_value should be changed to 0 of course. Extracting > the real meaning from the control setting is more obvious for the integer > control than a menu. And what if some hardware allows for rotations > other than 90 degrees? If the hardware can do rotations other than 90 degrees then we get into the area of video effects. In principle such a driver can implement this rotation control as an integer rather than a menu (apps are supposed to query the type of a control dynamically, after all). But for a case like this where there are only four values I think a menu-type control is much more user-friendly. Regards, Hans
On Tue, 27 Jan 2009, Hans Verkuil wrote: > On Tuesday 27 January 2009 19:08:43 Trent Piepho wrote: > > On Tue, 27 Jan 2009, Hans Verkuil wrote: > > > > [Shah, Hardik] Hi Hans, > > > > I got your above point. Now regarding the enum I am not sure about > > > > how to implement it. Are you suggesting me to remove the control ID > > > > for rotation and implement in some other way. Please let me know if > > > > I am missing something. Currently in driver I have implemented the > > > > rotation in below way { > > > > .id = V4L2_CID_ROTATION, > > > > .name = "Rotation", > > > > .minimum = 0, > > > > .maximum = 270, > > > > .step = 90, > > > > .default_value = -1, > > > > .flags = 0, > > > > .type = V4L2_CTRL_TYPE_INTEGER, > > > > You want me to change V4L2_CTRL_TYPE_INTEGER to some enum or > > > > something. > > > > > > Change it to V4L2_CTRL_TYPE_MENU. See: > > > http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec-single/v > > >4l2.html#VIDIOC-QUERYCTRL > > > > Thinking about it more, I think an integer control like this might make > > more sense. default_value should be changed to 0 of course. Extracting > > the real meaning from the control setting is more obvious for the integer > > control than a menu. And what if some hardware allows for rotations > > other than 90 degrees? > > If the hardware can do rotations other than 90 degrees then we get into the > area of video effects. In principle such a driver can implement this > rotation control as an integer rather than a menu (apps are supposed to > query the type of a control dynamically, after all). But for a case like > this where there are only four values I think a menu-type control is much > more user-friendly. How so? An application can easily tell from the range and step of the integer control that there are only four values. And it can easily tell what the values mean. For instance if the app wants to show icons for the rotations or have a command line parameter "rotation in degrees", it's easy to figure out what value the control should be set to. For a menu, what is the app supposed to do? Call atoi() on the menu entry names and hope that they parse corretly? Seems like a kludge to do that. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 27 Jan 2009, Shah, Hardik wrote: > > 90 and 270 degrees would modify the image size. How would that be handled in > > relationship with VIDIOC_S/G_FMT ? > [Shah, Hardik] Hi Laurent, > After setting the rotation degree user has to once again call the S_FMT > with the height and width to allow S_FMT to take care of reversing the > height and width of the panel and depending on that your crop window and > display window will be set if the rotation is set to 90 or 270. While the > get format will give you same height and width set earlier. Is this > answer to your question? That seems like a bad idea. Consider the case the hardware does scaling and rotation as separate steps. The hardware receives a Rec. 601 standard 720x480 image to it's scaling/rotation engine. You have a selected a 640x480 image rotated 0 degrees. This is ok. You change the rotation to 90. The hardware must re-program the scaler to produce a 480x640 image so that after rotation it will remain 640x480. Then you must call S_FMT to request 480x640, which re-programs the scaler back to 640x480. There are two problems here. First, it's inefficient to reprogram the scaler from 640x480 to 480x640 just to change it right back to 640x480 again. But there is a much more serious problem. Virtually all scaling hardware has various limitations of what it can do. For instance, it is very common that the hardware can only shrink images, not enlarge them. In the above example, changing the rotation from 0 to 90 will fail, since doing so would require scaling the 720x480 input to 480x640. The hardware can't do that as it involves enlarging from 480 lines to 640. How would one change the rotation? Use S_FMT to change to 480x640 first, then change rotation? That won't work because 480x640 at 0 degrees isn't supported. The basic problem here is that the limitations on image size for a non-rotated image are different from the limitations of a rotated image. I think the specification should say that changing the rotation is allowed (not required, allowed) to change the image size. Applications should call G_FMT after changing rotation to see if driver decided to alter the image size for efficiency or because of hardware limitations. Any given image size may be valid in some rotations but not in other rotations. The *meaning* of width and height, i.e. how they relate to layout of data in memory, do not change when the image is rotated. Since changing the image size during capture is problematic, it should be expected that trying to change the rotation during capture might return EBUSY. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 27 January 2009 19:30:18 Trent Piepho wrote: > On Tue, 27 Jan 2009, Hans Verkuil wrote: > > On Tuesday 27 January 2009 19:08:43 Trent Piepho wrote: > > > On Tue, 27 Jan 2009, Hans Verkuil wrote: > > > > > [Shah, Hardik] Hi Hans, > > > > > I got your above point. Now regarding the enum I am not sure > > > > > about how to implement it. Are you suggesting me to remove the > > > > > control ID for rotation and implement in some other way. Please > > > > > let me know if I am missing something. Currently in driver I have > > > > > implemented the rotation in below way { > > > > > .id = V4L2_CID_ROTATION, > > > > > .name = "Rotation", > > > > > .minimum = 0, > > > > > .maximum = 270, > > > > > .step = 90, > > > > > .default_value = -1, > > > > > .flags = 0, > > > > > .type = V4L2_CTRL_TYPE_INTEGER, > > > > > You want me to change V4L2_CTRL_TYPE_INTEGER to some enum or > > > > > something. > > > > > > > > Change it to V4L2_CTRL_TYPE_MENU. See: > > > > http://www.linuxtv.org/downloads/video4linux/API/V4L2_API/spec-sing > > > >le/v 4l2.html#VIDIOC-QUERYCTRL > > > > > > Thinking about it more, I think an integer control like this might > > > make more sense. default_value should be changed to 0 of course. > > > Extracting the real meaning from the control setting is more obvious > > > for the integer control than a menu. And what if some hardware > > > allows for rotations other than 90 degrees? > > > > If the hardware can do rotations other than 90 degrees then we get into > > the area of video effects. In principle such a driver can implement > > this rotation control as an integer rather than a menu (apps are > > supposed to query the type of a control dynamically, after all). But > > for a case like this where there are only four values I think a > > menu-type control is much more user-friendly. > > How so? An application can easily tell from the range and step of the > integer control that there are only four values. And it can easily tell > what the values mean. For instance if the app wants to show icons for > the rotations or have a command line parameter "rotation in degrees", > it's easy to figure out what value the control should be set to. For a > menu, what is the app supposed to do? Call atoi() on the menu entry > names and hope that they parse corretly? Seems like a kludge to do that. In the case of menus each possible menu item always corresponds with an enum defined in videodev2.h. So it is no problem doing what you want with a menu. That said, it is a good point that you can use an integer with an appropriate step size. I'd forgotten about that. Hmm, I really have no preference. Let's keep it as an integer after all. Regards, Hans
diff --git a/linux/drivers/media/video/v4l2-ioctl.c b/linux/drivers/media/video/v4l2-ioctl.c index 165bc90..7599da8 100644 --- a/linux/drivers/media/video/v4l2-ioctl.c +++ b/linux/drivers/media/video/v4l2-ioctl.c @@ -270,6 +270,8 @@ static const char *v4l2_ioctls[] = { [_IOC_NR(VIDIOC_DBG_G_CHIP_IDENT)] = "VIDIOC_DBG_G_CHIP_IDENT", [_IOC_NR(VIDIOC_S_HW_FREQ_SEEK)] = "VIDIOC_S_HW_FREQ_SEEK", #endif + [_IOC_NR(VIDIOC_S_COLOR_SPACE_CONV)] = "VIDIOC_S_COLOR_SPACE_CONV", + [_IOC_NR(VIDIOC_G_COLOR_SPACE_CONV)] = "VIDIOC_G_COLOR_SPACE_CONV", }; #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) @@ -1838,7 +1840,22 @@ static long __video_do_ioctl(struct file *file, } break; } - + case VIDIOC_S_COLOR_SPACE_CONV: + { + struct v4l2_color_space_conversion *p = arg; + if (!ops->vidioc_s_color_space_conv) + break; + ret = ops->vidioc_s_color_space_conv(file, fh, p); + break; + } + case VIDIOC_G_COLOR_SPACE_CONV: + { + struct v4l2_color_space_conversion *p = arg; + if (!ops->vidioc_g_color_space_conv) + break; + ret = ops->vidioc_g_color_space_conv(file, fh, p); + break; + } default: { if (!ops->vidioc_default) diff --git a/linux/include/linux/videodev2.h b/linux/include/linux/videodev2.h index b0c5010..9fbc3b0 100644 --- a/linux/include/linux/videodev2.h +++ b/linux/include/linux/videodev2.h @@ -879,8 +879,10 @@ enum v4l2_power_line_frequency { #define V4L2_CID_BACKLIGHT_COMPENSATION (V4L2_CID_BASE+28) #define V4L2_CID_CHROMA_AGC (V4L2_CID_BASE+29) #define V4L2_CID_COLOR_KILLER (V4L2_CID_BASE+30) +#define V4L2_CID_ROTATION (V4L2_CID_BASE+31) +#define V4L2_CID_BG_COLOR (V4L2_CID_BASE+32) /* last CID + 1 */ -#define V4L2_CID_LASTP1 (V4L2_CID_BASE+31) +#define V4L2_CID_LASTP1 (V4L2_CID_BASE+33) /* MPEG-class control IDs defined by V4L2 */ #define V4L2_CID_MPEG_BASE (V4L2_CTRL_CLASS_MPEG | 0x900) @@ -1192,6 +1194,17 @@ struct v4l2_hw_freq_seek { }; /* + * Color conversion + * User needs to pass pointer to color conversion matrix + * defined by hardware + */ +struct v4l2_color_space_conversion { + __s32 coefficients[3][3]; + __s32 const_factor; + __s32 offsets[3]; +}; + +/* * A U D I O */ struct v4l2_audio { @@ -1493,9 +1506,13 @@ struct v4l2_chip_ident_old { #endif #define VIDIOC_S_HW_FREQ_SEEK _IOW('V', 82, struct v4l2_hw_freq_seek) + +#define VIDIOC_S_COLOR_SPACE_CONV _IOW('V', 83, struct v4l2_color_space_conversion) +#define VIDIOC_G_COLOR_SPACE_CONV _IOR('V', 84, struct v4l2_color_space_conversion) /* Reminder: when adding new ioctls please add support for them to drivers/media/video/v4l2-compat-ioctl32.c as well! */ + #ifdef __OLD_VIDIOC_ /* for compatibility, will go away some day */ #define VIDIOC_OVERLAY_OLD _IOWR('V', 14, int) diff --git a/linux/include/media/v4l2-ioctl.h b/linux/include/media/v4l2-ioctl.h index b01c044..0c44ecf 100644 --- a/linux/include/media/v4l2-ioctl.h +++ b/linux/include/media/v4l2-ioctl.h @@ -241,6 +241,10 @@ struct v4l2_ioctl_ops { /* For other private ioctls */ long (*vidioc_default) (struct file *file, void *fh, int cmd, void *arg); + int (*vidioc_s_color_space_conv) (struct file *file, void *fh, + struct v4l2_color_space_conversion *a); + int (*vidioc_g_color_space_conv) (struct file *file, void *fh, + struct v4l2_color_space_conversion *a); };
1. Control ID added for rotation. Same as HFLIP. 2. Control ID added for setting background color on output device. 3. New ioctl added for setting the color space conversion from YUV to RGB. Signed-off-by: Brijesh Jadav <brijesh.j@ti.com> Signed-off-by: Hari Nagalla <hnagalla@ti.com> Signed-off-by: Hardik Shah <hardik.shah@ti.com> Signed-off-by: Manjunath Hadli <mrh@ti.com> Signed-off-by: R Sivaraj <sivaraj@ti.com> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> --- linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- linux/include/linux/videodev2.h | 19 ++++++++++++++++++- linux/include/media/v4l2-ioctl.h | 4 ++++ 3 files changed, 40 insertions(+), 2 deletions(-) -- 1.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html