| Message ID | 1233206626-19157-1-git-send-email-hardik.shah@ti.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Thu, 29 Jan 2009, Hardik Shah wrote: > 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. > 4. Updated the v4l2-common.c file according to comments. Wasn't there supposed to be some documentation? > + case V4L2_CID_BG_COLOR: > + /* Max value is 2^24 as RGB888 is used for background color */ > + return v4l2_ctrl_query_fill(qctrl, 0, 16777216, 1, 0); Wouldn't it make more sense to set background in the same colorspace as the selected format? -- 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: Trent Piepho [mailto:xyzzy@speakeasy.org] > Sent: Thursday, January 29, 2009 11:50 AM > To: Shah, Hardik > Cc: linux-media@vger.kernel.org; video4linux-list@redhat.com; Jadav, Brijesh > R; Nagalla, Hari; Hiremath, Vaibhav > Subject: Re: [PATCHv2] New V4L2 ioctls for OMAP class of Devices > > On Thu, 29 Jan 2009, Hardik Shah wrote: > > 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. > > 4. Updated the v4l2-common.c file according to comments. > > Wasn't there supposed to be some documentation? > > > + case V4L2_CID_BG_COLOR: > > + /* Max value is 2^24 as RGB888 is used for background color */ > > + return v4l2_ctrl_query_fill(qctrl, 0, 16777216, 1, 0); > > Wouldn't it make more sense to set background in the same colorspace as the > selected format? [Shah, Hardik] Background color setting can be done only in the RGB space as hardware doesn't understand YUV or RGB565 for the background color setting. And background color and pixel format are not related. If we want to have the background setting format same as the color format then driver will have to do the color conversion and that is not optimal. 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
Hello. > +#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) Do you mind if I ask a question about those ioctls? Because as far as I understand, we can use VIDIOC_S_FMT ioctl to convert colorspaces. Setting through colorspace member in v4l2_pix_format, we could change output colorspace. If there is some different use, can you tell me what it is? Regards, Nate
Hi Hardik, Just a minor point: it's enough to post to linux-media, no need to post to the video4linux list as well. I assume everyone involved in v4l-dvb has now subscribed to linux-media. On Thursday 29 January 2009 07:57:22 Shah, Hardik wrote: > > -----Original Message----- > > From: Trent Piepho [mailto:xyzzy@speakeasy.org] > > Sent: Thursday, January 29, 2009 11:50 AM > > To: Shah, Hardik > > Cc: linux-media@vger.kernel.org; video4linux-list@redhat.com; Jadav, > > Brijesh R; Nagalla, Hari; Hiremath, Vaibhav > > Subject: Re: [PATCHv2] New V4L2 ioctls for OMAP class of Devices > > > > On Thu, 29 Jan 2009, Hardik Shah wrote: > > > 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. > > > 4. Updated the v4l2-common.c file according to comments. > > > > Wasn't there supposed to be some documentation? > > > > > + case V4L2_CID_BG_COLOR: > > > + /* Max value is 2^24 as RGB888 is used for background color */ > > > + return v4l2_ctrl_query_fill(qctrl, 0, 16777216, 1, 0); > > > > Wouldn't it make more sense to set background in the same colorspace as > > the selected format? > > [Shah, Hardik] Background color setting can be done only in the RGB space > as hardware doesn't understand YUV or RGB565 for the background color > setting. And background color and pixel format are not related. If we > want to have the background setting format same as the color format then > driver will have to do the color conversion and that is not optimal. In the case of a chromakey the value is interpreted according to the pixel format of the associated framebuffer. However, if I understand the omap architecture correctly, this background color is pretty much independent from either graphics or videoplane pixel formats. As such it makes sense to fix it to a specific pixel format and let the driver transform it if it needs to. It is similar in that respect to the V4L2_CID_MPEG_VIDEO_MUTE_YUV control. The documentation needs to specify the format precisely, of course. I also noticed this: @@ -548,6 +553,7 @@ int v4l2_ctrl_query_fill(struct v4l2_queryctrl *qctrl, s32 min, s32 max, s32 ste case V4L2_CID_CONTRAST: case V4L2_CID_SATURATION: case V4L2_CID_HUE: + case V4L2_CID_BG_COLOR: qctrl->flags |= V4L2_CTRL_FLAG_SLIDER; break; } BG_COLOR is hardly a slider-like control! It's just a regular integer control. Regards, Hans
> -----Original Message----- > From: DongSoo Kim [mailto:dongsoo.kim@gmail.com] > Sent: Thursday, January 29, 2009 1:14 PM > To: Shah, Hardik > Cc: linux-media@vger.kernel.org; video4linux-list@redhat.com > Subject: Re: [PATCHv2] New V4L2 ioctls for OMAP class of Devices > > Hello. > > > +#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) > > Do you mind if I ask a question about those ioctls? > Because as far as I understand, we can use VIDIOC_S_FMT ioctl to convert > colorspaces. Setting through colorspace member in v4l2_pix_format, we > could change output colorspace. > If there is some different use, can you tell me what it is? > [Shah, Hardik] OMAP Display sub-system supports various pixel formats as inputs like YUV, UYVY, RGB24, RGB16 but the compositors which take these input format and displays on to the output devices like TV, LCD can only understand RGB format. Hardware has provision for converting any data taken in YUV or UYVY format to be converted into the RGB formats, before giving it to the output devices. To convert this hardware needs to be programmed with correct coefficient and offsets to convert from YUV to RGB. These coefficients are pretty standard but still some one may require altering it. For this new ioctl is added. Standard equation for converting from YUV or UYVY is | R | | RY RCr RCb | | Y - 16 | | G | = 1/K | Gy GCr Gcb | * | Cr - 128 | | B | | By BCr BCb | | Cb - 128 | Where Ry, Rcr, Rcb Gy, Gcr, Gcb, By, Bcr and Bcb are the programmable coefficients. But in future offsets like Y-16, Cr-128 or Cb-128 or constant like 1/K may also be programmable. So I have added this new ioctl. Regards, Hardik Shah > Regards, > Nate > > -- > ======================================================== > Dong Soo, Kim > Engineer > Mobile S/W Platform Lab. S/W centre > Telecommunication R&D Centre > Samsung Electronics CO., LTD. > e-mail : dongsoo.kim@gmail.com > dongsoo45.kim@samsung.com > ======================================================== -- 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 Thursday 29 January 2009 08:44:20 DongSoo Kim wrote: > Hello. > > > +#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) > > Do you mind if I ask a question about those ioctls? > Because as far as I understand, we can use VIDIOC_S_FMT ioctl to convert > colorspaces. Setting through colorspace member in v4l2_pix_format, we > could change output colorspace. Colorspace is read-only, you cannot set it. It just gives you the colorspace that the hardware uses by default. If you want to fully control the colorspace, then you need these ioctls. Regards, Hans > If there is some different use, can you tell me what it is? > > Regards, > Nate
On Thursday 29 January 2009 09:28:07 Shah, Hardik wrote: > > -----Original Message----- > > From: DongSoo Kim [mailto:dongsoo.kim@gmail.com] > > Sent: Thursday, January 29, 2009 1:14 PM > > To: Shah, Hardik > > Cc: linux-media@vger.kernel.org; video4linux-list@redhat.com > > Subject: Re: [PATCHv2] New V4L2 ioctls for OMAP class of Devices > > > > Hello. > > > > > +#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) > > > > Do you mind if I ask a question about those ioctls? > > Because as far as I understand, we can use VIDIOC_S_FMT ioctl to > > convert colorspaces. Setting through colorspace member in > > v4l2_pix_format, we could change output colorspace. > > If there is some different use, can you tell me what it is? > > [Shah, Hardik] OMAP Display sub-system supports various pixel formats as > inputs like YUV, UYVY, RGB24, RGB16 but the compositors which take these > input format and displays on to the output devices like TV, LCD can only > understand RGB format. Hardware has provision for converting any data > taken in YUV or UYVY format to be converted into the RGB formats, before > giving it to the output devices. To convert this hardware needs to be > programmed with correct coefficient and offsets to convert from YUV to > RGB. Does that mean that when I select a YUV format for output, I still need to call the color space conversion ioctl? That is not right. Selecting a format must setup the CSC with decent defaults. I assumed the CSC ioctls were only meant for fine-grained control: first you call S_FMT to select the pixelformat which sets up the CSC with defaults, then you call VIDIOC_G/S_COLOR_SPACE_CONV to modify them if needed. Regards, Hans > These coefficients are pretty standard but still some one may > require altering it. For this new ioctl is added. Standard equation for > converting from YUV or UYVY is > > | R | | RY RCr RCb | | Y - 16 | > | G | = 1/K | Gy GCr Gcb | * | Cr - 128 | > | B | | By BCr BCb | | Cb - 128 | > > Where Ry, Rcr, Rcb Gy, Gcr, Gcb, By, Bcr and Bcb are the programmable > coefficients. But in future offsets like Y-16, Cr-128 or Cb-128 or > constant like 1/K may also be programmable. So I have added this new > ioctl. > > Regards, > Hardik Shah > > > Regards, > > Nate > > > > -- > > ======================================================== > > Dong Soo, Kim > > Engineer > > Mobile S/W Platform Lab. S/W centre > > Telecommunication R&D Centre > > Samsung Electronics CO., LTD. > > e-mail : dongsoo.kim@gmail.com > > dongsoo45.kim@samsung.com > > ======================================================== > > -- > 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 Thu, 29 Jan 2009, Hans Verkuil wrote: > On Thursday 29 January 2009 08:44:20 DongSoo Kim wrote: > > Hello. > > > > > +#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) > > > > Do you mind if I ask a question about those ioctls? > > Because as far as I understand, we can use VIDIOC_S_FMT ioctl to convert > > colorspaces. Setting through colorspace member in v4l2_pix_format, we > > could change output colorspace. > > Colorspace is read-only, you cannot set it. It just gives you the colorspace > that the hardware uses by default. Is there a reason it must be read-only? > If you want to fully control the colorspace, then you need these ioctls. I don't know about "fully". I don't see anything about gamma correction. Since there is no documentation, it's also not clear if it can describe the full range luma the bt848 and cx88 can produce. -- 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 Thursday 29 January 2009 22:59:13 Trent Piepho wrote: > On Thu, 29 Jan 2009, Hans Verkuil wrote: > > On Thursday 29 January 2009 08:44:20 DongSoo Kim wrote: > > > Hello. > > > > > > > +#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) > > > > > > Do you mind if I ask a question about those ioctls? > > > Because as far as I understand, we can use VIDIOC_S_FMT ioctl to > > > convert colorspaces. Setting through colorspace member in > > > v4l2_pix_format, we could change output colorspace. > > > > Colorspace is read-only, you cannot set it. It just gives you the > > colorspace that the hardware uses by default. > > Is there a reason it must be read-only? The V4L2 spec. And I've no idea if we can safely change it to a read/write property or if there are apps out their that do not set this field. > > If you want to fully control the colorspace, then you need these > > ioctls. > > I don't know about "fully". I don't see anything about gamma correction. > Since there is no documentation, it's also not clear if it can describe > the full range luma the bt848 and cx88 can produce. I'm also waiting for full documentation on this. I do know that this type of matrix is common in other devices as well. I'm not sure if gamma correction can be done with CSC, but I think that should be done through a separate control anyway. However, I'm no expert. Regards, Hans
diff --git a/linux/drivers/media/video/v4l2-common.c b/linux/drivers/media/video/v4l2-common.c index 6cc7d40..b645821 100644 --- a/linux/drivers/media/video/v4l2-common.c +++ b/linux/drivers/media/video/v4l2-common.c @@ -78,7 +78,6 @@ MODULE_LICENSE("GPL"); * Video Standard Operations (contributed by Michael Schimek) */ - /* ----------------------------------------------------------------- */ /* priority handling */ @@ -413,6 +412,8 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_BACKLIGHT_COMPENSATION: return "Backlight Compensation"; case V4L2_CID_CHROMA_AGC: return "Chroma AGC"; case V4L2_CID_COLOR_KILLER: return "Color Killer"; + case V4L2_CID_ROTATION: return "Rotation"; + case V4L2_CID_BG_COLOR: return "Background color"; /* MPEG controls */ case V4L2_CID_MPEG_CLASS: return "MPEG Encoder Controls"; @@ -528,6 +529,10 @@ int v4l2_ctrl_query_fill(struct v4l2_queryctrl *qctrl, s32 min, s32 max, s32 ste qctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY; min = max = step = def = 0; break; + case V4L2_CID_BG_COLOR: + qctrl->type = V4L2_CTRL_TYPE_INTEGER; + step = 1; + break; default: qctrl->type = V4L2_CTRL_TYPE_INTEGER; break; @@ -548,6 +553,7 @@ int v4l2_ctrl_query_fill(struct v4l2_queryctrl *qctrl, s32 min, s32 max, s32 ste case V4L2_CID_CONTRAST: case V4L2_CID_SATURATION: case V4L2_CID_HUE: + case V4L2_CID_BG_COLOR: qctrl->flags |= V4L2_CTRL_FLAG_SLIDER; break; } @@ -586,6 +592,13 @@ int v4l2_ctrl_query_fill_std(struct v4l2_queryctrl *qctrl) return v4l2_ctrl_query_fill(qctrl, 0, 127, 1, 64); case V4L2_CID_HUE: return v4l2_ctrl_query_fill(qctrl, -128, 127, 1, 0); + case V4L2_CID_BG_COLOR: + /* Max value is 2^24 as RGB888 is used for background color */ + return v4l2_ctrl_query_fill(qctrl, 0, 16777216, 1, 0); + case V4L2_CID_ROTATION: + /* Standard rotation values supported will be 0, 90, 180, + 270 degree */ + return v4l2_ctrl_query_fill(qctrl, 0, 270, 90, 0); /* MPEG controls */ case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ: @@ -927,7 +940,6 @@ static struct i2c_client *v4l2_i2c_legacy_find_client(struct i2c_adapter *adap, } #endif - /* Load an i2c sub-device. It assumes that i2c_get_adapdata(adapter) returns the v4l2_device and that i2c_get_clientdata(client) returns the v4l2_subdev. */ 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 e5be28a..71e0f72 100644 --- a/linux/include/linux/videodev2.h +++ b/linux/include/linux/videodev2.h @@ -880,8 +880,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) @@ -1193,6 +1195,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 { @@ -1494,9 +1507,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. 4. Updated the v4l2-common.c file according to comments. 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: Vaibhav Hiremath <hvaibhav@ti.com> --- linux/drivers/media/video/v4l2-common.c | 16 ++++++++++++++-- linux/drivers/media/video/v4l2-ioctl.c | 19 ++++++++++++++++++- linux/include/linux/videodev2.h | 19 ++++++++++++++++++- linux/include/media/v4l2-ioctl.h | 4 ++++ 4 files changed, 54 insertions(+), 4 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