[PATCH/RFC] Add V4L2_CID_COLOR_ALPHA control for global color alpha

[PATCH/RFC] Add V4L2_CID_COLOR_ALPHA control for global color alpha

[Sylwester Nawrocki]

Hello,

This changeset adds new V4L2_CID_COLOR_ALPHA control that allows to configure 
per image plane color alpha value on the capture queue buffers.

There was a short discussion in the past about the global alpha control
support started by Jonghun Han:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg27128.html

The second patch adds the control to s5p-fimc video capture and mem-to-mem
driver.


Sylwester Nawrocki (2):
  v4l: Add a global color alpha control
  s5p-fimc: Add support for global color alpha configuration

 Documentation/DocBook/media/v4l/controls.xml       |   20 +++++--
 .../DocBook/media/v4l/pixfmt-packed-rgb.xml        |    7 ++-
 drivers/media/video/s5p-fimc/fimc-capture.c        |    4 ++
 drivers/media/video/s5p-fimc/fimc-core.c           |   49 ++++++++++++++++--
 drivers/media/video/s5p-fimc/fimc-core.h           |   13 ++++-
 drivers/media/video/s5p-fimc/fimc-reg.c            |   53 +++++++++++++++-----
 drivers/media/video/s5p-fimc/regs-fimc.h           |    5 ++
 drivers/media/video/v4l2-ctrls.c                   |    7 +++
 include/linux/videodev2.h                          |    6 +-
 9 files changed, 134 insertions(+), 30 deletions(-)


-- 
Regards,
 
Sylwester Nawrocki 
Samsung Poland R&D Center

[PATCH/RFC 1/2] v4l: Add a global color alpha control

[Sylwester Nawrocki]

This control is intended for video capture or memory-to-memory
devices that are capable of setting up the alpha conponent to
some arbitrary value.
The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
globally to a value in range from 0 to 255.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/DocBook/media/v4l/controls.xml       |   20 ++++++++++++++------
 .../DocBook/media/v4l/pixfmt-packed-rgb.xml        |    7 +++++--
 drivers/media/video/v4l2-ctrls.c                   |    7 +++++++
 include/linux/videodev2.h                          |    6 +++---
 4 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index 3bc5ee8..7f99222 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -324,12 +324,6 @@ minimum value disables backlight compensation.</entry>
 		(usually a microscope).</entry>
 	  </row>
 	  <row>
