Re: [PATCH 01/15] V4L: Extend V4L2_CID_COLORFX with more image effects
On 04/17/2012 01:28 PM, Sylwester Nawrocki wrote: > On 04/17/2012 12:51 PM, Rémi Denis-Courmont wrote: >> On Tue, 17 Apr 2012 12:09:42 +0200, Sylwester Nawrocki >> wrote: >>> This patch adds definition of additional color effects: >>> - V4L2_COLORFX_AQUA, >>> - V4L2_COLORFX_ART_FREEZE, >>> - V4L2_COLORFX_SILHOUETTE, >>> - V4L2_COLORFX_SOLARIZATION, >>> - V4L2_COLORFX_ANTIQUE, >> >> There starts to be a lot of different color effects with no obvious way to >> determine which ones the current device actually suppots. Should this not >> be a menu control instead? > > Fortunately this has been a menu control, since it was introduced. Only > the DocBook erroneously defined it as an enum. This patch also fixes that, > please see the DocBook part. > >>> - V4L2_COLORFX_ARBITRARY. >>> >>> The control's type in the documentation is changed from 'enum' to 'menu' >>> - V4L2_CID_COLORFX has always been a menu, not an integer type control. >>> >>> The V4L2_COLORFX_ARBITRARY option enables custom color effects, which >> are >>> impossible or impractical to define as the menu items. For example, the >>> devices may provide coefficients for Cb and Cr manipulation, which yield >>> many permutations, e.g. many slightly different color tints. Such >> devices >>> are better exporting their own API for precise control of non-standard >>> color effects. >> >> I don't understand why you need a number for this, if it's going to use >> another control anyway... ? > > In my use case, the hardware has 3 registers: one of them selects the colour > effect and two others determine Cr, Cb coefficients (probably I could use > V4L2_CID_RED_BALANCE, V4L2_CID_BLUE_BALANCE for that, but so far these are > just private controls). > > If I would have removed the V4L2_COLORFX_ARBITRARY item, another control > would have to be added (let's say boolean V4L2_PRIV_IMG_EFFECT). Just to > enable the "arbitrary" effect. > > Then, to enable the arbitrary effect V4L2_CID_COLORFX would have to be set > to V4L2_COLORFX_NONE, and V4L2_PRIV_IMG_EFFECT to true. > > The CB, CR coefficients are meaningful only when the arbitrary effect is > selected. So having another option in the menu, which drivers can just mask > if they don't support it, appeared better to me. > > It's a bit similar to gain/autogain scenario, where gain is active only when > autogain is off. > Maybe I should just add another private control (V4L2_PRIV_IMG_EFFECT) and > remove V4L2_COLORFX_ARBITRARY item. Instead of an imprecise V4L2_COLORFX_ARBITRARY, I'm considering adding V4L2_COLORFX_CHROMA_BALANCE item and document that it should be used together with V4L2_CID_RED_BALANCE and V4L2_CID_BLUE_BALANCE controls. Would something like this be acceptable ? I'd like to avoid (many) private controls if possible. --- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] V4L: Extend V4L2_CID_COLORFX with more image effects
On 04/17/2012 12:51 PM, Rémi Denis-Courmont wrote: > On Tue, 17 Apr 2012 12:09:42 +0200, Sylwester Nawrocki > wrote: >> This patch adds definition of additional color effects: >> - V4L2_COLORFX_AQUA, >> - V4L2_COLORFX_ART_FREEZE, >> - V4L2_COLORFX_SILHOUETTE, >> - V4L2_COLORFX_SOLARIZATION, >> - V4L2_COLORFX_ANTIQUE, > > There starts to be a lot of different color effects with no obvious way to > determine which ones the current device actually suppots. Should this not > be a menu control instead? Fortunately this has been a menu control, since it was introduced. Only the DocBook erroneously defined it as an enum. This patch also fixes that, please see the DocBook part. >> - V4L2_COLORFX_ARBITRARY. >> >> The control's type in the documentation is changed from 'enum' to 'menu' >> - V4L2_CID_COLORFX has always been a menu, not an integer type control. >> >> The V4L2_COLORFX_ARBITRARY option enables custom color effects, which > are >> impossible or impractical to define as the menu items. For example, the >> devices may provide coefficients for Cb and Cr manipulation, which yield >> many permutations, e.g. many slightly different color tints. Such > devices >> are better exporting their own API for precise control of non-standard >> color effects. > > I don't understand why you need a number for this, if it's going to use > another control anyway... ? In my use case, the hardware has 3 registers: one of them selects the colour effect and two others determine Cr, Cb coefficients (probably I could use V4L2_CID_RED_BALANCE, V4L2_CID_BLUE_BALANCE for that, but so far these are just private controls). If I would have removed the V4L2_COLORFX_ARBITRARY item, another control would have to be added (let's say boolean V4L2_PRIV_IMG_EFFECT). Just to enable the "arbitrary" effect. Then, to enable the arbitrary effect V4L2_CID_COLORFX would have to be set to V4L2_COLORFX_NONE, nnd V4L2_PRIV_IMG_EFFECT to true. The CB, CR coefficients are meaningful only when the arbitrary effect is selected. So having another option in the menu, which drivers can just mask if they don't support it, appeared better to me. It's a bit similar to gain/autogain scenario, where gain is active only when autogain is off. Maybe I should just add another private control (V4L2_PRIV_IMG_EFFECT) and remove V4L2_COLORFX_ARBITRARY item. --- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] V4L: Extend V4L2_CID_COLORFX with more image effects
On Tue, 17 Apr 2012 12:09:42 +0200, Sylwester Nawrocki wrote: > This patch adds definition of additional color effects: > - V4L2_COLORFX_AQUA, > - V4L2_COLORFX_ART_FREEZE, > - V4L2_COLORFX_SILHOUETTE, > - V4L2_COLORFX_SOLARIZATION, > - V4L2_COLORFX_ANTIQUE, There starts to be a lot of different color effects with no obvious way to determine which ones the current device actually suppots. Should this not be a menu control instead? > - V4L2_COLORFX_ARBITRARY. > > The control's type in the documentation is changed from 'enum' to 'menu' > - V4L2_CID_COLORFX has always been a menu, not an integer type control. > > The V4L2_COLORFX_ARBITRARY option enables custom color effects, which are > impossible or impractical to define as the menu items. For example, the > devices may provide coefficients for Cb and Cr manipulation, which yield > many permutations, e.g. many slightly different color tints. Such devices > are better exporting their own API for precise control of non-standard > color effects. I don't understand why you need a number for this, if it's going to use another control anyway... ? -- Rémi Denis-Courmont Sent from my collocated server -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/15] V4L: Extend V4L2_CID_COLORFX with more image effects
This patch adds definition of additional color effects:
- V4L2_COLORFX_AQUA,
- V4L2_COLORFX_ART_FREEZE,
- V4L2_COLORFX_SILHOUETTE,
- V4L2_COLORFX_SOLARIZATION,
- V4L2_COLORFX_ANTIQUE,
- V4L2_COLORFX_ARBITRARY.
The control's type in the documentation is changed from 'enum' to 'menu'
- V4L2_CID_COLORFX has always been a menu, not an integer type control.
The V4L2_COLORFX_ARBITRARY option enables custom color effects, which are
impossible or impractical to define as the menu items. For example, the
devices may provide coefficients for Cb and Cr manipulation, which yield
many permutations, e.g. many slightly different color tints. Such devices
are better exporting their own API for precise control of non-standard
color effects.
Signed-off-by: Sylwester Nawrocki
Signed-off-by: Kyungmin Park
---
Documentation/DocBook/media/v4l/controls.xml | 91 ++
drivers/media/video/v4l2-ctrls.c |6 ++
include/linux/videodev2.h| 26 +---
3 files changed, 100 insertions(+), 23 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/controls.xml
b/Documentation/DocBook/media/v4l/controls.xml
index 6e2a2c6..0e2040d 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -284,19 +284,84 @@ minimum value disables backlight compensation.
V4L2_CID_COLORFX
- enum
- Selects a color effect. Possible values for
-enum v4l2_colorfx are:
-V4L2_COLORFX_NONE (0),
-V4L2_COLORFX_BW (1),
-V4L2_COLORFX_SEPIA (2),
-V4L2_COLORFX_NEGATIVE (3),
-V4L2_COLORFX_EMBOSS (4),
-V4L2_COLORFX_SKETCH (5),
-V4L2_COLORFX_SKY_BLUE (6),
-V4L2_COLORFX_GRASS_GREEN (7),
-V4L2_COLORFX_SKIN_WHITEN (8) and
-V4L2_COLORFX_VIVID (9).
+ menu
+ Selects a color effect. The following values are defined:
+
+
+
+
+
+
+
+ V4L2_COLORFX_NONE
+ Color effect is disabled.
+
+
+ V4L2_COLORFX_ANTIQUE
+ An aging (old photo) effect.
+
+
+
V4L2_COLORFX_ART_FREEZE
+ Frost color effect.
+
+
+ V4L2_COLORFX_AQUA
+ Water color, cool tone.
+
+
+ V4L2_COLORFX_BW
+ Black and white.
+
+
+ V4L2_COLORFX_EMBOSS
+ Emboss, the highlights and shadows replace light/dark
boundaries
+ and low contrast areas are set to a gray background.
+
+
+
V4L2_COLORFX_GRASS_GREEN
+ Grass green.
+
+
+
V4L2_COLORFX_NEGATIVE
+ Negative.
+
+
+ V4L2_COLORFX_SEPIA
+ Sepia tone.
+
+
+ V4L2_COLORFX_SKETCH
+ Sketch.
+
+
+
V4L2_COLORFX_SKIN_WHITEN
+ Skin whiten.
+
+
+
V4L2_COLORFX_SKY_BLUE
+ Sky blue.
+
+
+
V4L2_COLORFX_SOLARIZATION
+ Solarization, the image is partially reversed in tone,
+ only color values above or below a certain threshold are
inverted.
+
+
+
+
V4L2_COLORFX_SILHOUETTE
+ Silhouette (outline).
+
+
+ V4L2_COLORFX_VIVID
+ Vivid colors.
+
+
+
V4L2_COLORFX_ARBITRARY
+ Arbitrary color effect, determined by other means, e.g.
+with driver private controls.
+
+
+
V4L2_CID_ROTATE
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 34ff32d..13e6e8d 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -241,6 +241,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
"Grass Green",
"Skin Whiten",
"Vivid",
+ "Aqua",
+ "Art Freeze",
+ "Silhouette",
+ "Solarization",
+ "Antique",
+ "Arbitrary",
NULL
};
static const char * const tune_preemphasis[] = {
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 2503857..95ce588 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1269,16 +1269,22 @@ enum v4l2_power_line_frequency {
#define V4L2_CID_COLOR_KILLER (V4L2_CID_BA