-	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
-	    <entry></entry>
-	    <entry>End of the predefined control IDs (currently
-<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
-	  </row>
-	  <row>
 	    <entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant></entry>
 	    <entry>integer</entry>
 	    <entry>This is a read-only control that can be read by the application
@@ -345,6 +339,20 @@ and used as a hint to determine the number of OUTPUT buffers to pass to REQBUFS.
 The value is the minimum number of OUTPUT buffers that is necessary for hardware
 to work.</entry>
 	  </row>
+	  <row id="v4l2-color-alpha">
+	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
+	    <entry>integer</entry>
+	    <entry> Sets the color alpha component on the capture device. It is
+	    applicable to any pixel formats that contain the alpha component,
+	    e.g. <link linkend="rgb-formats">packed RGB image formats</link>.
+	    </entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
+	    <entry></entry>
+	    <entry>End of the predefined control IDs (currently
+	      <constant>V4L2_CID_COLOR_ALPHA</constant> + 1).</entry>
+	  </row>
 	  <row>
 	    <entry><constant>V4L2_CID_PRIVATE_BASE</constant></entry>
 	    <entry></entry>
diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
index 4db272b..da4c360 100644
--- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
+++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
@@ -428,8 +428,11 @@ colorspace <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
     <para>Bit 7 is the most significant bit. The value of a = alpha
 bits is undefined when reading from the driver, ignored when writing
 to the driver, except when alpha blending has been negotiated for a
-<link linkend="overlay">Video Overlay</link> or <link
-linkend="osd">Video Output Overlay</link>.</para>
+<link linkend="overlay">Video Overlay</link> or <link linkend="osd">
+Video Output Overlay</link> or when global alpha has been configured
+for a <link linkend="capture">Video Capture</link> by means of
+<link linkend="v4l2-color-alpha"> <constant>V4L2_CID_COLOR_ALPHA
+</constant> </link> control.</para>
 
     <example>
       <title><constant>V4L2_PIX_FMT_BGR24</constant> 4 &times; 4 pixel
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 5552f81..bd90955 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -466,6 +466,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_ILLUMINATORS_2:		return "Illuminator 2";
 	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:	return "Minimum Number of Capture Buffers";
 	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return "Minimum Number of Output Buffers";
+	case V4L2_CID_COLOR_ALPHA:		return "Color Alpha";
 
 	/* MPEG controls */
 	/* Keep the order of the 'case's the same as in videodev2.h! */
@@ -714,6 +715,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		/* Max is calculated as RGB888 that is 2^24 */
 		*max = 0xFFFFFF;
 		break;
+	case V4L2_CID_COLOR_ALPHA:
+		*type = V4L2_CTRL_TYPE_INTEGER;
+		*step = 1;
+		*min = 0;
+		*max = 0xff;
+		break;
 	case V4L2_CID_FLASH_FAULT:
 		*type = V4L2_CTRL_TYPE_BITMASK;
 		break;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 4b752d5..42192c1 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1204,10 +1204,10 @@ enum v4l2_colorfx {
 #define V4L2_CID_MIN_BUFFERS_FOR_CAPTURE	(V4L2_CID_BASE+39)
 #define V4L2_CID_MIN_BUFFERS_FOR_OUTPUT		(V4L2_CID_BASE+40)
 
-/* last CID + 1 */
-#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+41)
+#define V4L2_CID_COLOR_ALPHA			(V4L2_CID_BASE+41)
 
-/* Minimum number of buffer neede by the device */
+/* last CID + 1 */
+#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+42)
 
 /*  MPEG-class control IDs defined by V4L2 */
 #define V4L2_CID_MPEG_BASE 			(V4L2_CTRL_CLASS_MPEG | 0x900)
-- 
1.7.7.2

[PATCH 2/2] s5p-fimc: Add support for global color alpha configuration

[Sylwester Nawrocki]

On Exynos SoCs the FIMC IP allows to configure globally the alpha
color channel for V4L2_PIX_FMT_RGB32, V4L2_PIX_FMT_RGB555 and
V4L2_PIX_FMT_RGB444 pixel formats. This patch adds a v4l2 control
in order to let the applications control the global alpha value.

The alpha value range depends on the pixel format, for RGB32
it's 0..255, for RGB555 - 0..1 and for RGB444 - 0..7. The v4l2
control range is always 0..255 and the driver will ignore most
significant bits of the alpha value where the alpha channel width
is less than 8 bits. The applications need to match the alpha
channel data width and the pixel format.

An option is added to the variant description data structure
so an additional control is created only where really supported
in hardware.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/s5p-fimc/fimc-capture.c |    4 ++
 drivers/media/video/s5p-fimc/fimc-core.c    |   49 ++++++++++++++++++++++---
 drivers/media/video/s5p-fimc/fimc-core.h    |   13 ++++++-
 drivers/media/video/s5p-fimc/fimc-reg.c     |   53 +++++++++++++++++++++------
 drivers/media/video/s5p-fimc/regs-fimc.h    |    5 +++
 5 files changed, 105 insertions(+), 19 deletions(-)

diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c
index 82d9ab6..70176e5 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -63,6 +63,8 @@ static int fimc_init_capture(struct fimc_dev *fimc)
 		fimc_hw_set_effect(ctx, false);
 		fimc_hw_set_output_path(ctx);
 		fimc_hw_set_out_dma(ctx);
+		if (fimc->variant->has_alpha)
+			fimc_hw_set_rgb_alpha(ctx);
 		clear_bit(ST_CAPT_APPLY_CFG, &fimc->state);
 	}
 	spin_unlock_irqrestore(&fimc->slock, flags);
@@ -154,6 +156,8 @@ int fimc_capture_config_update(struct fimc_ctx *ctx)
 		fimc_hw_set_rotation(ctx);
 		fimc_prepare_dma_offset(ctx, &ctx->d_frame);
 		fimc_hw_set_out_dma(ctx);
+		if (fimc->variant->has_alpha)
+			fimc_hw_set_rgb_alpha(ctx);
 		clear_bit(ST_CAPT_APPLY_CFG, &fimc->state);
 	}
 	spin_unlock(&ctx->slock);
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c b/drivers/media/video/s5p-fimc/fimc-core.c
index 567e9ea..20f9da8 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -52,13 +52,29 @@ static struct fimc_fmt fimc_formats[] = {
 		.colplanes	= 1,
 		.flags		= FMT_FLAGS_M2M,
 	}, {
-		.name		= "XRGB-8-8-8-8, 32 bpp",
+		.name		= "XRGB8888, 32 bpp",
 		.fourcc		= V4L2_PIX_FMT_RGB32,
 		.depth		= { 32 },
 		.color		= S5P_FIMC_RGB888,
 		.memplanes	= 1,
 		.colplanes	= 1,
-		.flags		= FMT_FLAGS_M2M,
+		.flags		= FMT_FLAGS_M2M | FMT_HAS_ALPHA,
+	}, {
+		.name		= "ARGB1555",
+		.fourcc		= V4L2_PIX_FMT_RGB555,
+		.depth		= { 16 },
+		.color		= S5P_FIMC_RGB555,
+		.memplanes	= 1,
+		.colplanes	= 1,
+		.flags		= FMT_FLAGS_M2M | FMT_HAS_ALPHA,
+	}, {
+		.name		= "ARGB4444",
+		.fourcc		= V4L2_PIX_FMT_RGB444,
+		.depth		= { 16 },
+		.color		= S5P_FIMC_RGB444,
+		.memplanes	= 1,
+		.colplanes	= 1,
+		.flags		= FMT_FLAGS_M2M | FMT_HAS_ALPHA,
 	}, {
 		.name		= "YUV 4:2:2 packed, YCbYCr",
 		.fourcc		= V4L2_PIX_FMT_YUYV,
@@ -652,8 +668,11 @@ static void fimc_dma_run(void *priv)
 	if (ctx->state & (FIMC_DST_ADDR | FIMC_PARAMS))
 		fimc_hw_set_output_addr(fimc, &ctx->d_frame.paddr, -1);
 
-	if (ctx->state & FIMC_PARAMS)
+	if (ctx->state & FIMC_PARAMS) {
 		fimc_hw_set_out_dma(ctx);
+		if (fimc->variant->has_alpha)
+			fimc_hw_set_rgb_alpha(ctx);
+	}
 
 	fimc_activate_capture(ctx);
 
@@ -790,6 +809,11 @@ static int fimc_s_ctrl(struct v4l2_ctrl *ctrl)
 		ctx->rotation = ctrl->val;
 		break;
 
+	case V4L2_CID_COLOR_ALPHA:
+		spin_lock_irqsave(&ctx->slock, flags);
+		ctx->d_frame.alpha = ctrl->val;
+		break;
+
 	default:
 		v4l2_err(fimc->v4l2_dev, "Invalid control: 0x%X\n", ctrl->id);
 		return -EINVAL;
@@ -806,9 +830,11 @@ static const struct v4l2_ctrl_ops fimc_ctrl_ops = {
 
 int fimc_ctrls_create(struct fimc_ctx *ctx)
 {
+	struct samsung_fimc_variant *variant = ctx->fimc_dev->variant;
+
 	if (ctx->ctrls_rdy)
 		return 0;
-	v4l2_ctrl_handler_init(&ctx->ctrl_handler, 3);
+	v4l2_ctrl_handler_init(&ctx->ctrl_handler, 4);
 
 	ctx->ctrl_rotate = v4l2_ctrl_new_std(&ctx->ctrl_handler, &fimc_ctrl_ops,
 				     V4L2_CID_HFLIP, 0, 1, 1, 0);
@@ -816,6 +842,14 @@ int fimc_ctrls_create(struct fimc_ctx *ctx)
 				    V4L2_CID_VFLIP, 0, 1, 1, 0);
 	ctx->ctrl_vflip = v4l2_ctrl_new_std(&ctx->ctrl_handler, &fimc_ctrl_ops,
 				    V4L2_CID_ROTATE, 0, 270, 90, 0);
+
+	if (variant->has_alpha)
+		ctx->ctrl_alpha = v4l2_ctrl_new_std(&ctx->ctrl_handler,
+				    &fimc_ctrl_ops, V4L2_CID_COLOR_ALPHA,
+				    0, 0xff, 1, 0);
+	else
+		ctx->ctrl_alpha = NULL;
+
 	ctx->ctrls_rdy = ctx->ctrl_handler.error == 0;
 
 	return ctx->ctrl_handler.error;
@@ -838,6 +872,8 @@ void fimc_ctrls_activate(struct fimc_ctx *ctx, bool active)
 	v4l2_ctrl_activate(ctx->ctrl_rotate, active);
 	v4l2_ctrl_activate(ctx->ctrl_hflip, active);
 	v4l2_ctrl_activate(ctx->ctrl_vflip, active);
+	if (ctx->ctrl_alpha)
+		v4l2_ctrl_activate(ctx->ctrl_alpha, active);
 
 	if (active) {
 		ctx->rotation = ctx->ctrl_rotate->val;
@@ -1374,6 +1410,8 @@ static int fimc_m2m_open(struct file *file)
 	if (!ctx)
 		return -ENOMEM;
 	v4l2_fh_init(&ctx->fh, fimc->m2m.vfd);
+	ctx->fimc_dev = fimc;
+
 	ret = fimc_ctrls_create(ctx);
 	if (ret)
 		goto error_fh;
@@ -1383,7 +1421,6 @@ static int fimc_m2m_open(struct file *file)
 	file->private_data = &ctx->fh;
 	v4l2_fh_add(&ctx->fh);
 
-	ctx->fimc_dev = fimc;
 	/* Default color format */
 	ctx->s_frame.fmt = &fimc_formats[0];
 	ctx->d_frame.fmt = &fimc_formats[0];
@@ -1892,6 +1929,7 @@ static struct samsung_fimc_variant fimc0_variant_exynos4 = {
 	.has_cam_if	 = 1,
 	.has_cistatus2	 = 1,
 	.has_mainscaler_ext = 1,
+	.has_alpha	 = 1,
 	.min_inp_pixsize = 16,
 	.min_out_pixsize = 16,
 	.hor_offs_align	 = 2,
@@ -1905,6 +1943,7 @@ static struct samsung_fimc_variant fimc3_variant_exynos4 = {
 	.has_cam_if	 = 1,
 	.has_cistatus2	 = 1,
 	.has_mainscaler_ext = 1,
+	.has_alpha	 = 1,
 	.min_inp_pixsize = 16,
 	.min_out_pixsize = 16,
 	.hor_offs_align	 = 2,
diff --git a/drivers/media/video/s5p-fimc/fimc-core.h b/drivers/media/video/s5p-fimc/fimc-core.h
index c7f01c4..9d1f669 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.h
+++ b/drivers/media/video/s5p-fimc/fimc-core.h
@@ -85,11 +85,14 @@ enum fimc_datapath {
 };
 
 enum fimc_color_fmt {
-	S5P_FIMC_RGB565 = 0x10,
+	S5P_FIMC_RGB444 = 0x10,
+	S5P_FIMC_RGB555,
+	S5P_FIMC_RGB565,
 	S5P_FIMC_RGB666,
 	S5P_FIMC_RGB888,
 	S5P_FIMC_RGB30_LOCAL,
 	S5P_FIMC_YCBCR420 = 0x20,
+	S5P_FIMC_YCBCR422,
 	S5P_FIMC_YCBYCR422,
 	S5P_FIMC_YCRYCB422,
 	S5P_FIMC_CBYCRY422,
@@ -162,6 +165,7 @@ struct fimc_fmt {
 	u16	flags;
 #define FMT_FLAGS_CAM	(1 << 0)
 #define FMT_FLAGS_M2M	(1 << 1)
+#define FMT_HAS_ALPHA	(1 << 2)
 };
 
 /**
@@ -283,6 +287,7 @@ struct fimc_frame {
 	struct fimc_addr	paddr;
 	struct fimc_dma_offset	dma_offset;
 	struct fimc_fmt		*fmt;
+	u8			alpha;
 };
 
 /**
@@ -387,6 +392,7 @@ struct samsung_fimc_variant {
 	unsigned int	has_cistatus2:1;
 	unsigned int	has_mainscaler_ext:1;
 	unsigned int	has_cam_if:1;
+	unsigned int	has_alpha:1;
 	struct fimc_pix_limit *pix_limit;
 	u16		min_inp_pixsize;
 	u16		min_out_pixsize;
@@ -482,7 +488,8 @@ struct fimc_dev {
  * @ctrl_handler:	v4l2 controls handler
  * @ctrl_rotate		image rotation control
  * @ctrl_hflip		horizontal flip control
- * @ctrl_vflip		vartical flip control
+ * @ctrl_vflip		vertical flip control
+ * @ctrl_alpha		RGB alpha control
  * @ctrls_rdy:		true if the control handler is initialized
  */
 struct fimc_ctx {
@@ -509,6 +516,7 @@ struct fimc_ctx {
 	struct v4l2_ctrl	*ctrl_rotate;
 	struct v4l2_ctrl	*ctrl_hflip;
 	struct v4l2_ctrl	*ctrl_vflip;
+	struct v4l2_ctrl	*ctrl_alpha;
 	bool			ctrls_rdy;
 };
 
@@ -674,6 +682,7 @@ void fimc_hw_set_prescaler(struct fimc_ctx *ctx);
 void fimc_hw_set_mainscaler(struct fimc_ctx *ctx);
 void fimc_hw_en_capture(struct fimc_ctx *ctx);
 void fimc_hw_set_effect(struct fimc_ctx *ctx, bool active);
+void fimc_hw_set_rgb_alpha(struct fimc_ctx *ctx);
 void fimc_hw_set_in_dma(struct fimc_ctx *ctx);
 void fimc_hw_set_input_path(struct fimc_ctx *ctx);
 void fimc_hw_set_output_path(struct fimc_ctx *ctx);
diff --git a/drivers/media/video/s5p-fimc/fimc-reg.c b/drivers/media/video/s5p-fimc/fimc-reg.c
index 44f5c2d..15466d0 100644
--- a/drivers/media/video/s5p-fimc/fimc-reg.c
+++ b/drivers/media/video/s5p-fimc/fimc-reg.c
@@ -117,7 +117,7 @@ void fimc_hw_set_target_format(struct fimc_ctx *ctx)
 		  S5P_CITRGFMT_VSIZE_MASK);
 
 	switch (frame->fmt->color) {
-	case S5P_FIMC_RGB565...S5P_FIMC_RGB888:
+	case S5P_FIMC_RGB444...S5P_FIMC_RGB888:
 		cfg |= S5P_CITRGFMT_RGB;
 		break;
 	case S5P_FIMC_YCBCR420:
@@ -175,6 +175,7 @@ void fimc_hw_set_out_dma(struct fimc_ctx *ctx)
 	struct fimc_dev *dev = ctx->fimc_dev;
 	struct fimc_frame *frame = &ctx->d_frame;
 	struct fimc_dma_offset *offset = &frame->dma_offset;
+	struct fimc_fmt *fmt = frame->fmt;
 
 	/* Set the input dma offsets. */
 	cfg = 0;
@@ -198,15 +199,22 @@ void fimc_hw_set_out_dma(struct fimc_ctx *ctx)
 	cfg = readl(dev->regs + S5P_CIOCTRL);
 
 	cfg &= ~(S5P_CIOCTRL_ORDER2P_MASK | S5P_CIOCTRL_ORDER422_MASK |
-		 S5P_CIOCTRL_YCBCR_PLANE_MASK);
+		 S5P_CIOCTRL_YCBCR_PLANE_MASK | S5P_CIOCTRL_RGB16FMT_MASK);
 
-	if (frame->fmt->colplanes == 1)
+	if (fmt->colplanes == 1)
 		cfg |= ctx->out_order_1p;
-	else if (frame->fmt->colplanes == 2)
+	else if (fmt->colplanes == 2)
 		cfg |= ctx->out_order_2p | S5P_CIOCTRL_YCBCR_2PLANE;
-	else if (frame->fmt->colplanes == 3)
+	else if (fmt->colplanes == 3)
 		cfg |= S5P_CIOCTRL_YCBCR_3PLANE;
 
+	if (fmt->color == S5P_FIMC_RGB565)
+		cfg |= S5P_CIOCTRL_RGB565;
+	else if (fmt->color == S5P_FIMC_RGB555)
+		cfg |= S5P_CIOCTRL_ARGB1555;
+	else if (fmt->color == S5P_FIMC_RGB444)
+		cfg |= S5P_CIOCTRL_ARGB4444;
+
 	writel(cfg, dev->regs + S5P_CIOCTRL);
 }
 
@@ -278,22 +286,28 @@ static void fimc_hw_set_scaler(struct fimc_ctx *ctx)
 	if (sc->copy_mode)
 		cfg |= S5P_CISCCTRL_ONE2ONE;
 
-
 	if (ctx->in_path == FIMC_DMA) {
-		if (src_frame->fmt->color == S5P_FIMC_RGB565)
+		switch (src_frame->fmt->color) {
+		case S5P_FIMC_RGB565:
 			cfg |= S5P_CISCCTRL_INRGB_FMT_RGB565;
-		else if (src_frame->fmt->color == S5P_FIMC_RGB666)
+			break;
+		case S5P_FIMC_RGB666:
 			cfg |= S5P_CISCCTRL_INRGB_FMT_RGB666;
-		else if (src_frame->fmt->color == S5P_FIMC_RGB888)
+			break;
+		case S5P_FIMC_RGB888:
 			cfg |= S5P_CISCCTRL_INRGB_FMT_RGB888;
+			break;
+		}
 	}
 
 	if (ctx->out_path == FIMC_DMA) {
-		if (dst_frame->fmt->color == S5P_FIMC_RGB565)
+		u32 color = dst_frame->fmt->color;
+
+		if (color >= S5P_FIMC_RGB444 && color <= S5P_FIMC_RGB565)
 			cfg |= S5P_CISCCTRL_OUTRGB_FMT_RGB565;
-		else if (dst_frame->fmt->color == S5P_FIMC_RGB666)
+		else if (color == S5P_FIMC_RGB666)
 			cfg |= S5P_CISCCTRL_OUTRGB_FMT_RGB666;
-		else if (dst_frame->fmt->color == S5P_FIMC_RGB888)
+		else if (color == S5P_FIMC_RGB888)
 			cfg |= S5P_CISCCTRL_OUTRGB_FMT_RGB888;
 	} else {
 		cfg |= S5P_CISCCTRL_OUTRGB_FMT_RGB888;
@@ -379,6 +393,21 @@ void fimc_hw_set_effect(struct fimc_ctx *ctx, bool active)
 	writel(cfg, dev->regs + S5P_CIIMGEFF);
 }
 
+void fimc_hw_set_rgb_alpha(struct fimc_ctx *ctx)
+{
+	struct fimc_dev *dev = ctx->fimc_dev;
+	struct fimc_frame *frame = &ctx->d_frame;
+	u32 cfg;
+
+	if (!(frame->fmt->flags & FMT_HAS_ALPHA))
+		return;
+
+	cfg = readl(dev->regs + S5P_CIOCTRL);
+	cfg &= ~S5P_CIOCTRL_ALPHA_OUT_MASK;
+	cfg |= (frame->alpha << 4);
+	writel(cfg, dev->regs + S5P_CIOCTRL);
+}
+
 static void fimc_hw_set_in_dma_size(struct fimc_ctx *ctx)
 {
 	struct fimc_dev *dev = ctx->fimc_dev;
diff --git a/drivers/media/video/s5p-fimc/regs-fimc.h b/drivers/media/video/s5p-fimc/regs-fimc.h
index c8e3b94..c7a5bc5 100644
--- a/drivers/media/video/s5p-fimc/regs-fimc.h
+++ b/drivers/media/video/s5p-fimc/regs-fimc.h
@@ -107,6 +107,11 @@
 #define S5P_CIOCTRL_YCBCR_3PLANE	(0 << 3)
 #define S5P_CIOCTRL_YCBCR_2PLANE	(1 << 3)
 #define S5P_CIOCTRL_YCBCR_PLANE_MASK	(1 << 3)
+#define S5P_CIOCTRL_ALPHA_OUT_MASK	(0xff << 4)
+#define S5P_CIOCTRL_RGB16FMT_MASK	(3 << 16)
+#define S5P_CIOCTRL_RGB565		(0 << 16)
+#define S5P_CIOCTRL_ARGB1555		(1 << 16)
+#define S5P_CIOCTRL_ARGB4444		(2 << 16)
 #define S5P_CIOCTRL_ORDER2P_SHIFT	(24)
 #define S5P_CIOCTRL_ORDER2P_MASK	(3 << 24)
 #define S5P_CIOCTRL_ORDER422_2P_LSB_CRCB (0 << 24)
-- 
1.7.7.2

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Hans Verkuil]

On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
> This control is intended for video capture or memory-to-memory
> devices that are capable of setting up the alpha conponent to
> some arbitrary value.
> The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
> globally to a value in range from 0 to 255.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  Documentation/DocBook/media/v4l/controls.xml       |   20 ++++++++++++++------
>  .../DocBook/media/v4l/pixfmt-packed-rgb.xml        |    7 +++++--
>  drivers/media/video/v4l2-ctrls.c                   |    7 +++++++
>  include/linux/videodev2.h                          |    6 +++---
>  4 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> index 3bc5ee8..7f99222 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -324,12 +324,6 @@ minimum value disables backlight compensation.</entry>
>  		(usually a microscope).</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> -	    <entry></entry>
> -	    <entry>End of the predefined control IDs (currently
> -<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
> -	  </row>
> -	  <row>
>  	    <entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant></entry>
>  	    <entry>integer</entry>
>  	    <entry>This is a read-only control that can be read by the application
> @@ -345,6 +339,20 @@ and used as a hint to determine the number of OUTPUT buffers to pass to REQBUFS.
>  The value is the minimum number of OUTPUT buffers that is necessary for hardware
>  to work.</entry>
>  	  </row>
> +	  <row id="v4l2-color-alpha">
> +	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
> +	    <entry>integer</entry>
> +	    <entry> Sets the color alpha component on the capture device. It is
> +	    applicable to any pixel formats that contain the alpha component,
> +	    e.g. <link linkend="rgb-formats">packed RGB image formats</link>.
> +	    </entry>
> +	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> +	    <entry></entry>
> +	    <entry>End of the predefined control IDs (currently
> +	      <constant>V4L2_CID_COLOR_ALPHA</constant> + 1).</entry>
> +	  </row>
>  	  <row>
>  	    <entry><constant>V4L2_CID_PRIVATE_BASE</constant></entry>
>  	    <entry></entry>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> index 4db272b..da4c360 100644
> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> @@ -428,8 +428,11 @@ colorspace <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
>      <para>Bit 7 is the most significant bit. The value of a = alpha
>  bits is undefined when reading from the driver, ignored when writing
>  to the driver, except when alpha blending has been negotiated for a
> -<link linkend="overlay">Video Overlay</link> or <link
> -linkend="osd">Video Output Overlay</link>.</para>
> +<link linkend="overlay">Video Overlay</link> or <link linkend="osd">
> +Video Output Overlay</link> or when global alpha has been configured
> +for a <link linkend="capture">Video Capture</link> by means of
> +<link linkend="v4l2-color-alpha"> <constant>V4L2_CID_COLOR_ALPHA
> +</constant> </link> control.</para>
>  
>      <example>
>        <title><constant>V4L2_PIX_FMT_BGR24</constant> 4 &times; 4 pixel
> diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
> index 5552f81..bd90955 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -466,6 +466,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_ILLUMINATORS_2:		return "Illuminator 2";
>  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:	return "Minimum Number of Capture Buffers";
>  	case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return "Minimum Number of Output Buffers";
> +	case V4L2_CID_COLOR_ALPHA:		return "Color Alpha";

I prefer CID_ALPHA_COLOR and string "Alpha Color". I think it is more
natural than the other way around.

Other than that I'm OK with this.

Regards,

	Hans

>  
>  	/* MPEG controls */
>  	/* Keep the order of the 'case's the same as in videodev2.h! */
> @@ -714,6 +715,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  		/* Max is calculated as RGB888 that is 2^24 */
>  		*max = 0xFFFFFF;
>  		break;
> +	case V4L2_CID_COLOR_ALPHA:
> +		*type = V4L2_CTRL_TYPE_INTEGER;
> +		*step = 1;
> +		*min = 0;
> +		*max = 0xff;
> +		break;
>  	case V4L2_CID_FLASH_FAULT:
>  		*type = V4L2_CTRL_TYPE_BITMASK;
>  		break;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 4b752d5..42192c1 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1204,10 +1204,10 @@ enum v4l2_colorfx {
>  #define V4L2_CID_MIN_BUFFERS_FOR_CAPTURE	(V4L2_CID_BASE+39)
>  #define V4L2_CID_MIN_BUFFERS_FOR_OUTPUT		(V4L2_CID_BASE+40)
>  
> -/* last CID + 1 */
> -#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+41)
> +#define V4L2_CID_COLOR_ALPHA			(V4L2_CID_BASE+41)
>  
> -/* Minimum number of buffer neede by the device */
> +/* last CID + 1 */
> +#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+42)
>  
>  /*  MPEG-class control IDs defined by V4L2 */
>  #define V4L2_CID_MPEG_BASE 			(V4L2_CTRL_CLASS_MPEG | 0x900)
> 

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Laurent Pinchart]

Hi Sylwester and Hans,

On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
> > This control is intended for video capture or memory-to-memory
> > devices that are capable of setting up the alpha conponent to
> > some arbitrary value.
> > The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
> > globally to a value in range from 0 to 255.
> > 
> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > 
> >  Documentation/DocBook/media/v4l/controls.xml       |   20
> >  ++++++++++++++------ .../DocBook/media/v4l/pixfmt-packed-rgb.xml       
> >  |    7 +++++-- drivers/media/video/v4l2-ctrls.c                   |   
> >  7 +++++++ include/linux/videodev2.h                          |    6
> >  +++--- 4 files changed, 29 insertions(+), 11 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/controls.xml
> > b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..7f99222
> > 100644
> > --- a/Documentation/DocBook/media/v4l/controls.xml
> > +++ b/Documentation/DocBook/media/v4l/controls.xml
> > @@ -324,12 +324,6 @@ minimum value disables backlight
> > compensation.</entry>
> > 
> >  		(usually a microscope).</entry>
> >  		
> >  	  </row>
> >  	  <row>
> > 
> > -	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> > -	    <entry></entry>
> > -	    <entry>End of the predefined control IDs (currently
> > -<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
> > -	  </row>
> > -	  <row>
> > 
> >  	    <entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant></entry
> >  	    > <entry>integer</entry>
> >  	    <entry>This is a read-only control that can be read by the
> >  	    application
> > 
> > @@ -345,6 +339,20 @@ and used as a hint to determine the number of OUTPUT
> > buffers to pass to REQBUFS.
> > 
> >  The value is the minimum number of OUTPUT buffers that is necessary for
> >  hardware to work.</entry>
> >  
> >  	  </row>
> > 
> > +	  <row id="v4l2-color-alpha">
> > +	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
> > +	    <entry>integer</entry>
> > +	    <entry> Sets the color alpha component on the capture device. It is
> > +	    applicable to any pixel formats that contain the alpha component,
> > +	    e.g. <link linkend="rgb-formats">packed RGB image formats</link>.
> > +	    </entry>

As the alpha value is global, isn't it applicable to formats with no alpha 
component as well ?

> > +	  </row>
> > +	  <row>
> > +	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> > +	    <entry></entry>
> > +	    <entry>End of the predefined control IDs (currently
> > +	      <constant>V4L2_CID_COLOR_ALPHA</constant> + 1).</entry>
> > +	  </row>
> > 
> >  	  <row>
> >  	  
> >  	    <entry><constant>V4L2_CID_PRIVATE_BASE</constant></entry>
> >  	    <entry></entry>
> > 
> > diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
> > 4db272b..da4c360 100644
> > --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > @@ -428,8 +428,11 @@ colorspace
> > <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
> > 
> >      <para>Bit 7 is the most significant bit. The value of a = alpha
> >  
> >  bits is undefined when reading from the driver, ignored when writing
> >  to the driver, except when alpha blending has been negotiated for a
> > 
> > -<link linkend="overlay">Video Overlay</link> or <link
> > -linkend="osd">Video Output Overlay</link>.</para>
> > +<link linkend="overlay">Video Overlay</link> or <link linkend="osd">
> > +Video Output Overlay</link> or when global alpha has been configured
> > +for a <link linkend="capture">Video Capture</link> by means of
> > +<link linkend="v4l2-color-alpha"> <constant>V4L2_CID_COLOR_ALPHA
> > +</constant> </link> control.</para>
> > 
> >      <example>
> >      
> >        <title><constant>V4L2_PIX_FMT_BGR24</constant> 4 &times; 4 pixel
> > 
> > diff --git a/drivers/media/video/v4l2-ctrls.c
> > b/drivers/media/video/v4l2-ctrls.c index 5552f81..bd90955 100644
> > --- a/drivers/media/video/v4l2-ctrls.c
> > +++ b/drivers/media/video/v4l2-ctrls.c
> > @@ -466,6 +466,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > 
> >  	case V4L2_CID_ILLUMINATORS_2:		return "Illuminator 2";
> >  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:	return "Minimum Number of
> >  	Capture Buffers"; case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return
> >  	"Minimum Number of Output Buffers";
> > 
> > +	case V4L2_CID_COLOR_ALPHA:		return "Color Alpha";
> 
> I prefer CID_ALPHA_COLOR and string "Alpha Color". I think it is more
> natural than the other way around.

I'm not too found of "color" in the name. Is the alpha value considered as a 
color ?

> Other than that I'm OK with this.
> 
> Regards,
> 
> 	Hans
> 
> >  	/* MPEG controls */
> >  	/* Keep the order of the 'case's the same as in videodev2.h! */
> > 
> > @@ -714,6 +715,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> > v4l2_ctrl_type *type,
> > 
> >  		/* Max is calculated as RGB888 that is 2^24 */
> >  		*max = 0xFFFFFF;
> >  		break;
> > 
> > +	case V4L2_CID_COLOR_ALPHA:
> > +		*type = V4L2_CTRL_TYPE_INTEGER;
> > +		*step = 1;
> > +		*min = 0;
> > +		*max = 0xff;
> > +		break;
> > 
> >  	case V4L2_CID_FLASH_FAULT:
> >  		*type = V4L2_CTRL_TYPE_BITMASK;
> >  		break;
> > 
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index 4b752d5..42192c1 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -1204,10 +1204,10 @@ enum v4l2_colorfx {
> > 
> >  #define V4L2_CID_MIN_BUFFERS_FOR_CAPTURE	(V4L2_CID_BASE+39)
> >  #define V4L2_CID_MIN_BUFFERS_FOR_OUTPUT		(V4L2_CID_BASE+40)
> > 
> > -/* last CID + 1 */
> > -#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+41)
> > +#define V4L2_CID_COLOR_ALPHA			(V4L2_CID_BASE+41)
> > 
> > -/* Minimum number of buffer neede by the device */
> > +/* last CID + 1 */
> > +#define V4L2_CID_LASTP1                         (V4L2_CID_BASE+42)
> > 
> >  /*  MPEG-class control IDs defined by V4L2 */
> >  #define V4L2_CID_MPEG_BASE 			(V4L2_CTRL_CLASS_MPEG | 0x900)

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Sylwester Nawrocki]

Hi Laurent,

On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
> Hi Sylwester and Hans,
> 
> On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
>> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
>>> This control is intended for video capture or memory-to-memory
>>> devices that are capable of setting up the alpha conponent to
>>> some arbitrary value.
>>> The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
>>> globally to a value in range from 0 to 255.
>>>
>>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>>
>>>  Documentation/DocBook/media/v4l/controls.xml       |   20
>>>  ++++++++++++++------ .../DocBook/media/v4l/pixfmt-packed-rgb.xml       
>>>  |    7 +++++-- drivers/media/video/v4l2-ctrls.c                   |   
>>>  7 +++++++ include/linux/videodev2.h                          |    6
>>>  +++--- 4 files changed, 29 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml
>>> b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..7f99222
>>> 100644
>>> --- a/Documentation/DocBook/media/v4l/controls.xml
>>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>>> @@ -324,12 +324,6 @@ minimum value disables backlight
>>> compensation.</entry>
>>>
>>>  		(usually a microscope).</entry>
>>>  		
>>>  	  </row>
>>>  	  <row>
>>>
>>> -	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
>>> -	    <entry></entry>
>>> -	    <entry>End of the predefined control IDs (currently
>>> -<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
>>> -	  </row>
>>> -	  <row>
>>>
>>>  	    <entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant></entry
>>>  	    > <entry>integer</entry>
>>>  	    <entry>This is a read-only control that can be read by the
>>>  	    application
>>>
>>> @@ -345,6 +339,20 @@ and used as a hint to determine the number of OUTPUT
>>> buffers to pass to REQBUFS.
>>>
>>>  The value is the minimum number of OUTPUT buffers that is necessary for
>>>  hardware to work.</entry>
>>>  
>>>  	  </row>
>>>
>>> +	  <row id="v4l2-color-alpha">
>>> +	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
>>> +	    <entry>integer</entry>
>>> +	    <entry> Sets the color alpha component on the capture device. It is
>>> +	    applicable to any pixel formats that contain the alpha component,
>>> +	    e.g. <link linkend="rgb-formats">packed RGB image formats</link>.
>>> +	    </entry>
> 
> As the alpha value is global, isn't it applicable to formats with no alpha 
> component as well ?

Hmm, I can't say no.. The control was intended as a means of setting up
the alpha value for packed RGB formats:
http://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html#rgb-formats

However it could well be used for formats with no alpha. Do you think
the second sentence above should be removed or should something else be
added to indicate it doesn't necessarily have to have a connection
with ARGB color formats ?

> 
>>> +	  </row>
>>> +	  <row>
>>> +	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
>>> +	    <entry></entry>
>>> +	    <entry>End of the predefined control IDs (currently
>>> +	      <constant>V4L2_CID_COLOR_ALPHA</constant> + 1).</entry>
>>> +	  </row>
>>>
>>>  	  <row>
>>>  	  
>>>  	    <entry><constant>V4L2_CID_PRIVATE_BASE</constant></entry>
>>>  	    <entry></entry>
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
>>> 4db272b..da4c360 100644
>>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>> @@ -428,8 +428,11 @@ colorspace
>>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
>>>
>>>      <para>Bit 7 is the most significant bit. The value of a = alpha
>>>  
>>>  bits is undefined when reading from the driver, ignored when writing
>>>  to the driver, except when alpha blending has been negotiated for a
>>>
>>> -<link linkend="overlay">Video Overlay</link> or <link
>>> -linkend="osd">Video Output Overlay</link>.</para>
>>> +<link linkend="overlay">Video Overlay</link> or <link linkend="osd">
>>> +Video Output Overlay</link> or when global alpha has been configured
>>> +for a <link linkend="capture">Video Capture</link> by means of
>>> +<link linkend="v4l2-color-alpha"> <constant>V4L2_CID_COLOR_ALPHA
>>> +</constant> </link> control.</para>
>>>
>>>      <example>
>>>      
>>>        <title><constant>V4L2_PIX_FMT_BGR24</constant> 4 &times; 4 pixel
>>>
>>> diff --git a/drivers/media/video/v4l2-ctrls.c
>>> b/drivers/media/video/v4l2-ctrls.c index 5552f81..bd90955 100644
>>> --- a/drivers/media/video/v4l2-ctrls.c
>>> +++ b/drivers/media/video/v4l2-ctrls.c
>>> @@ -466,6 +466,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>>
>>>  	case V4L2_CID_ILLUMINATORS_2:		return "Illuminator 2";
>>>  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:	return "Minimum Number of
>>>  	Capture Buffers"; case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return
>>>  	"Minimum Number of Output Buffers";
>>>
>>> +	case V4L2_CID_COLOR_ALPHA:		return "Color Alpha";
>>
>> I prefer CID_ALPHA_COLOR and string "Alpha Color". I think it is more
>> natural than the other way around.

OK, I guess you're right. And Google returns about twice as many hits for
"alpha color" than for "color alpha"...

> 
> I'm not too found of "color" in the name. Is the alpha value considered as a 
> color ?

Certainly it isn't, but Alpha alone looks a bit odd. It's too generic IMHO.

> 
>> Other than that I'm OK with this.
>>
>> Regards,
>>
>> 	Hans

--
Thanks,
Sylwester

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Hans Verkuil]

On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
> Hi Laurent,
> 
> On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
> > Hi Sylwester and Hans,
> > 
> > On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
> >> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
> >>> This control is intended for video capture or memory-to-memory
> >>> devices that are capable of setting up the alpha conponent to
> >>> some arbitrary value.
> >>> The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
> >>> globally to a value in range from 0 to 255.
> >>>
> >>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> >>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>> ---
> >>>
> >>>  Documentation/DocBook/media/v4l/controls.xml       |   20
> >>>  ++++++++++++++------ .../DocBook/media/v4l/pixfmt-packed-rgb.xml       
> >>>  |    7 +++++-- drivers/media/video/v4l2-ctrls.c                   |   
> >>>  7 +++++++ include/linux/videodev2.h                          |    6
> >>>  +++--- 4 files changed, 29 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/Documentation/DocBook/media/v4l/controls.xml
> >>> b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..7f99222
> >>> 100644
> >>> --- a/Documentation/DocBook/media/v4l/controls.xml
> >>> +++ b/Documentation/DocBook/media/v4l/controls.xml
> >>> @@ -324,12 +324,6 @@ minimum value disables backlight
> >>> compensation.</entry>
> >>>
> >>>  		(usually a microscope).</entry>
> >>>  		
> >>>  	  </row>
> >>>  	  <row>
> >>>
> >>> -	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> >>> -	    <entry></entry>
> >>> -	    <entry>End of the predefined control IDs (currently
> >>> -<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
> >>> -	  </row>
> >>> -	  <row>
> >>>
> >>>  	    <entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant></entry
> >>>  	    > <entry>integer</entry>
> >>>  	    <entry>This is a read-only control that can be read by the
> >>>  	    application
> >>>
> >>> @@ -345,6 +339,20 @@ and used as a hint to determine the number of OUTPUT
> >>> buffers to pass to REQBUFS.
> >>>
> >>>  The value is the minimum number of OUTPUT buffers that is necessary for
> >>>  hardware to work.</entry>
> >>>  
> >>>  	  </row>
> >>>
> >>> +	  <row id="v4l2-color-alpha">
> >>> +	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
> >>> +	    <entry>integer</entry>
> >>> +	    <entry> Sets the color alpha component on the capture device. It is
> >>> +	    applicable to any pixel formats that contain the alpha component,
> >>> +	    e.g. <link linkend="rgb-formats">packed RGB image formats</link>.
> >>> +	    </entry>
> > 
> > As the alpha value is global, isn't it applicable to formats with no alpha 
> > component as well ?
> 
> Hmm, I can't say no.. The control was intended as a means of setting up
> the alpha value for packed RGB formats:
> http://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html#rgb-formats
> 
> However it could well be used for formats with no alpha. Do you think
> the second sentence above should be removed or should something else be
> added to indicate it doesn't necessarily have to have a connection
> with ARGB color formats ?

Huh? How can this be used for formats without an alpha channel?

> 
> > 
> >>> +	  </row>
> >>> +	  <row>
> >>> +	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> >>> +	    <entry></entry>
> >>> +	    <entry>End of the predefined control IDs (currently
> >>> +	      <constant>V4L2_CID_COLOR_ALPHA</constant> + 1).</entry>
> >>> +	  </row>
> >>>
> >>>  	  <row>
> >>>  	  
> >>>  	    <entry><constant>V4L2_CID_PRIVATE_BASE</constant></entry>
> >>>  	    <entry></entry>
> >>>
> >>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
> >>> 4db272b..da4c360 100644
> >>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> >>> @@ -428,8 +428,11 @@ colorspace
> >>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
> >>>
> >>>      <para>Bit 7 is the most significant bit. The value of a = alpha
> >>>  
> >>>  bits is undefined when reading from the driver, ignored when writing
> >>>  to the driver, except when alpha blending has been negotiated for a
> >>>
> >>> -<link linkend="overlay">Video Overlay</link> or <link
> >>> -linkend="osd">Video Output Overlay</link>.</para>
> >>> +<link linkend="overlay">Video Overlay</link> or <link linkend="osd">
> >>> +Video Output Overlay</link> or when global alpha has been configured
> >>> +for a <link linkend="capture">Video Capture</link> by means of
> >>> +<link linkend="v4l2-color-alpha"> <constant>V4L2_CID_COLOR_ALPHA
> >>> +</constant> </link> control.</para>
> >>>
> >>>      <example>
> >>>      
> >>>        <title><constant>V4L2_PIX_FMT_BGR24</constant> 4 &times; 4 pixel
> >>>
> >>> diff --git a/drivers/media/video/v4l2-ctrls.c
> >>> b/drivers/media/video/v4l2-ctrls.c index 5552f81..bd90955 100644
> >>> --- a/drivers/media/video/v4l2-ctrls.c
> >>> +++ b/drivers/media/video/v4l2-ctrls.c
> >>> @@ -466,6 +466,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> >>>
> >>>  	case V4L2_CID_ILLUMINATORS_2:		return "Illuminator 2";
> >>>  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:	return "Minimum Number of
> >>>  	Capture Buffers"; case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return
> >>>  	"Minimum Number of Output Buffers";
> >>>
> >>> +	case V4L2_CID_COLOR_ALPHA:		return "Color Alpha";
> >>
> >> I prefer CID_ALPHA_COLOR and string "Alpha Color". I think it is more
> >> natural than the other way around.
> 
> OK, I guess you're right. And Google returns about twice as many hits for
> "alpha color" than for "color alpha"...
> 
> > 
> > I'm not too found of "color" in the name. Is the alpha value considered as a 
> > color ?
> 
> Certainly it isn't, but Alpha alone looks a bit odd. It's too generic IMHO.

How about V4L2_CID_ALPHA_COMPONENT?

Regards,

	Hans

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Laurent Pinchart]

Hi Hans,

On Thursday 24 November 2011 12:49:00 Hans Verkuil wrote:
> On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
> > On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
> > > On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
> > >> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
> > >>> This control is intended for video capture or memory-to-memory
> > >>> devices that are capable of setting up the alpha conponent to
> > >>> some arbitrary value.
> > >>> The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
> > >>> globally to a value in range from 0 to 255.
> > >>> 
> > >>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > >>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > >>> ---
> > >>> 
> > >>>  Documentation/DocBook/media/v4l/controls.xml       |   20
> > >>>  ++++++++++++++------ .../DocBook/media/v4l/pixfmt-packed-rgb.xml
> > >>>  
> > >>>  |    7 +++++-- drivers/media/video/v4l2-ctrls.c                   |
> > >>>  
> > >>>  7 +++++++ include/linux/videodev2.h                          |    6
> > >>>  +++--- 4 files changed, 29 insertions(+), 11 deletions(-)
> > >>> 
> > >>> diff --git a/Documentation/DocBook/media/v4l/controls.xml
> > >>> b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..7f99222
> > >>> 100644
> > >>> --- a/Documentation/DocBook/media/v4l/controls.xml
> > >>> +++ b/Documentation/DocBook/media/v4l/controls.xml
> > >>> @@ -324,12 +324,6 @@ minimum value disables backlight
> > >>> compensation.</entry>
> > >>> 
> > >>>  		(usually a microscope).</entry>
> > >>>  		
> > >>>  	  </row>
> > >>>  	  <row>
> > >>> 
> > >>> -	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> > >>> -	    <entry></entry>
> > >>> -	    <entry>End of the predefined control IDs (currently
> > >>> -<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
> > >>> -	  </row>
> > >>> -	  <row>
> > >>> 
> > >>>  	    <entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant></e
> > >>>  	    ntry
> > >>>  	    
> > >>>  	    > <entry>integer</entry>
> > >>>  	    
> > >>>  	    <entry>This is a read-only control that can be read by the
> > >>>  	    application
> > >>> 
> > >>> @@ -345,6 +339,20 @@ and used as a hint to determine the number of
> > >>> OUTPUT buffers to pass to REQBUFS.
> > >>> 
> > >>>  The value is the minimum number of OUTPUT buffers that is necessary
> > >>>  for hardware to work.</entry>
> > >>>  
> > >>>  	  </row>
> > >>> 
> > >>> +	  <row id="v4l2-color-alpha">
> > >>> +	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
> > >>> +	    <entry>integer</entry>
> > >>> +	    <entry> Sets the color alpha component on the capture device.
> > >>> It is +	    applicable to any pixel formats that contain the alpha
> > >>> component, +	    e.g. <link linkend="rgb-formats">packed RGB image
> > >>> formats</link>. +	    </entry>
> > > 
> > > As the alpha value is global, isn't it applicable to formats with no
> > > alpha component as well ?
> > 
> > Hmm, I can't say no.. The control was intended as a means of setting up
> > the alpha value for packed RGB formats:
> > http://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html#rgb-formats
> > 
> > However it could well be used for formats with no alpha. Do you think
> > the second sentence above should be removed or should something else be
> > added to indicate it doesn't necessarily have to have a connection
> > with ARGB color formats ?

I think we should make it explicit that this global alpha value is applied in 
addition to a possibly per-pixel alpha value (if available in the selected 
format).

> Huh? How can this be used for formats without an alpha channel?

If my understanding is correct, this control sets a global alpha value for the 
whole overlay. For instance, with V4L2_CID_COLOR_ALPHA set to 0.5, an overlay 
using a non-alpha format (such as YUYV), or an overlay using an alpha format  
with the alpha value set to 1 for every pixel, would be half transparent.

In other words, the resulting alpha value is the product of the global alpha 
value and the per-pixel alpha value. Non-alpha formats have an implicit per-
pixel alpha value equal to 1 for every pixel.

> > >>> +	  </row>
> > >>> +	  <row>
> > >>> +	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> > >>> +	    <entry></entry>
> > >>> +	    <entry>End of the predefined control IDs (currently
> > >>> +	      <constant>V4L2_CID_COLOR_ALPHA</constant> + 1).</entry>
> > >>> +	  </row>
> > >>> 
> > >>>  	  <row>
> > >>>  	  
> > >>>  	    <entry><constant>V4L2_CID_PRIVATE_BASE</constant></entry>
> > >>>  	    <entry></entry>
> > >>> 
> > >>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > >>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
> > >>> 4db272b..da4c360 100644
> > >>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > >>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > >>> @@ -428,8 +428,11 @@ colorspace
> > >>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
> > >>> 
> > >>>      <para>Bit 7 is the most significant bit. The value of a = alpha
> > >>>  
> > >>>  bits is undefined when reading from the driver, ignored when writing
> > >>>  to the driver, except when alpha blending has been negotiated for a
> > >>> 
> > >>> -<link linkend="overlay">Video Overlay</link> or <link
> > >>> -linkend="osd">Video Output Overlay</link>.</para>
> > >>> +<link linkend="overlay">Video Overlay</link> or <link linkend="osd">
> > >>> +Video Output Overlay</link> or when global alpha has been configured
> > >>> +for a <link linkend="capture">Video Capture</link> by means of
> > >>> +<link linkend="v4l2-color-alpha"> <constant>V4L2_CID_COLOR_ALPHA
> > >>> +</constant> </link> control.</para>
> > >>> 
> > >>>      <example>
> > >>>      
> > >>>        <title><constant>V4L2_PIX_FMT_BGR24</constant> 4 &times; 4
> > >>>        pixel
> > >>> 
> > >>> diff --git a/drivers/media/video/v4l2-ctrls.c
> > >>> b/drivers/media/video/v4l2-ctrls.c index 5552f81..bd90955 100644
> > >>> --- a/drivers/media/video/v4l2-ctrls.c
> > >>> +++ b/drivers/media/video/v4l2-ctrls.c
> > >>> @@ -466,6 +466,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > >>> 
> > >>>  	case V4L2_CID_ILLUMINATORS_2:		return "Illuminator 2";
> > >>>  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:	return "Minimum Number of
> > >>>  	Capture Buffers"; case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return
> > >>>  	"Minimum Number of Output Buffers";
> > >>> 
> > >>> +	case V4L2_CID_COLOR_ALPHA:		return "Color Alpha";
> > >> 
> > >> I prefer CID_ALPHA_COLOR and string "Alpha Color". I think it is more
> > >> natural than the other way around.
> > 
> > OK, I guess you're right. And Google returns about twice as many hits for
> > "alpha color" than for "color alpha"...
> > 
> > > I'm not too found of "color" in the name. Is the alpha value considered
> > > as a color ?
> > 
> > Certainly it isn't, but Alpha alone looks a bit odd. It's too generic
> > IMHO.
> 
> How about V4L2_CID_ALPHA_COMPONENT?

Or V4L2_CID_GLOBAL_ALPHA ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Hans Verkuil]

On Thursday, November 24, 2011 13:06:09 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Thursday 24 November 2011 12:49:00 Hans Verkuil wrote:
> > On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
> > > On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
> > > > On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
> > > >> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
> > > >>> This control is intended for video capture or memory-to-memory
> > > >>> devices that are capable of setting up the alpha conponent to
> > > >>> some arbitrary value.
> > > >>> The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
> > > >>> globally to a value in range from 0 to 255.
> > > >>> 
> > > >>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > > >>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > >>> ---
> > > >>> 
> > > >>>  Documentation/DocBook/media/v4l/controls.xml       |   20
> > > >>>  ++++++++++++++------ .../DocBook/media/v4l/pixfmt-packed-rgb.xml
> > > >>>  
> > > >>>  |    7 +++++-- drivers/media/video/v4l2-ctrls.c                   |
> > > >>>  
> > > >>>  7 +++++++ include/linux/videodev2.h                          |    6
> > > >>>  +++--- 4 files changed, 29 insertions(+), 11 deletions(-)
> > > >>> 
> > > >>> diff --git a/Documentation/DocBook/media/v4l/controls.xml
> > > >>> b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..7f99222
> > > >>> 100644
> > > >>> --- a/Documentation/DocBook/media/v4l/controls.xml
> > > >>> +++ b/Documentation/DocBook/media/v4l/controls.xml
> > > >>> @@ -324,12 +324,6 @@ minimum value disables backlight
> > > >>> compensation.</entry>
> > > >>> 
> > > >>>  		(usually a microscope).</entry>
> > > >>>  		
> > > >>>  	  </row>
> > > >>>  	  <row>
> > > >>> 
> > > >>> -	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> > > >>> -	    <entry></entry>
> > > >>> -	    <entry>End of the predefined control IDs (currently
> > > >>> -<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
> > > >>> -	  </row>
> > > >>> -	  <row>
> > > >>> 
> > > >>>  	    <entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant></e
> > > >>>  	    ntry
> > > >>>  	    
> > > >>>  	    > <entry>integer</entry>
> > > >>>  	    
> > > >>>  	    <entry>This is a read-only control that can be read by the
> > > >>>  	    application
> > > >>> 
> > > >>> @@ -345,6 +339,20 @@ and used as a hint to determine the number of
> > > >>> OUTPUT buffers to pass to REQBUFS.
> > > >>> 
> > > >>>  The value is the minimum number of OUTPUT buffers that is necessary
> > > >>>  for hardware to work.</entry>
> > > >>>  
> > > >>>  	  </row>
> > > >>> 
> > > >>> +	  <row id="v4l2-color-alpha">
> > > >>> +	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
> > > >>> +	    <entry>integer</entry>
> > > >>> +	    <entry> Sets the color alpha component on the capture device.
> > > >>> It is +	    applicable to any pixel formats that contain the alpha
> > > >>> component, +	    e.g. <link linkend="rgb-formats">packed RGB image
> > > >>> formats</link>. +	    </entry>
> > > > 
> > > > As the alpha value is global, isn't it applicable to formats with no
> > > > alpha component as well ?
> > > 
> > > Hmm, I can't say no.. The control was intended as a means of setting up
> > > the alpha value for packed RGB formats:
> > > http://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html#rgb-formats
> > > 
> > > However it could well be used for formats with no alpha. Do you think
> > > the second sentence above should be removed or should something else be
> > > added to indicate it doesn't necessarily have to have a connection
> > > with ARGB color formats ?
> 
> I think we should make it explicit that this global alpha value is applied in 
> addition to a possibly per-pixel alpha value (if available in the selected 
> format).
> 
> > Huh? How can this be used for formats without an alpha channel?
> 
> If my understanding is correct, this control sets a global alpha value for the 
> whole overlay. For instance, with V4L2_CID_COLOR_ALPHA set to 0.5, an overlay 
> using a non-alpha format (such as YUYV), or an overlay using an alpha format  
> with the alpha value set to 1 for every pixel, would be half transparent.
> 
> In other words, the resulting alpha value is the product of the global alpha 
> value and the per-pixel alpha value. Non-alpha formats have an implicit per-
> pixel alpha value equal to 1 for every pixel.

Well, if that's the case, then we already have an API for that
(http://hverkuil.home.xs4all.nl/spec/media.html#v4l2-window, field global_alpha).

It was my understanding that this is used with a mem2mem device where you
just want to fill in the alpha channel to the desired value. It's not used
inside the device at all (that happens later in the pipeline).

Regards,

	Hans

> > > >>> +	  </row>
> > > >>> +	  <row>
> > > >>> +	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> > > >>> +	    <entry></entry>
> > > >>> +	    <entry>End of the predefined control IDs (currently
> > > >>> +	      <constant>V4L2_CID_COLOR_ALPHA</constant> + 1).</entry>
> > > >>> +	  </row>
> > > >>> 
> > > >>>  	  <row>
> > > >>>  	  
> > > >>>  	    <entry><constant>V4L2_CID_PRIVATE_BASE</constant></entry>
> > > >>>  	    <entry></entry>
> > > >>> 
> > > >>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > > >>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
> > > >>> 4db272b..da4c360 100644
> > > >>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > > >>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
> > > >>> @@ -428,8 +428,11 @@ colorspace
> > > >>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
> > > >>> 
> > > >>>      <para>Bit 7 is the most significant bit. The value of a = alpha
> > > >>>  
> > > >>>  bits is undefined when reading from the driver, ignored when writing
> > > >>>  to the driver, except when alpha blending has been negotiated for a
> > > >>> 
> > > >>> -<link linkend="overlay">Video Overlay</link> or <link
> > > >>> -linkend="osd">Video Output Overlay</link>.</para>
> > > >>> +<link linkend="overlay">Video Overlay</link> or <link linkend="osd">
> > > >>> +Video Output Overlay</link> or when global alpha has been configured
> > > >>> +for a <link linkend="capture">Video Capture</link> by means of
> > > >>> +<link linkend="v4l2-color-alpha"> <constant>V4L2_CID_COLOR_ALPHA
> > > >>> +</constant> </link> control.</para>
> > > >>> 
> > > >>>      <example>
> > > >>>      
> > > >>>        <title><constant>V4L2_PIX_FMT_BGR24</constant> 4 &times; 4
> > > >>>        pixel
> > > >>> 
> > > >>> diff --git a/drivers/media/video/v4l2-ctrls.c
> > > >>> b/drivers/media/video/v4l2-ctrls.c index 5552f81..bd90955 100644
> > > >>> --- a/drivers/media/video/v4l2-ctrls.c
> > > >>> +++ b/drivers/media/video/v4l2-ctrls.c
> > > >>> @@ -466,6 +466,7 @@ const char *v4l2_ctrl_get_name(u32 id)
> > > >>> 
> > > >>>  	case V4L2_CID_ILLUMINATORS_2:		return "Illuminator 2";
> > > >>>  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:	return "Minimum Number of
> > > >>>  	Capture Buffers"; case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return
> > > >>>  	"Minimum Number of Output Buffers";
> > > >>> 
> > > >>> +	case V4L2_CID_COLOR_ALPHA:		return "Color Alpha";
> > > >> 
> > > >> I prefer CID_ALPHA_COLOR and string "Alpha Color". I think it is more
> > > >> natural than the other way around.
> > > 
> > > OK, I guess you're right. And Google returns about twice as many hits for
> > > "alpha color" than for "color alpha"...
> > > 
> > > > I'm not too found of "color" in the name. Is the alpha value considered
> > > > as a color ?
> > > 
> > > Certainly it isn't, but Alpha alone looks a bit odd. It's too generic
> > > IMHO.
> > 
> > How about V4L2_CID_ALPHA_COMPONENT?
> 
> Or V4L2_CID_GLOBAL_ALPHA ?
> 
> 

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Sylwester Nawrocki]

On 11/24/2011 01:06 PM, Laurent Pinchart wrote:
> On Thursday 24 November 2011 12:49:00 Hans Verkuil wrote:
>> On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
>>> On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
>>>> On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
>>>>> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
>>>>>> This control is intended for video capture or memory-to-memory
>>>>>> devices that are capable of setting up the alpha conponent to
>>>>>> some arbitrary value.
>>>>>> The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
>>>>>> globally to a value in range from 0 to 255.
>>>>>>
>>>>>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>> ---
>>>>>>
>>>>>>  Documentation/DocBook/media/v4l/controls.xml       |   20
>>>>>>  ++++++++++++++------ .../DocBook/media/v4l/pixfmt-packed-rgb.xml
>>>>>>  
>>>>>>  |    7 +++++-- drivers/media/video/v4l2-ctrls.c                   |
>>>>>>  
>>>>>>  7 +++++++ include/linux/videodev2.h                          |    6
>>>>>>  +++--- 4 files changed, 29 insertions(+), 11 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml
>>>>>> b/Documentation/DocBook/media/v4l/controls.xml index 3bc5ee8..7f99222
>>>>>> 100644
>>>>>> --- a/Documentation/DocBook/media/v4l/controls.xml
>>>>>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>>>>>> @@ -324,12 +324,6 @@ minimum value disables backlight
>>>>>> compensation.</entry>
>>>>>>
>>>>>>  		(usually a microscope).</entry>
>>>>>>  		
>>>>>>  	  </row>
>>>>>>  	  <row>
>>>>>>
>>>>>> -	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
>>>>>> -	    <entry></entry>
>>>>>> -	    <entry>End of the predefined control IDs (currently
>>>>>> -<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
>>>>>> -	  </row>
>>>>>> -	  <row>
>>>>>>
>>>>>>  	    <entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant></e
>>>>>>  	    ntry
>>>>>>  	    
>>>>>>  	    > <entry>integer</entry>
>>>>>>  	    
>>>>>>  	    <entry>This is a read-only control that can be read by the
>>>>>>  	    application
>>>>>>
>>>>>> @@ -345,6 +339,20 @@ and used as a hint to determine the number of
>>>>>> OUTPUT buffers to pass to REQBUFS.
>>>>>>
>>>>>>  The value is the minimum number of OUTPUT buffers that is necessary
>>>>>>  for hardware to work.</entry>
>>>>>>  
>>>>>>  	  </row>
>>>>>>
>>>>>> +	  <row id="v4l2-color-alpha">
>>>>>> +	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
>>>>>> +	    <entry>integer</entry>
>>>>>> +	    <entry> Sets the color alpha component on the capture device.
>>>>>> It is +	    applicable to any pixel formats that contain the alpha
>>>>>> component, +	    e.g. <link linkend="rgb-formats">packed RGB image
>>>>>> formats</link>. +	    </entry>
>>>>
>>>> As the alpha value is global, isn't it applicable to formats with no
>>>> alpha component as well ?
>>>
>>> Hmm, I can't say no.. The control was intended as a means of setting up
>>> the alpha value for packed RGB formats:
>>> http://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html#rgb-formats
>>>
>>> However it could well be used for formats with no alpha. Do you think
>>> the second sentence above should be removed or should something else be
>>> added to indicate it doesn't necessarily have to have a connection
>>> with ARGB color formats ?
> 
> I think we should make it explicit that this global alpha value is applied in 
> addition to a possibly per-pixel alpha value (if available in the selected 
> format).

I'm not sure if it was the intention. If pixel format has alpha component
this control would set per-pixel alpha to an arbitrary control value.

If the pixel format doesn't have alpha, then what this control would do
- I'm not sure.

> 
>> Huh? How can this be used for formats without an alpha channel?
> 
> If my understanding is correct, this control sets a global alpha value for the 
> whole overlay. For instance, with V4L2_CID_COLOR_ALPHA set to 0.5, an overlay 

More precisely it sets a per-pixel alpha to some arbitrary value. For instance
in this case the A value for each pixel with ARGB8888 pixel format would be 128.

> using a non-alpha format (such as YUYV), or an overlay using an alpha format  
> with the alpha value set to 1 for every pixel, would be half transparent.
> 
> In other words, the resulting alpha value is the product of the global alpha 
> value and the per-pixel alpha value. Non-alpha formats have an implicit per-
> pixel alpha value equal to 1 for every pixel.
> 
>>>>>> +	  </row>
>>>>>> +	  <row>
>>>>>> +	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
>>>>>> +	    <entry></entry>
>>>>>> +	    <entry>End of the predefined control IDs (currently
>>>>>> +	      <constant>V4L2_CID_COLOR_ALPHA</constant> + 1).</entry>
>>>>>> +	  </row>
>>>>>>
>>>>>>  	  <row>
>>>>>>  	  
>>>>>>  	    <entry><constant>V4L2_CID_PRIVATE_BASE</constant></entry>
>>>>>>  	    <entry></entry>
>>>>>>
>>>>>> diff --git a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>>>>> b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml index
>>>>>> 4db272b..da4c360 100644
>>>>>> --- a/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>>>>> +++ b/Documentation/DocBook/media/v4l/pixfmt-packed-rgb.xml
>>>>>> @@ -428,8 +428,11 @@ colorspace
>>>>>> <constant>V4L2_COLORSPACE_SRGB</constant>.</para>
>>>>>>
>>>>>>      <para>Bit 7 is the most significant bit. The value of a = alpha
>>>>>>  
>>>>>>  bits is undefined when reading from the driver, ignored when writing
>>>>>>  to the driver, except when alpha blending has been negotiated for a
>>>>>>
>>>>>> -<link linkend="overlay">Video Overlay</link> or <link
>>>>>> -linkend="osd">Video Output Overlay</link>.</para>
>>>>>> +<link linkend="overlay">Video Overlay</link> or <link linkend="osd">
>>>>>> +Video Output Overlay</link> or when global alpha has been configured
>>>>>> +for a <link linkend="capture">Video Capture</link> by means of
>>>>>> +<link linkend="v4l2-color-alpha"> <constant>V4L2_CID_COLOR_ALPHA
>>>>>> +</constant> </link> control.</para>
>>>>>>
>>>>>>      <example>
>>>>>>      
>>>>>>        <title><constant>V4L2_PIX_FMT_BGR24</constant> 4 &times; 4
>>>>>>        pixel
>>>>>>
>>>>>> diff --git a/drivers/media/video/v4l2-ctrls.c
>>>>>> b/drivers/media/video/v4l2-ctrls.c index 5552f81..bd90955 100644
>>>>>> --- a/drivers/media/video/v4l2-ctrls.c
>>>>>> +++ b/drivers/media/video/v4l2-ctrls.c
>>>>>> @@ -466,6 +466,7 @@ const char *v4l2_ctrl_get_name(u32 id)
>>>>>>
>>>>>>  	case V4L2_CID_ILLUMINATORS_2:		return "Illuminator 2";
>>>>>>  	case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:	return "Minimum Number of
>>>>>>  	Capture Buffers"; case V4L2_CID_MIN_BUFFERS_FOR_OUTPUT:	return
>>>>>>  	"Minimum Number of Output Buffers";
>>>>>>
>>>>>> +	case V4L2_CID_COLOR_ALPHA:		return "Color Alpha";
>>>>>
>>>>> I prefer CID_ALPHA_COLOR and string "Alpha Color". I think it is more
>>>>> natural than the other way around.
>>>
>>> OK, I guess you're right. And Google returns about twice as many hits for
>>> "alpha color" than for "color alpha"...
>>>
>>>> I'm not too found of "color" in the name. Is the alpha value considered
>>>> as a color ?
>>>
>>> Certainly it isn't, but Alpha alone looks a bit odd. It's too generic
>>> IMHO.
>>
>> How about V4L2_CID_ALPHA_COMPONENT?
> 
> Or V4L2_CID_GLOBAL_ALPHA ?

Although V4L2_CID_GLOBAL_ALPHA sounds good, V4L2_CID_ALPHA_COMPONENT reflects 
more precisely what was my intention. The control was supposed to set the alpha
component for ARGB formats.
 

-- 
Regards,
Sylwester

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Sylwester Nawrocki]

On 11/24/2011 12:49 PM, Hans Verkuil wrote:
> On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
>> On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
>>> On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
>>>> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
...
> How about V4L2_CID_ALPHA_COMPONENT?

This one seems the best fit for me. I'll redo the patches and send
them out tomorrow. Unless someone comes up with even better name.

Thanks for your comment Hans and Laurent!

--
Regards,
Sylwester

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Laurent Pinchart]

Hi,

On Thursday 24 November 2011 13:22:10 Hans Verkuil wrote:
> On Thursday, November 24, 2011 13:06:09 Laurent Pinchart wrote:
> > On Thursday 24 November 2011 12:49:00 Hans Verkuil wrote:
> > > On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
> > > > On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
> > > > > On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
> > > > >> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
> > > > >>> This control is intended for video capture or memory-to-memory
> > > > >>> devices that are capable of setting up the alpha conponent to
> > > > >>> some arbitrary value.
> > > > >>> The V4L2_CID_COLOR_ALPHA control allows to set the alpha channel
> > > > >>> globally to a value in range from 0 to 255.
> > > > >>> 
> > > > >>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > > > >>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > > >>> ---
> > > > >>> 
> > > > >>>  Documentation/DocBook/media/v4l/controls.xml       |   20
> > > > >>>  ++++++++++++++------ .../DocBook/media/v4l/pixfmt-packed-rgb.xml
> > > > >>>  
> > > > >>>  |    7 +++++-- drivers/media/video/v4l2-ctrls.c                 
> > > > >>>  |     |
> > > > >>>  
> > > > >>>  7 +++++++ include/linux/videodev2.h                          |  
> > > > >>>   6 +++--- 4 files changed, 29 insertions(+), 11 deletions(-)
> > > > >>> 
> > > > >>> diff --git a/Documentation/DocBook/media/v4l/controls.xml
> > > > >>> b/Documentation/DocBook/media/v4l/controls.xml index
> > > > >>> 3bc5ee8..7f99222 100644
> > > > >>> --- a/Documentation/DocBook/media/v4l/controls.xml
> > > > >>> +++ b/Documentation/DocBook/media/v4l/controls.xml
> > > > >>> @@ -324,12 +324,6 @@ minimum value disables backlight
> > > > >>> compensation.</entry>
> > > > >>> 
> > > > >>>  		(usually a microscope).</entry>
> > > > >>>  		
> > > > >>>  	  </row>
> > > > >>>  	  <row>
> > > > >>> 
> > > > >>> -	    <entry><constant>V4L2_CID_LASTP1</constant></entry>
> > > > >>> -	    <entry></entry>
> > > > >>> -	    <entry>End of the predefined control IDs (currently
> > > > >>> -<constant>V4L2_CID_ILLUMINATORS_2</constant> + 1).</entry>
> > > > >>> -	  </row>
> > > > >>> -	  <row>
> > > > >>> 
> > > > >>>  	    
<entry><constant>V4L2_CID_MIN_BUFFERS_FOR_CAPTURE</constant
> > > > >>>  	    ></e ntry
> > > > >>>  	    
> > > > >>>  	    > <entry>integer</entry>
> > > > >>>  	    
> > > > >>>  	    <entry>This is a read-only control that can be read by the
> > > > >>>  	    application
> > > > >>> 
> > > > >>> @@ -345,6 +339,20 @@ and used as a hint to determine the number
> > > > >>> of OUTPUT buffers to pass to REQBUFS.
> > > > >>> 
> > > > >>>  The value is the minimum number of OUTPUT buffers that is
> > > > >>>  necessary for hardware to work.</entry>
> > > > >>>  
> > > > >>>  	  </row>
> > > > >>> 
> > > > >>> +	  <row id="v4l2-color-alpha">
> > > > >>> +	    <entry><constant>V4L2_CID_COLOR_ALPHA</constant></entry>
> > > > >>> +	    <entry>integer</entry>
> > > > >>> +	    <entry> Sets the color alpha component on the capture
> > > > >>> device. It is +	    applicable to any pixel formats that contain
> > > > >>> the alpha component, +	    e.g. <link
> > > > >>> linkend="rgb-formats">packed RGB image formats</link>. +	   
> > > > >>> </entry>
> > > > > 
> > > > > As the alpha value is global, isn't it applicable to formats with
> > > > > no alpha component as well ?
> > > > 
> > > > Hmm, I can't say no.. The control was intended as a means of setting
> > > > up the alpha value for packed RGB formats:
> > > > http://linuxtv.org/downloads/v4l-dvb-apis/packed-rgb.html#rgb-formats
> > > > 
> > > > However it could well be used for formats with no alpha. Do you think
> > > > the second sentence above should be removed or should something else
> > > > be added to indicate it doesn't necessarily have to have a
> > > > connection with ARGB color formats ?
> > 
> > I think we should make it explicit that this global alpha value is
> > applied in addition to a possibly per-pixel alpha value (if available in
> > the selected format).
> > 
> > > Huh? How can this be used for formats without an alpha channel?
> > 
> > If my understanding is correct, this control sets a global alpha value
> > for the whole overlay. For instance, with V4L2_CID_COLOR_ALPHA set to
> > 0.5, an overlay using a non-alpha format (such as YUYV), or an overlay
> > using an alpha format with the alpha value set to 1 for every pixel,
> > would be half transparent.
> > 
> > In other words, the resulting alpha value is the product of the global
> > alpha value and the per-pixel alpha value. Non-alpha formats have an
> > implicit per- pixel alpha value equal to 1 for every pixel.
> 
> Well, if that's the case, then we already have an API for that
> (http://hverkuil.home.xs4all.nl/spec/media.html#v4l2-window, field
> global_alpha).
> 
> It was my understanding that this is used with a mem2mem device where you
> just want to fill in the alpha channel to the desired value. It's not used
> inside the device at all (that happens later in the pipeline).

OK, now I understand. Maybe the documentation should describe this a bit more 
explicitly ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Sylwester Nawrocki]

On 11/24/2011 03:00 PM, Laurent Pinchart wrote:
> On Thursday 24 November 2011 13:22:10 Hans Verkuil wrote:
>> On Thursday, November 24, 2011 13:06:09 Laurent Pinchart wrote:
>>> On Thursday 24 November 2011 12:49:00 Hans Verkuil wrote:
>>>> On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
>>>>> On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
>>>>>> On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
>>>>>>> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
>> Well, if that's the case, then we already have an API for that
>> (http://hverkuil.home.xs4all.nl/spec/media.html#v4l2-window, field
>> global_alpha).
>>
>> It was my understanding that this is used with a mem2mem device where you
>> just want to fill in the alpha channel to the desired value. It's not used
>> inside the device at all (that happens later in the pipeline).
> 
> OK, now I understand. Maybe the documentation should describe this a bit more 
> explicitly ?

I've modified the control description so now it is:

V4L2_CID_ALPHA_COMPONENT	
integer	

Sets the alpha color component on the capture device or on the capture buffer
queue of a mem-to-mem device. It is applicable to any pixel formats that
contain the alpha component, e.g. _packed RGB image_ formats.


And the part below Table 2.6

Bit 7 is the most significant bit. The value of a = alpha bits is undefined
when reading from the driver, ignored when writing to the driver, except when
alpha blending has been negotiated for a Video Overlay or Video Output Overlay
or when alpha component has been configured for a Video Capture by means of
V4L2_CID_ALPHA_COMPONENT control.


--
Regards,
Sylwester

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Laurent Pinchart]

Hi Sylwester,

On Thursday 24 November 2011 16:53:12 Sylwester Nawrocki wrote:
> On 11/24/2011 03:00 PM, Laurent Pinchart wrote:
> > On Thursday 24 November 2011 13:22:10 Hans Verkuil wrote:
> >> On Thursday, November 24, 2011 13:06:09 Laurent Pinchart wrote:
> >>> On Thursday 24 November 2011 12:49:00 Hans Verkuil wrote:
> >>>> On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
> >>>>> On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
> >>>>>> On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
> >>>>>>> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
> >> Well, if that's the case, then we already have an API for that
> >> (http://hverkuil.home.xs4all.nl/spec/media.html#v4l2-window, field
> >> global_alpha).
> >> 
> >> It was my understanding that this is used with a mem2mem device where
> >> you just want to fill in the alpha channel to the desired value. It's
> >> not used inside the device at all (that happens later in the pipeline).
> > 
> > OK, now I understand. Maybe the documentation should describe this a bit
> > more explicitly ?
> 
> I've modified the control description so now it is:
> 
> V4L2_CID_ALPHA_COMPONENT
> integer
> 

What about clarifying it further with something like

"When a mem-to-mem device produces a frame format that includes an alpha 
component (e.g. _packed RGB image_ formats), the alpha value is not defined by 
the mem-to-mem input data. This control lets you select the alpha component 
value of all pixels in such a case. It is applicable to any pixel format that 
contains an alpha component."

> And the part below Table 2.6
> 
> Bit 7 is the most significant bit. The value of a = alpha bits is undefined
> when reading from the driver, ignored when writing to the driver, except
> when alpha blending has been negotiated for a Video Overlay or Video
> Output Overlay or when alpha component has been configured for a Video
> Capture by means of V4L2_CID_ALPHA_COMPONENT control.

-- 
Regards,

Laurent Pinchart

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Sylwester Nawrocki]

Hi Laurent,

On 11/25/2011 01:57 PM, Laurent Pinchart wrote:
> On Thursday 24 November 2011 16:53:12 Sylwester Nawrocki wrote:
>> On 11/24/2011 03:00 PM, Laurent Pinchart wrote:
>>> On Thursday 24 November 2011 13:22:10 Hans Verkuil wrote:
>>>> On Thursday, November 24, 2011 13:06:09 Laurent Pinchart wrote:
>>>>> On Thursday 24 November 2011 12:49:00 Hans Verkuil wrote:
>>>>>> On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
>>>>>>> On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
>>>>>>>> On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
>>>>>>>>> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
>>>> Well, if that's the case, then we already have an API for that
>>>> (http://hverkuil.home.xs4all.nl/spec/media.html#v4l2-window, field
>>>> global_alpha).
>>>>
>>>> It was my understanding that this is used with a mem2mem device where
>>>> you just want to fill in the alpha channel to the desired value. It's
>>>> not used inside the device at all (that happens later in the pipeline).
>>>
>>> OK, now I understand. Maybe the documentation should describe this a bit
>>> more explicitly ?
>>
>> I've modified the control description so now it is:
>>
>> V4L2_CID_ALPHA_COMPONENT
>> integer
>>
> 
> What about clarifying it further with something like
> 
> "When a mem-to-mem device produces a frame format that includes an alpha 
> component (e.g. _packed RGB image_ formats), the alpha value is not defined by 
> the mem-to-mem input data. This control lets you select the alpha component 
> value of all pixels in such a case. It is applicable to any pixel format that 
> contains an alpha component."

Thanks for the help. Since there are also mem-to-mem devices that account an input 
alpha when producing an output frame (e.g. S5P SoC G2D IP block supports alpha 
blending), I would change that to:


 "When a mem-to-mem device produces a frame format that includes an alpha 
 component (e.g. _packed RGB image_ formats) and the alpha value is not defined by 
 the mem-to-mem input data this control lets you select the alpha component 
 value of all pixels. It is applicable to any pixel format that contains an alpha
 component."

OR

 "When data format of a frame produced by a mem-to-mem device includes an alpha 
 component (e.g. _packed RGB image_ formats) and the alpha value is not defined
 by the mem-to-mem input data this control lets you select the alpha component
 value of all pixels. The control is applicable to any pixel format that contains
 an alpha component."

How do you think ?

> 
>> And the part below Table 2.6
>>
>> Bit 7 is the most significant bit. The value of a = alpha bits is undefined
>> when reading from the driver, ignored when writing to the driver, except
>> when alpha blending has been negotiated for a Video Overlay or Video
>> Output Overlay or when alpha component has been configured for a Video
>> Capture by means of V4L2_CID_ALPHA_COMPONENT control.
 
--
Thanks,
Sylwester

Re: [PATCH/RFC 1/2] v4l: Add a global color alpha control

[Laurent Pinchart]

Hi Sylwester,

On Friday 25 November 2011 14:24:47 Sylwester Nawrocki wrote:
> On 11/25/2011 01:57 PM, Laurent Pinchart wrote:
> > On Thursday 24 November 2011 16:53:12 Sylwester Nawrocki wrote:
> >> On 11/24/2011 03:00 PM, Laurent Pinchart wrote:
> >>> On Thursday 24 November 2011 13:22:10 Hans Verkuil wrote:
> >>>> On Thursday, November 24, 2011 13:06:09 Laurent Pinchart wrote:
> >>>>> On Thursday 24 November 2011 12:49:00 Hans Verkuil wrote:
> >>>>>> On Thursday, November 24, 2011 12:39:54 Sylwester Nawrocki wrote:
> >>>>>>> On 11/24/2011 12:09 PM, Laurent Pinchart wrote:
> >>>>>>>> On Thursday 24 November 2011 12:00:45 Hans Verkuil wrote:
> >>>>>>>>> On Thursday, November 24, 2011 11:53:16 Sylwester Nawrocki wrote:
> >>>> Well, if that's the case, then we already have an API for that
> >>>> (http://hverkuil.home.xs4all.nl/spec/media.html#v4l2-window, field
> >>>> global_alpha).
> >>>> 
> >>>> It was my understanding that this is used with a mem2mem device where
> >>>> you just want to fill in the alpha channel to the desired value. It's
> >>>> not used inside the device at all (that happens later in the
> >>>> pipeline).
> >>> 
> >>> OK, now I understand. Maybe the documentation should describe this a
> >>> bit more explicitly ?
> >> 
> >> I've modified the control description so now it is:
> >> 
> >> V4L2_CID_ALPHA_COMPONENT
> >> integer
> > 
> > What about clarifying it further with something like
> > 
> > "When a mem-to-mem device produces a frame format that includes an alpha
> > component (e.g. _packed RGB image_ formats), the alpha value is not
> > defined by the mem-to-mem input data. This control lets you select the
> > alpha component value of all pixels in such a case. It is applicable to
> > any pixel format that contains an alpha component."
> 
> Thanks for the help. Since there are also mem-to-mem devices that account
> an input alpha when producing an output frame (e.g. S5P SoC G2D IP block
> supports alpha blending), I would change that to:
> 
> 
>  "When a mem-to-mem device produces a frame format that includes an alpha
>  component (e.g. _packed RGB image_ formats) and the alpha value is not
> defined by the mem-to-mem input data this control lets you select the
> alpha component value of all pixels. It is applicable to any pixel format
> that contains an alpha component."

That sounds good to me.

> 
> OR
> 
>  "When data format of a frame produced by a mem-to-mem device includes an
> alpha component (e.g. _packed RGB image_ formats) and the alpha value is
> not defined by the mem-to-mem input data this control lets you select the
> alpha component value of all pixels. The control is applicable to any
> pixel format that contains an alpha component."
> 
> How do you think ?
> 
> >> And the part below Table 2.6
> >> 
> >> Bit 7 is the most significant bit. The value of a = alpha bits is
> >> undefined when reading from the driver, ignored when writing to the
> >> driver, except when alpha blending has been negotiated for a Video
> >> Overlay or Video Output Overlay or when alpha component has been
> >> configured for a Video Capture by means of V4L2_CID_ALPHA_COMPONENT
> >> control.

-- 
Regards,

Laurent Pinchart