Patch for Brightness Contrast Hue and Saturation

Boris Borisov

unread,

Mar 18, 2009, 6:37:53 PM3/18/09

to micr...@googlegroups.com

Hello,
Try this patch. I test with amsn and all seems OK. Hue, saturation
brightness and contrast is controlled as I expect.

hue_sat.patch

Stefan Krastanov

unread,

Mar 18, 2009, 7:06:30 PM3/18/09

to micr...@googlegroups.com

The patch is working great on my cam 0C45:6270 MT9V011.
Just the default hue is just a little bit green.

2009/3/18 Boris Borisov <lz1...@gmail.com>

Hello,
Try this patch. I test with amsn and all seems OK. Hue, saturation
brightness and contrast is controlled as I expect.

diff --git a/sn9c20x-bridge.c b/sn9c20x-bridge.c
index dfbf87f..144fe1e 100644
--- a/sn9c20x-bridge.c
+++ b/sn9c20x-bridge.c
@@ -30,6 +30,290 @@
#include "sn9c20x.h"
#include "sn9c20x-bridge.h"

+
+//Coordinate aray for eliptic HSV corrections
+
+ int RX[]={ 41, 44, 46, 48, 50, 52, 54, 56,
+ 58, 60, 62, 64, 66, 68, 70, 72,
+ 74, 76, 78, 80, 81, 83, 85, 87,
+ 88, 90, 92, 93, 95, 97, 98, 100,
+ 101, 102, 104, 105, 107, 108, 109, 110,
+ 112, 113, 114, 115, 116, 117, 118, 119,
+ 120, 121, 122, 123, 123, 124, 125, 125,
+ 126, 127, 127, 128, 128, 129, 129, 129,
+ 130, 130, 130, 130, 131, 131, 131, 131,
+ 131, 131, 131, 131, 130, 130, 130, 130,
+ 129, 129, 129, 128, 128, 127, 127, 126,
+ 125, 125, 124, 123, 122, 122, 121, 120,
+ 119, 118, 117, 116, 115, 114, 112, 111,
+ 110, 109, 107, 106, 105, 103, 102, 101,
+ 99, 98, 96, 94, 93, 91, 90, 88,
+ 86, 84, 83, 81, 79, 77, 75, 74,
+ 72, 70, 68, 66, 64, 62, 60, 58,
+ 56, 54, 52, 49, 47, 45, 43, 41,
+ 39, 36, 34, 32, 30, 28, 25, 23,
+ 21, 19, 16, 14, 12, 9, 7, 5,
+ 3, 0, -1, -3, -6, -8, -10, -12,
+ -15, -17, -19, -22, -24, -26, -28, -30,
+ -33, -35, -37, -39, -41, -44, -46, -48,
+ -50, -52, -54, -56, -58, -60, -62, -64,
+ -66, -68, -70, -72, -74, -76, -78, -80,
+ -81, -83, -85, -87, -88, -90, -92, -93,
+ -95, -97, -98,-100,-101,-102,-104,-105,
+ -107,-108,-109,-110,-112,-113,-114,-115,
+ -116,-117,-118,-119,-120,-121,-122,-123,
+ -123,-124,-125,-125,-126,-127,-127,-128,
+ -128,-128,-128,-128,-128,-128,-128,-128,
+ -128,-128,-128,-128,-128,-128,-128,-128,
+ -128,-128,-128,-128,-128,-128,-128,-128,
+ -128,-127,-127,-126,-125,-125,-124,-123,
+ -122,-122,-121,-120,-119,-118,-117,-116,
+ -115,-114,-112,-111,-110,-109,-107,-106,
+ -105,-103,-102,-101, -99, -98, -96, -94,
+ -93, -91, -90, -88, -86, -84, -83, -81,
+ -79, -77, -75, -74, -72, -70, -68, -66,
+ -64, -62, -60, -58, -56, -54, -52, -49,
+ -47, -45, -43, -41, -39, -36, -34, -32,
+ -30, -28, -25, -23, -21, -19, -16, -14,
+ -12, -9, -7, -5, -3, 0, 1, 3,
+ 6, 8, 10, 12, 15, 17, 19, 22,
+ 24, 26, 28, 30, 33, 35, 37, 39, 41};
+
+ int RY[]={ 82, 80, 78, 76, 74, 73, 71, 69,
+ 67, 65, 63, 61, 58, 56, 54, 52,
+ 50, 48, 46, 44, 41, 39, 37, 35,
+ 32, 30, 28, 26, 23, 21, 19, 16,
+ 14, 12, 10, 7, 5, 3, 0, -1,
+ -3, -6, -8, -10, -13, -15, -17, -19,
+ -22, -24, -26, -29, -31, -33, -35, -38,
+ -40, -42, -44, -46, -48, -51, -53, -55,
+ -57, -59, -61, -63, -65, -67, -69, -71,
+ -73, -75, -77, -79, -81, -82, -84, -86,
+ -88, -89, -91, -93, -94, -96, -98, -99,
+ -101,-102,-104,-105,-106,-108,-109,-110,
+ -112,-113,-114,-115,-116,-117,-119,-120,
+ -120,-121,-122,-123,-124,-125,-126,-126,
+ -127,-128,-128,-128,-128,-128,-128,-128,
+ -128,-128,-128,-128,-128,-128,-128,-128,
+ -128,-128,-128,-128,-128,-128,-128,-128,
+ -128,-128,-128,-128,-128,-128,-128,-128,
+ -127,-127,-126,-125,-125,-124,-123,-122,
+ -121,-120,-119,-118,-117,-116,-115,-114,
+ -113,-111,-110,-109,-107,-106,-105,-103,
+ -102,-100, -99, -97, -96, -94, -92, -91,
+ -89, -87, -85, -84, -82, -80, -78, -76,
+ -74, -73, -71, -69, -67, -65, -63, -61,
+ -58, -56, -54, -52, -50, -48, -46, -44,
+ -41, -39, -37, -35, -32, -30, -28, -26,
+ -23, -21, -19, -16, -14, -12, -10, -7,
+ -5, -3, 0, 1, 3, 6, 8, 10,
+ 13, 15, 17, 19, 22, 24, 26, 29,
+ 31, 33, 35, 38, 40, 42, 44, 46,
+ 48, 51, 53, 55, 57, 59, 61, 63,
+ 65, 67, 69, 71, 73, 75, 77, 79,
+ 81, 82, 84, 86, 88, 89, 91, 93,
+ 94, 96, 98, 99, 101, 102, 104, 105,
+ 106, 108, 109, 110, 112, 113, 114, 115,
+ 116, 117, 119, 120, 120, 121, 122, 123,
+ 124, 125, 126, 126, 127, 128, 128, 129,
+ 129, 130, 130, 131, 131, 131, 131, 132,
+ 132, 132, 132, 132, 132, 132, 132, 132,
+ 132, 132, 132, 131, 131, 131, 130, 130,
+ 130, 129, 129, 128, 127, 127, 126, 125,
+ 125, 124, 123, 122, 121, 120, 119, 118,
+ 117, 116, 115, 114, 113, 111, 110, 109,
+ 107, 106, 105, 103, 102, 100, 99, 97,
+ 96, 94, 92, 91, 89, 87, 85, 84, 82};
+
+
+ int GX[]={-124,-124,-125,-125,-125,-125,-125,-125,
+ -125,-126,-126,-125,-125,-125,-125,-125,
+ -125,-124,-124,-124,-123,-123,-122,-122,
+ -121,-121,-120,-120,-119,-118,-117,-117,
+ -116,-115,-114,-113,-112,-111,-110,-109,
+ -108,-107,-105,-104,-103,-102,-100, -99,
+ -98, -96, -95, -93, -92, -91, -89, -87,
+ -86, -84, -83, -81, -79, -77, -76, -74,
+ -72, -70, -69, -67, -65, -63, -61, -59,
+ -57, -55, -53, -51, -49, -47, -45, -43,
+ -41, -39, -37, -35, -33, -30, -28, -26,
+ -24, -22, -20, -18, -15, -13, -11, -9,
+ -7, -4, -2, 0, 1, 3, 6, 8,
+ 10, 12, 14, 17, 19, 21, 23, 25,
+ 27, 29, 32, 34, 36, 38, 40, 42,
+ 44, 46, 48, 50, 52, 54, 56, 58,
+ 60, 62, 64, 66, 68, 70, 71, 73,
+ 75, 77, 78, 80, 82, 83, 85, 87,
+ 88, 90, 91, 93, 94, 96, 97, 98,
+ 100, 101, 102, 104, 105, 106, 107, 108,
+ 109, 111, 112, 113, 113, 114, 115, 116,
+ 117, 118, 118, 119, 120, 120, 121, 122,
+ 122, 123, 123, 124, 124, 124, 125, 125,
+ 125, 125, 125, 125, 125, 126, 126, 125,
+ 125, 125, 125, 125, 125, 124, 124, 124,
+ 123, 123, 122, 122, 121, 121, 120, 120,
+ 119, 118, 117, 117, 116, 115, 114, 113,
+ 112, 111, 110, 109, 108, 107, 105, 104,
+ 103, 102, 100, 99, 98, 96, 95, 93,
+ 92, 91, 89, 87, 86, 84, 83, 81,
+ 79, 77, 76, 74, 72, 70, 69, 67,
+ 65, 63, 61, 59, 57, 55, 53, 51,
+ 49, 47, 45, 43, 41, 39, 37, 35,
+ 33, 30, 28, 26, 24, 22, 20, 18,
+ 15, 13, 11, 9, 7, 4, 2, 0,
+ -1, -3, -6, -8, -10, -12, -14, -17,
+ -19, -21, -23, -25, -27, -29, -32, -34,
+ -36, -38, -40, -42, -44, -46, -48, -50,
+ -52, -54, -56, -58, -60, -62, -64, -66,
+ -68, -70, -71, -73, -75, -77, -78, -80,
+ -82, -83, -85, -87, -88, -90, -91, -93,
+ -94, -96, -97, -98,-100,-101,-102,-104,
+ -105,-106,-107,-108,-109,-111,-112,-113,
+ -113,-114,-115,-116,-117,-118,-118,-119,
+ -120,-120,-121,-122,-122,-123,-123,-124,-124};
+
+ int GY[]={-100, -99, -98, -97, -95, -94, -93, -91,
+ -90, -89, -87, -86, -84, -83, -81, -80,
+ -78, -76, -75, -73, -71, -70, -68, -66,
+ -64, -63, -61, -59, -57, -55, -53, -51,
+ -49, -48, -46, -44, -42, -40, -38, -36,
+ -34, -32, -30, -27, -25, -23, -21, -19,
+ -17, -15, -13, -11, -9, -7, -4, -2,
+ 0, 1, 3, 5, 7, 9, 11, 14,
+ 16, 18, 20, 22, 24, 26, 28, 30,
+ 32, 34, 36, 38, 40, 42, 44, 46,
+ 48, 50, 52, 54, 56, 58, 59, 61,
+ 63, 65, 67, 68, 70, 72, 74, 75,
+ 77, 78, 80, 82, 83, 85, 86, 88,
+ 89, 90, 92, 93, 95, 96, 97, 98,
+ 100, 101, 102, 103, 104, 105, 106, 107,
+ 108, 109, 110, 111, 112, 112, 113, 114,
+ 115, 115, 116, 116, 117, 117, 118, 118,
+ 119, 119, 119, 120, 120, 120, 120, 120,
+ 121, 121, 121, 121, 121, 121, 120, 120,
+ 120, 120, 120, 119, 119, 119, 118, 118,
+ 117, 117, 116, 116, 115, 114, 114, 113,
+ 112, 111, 111, 110, 109, 108, 107, 106,
+ 105, 104, 103, 102, 100, 99, 98, 97,
+ 95, 94, 93, 91, 90, 89, 87, 86,
+ 84, 83, 81, 80, 78, 76, 75, 73,
+ 71, 70, 68, 66, 64, 63, 61, 59,
+ 57, 55, 53, 51, 49, 48, 46, 44,
+ 42, 40, 38, 36, 34, 32, 30, 27,
+ 25, 23, 21, 19, 17, 15, 13, 11,
+ 9, 7, 4, 2, 0, -1, -3, -5,
+ -7, -9, -11, -14, -16, -18, -20, -22,
+ -24, -26, -28, -30, -32, -34, -36, -38,
+ -40, -42, -44, -46, -48, -50, -52, -54,
+ -56, -58, -59, -61, -63, -65, -67, -68,
+ -70, -72, -74, -75, -77, -78, -80, -82,
+ -83, -85, -86, -88, -89, -90, -92, -93,
+ -95, -96, -97, -98,-100,-101,-102,-103,
+ -104,-105,-106,-107,-108,-109,-110,-111,
+ -112,-112,-113,-114,-115,-115,-116,-116,
+ -117,-117,-118,-118,-119,-119,-119,-120,
+ -120,-120,-120,-120,-121,-121,-121,-121,
+ -121,-121,-120,-120,-120,-120,-120,-119,
+ -119,-119,-118,-118,-117,-117,-116,-116,
+ -115,-114,-114,-113,-112,-111,-111,-110,
+ -109,-108,-107,-106,-105,-104,-103,-102,-100};
+
+ int BX[]={ 112, 113, 114, 114, 115, 116, 117, 117,
+ 118, 118, 119, 119, 120, 120, 120, 121,
+ 121, 121, 122, 122, 122, 122, 122, 122,
+ 122, 122, 122, 122, 122, 122, 121, 121,
+ 121, 120, 120, 120, 119, 119, 118, 118,
+ 117, 116, 116, 115, 114, 113, 113, 112,
+ 111, 110, 109, 108, 107, 106, 105, 104,
+ 103, 102, 100, 99, 98, 97, 95, 94,
+ 93, 91, 90, 88, 87, 85, 84, 82,
+ 80, 79, 77, 76, 74, 72, 70, 69,
+ 67, 65, 63, 61, 60, 58, 56, 54,
+ 52, 50, 48, 46, 44, 42, 40, 38,
+ 36, 34, 32, 30, 28, 26, 24, 22,
+ 19, 17, 15, 13, 11, 9, 7, 5,
+ 2, 0, -1, -3, -5, -7, -9, -12,
+ -14, -16, -18, -20, -22, -24, -26, -28,
+ -31, -33, -35, -37, -39, -41, -43, -45,
+ -47, -49, -51, -53, -54, -56, -58, -60,
+ -62, -64, -66, -67, -69, -71, -73, -74,
+ -76, -78, -79, -81, -83, -84, -86, -87,
+ -89, -90, -92, -93, -94, -96, -97, -98,
+ -99,-101,-102,-103,-104,-105,-106,-107,
+ -108,-109,-110,-111,-112,-113,-114,-114,
+ -115,-116,-117,-117,-118,-118,-119,-119,
+ -120,-120,-120,-121,-121,-121,-122,-122,
+ -122,-122,-122,-122,-122,-122,-122,-122,
+ -122,-122,-121,-121,-121,-120,-120,-120,
+ -119,-119,-118,-118,-117,-116,-116,-115,
+ -114,-113,-113,-112,-111,-110,-109,-108,
+ -107,-106,-105,-104,-103,-102,-100, -99,
+ -98, -97, -95, -94, -93, -91, -90, -88,
+ -87, -85, -84, -82, -80, -79, -77, -76,
+ -74, -72, -70, -69, -67, -65, -63, -61,
+ -60, -58, -56, -54, -52, -50, -48, -46,
+ -44, -42, -40, -38, -36, -34, -32, -30,
+ -28, -26, -24, -22, -19, -17, -15, -13,
+ -11, -9, -7, -5, -2, 0, 1, 3,
+ 5, 7, 9, 12, 14, 16, 18, 20,
+ 22, 24, 26, 28, 31, 33, 35, 37,
+ 39, 41, 43, 45, 47, 49, 51, 53,
+ 54, 56, 58, 60, 62, 64, 66, 67,
+ 69, 71, 73, 74, 76, 78, 79, 81,
+ 83, 84, 86, 87, 89, 90, 92, 93,
+ 94, 96, 97, 98, 99, 101, 102, 103,
+ 104, 105, 106, 107, 108, 109, 110, 111, 112};
+
+
+ int BY[]={ -11, -13, -15, -17, -19, -21, -23, -25,
+ -27, -29, -31, -33, -35, -37, -39, -41,
+ -43, -45, -46, -48, -50, -52, -54, -55,
+ -57, -59, -61, -62, -64, -66, -67, -69,
+ -71, -72, -74, -75, -77, -78, -80, -81,
+ -83, -84, -86, -87, -88, -90, -91, -92,
+ -93, -95, -96, -97, -98, -99,-100,-101,
+ -102,-103,-104,-105,-106,-106,-107,-108,
+ -109,-109,-110,-111,-111,-112,-112,-113,
+ -113,-114,-114,-114,-115,-115,-115,-115,
+ -116,-116,-116,-116,-116,-116,-116,-116,
+ -116,-115,-115,-115,-115,-114,-114,-114,
+ -113,-113,-112,-112,-111,-111,-110,-110,
+ -109,-108,-108,-107,-106,-105,-104,-103,
+ -102,-101,-100, -99, -98, -97, -96, -95,
+ -94, -93, -91, -90, -89, -88, -86, -85,
+ -84, -82, -81, -79, -78, -76, -75, -73,
+ -71, -70, -68, -67, -65, -63, -62, -60,
+ -58, -56, -55, -53, -51, -49, -47, -45,
+ -44, -42, -40, -38, -36, -34, -32, -30,
+ -28, -26, -24, -22, -20, -18, -16, -14,
+ -12, -10, -8, -6, -4, -2, 0, 1,
+ 3, 5, 7, 9, 11, 13, 15, 17,
+ 19, 21, 23, 25, 27, 29, 31, 33,
+ 35, 37, 39, 41, 43, 45, 46, 48,
+ 50, 52, 54, 55, 57, 59, 61, 62,
+ 64, 66, 67, 69, 71, 72, 74, 75,
+ 77, 78, 80, 81, 83, 84, 86, 87,
+ 88, 90, 91, 92, 93, 95, 96, 97,
+ 98, 99, 100, 101, 102, 103, 104, 105,
+ 106, 106, 107, 108, 109, 109, 110, 111,
+ 111, 112, 112, 113, 113, 114, 114, 114,
+ 115, 115, 115, 115, 116, 116, 116, 116,
+ 116, 116, 116, 116, 116, 115, 115, 115,
+ 115, 114, 114, 114, 113, 113, 112, 112,
+ 111, 111, 110, 110, 109, 108, 108, 107,
+ 106, 105, 104, 103, 102, 101, 100, 99,
+ 98, 97, 96, 95, 94, 93, 91, 90,
+ 89, 88, 86, 85, 84, 82, 81, 79,
+ 78, 76, 75, 73, 71, 70, 68, 67,
+ 65, 63, 62, 60, 58, 56, 55, 53,
+ 51, 49, 47, 45, 44, 42, 40, 38,
+ 36, 34, 32, 30, 28, 26, 24, 22,
+ 20, 18, 16, 14, 12, 10, 8, 6,
+ 4, 2, 0, -1, -3, -5, -7, -9, -11};
+
+
+
+
int sn9c20x_set_camera_control(struct usb_sn9c20x *dev,
__u32 control, __u32 value)
{
@@ -53,6 +337,18 @@ int sn9c20x_set_camera_control(struct usb_sn9c20x *dev,
ret = dev->camera.set_gamma(dev);
}
break;
+ case V4L2_CID_SATURATION:
+ if (dev->camera.set_saturation) {
+ dev->vsettings.colour = value;
+ ret = dev->camera.set_saturation(dev);
+ }
+ break;
+ case V4L2_CID_HUE:
+ if (dev->camera.set_hue) {
+ dev->vsettings.hue = value;
+ ret = dev->camera.set_hue(dev);
+ }
+ break;
case V4L2_CID_SHARPNESS:
if (dev->camera.set_sharpness) {
dev->vsettings.sharpness = value;
@@ -483,33 +779,101 @@ int sn9c20x_write_i2c_array(struct usb_sn9c20x *dev,
}

/**
- * @brief Set contrast inside sn9c20x chip
+ * @brief Calculate and set optical parameters
*
- * @author Comer352l
+ * @author Comer352l, Boris Borisov
*
* @param dev Pointer to the device
*
* @return Zero (success) or negative (USB-error value)
*
*/
-int sn9c20x_set_contrast(struct usb_sn9c20x *dev)
+int sn9c20x_set_optical_parameters(struct usb_sn9c20x *dev)
{
- /* from 0x26 to 0x4b */
- __u8 brightness_contrast[21] = {0x16, 0x0, 0x2b, 0x0, 0x8, 0x0, 0xf6, 0x0f,
+ __u8 optical_parameters[21]= {0x16, 0x0, 0x2b, 0x0, 0x8, 0x0, 0xf6, 0x0f,
0xd2, 0x0f, 0x38, 0x0, 0x34, 0x0, 0xcf, 0x0f,
0xfd, 0x0f, 0x0, 0x0, 0x0};
+
+
__u8 contrast_val = (dev->vsettings.contrast) * 0x25 / 0x100;
__u8 brightness_val = dev->vsettings.brightness;
+ __u8 ret=0;
+// __u8 value;
+// __u8 i;
+ long tmp_coordinate;

+
+ if(dev->vsettings.hue<0)
+ dev->vsettings.hue=0;
+
+ if(dev->vsettings.hue>359)
+ dev->vsettings.hue=360;
+
+ //Brighness and contrast settings
brightness_val -= 0x80;
- brightness_contrast[18] = brightness_val;
+ optical_parameters[18] = brightness_val;

contrast_val += 0x26;
- brightness_contrast[2] = contrast_val;
- brightness_contrast[0] = 0x13 + (brightness_contrast[2] - 0x26) * 0x13 / 0x25;
- brightness_contrast[4] = 0x7 + (brightness_contrast[2] - 0x26) * 0x7 / 0x25;
+ optical_parameters[2] = contrast_val;
+ optical_parameters[0] = 0x13 + (optical_parameters[2] - 0x26) * 0x13 / 0x25;
+ optical_parameters[4] = 0x7 + (optical_parameters[2] - 0x26) * 0x7 / 0x25;
+
+ //Calculate HUE and SATURATION
+ tmp_coordinate=(long)(RX[dev->vsettings.hue]);
+ tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
+ optical_parameters[6]=(unsigned char)(tmp_coordinate&0xff);
+ optical_parameters[7]=(unsigned char)((tmp_coordinate>>8)&0x0f);
+
+ tmp_coordinate=(long)(RY[dev->vsettings.hue]);
+ tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
+ optical_parameters[8]=(unsigned char)(tmp_coordinate&0xff);
+ optical_parameters[9]=(unsigned char)((tmp_coordinate>>8)&0x0f);
+
+ tmp_coordinate=(long)(GX[dev->vsettings.hue]);
+ tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
+ optical_parameters[10]=(unsigned char)(tmp_coordinate&0xff);
+ optical_parameters[11]=(unsigned char)((tmp_coordinate>>8)&0x0f);
+
+ tmp_coordinate=(long)(GY[dev->vsettings.hue]);
+ tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
+ optical_parameters[12]=(unsigned char)(tmp_coordinate&0xff);
+ optical_parameters[13]=(unsigned char)((tmp_coordinate>>8)&0x0f);
+
+ tmp_coordinate=(long)(BX[dev->vsettings.hue]);
+ tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
+ optical_parameters[14]=(unsigned char)(tmp_coordinate&0xff);
+ optical_parameters[15]=(unsigned char)((tmp_coordinate>>8)&0x0f);
+
+ tmp_coordinate=(long)(BY[dev->vsettings.hue]);
+ tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
+ optical_parameters[16]=(unsigned char)(tmp_coordinate&0xff);
+ optical_parameters[17]=(unsigned char)((tmp_coordinate>>8)&0x0f);
+
+
+ ret=0;
+
+// for(i=0;i<21;i++)
+// {
+// value=optical_parameters[i];
+// ret|=usb_sn9c20x_control_write(dev, 0x10e1+i, &value, 1);
+// }
+ ret=usb_sn9c20x_control_write(dev, 0x10e1, optical_parameters, 21);
+ return(ret);
+}

- return usb_sn9c20x_control_write(dev, 0x10e1, brightness_contrast, 21);
+/**
+ * @brief Set contrast inside sn9c20x chip
+ *
+ * @author Comer352l
+ *
+ * @param dev Pointer to the device
+ *
+ * @return Zero (success) or negative (USB-error value)
+ *
+ */
+int sn9c20x_set_contrast(struct usb_sn9c20x *dev)
+{
+ return sn9c20x_set_optical_parameters(dev);
}

/**
@@ -526,7 +890,7 @@ int sn9c20x_set_contrast(struct usb_sn9c20x *dev)
*/
int sn9c20x_set_brightness(struct usb_sn9c20x *dev)
{
- return sn9c20x_set_contrast(dev);
+ return sn9c20x_set_optical_parameters(dev);
}

/**
@@ -571,6 +935,36 @@ int sn9c20x_set_gamma(struct usb_sn9c20x *dev)
}

/**
+ * @brief Set saturation inside sn9c20x chip
+ *
+ * @author Neekhil
+ *
+ * @param dev Pointer to the device
+ *
+ * @return Zero (success) or negative (USB-error value)
+ *
+ */
+int sn9c20x_set_saturation(struct usb_sn9c20x *dev)
+{
+ return sn9c20x_set_optical_parameters(dev);
+}
+
+/**
+ * @brief Set hue inside sn9c20x chip
+ *
+ * @author Boris Borisov
+ *
+ * @param dev Pointer to the device
+ *
+ * @return Zero (success) or negative (USB-error value)
+ *
+ */
+int sn9c20x_set_hue(struct usb_sn9c20x *dev)
+{
+ return sn9c20x_set_optical_parameters(dev);
+}
+
+/**
* @brief Set sharpness inside sn9c20x chip
*
* @author Comer352l
@@ -906,9 +1300,11 @@ int sn9c20x_initialize(struct usb_sn9c20x *dev)
dev->camera.set_contrast = sn9c20x_set_contrast;
dev->camera.set_brightness = sn9c20x_set_brightness;
dev->camera.set_gamma = sn9c20x_set_gamma;
+ dev->camera.set_saturation = sn9c20x_set_saturation;
dev->camera.set_sharpness = sn9c20x_set_sharpness;
dev->camera.set_red_gain = sn9c20x_set_red_gain;
dev->camera.set_blue_gain = sn9c20x_set_blue_gain;
+ dev->camera.set_hue = sn9c20x_set_hue;

ret = sn9c20x_i2c_initialize(dev);
if (ret < 0)
diff --git a/sn9c20x-dev.c b/sn9c20x-dev.c
index 11ac1f3..dd1eabf 100644
--- a/sn9c20x-dev.c
+++ b/sn9c20x-dev.c
@@ -173,7 +173,6 @@ int sn9c20x_initialize_sensor(struct usb_sn9c20x *dev)
sn9c20x_write_i2c_array(dev, ov7660_init, 0);
dev->camera.set_exposure = ov_set_exposure;
dev->camera.set_auto_gain = ov_set_autogain;
- dev->camera.set_gain = ov9650_set_gain;
dev->camera.hstart = 1;
dev->camera.vstart = 1;
UDIA_INFO("Detected OV7660 Sensor.\n");
diff --git a/sn9c20x-sysfs.c b/sn9c20x-sysfs.c
index 15bb13c..d9190e0 100644
--- a/sn9c20x-sysfs.c
+++ b/sn9c20x-sysfs.c
@@ -400,30 +400,32 @@ static ssize_t store_contrast(struct device *class, struct device_attribute *att
}

+
+
/**
- * @brief show_whitebalance
+ * @brief show_saturation
*
* @param class Class device
* @param attr
- * @retval buf Adress of buffer with the 'whitebalance' value
+ * @retval buf Adress of buffer with the 'contrast' value
*
* @returns Size of buffer
*/
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)
-static ssize_t show_gamma(struct class_device *class, char *buf)
+static ssize_t show_saturation(struct class_device *class, char *buf)
#else
-static ssize_t show_gamma(struct device *class, struct device_attribute *attr, char *buf)
+static ssize_t show_saturation(struct device *class, struct device_attribute *attr, char *buf)
#endif
{
struct video_device *vdev = to_video_device(class);
struct usb_sn9c20x *dev = video_get_drvdata(vdev);

- return sprintf(buf, "%X\n", dev->vsettings.gamma);
+ return sprintf(buf, "%X\n", dev->vsettings.colour);
}

/**
- * @brief store_gamma
+ * @brief store_saturation
*
* @param class Class device
* @param buf Buffer
@@ -433,9 +435,9 @@ static ssize_t show_gamma(struct device *class, struct device_attribute *attr, c
* @returns Size of buffer
*/
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)
-static ssize_t store_gamma(struct class_device *class, const char *buf, size_t count)
+static ssize_t store_saturation(struct class_device *class, const char *buf, size_t count)
#else
-static ssize_t store_gamma(struct device *class, struct device_attribute *attr,
+static ssize_t store_saturation(struct device *class, struct device_attribute *attr,
const char *buf, size_t count)
#endif
{
@@ -448,37 +450,38 @@ static ssize_t store_gamma(struct device *class, struct device_attribute *attr,
value = simple_strtoul(buf, &endp, 16);

sn9c20x_set_camera_control(dev,
- V4L2_CID_GAMMA,
+ V4L2_CID_SATURATION,
value);

return strlen(buf);
}

-/*
- * @brief show_colour
+
+/**
+ * @brief show_hue
*
* @param class Class device
* @param attr
- * @retval buf Adress of buffer with the 'colour' value
+ * @retval buf Adress of buffer with the 'contrast' value
*
* @returns Size of buffer
*/
-/*#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)
-static ssize_t show_colour(struct class_device *class, char *buf)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)
+static ssize_t show_hue(struct class_device *class, char *buf)
#else
-static ssize_t show_colour(struct device *class, struct device_attribute *attr, char *buf)
+static ssize_t show_hue(struct device *class, struct device_attribute *attr, char *buf)
#endif
{
struct video_device *vdev = to_video_device(class);
struct usb_sn9c20x *dev = video_get_drvdata(vdev);

- return sprintf(buf, "%X\n", dev->vsettings.colour);
+ return sprintf(buf, "%X\n", dev->vsettings.hue);
}
-*/

-/*
- * @brief store_colour
+
+/**
+ * @brief store_hue
*
* @param class Class device
* @param buf Buffer
@@ -487,10 +490,10 @@ static ssize_t show_colour(struct device *class, struct device_attribute *attr,
*
* @returns Size of buffer
*/
-/*#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)
-static ssize_t store_colour(struct class_device *class, const char *buf, size_t count)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)
+static ssize_t store_hue(struct class_device *class, const char *buf, size_t count)
#else
-static ssize_t store_colour(struct device *class, struct device_attribute *attr,
+static ssize_t store_hue(struct device *class, struct device_attribute *attr,
const char *buf, size_t count)
#endif
{
@@ -502,13 +505,72 @@ static ssize_t store_colour(struct device *class, struct device_attribute *attr,

value = simple_strtoul(buf, &endp, 16);

- dev->vsettings.colour = (int) value;
+ sn9c20x_set_camera_control(dev,
+ V4L2_CID_HUE,
+ value);
+
+ return strlen(buf);
+}

-/ dev_sn9c20x_set_camera_quality(dev); /
+/////////////////////////////////////
+
+
+
+/**
+ * @brief show_whitebalance
+ *
+ * @param class Class device
+ * @param attr
+ * @retval buf Adress of buffer with the 'whitebalance' value
+ *
+ * @returns Size of buffer
+ */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)
+static ssize_t show_gamma(struct class_device *class, char *buf)
+#else
+static ssize_t show_gamma(struct device *class, struct device_attribute *attr, char *buf)
+#endif
+{
+ struct video_device *vdev = to_video_device(class);
+ struct usb_sn9c20x *dev = video_get_drvdata(vdev);
+
+ return sprintf(buf, "%X\n", dev->vsettings.gamma);
+}
+
+
+/**
+ * @brief store_gamma
+ *
+ * @param class Class device
+ * @param buf Buffer
+ * @param count Counter
+ * @param attr
+ *
+ * @returns Size of buffer
+ */
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)
+static ssize_t store_gamma(struct class_device *class, const char *buf, size_t count)
+#else
+static ssize_t store_gamma(struct device *class, struct device_attribute *attr,
+ const char *buf, size_t count)
+#endif
+{
+ char *endp;
+ unsigned long value;
+
+ struct video_device *vdev = to_video_device(class);
+ struct usb_sn9c20x *dev = video_get_drvdata(vdev);
+
+ value = simple_strtoul(buf, &endp, 16);
+
+ sn9c20x_set_camera_control(dev,
+ V4L2_CID_GAMMA,
+ value);

return strlen(buf);
}
-*/
+
+

/**
@@ -802,10 +864,11 @@ static CLASS_DEVICE_ATTR(fps, S_IRUGO, show_fps, NULL); /**< FPS value */
static CLASS_DEVICE_ATTR(brightness, S_IRUGO | S_IWUGO, show_brightness, store_brightness); /**< Brightness value */
static CLASS_DEVICE_ATTR(exposure, S_IRUGO | S_IWUGO, show_exposure, store_exposure); /**< Exposure value */
static CLASS_DEVICE_ATTR(gain, S_IRUGO | S_IWUGO, show_gain, store_gain); /**< Gain value */
+static CLASS_DEVICE_ATTR(saturation, S_IRUGO | S_IWUGO, show_saturation, store_saturation); /**< Saturation value */
+static CLASS_DEVICE_ATTR(hue, S_IRUGO | S_IWUGO, show_hue, store_hue); /**< Hue value */
static CLASS_DEVICE_ATTR(contrast, S_IRUGO | S_IWUGO, show_contrast, store_contrast); /**< Contrast value */
static CLASS_DEVICE_ATTR(gamma, S_IRUGO | S_IWUGO, show_gamma, store_gamma); /**< Gamma value */
static CLASS_DEVICE_ATTR(sharpness, S_IRUGO | S_IWUGO, show_sharpness, store_sharpness); /**< Sharpness value */
-/*static CLASS_DEVICE_ATTR(colour, S_IRUGO | S_IWUGO, show_colour, store_colour); /< Hue value */
static CLASS_DEVICE_ATTR(hflip, S_IRUGO | S_IWUGO, show_hflip, store_hflip); /**< Horizontal flip value */
static CLASS_DEVICE_ATTR(vflip, S_IRUGO | S_IWUGO, show_vflip, store_vflip); /**< Vertical flip value */
static CLASS_DEVICE_ATTR(auto_exposure, S_IRUGO | S_IWUGO, show_autoexposure, store_autoexposure); /**< Automatic exposure control value */
@@ -819,9 +882,10 @@ static DEVICE_ATTR(brightness, S_IRUGO | S_IWUGO, show_brightness, store_brightn
static DEVICE_ATTR(exposure, S_IRUGO | S_IWUGO, show_exposure, store_exposure); /**< Exposure value */
static DEVICE_ATTR(gain, S_IRUGO | S_IWUGO, show_gain, store_gain); /**< Gain value */
static DEVICE_ATTR(contrast, S_IRUGO | S_IWUGO, show_contrast, store_contrast); /**< Contrast value */
-static DEVICE_ATTR(gamma, S_IRUGO | S_IWUGO, show_gamma, store_gamma); /**< Gamma value */
+static DEVICE_ATTR(saturation, S_IRUGO | S_IWUGO, show_saturation, store_saturation); /**< Saturation value */
+static DEVICE_ATTR(hue, S_IRUGO | S_IWUGO, show_hue, store_hue); /**< Hue value */
+static DEVICE_ATTR(gamma, S_IRUGO | S_IWUGO, show_gamma, store_gamma); /**< Gamma value */
static DEVICE_ATTR(sharpness, S_IRUGO | S_IWUGO, show_sharpness, store_sharpness); /**< Sharpness value */
-/*static DEVICE_ATTR(colour, S_IRUGO | S_IWUGO, show_colour, store_colour); /< Hue value */
static DEVICE_ATTR(hflip, S_IRUGO | S_IWUGO, show_hflip, store_hflip); /**< Horizontal flip value */
static DEVICE_ATTR(vflip, S_IRUGO | S_IWUGO, show_vflip, store_vflip); /**< Vertical flip value */
static DEVICE_ATTR(auto_exposure, S_IRUGO | S_IWUGO, show_autoexposure, store_autoexposure); /**< Automatic exposure control value */
@@ -851,9 +915,10 @@ int sn9c20x_create_sysfs_files(struct video_device *vdev)
ret = video_device_create_file(vdev, &class_device_attr_exposure);
ret = video_device_create_file(vdev, &class_device_attr_gain);
ret = video_device_create_file(vdev, &class_device_attr_contrast);
+ ret = video_device_create_file(vdev, &class_device_attr_saturation);
+ ret = video_device_create_file(vdev, &class_device_attr_hue);
ret = video_device_create_file(vdev, &class_device_attr_gamma);
ret = video_device_create_file(vdev, &class_device_attr_sharpness);
- /*ret = video_device_create_file(vdev, &class_device_attr_colour);*/
ret = video_device_create_file(vdev, &class_device_attr_hflip);
ret = video_device_create_file(vdev, &class_device_attr_vflip);
ret = video_device_create_file(vdev, &class_device_attr_auto_exposure);
@@ -867,9 +932,10 @@ int sn9c20x_create_sysfs_files(struct video_device *vdev)
ret = video_device_create_file(vdev, &dev_attr_exposure);
ret = video_device_create_file(vdev, &dev_attr_gain);
ret = video_device_create_file(vdev, &dev_attr_contrast);
+ ret = video_device_create_file(vdev, &dev_attr_saturation);
+ ret = video_device_create_file(vdev, &dev_attr_hue);
ret = video_device_create_file(vdev, &dev_attr_gamma);
ret = video_device_create_file(vdev, &dev_attr_sharpness);
- /*ret = video_device_create_file(vdev, &dev_attr_colour);*/
ret = video_device_create_file(vdev, &dev_attr_hflip);
ret = video_device_create_file(vdev, &dev_attr_vflip);
ret = video_device_create_file(vdev, &dev_attr_auto_exposure);
@@ -883,9 +949,10 @@ int sn9c20x_create_sysfs_files(struct video_device *vdev)
ret = device_create_file(&vdev->dev, &dev_attr_exposure);
ret = device_create_file(&vdev->dev, &dev_attr_gain);
ret = device_create_file(&vdev->dev, &dev_attr_contrast);
+ ret = device_create_file(&vdev->dev, &dev_attr_saturation);
+ ret = device_create_file(&vdev->dev, &dev_attr_hue);
ret = device_create_file(&vdev->dev, &dev_attr_gamma);
ret = device_create_file(&vdev->dev, &dev_attr_sharpness);
- /*ret = video_device_create_file(vdev, &dev_attr_colour);*/
ret = device_create_file(&vdev->dev, &dev_attr_hflip);
ret = device_create_file(&vdev->dev, &dev_attr_vflip);
ret = device_create_file(&vdev->dev, &dev_attr_auto_exposure);
@@ -915,9 +982,10 @@ void sn9c20x_remove_sysfs_files(struct video_device *vdev)
video_device_remove_file(vdev, &class_device_attr_exposure);
video_device_remove_file(vdev, &class_device_attr_gain);
video_device_remove_file(vdev, &class_device_attr_contrast);
+ video_device_remove_file(vdev, &class_device_attr_saturation);
+ video_device_remove_file(vdev, &class_device_attr_hue);
video_device_remove_file(vdev, &class_device_attr_gamma);
video_device_remove_file(vdev, &class_device_attr_sharpness);
- /*video_device_remove_file(vdev, &class_device_attr_colour);*/
video_device_remove_file(vdev, &class_device_attr_hflip);
video_device_remove_file(vdev, &class_device_attr_vflip);
video_device_remove_file(vdev, &class_device_attr_auto_exposure);
@@ -931,9 +999,10 @@ void sn9c20x_remove_sysfs_files(struct video_device *vdev)
video_device_remove_file(vdev, &dev_attr_exposure);
video_device_remove_file(vdev, &dev_attr_gain);
video_device_remove_file(vdev, &dev_attr_contrast);
+ video_device_remove_file(vdev, &dev_attr_saturation);
+ video_device_remove_file(vdev, &dev_attr_hue);
video_device_remove_file(vdev, &dev_attr_gamma);
video_device_remove_file(vdev, &dev_attr_sharpness);
- /*video_device_remove_file(vdev, &dev_attr_colour);*/
video_device_remove_file(vdev, &dev_attr_hflip);
video_device_remove_file(vdev, &dev_attr_vflip);
video_device_remove_file(vdev, &dev_attr_auto_exposure);
@@ -947,9 +1016,10 @@ void sn9c20x_remove_sysfs_files(struct video_device *vdev)
device_remove_file(&vdev->dev, &dev_attr_exposure);
device_remove_file(&vdev->dev, &dev_attr_gain);
device_remove_file(&vdev->dev, &dev_attr_contrast);
+ device_remove_file(&vdev->dev, &dev_attr_saturation);
+ device_remove_file(&vdev->dev, &dev_attr_hue);
device_remove_file(&vdev->dev, &dev_attr_gamma);
device_remove_file(&vdev->dev, &dev_attr_sharpness);
- /*video_device_remove_file(vdev, &dev_attr_colour);*/
device_remove_file(&vdev->dev, &dev_attr_hflip);
device_remove_file(&vdev->dev, &dev_attr_vflip);
device_remove_file(&vdev->dev, &dev_attr_auto_exposure);
diff --git a/sn9c20x-usb.c b/sn9c20x-usb.c
index 17d8c2e..252995d 100644
--- a/sn9c20x-usb.c
+++ b/sn9c20x-usb.c
@@ -96,6 +96,18 @@ static __u16 brightness = SN9C20X_PERCENT(50, 0xFF);
static __u16 gamma = SN9C20X_PERCENT(20, 0xFF);

/**
+ * @var saturation
+ * Module parameter to set the gamma
+ */
+static __u16 saturation = SN9C20X_PERCENT(50, 0xFF);
+
+/**
+ * @var hue
+ * Module parameter to set the gamma
+ */
+static __u16 hue = SN9C20X_PERCENT(50, 0x168);
+
+/**
* @var contrast
* Module parameter to set the contrast
*/
@@ -111,7 +123,7 @@ static __u16 exposure = SN9C20X_PERCENT(20, 0xFF);
* @var gain
* Module parameter to set the gain
*/
-static __u16 gain = SN9C20X_PERCENT(20, 0xFF);
+static int gain = SN9C20X_PERCENT(20, 0xFF);

/**
* @var sharpness
@@ -720,6 +732,8 @@ static int usb_sn9c20x_default_settings(struct usb_sn9c20x *dev)
v4l2_set_control_default(dev, V4L2_CID_BRIGHTNESS, brightness);
v4l2_set_control_default(dev, V4L2_CID_CONTRAST, contrast);
v4l2_set_control_default(dev, V4L2_CID_GAMMA, gamma);
+ v4l2_set_control_default(dev, V4L2_CID_SATURATION, saturation);
+ v4l2_set_control_default(dev, V4L2_CID_HUE, hue);
v4l2_set_control_default(dev, V4L2_CID_SHARPNESS, sharpness);
v4l2_set_control_default(dev, V4L2_CID_RED_BALANCE, red_gain);
v4l2_set_control_default(dev, V4L2_CID_BLUE_BALANCE, blue_gain);
@@ -898,6 +912,7 @@ module_param(auto_gain, byte, 0444); /**< @brief Module parameter automatic gai
module_param(auto_whitebalance, byte, 0444); /**< @brief Module parameter automatic whitebalance control */
module_param(brightness, ushort, 0444); /**< @brief Module parameter brightness */
module_param(gamma, ushort, 0444); /**< @brief Module parameter gamma */
+module_param(saturation, ushort, 0444); /**< @brief Module parameter saturation */
module_param(contrast, ushort, 0444); /**< @brief Module parameter contrast */
module_param(exposure, ushort, 0444); /**< @brief Module parameter exposure */
module_param(sharpness, ushort, 0444); /**< @brief Module parameter sharpness */
@@ -1046,6 +1061,7 @@ MODULE_PARM_DESC(auto_gain, "Automatic gain control"); /**< @brief Description
MODULE_PARM_DESC(auto_whitebalance, "Automatic whitebalance"); /**< @brief Description of 'auto_whitebalance' parameter */
MODULE_PARM_DESC(brightness, "Brightness setting"); /**< @brief Description of 'brightness' parameter */
MODULE_PARM_DESC(gamma, "Gamma setting"); /**< @brief Description of 'gamma' parameter */
+MODULE_PARM_DESC(saturation, "Saturation setting"); /**< @brief Description of 'saturation' parameter */
MODULE_PARM_DESC(exposure, "Exposure setting"); /**< @brief Description of 'exposure' parameter */
MODULE_PARM_DESC(gain, "Gain setting"); /**< @brief Description of 'gain' parameter */
MODULE_PARM_DESC(contrast, "Contrast setting"); /**< @brief Description of 'contrast' parameter */
diff --git a/sn9c20x-v4l2.c b/sn9c20x-v4l2.c
index b762678..a41bb09 100644
--- a/sn9c20x-v4l2.c
+++ b/sn9c20x-v4l2.c
@@ -68,7 +68,7 @@ static struct v4l2_queryctrl sn9c20x_controls[] = {
.maximum = 0xff,
.step = 1,
},
-/*
+
{
.id = V4L2_CID_SATURATION,
.type = V4L2_CTRL_TYPE_INTEGER,
@@ -77,7 +77,16 @@ static struct v4l2_queryctrl sn9c20x_controls[] = {
.maximum = 0xff,
.step = 1,
},
-*/
+
+ {
+ .id = V4L2_CID_HUE,
+ .type = V4L2_CTRL_TYPE_INTEGER,
+ .name = "Hue",
+ .minimum = 0,
+ .maximum = 360,
+ .step = 1,
+ },
+
{
.id = V4L2_CID_CONTRAST,
.type = V4L2_CTRL_TYPE_INTEGER,
@@ -856,11 +865,15 @@ int sn9c20x_vidioc_g_ctrl(struct file *file, void *priv,
case V4L2_CID_GAMMA:
ctrl->value = dev->vsettings.gamma;
break;
-/*
+
case V4L2_CID_SATURATION:
ctrl->value = dev->vsettings.colour;
break;
-*/
+
+ case V4L2_CID_HUE:
+ ctrl->value = dev->vsettings.hue;
+ break;
+
case V4L2_CID_CONTRAST:
ctrl->value = dev->vsettings.contrast;
break;
diff --git a/sn9c20x.h b/sn9c20x.h
index 6a846a4..b26ed0c 100644
--- a/sn9c20x.h
+++ b/sn9c20x.h
@@ -326,9 +326,11 @@ struct sn9c20x_camera {
int (*set_contrast) (struct usb_sn9c20x *dev);
int (*set_brightness) (struct usb_sn9c20x *dev);
int (*set_gamma) (struct usb_sn9c20x *dev);
+ int (*set_saturation) (struct usb_sn9c20x *dev);
int (*set_sharpness) (struct usb_sn9c20x *dev);
int (*set_red_gain) (struct usb_sn9c20x *dev);
int (*set_blue_gain) (struct usb_sn9c20x *dev);
+ int (*set_hue) (struct usb_sn9c20x *dev);
};

/**

Brian Johnson

unread,

Mar 19, 2009, 1:02:24 AM3/19/09

to micr...@googlegroups.com

Works fairly well for me as well. I have a couple of issues though.
First there are many errors reported by checkpatch.pl. Next i think
hue should actually go from -180 to 180, this will make a hue of zero
represent the standard default hue with a range of 180 either left or
right. I think that is somewhat more intuitive. i attached a quick
patch that adds this to your code. Also while you added support for
hue under sysfs you are missing the code to add a hue module
parameter. Other then these issue it looks pretty good.

2009/3/18 Stefan Krastanov <krastano...@gmail.com>:

hue_range.patch

Boris Borisov

unread,

Mar 19, 2009, 2:56:27 AM3/19/09

to micr...@googlegroups.com

I agree. The mistake is my because I working in night and firs version is similar as you patch and hue betweeh -180 and 180 degree. But after coup of mistakes with overindexing of sinus array and many freezes of of kernel I forgot to return the values -180 - 180. Thanks Brian
Please test my patch and Brian's and if all is OK i wil be push into git this evening

2009/3/19 Brian Johnson <bri...@gmail.com>

Brian Johnson

unread,

Mar 19, 2009, 5:58:14 AM3/19/09

to micr...@googlegroups.com

Boris,

Can you post a fixed patch ithat includes the -180 - 180 fix plus
fxing th checkpatch.pl errors and adding the code to add hue as a
module param?

If you post the patch in the thread i created called weekend patches
it willl give us till the end of sunday for people to hoopefully look
at and review the patch before actually committing it.

2009/3/19 Boris Borisov <lz1...@gmail.com>:

Boris Borisov

unread,

Mar 19, 2009, 6:17:09 AM3/19/09

to micr...@googlegroups.com

Yes but after 19.00 BG time because i'm on my work.

2009/3/19 Brian Johnson <bri...@gmail.com>

Boris Borisov

unread,

Mar 19, 2009, 1:54:33 PM3/19/09

to micr...@googlegroups.com

Brian,
I get the current version of driver and apply the my patc and your
changes for range. But from V4L if hue is negative the set function is
not call. I can't find where is mistake. I attach the patches. Also from
sysfs the values is only positive.

Brian Johnson wrote:
> Boris,
>
> Can you post a fixed patch ithat includes the -180 - 180 fix plus
> fxing th checkpatch.pl errors and adding the code to add hue as a
> module param?
>
> If you post the patch in the thread i created called weekend patches
> it willl give us till the end of sunday for people to hoopefully look
> at and review the patch before actually committing it.
>
> 2009/3/19 Boris Borisov <lz1...@gmail.com>:
>
>> I agree. The mistake is my because I working in night and firs version is
>> similar as you patch and hue betweeh -180 and 180 degree. But after coup of
>>

snip

hue_sat_range.patch

Brian Johnson

unread,

Mar 19, 2009, 3:29:08 PM3/19/09

to micr...@googlegroups.com

Boris,
I'm not sure what you mean by the function is not being called when
hue is negative using this patch it certainly seems to work with
negative hues for me. About the issue with sysfs i did a quick
modification that should fix that

2009/3/19 Boris Borisov <lz1...@gmail.com>:

> diff --git a/sn9c20x-bridge.c b/sn9c20x-bridge.c
> index dfbf87f..d4e3c6a 100644
> --- a/sn9c20x-bridge.c
> +++ b/sn9c20x-bridge.c
> @@ -30,8 +30,292 @@

> - __u32 control, __u32 value)
> + __u32 control, __s32 value)
> {
> int ret = -EINVAL;
> switch (control) {

> @@ -53,6 +337,18 @@ int sn9c20x_set_camera_control(struct usb_sn9c20x *dev,
> ret = dev->camera.set_gamma(dev);
> }
> break;
> + case V4L2_CID_SATURATION:
> + if (dev->camera.set_saturation) {
> + dev->vsettings.colour = value;
> + ret = dev->camera.set_saturation(dev);
> + }
> + break;
> + case V4L2_CID_HUE:
> + if (dev->camera.set_hue) {
> + dev->vsettings.hue = value;
> + ret = dev->camera.set_hue(dev);
> + }
> + break;
> case V4L2_CID_SHARPNESS:
> if (dev->camera.set_sharpness) {
> dev->vsettings.sharpness = value;

> @@ -483,33 +779,97 @@ int sn9c20x_write_i2c_array(struct usb_sn9c20x *dev,

> }
>
> /**
> - * @brief Set contrast inside sn9c20x chip
> + * @brief Calculate and set optical parameters
> *
> - * @author Comer352l
> + * @author Comer352l, Boris Borisov
> *
> * @param dev Pointer to the device
> *
> * @return Zero (success) or negative (USB-error value)
> *
> */
> -int sn9c20x_set_contrast(struct usb_sn9c20x *dev)
> +int sn9c20x_set_optical_parameters(struct usb_sn9c20x *dev)
> {
> - /* from 0x26 to 0x4b */
> - __u8 brightness_contrast[21] = {0x16, 0x0, 0x2b, 0x0, 0x8, 0x0, 0xf6, 0x0f,
> + __u8 optical_parameters[21]= {0x16, 0x0, 0x2b, 0x0, 0x8, 0x0, 0xf6, 0x0f,
> 0xd2, 0x0f, 0x38, 0x0, 0x34, 0x0, 0xcf, 0x0f,
> 0xfd, 0x0f, 0x0, 0x0, 0x0};
> +
> +
> __u8 contrast_val = (dev->vsettings.contrast) * 0x25 / 0x100;
> __u8 brightness_val = dev->vsettings.brightness;
> + __u8 ret=0;

> + __s16 value;
> + long tmp_coordinate;
>
> +
> + if(dev->vsettings.hue < -179)
> + dev->vsettings.hue= -180;
> +
> + if(dev->vsettings.hue > 179)
> + dev->vsettings.hue = 180;
> +
> + value = 180 + dev->vsettings.hue;
> +
> + UDIA_INFO("Hue Value: %d\n", value);
> + UDIA_INFO("Hue vsettings.hue: %d\n", value);

> + //Brighness and contrast settings
> brightness_val -= 0x80;
> - brightness_contrast[18] = brightness_val;
> + optical_parameters[18] = brightness_val;
>
> contrast_val += 0x26;
> - brightness_contrast[2] = contrast_val;
> - brightness_contrast[0] = 0x13 + (brightness_contrast[2] - 0x26) * 0x13 / 0x25;
> - brightness_contrast[4] = 0x7 + (brightness_contrast[2] - 0x26) * 0x7 / 0x25;
> + optical_parameters[2] = contrast_val;
> + optical_parameters[0] = 0x13 + (optical_parameters[2] - 0x26) * 0x13 / 0x25;
> + optical_parameters[4] = 0x7 + (optical_parameters[2] - 0x26) * 0x7 / 0x25;
> +
> + //Calculate HUE and SATURATION

> + tmp_coordinate=(long)(RX[value]);

> + tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
> + optical_parameters[6]=(unsigned char)(tmp_coordinate&0xff);
> + optical_parameters[7]=(unsigned char)((tmp_coordinate>>8)&0x0f);
> +

> + tmp_coordinate=(long)(RY[value]);

> + tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
> + optical_parameters[8]=(unsigned char)(tmp_coordinate&0xff);
> + optical_parameters[9]=(unsigned char)((tmp_coordinate>>8)&0x0f);
> +

> + tmp_coordinate=(long)(GX[value]);

> + tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
> + optical_parameters[10]=(unsigned char)(tmp_coordinate&0xff);
> + optical_parameters[11]=(unsigned char)((tmp_coordinate>>8)&0x0f);
> +

> + tmp_coordinate=(long)(GY[value]);

> + tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
> + optical_parameters[12]=(unsigned char)(tmp_coordinate&0xff);
> + optical_parameters[13]=(unsigned char)((tmp_coordinate>>8)&0x0f);
> +

> + tmp_coordinate=(long)(BX[value]);

> + tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
> + optical_parameters[14]=(unsigned char)(tmp_coordinate&0xff);
> + optical_parameters[15]=(unsigned char)((tmp_coordinate>>8)&0x0f);
> +

> + tmp_coordinate=(long)(BY[value]);

> + tmp_coordinate=(tmp_coordinate*dev->vsettings.colour) >> 8;
> + optical_parameters[16]=(unsigned char)(tmp_coordinate&0xff);
> + optical_parameters[17]=(unsigned char)((tmp_coordinate>>8)&0x0f);
> +
> +

> + ret=usb_sn9c20x_control_write(dev, 0x10e1, optical_parameters, 21);
> + return(ret);
> +}
>
> - return usb_sn9c20x_control_write(dev, 0x10e1, brightness_contrast, 21);
> +/**
> + * @brief Set contrast inside sn9c20x chip
> + *
> + * @author Comer352l
> + *
> + * @param dev Pointer to the device
> + *
> + * @return Zero (success) or negative (USB-error value)
> + *
> + */
> +int sn9c20x_set_contrast(struct usb_sn9c20x *dev)
> +{
> + return sn9c20x_set_optical_parameters(dev);
> }
>
> /**

> @@ -526,7 +886,7 @@ int sn9c20x_set_contrast(struct usb_sn9c20x *dev)

> */
> int sn9c20x_set_brightness(struct usb_sn9c20x *dev)
> {
> - return sn9c20x_set_contrast(dev);
> + return sn9c20x_set_optical_parameters(dev);
> }
>
> /**

> @@ -571,6 +931,36 @@ int sn9c20x_set_gamma(struct usb_sn9c20x *dev)

> @@ -906,9 +1296,11 @@ int sn9c20x_initialize(struct usb_sn9c20x *dev)

> dev->camera.set_contrast = sn9c20x_set_contrast;
> dev->camera.set_brightness = sn9c20x_set_brightness;
> dev->camera.set_gamma = sn9c20x_set_gamma;
> + dev->camera.set_saturation = sn9c20x_set_saturation;
> dev->camera.set_sharpness = sn9c20x_set_sharpness;
> dev->camera.set_red_gain = sn9c20x_set_red_gain;
> dev->camera.set_blue_gain = sn9c20x_set_blue_gain;
> + dev->camera.set_hue = sn9c20x_set_hue;
>
> ret = sn9c20x_i2c_initialize(dev);
> if (ret < 0)

> diff --git a/sn9c20x-bridge.h b/sn9c20x-bridge.h
> index 029042c..54ea809 100644
> --- a/sn9c20x-bridge.h
> +++ b/sn9c20x-bridge.h
> @@ -47,7 +47,7 @@
>
> int sn9c20x_initialize(struct usb_sn9c20x *dev);
> int sn9c20x_set_LEDs(struct usb_sn9c20x *dev, int enable);
> -int sn9c20x_set_camera_control(struct usb_sn9c20x *dev, __u32 control, __u32 value);
> +int sn9c20x_set_camera_control(struct usb_sn9c20x *dev, __u32 control, __s32 value);
> int sn9c20x_enable_video(struct usb_sn9c20x *dev, int enable);
> int sn9c20x_i2c_initialize(struct usb_sn9c20x *dev);

>
> diff --git a/sn9c20x-dev.c b/sn9c20x-dev.c
> index 11ac1f3..dd1eabf 100644
> --- a/sn9c20x-dev.c
> +++ b/sn9c20x-dev.c
> @@ -173,7 +173,6 @@ int sn9c20x_initialize_sensor(struct usb_sn9c20x *dev)
> sn9c20x_write_i2c_array(dev, ov7660_init, 0);
> dev->camera.set_exposure = ov_set_exposure;
> dev->camera.set_auto_gain = ov_set_autogain;
> - dev->camera.set_gain = ov9650_set_gain;
> dev->camera.hstart = 1;
> dev->camera.vstart = 1;
> UDIA_INFO("Detected OV7660 Sensor.\n");
> diff --git a/sn9c20x-sysfs.c b/sn9c20x-sysfs.c

> index 15bb13c..16b1f27 100644

> @@ -502,13 +505,71 @@ static ssize_t store_colour(struct device *class, struct device_attribute *attr,

>
> value = simple_strtoul(buf, &endp, 16);
>
> - dev->vsettings.colour = (int) value;
> + sn9c20x_set_camera_control(dev,
> + V4L2_CID_HUE,
> + value);
> +
> + return strlen(buf);
> +}
>
> -/ dev_sn9c20x_set_camera_quality(dev); /

> @@ -802,10 +863,11 @@ static CLASS_DEVICE_ATTR(fps, S_IRUGO, show_fps, NULL); /**< FPS value */

> static CLASS_DEVICE_ATTR(brightness, S_IRUGO | S_IWUGO, show_brightness, store_brightness); /**< Brightness value */
> static CLASS_DEVICE_ATTR(exposure, S_IRUGO | S_IWUGO, show_exposure, store_exposure); /**< Exposure value */
> static CLASS_DEVICE_ATTR(gain, S_IRUGO | S_IWUGO, show_gain, store_gain); /**< Gain value */
> +static CLASS_DEVICE_ATTR(saturation, S_IRUGO | S_IWUGO, show_saturation, store_saturation); /**< Saturation value */
> +static CLASS_DEVICE_ATTR(hue, S_IRUGO | S_IWUGO, show_hue, store_hue); /**< Hue value */
> static CLASS_DEVICE_ATTR(contrast, S_IRUGO | S_IWUGO, show_contrast, store_contrast); /**< Contrast value */
> static CLASS_DEVICE_ATTR(gamma, S_IRUGO | S_IWUGO, show_gamma, store_gamma); /**< Gamma value */
> static CLASS_DEVICE_ATTR(sharpness, S_IRUGO | S_IWUGO, show_sharpness, store_sharpness); /**< Sharpness value */
> -/*static CLASS_DEVICE_ATTR(colour, S_IRUGO | S_IWUGO, show_colour, store_colour); /< Hue value */
> static CLASS_DEVICE_ATTR(hflip, S_IRUGO | S_IWUGO, show_hflip, store_hflip); /**< Horizontal flip value */
> static CLASS_DEVICE_ATTR(vflip, S_IRUGO | S_IWUGO, show_vflip, store_vflip); /**< Vertical flip value */
> static CLASS_DEVICE_ATTR(auto_exposure, S_IRUGO | S_IWUGO, show_autoexposure, store_autoexposure); /**< Automatic exposure control value */

> @@ -819,9 +881,10 @@ static DEVICE_ATTR(brightness, S_IRUGO | S_IWUGO, show_brightness, store_brightn

> static DEVICE_ATTR(exposure, S_IRUGO | S_IWUGO, show_exposure, store_exposure); /**< Exposure value */
> static DEVICE_ATTR(gain, S_IRUGO | S_IWUGO, show_gain, store_gain); /**< Gain value */
> static DEVICE_ATTR(contrast, S_IRUGO | S_IWUGO, show_contrast, store_contrast); /**< Contrast value */
> -static DEVICE_ATTR(gamma, S_IRUGO | S_IWUGO, show_gamma, store_gamma); /**< Gamma value */
> +static DEVICE_ATTR(saturation, S_IRUGO | S_IWUGO, show_saturation, store_saturation); /**< Saturation value */
> +static DEVICE_ATTR(hue, S_IRUGO | S_IWUGO, show_hue, store_hue); /**< Hue value */
> +static DEVICE_ATTR(gamma, S_IRUGO | S_IWUGO, show_gamma, store_gamma); /**< Gamma value */
> static DEVICE_ATTR(sharpness, S_IRUGO | S_IWUGO, show_sharpness, store_sharpness); /**< Sharpness value */
> -/*static DEVICE_ATTR(colour, S_IRUGO | S_IWUGO, show_colour, store_colour); /< Hue value */
> static DEVICE_ATTR(hflip, S_IRUGO | S_IWUGO, show_hflip, store_hflip); /**< Horizontal flip value */
> static DEVICE_ATTR(vflip, S_IRUGO | S_IWUGO, show_vflip, store_vflip); /**< Vertical flip value */
> static DEVICE_ATTR(auto_exposure, S_IRUGO | S_IWUGO, show_autoexposure, store_autoexposure); /**< Automatic exposure control value */

> @@ -851,9 +914,10 @@ int sn9c20x_create_sysfs_files(struct video_device *vdev)

> ret = video_device_create_file(vdev, &class_device_attr_exposure);
> ret = video_device_create_file(vdev, &class_device_attr_gain);
> ret = video_device_create_file(vdev, &class_device_attr_contrast);
> + ret = video_device_create_file(vdev, &class_device_attr_saturation);
> + ret = video_device_create_file(vdev, &class_device_attr_hue);
> ret = video_device_create_file(vdev, &class_device_attr_gamma);
> ret = video_device_create_file(vdev, &class_device_attr_sharpness);
> - /*ret = video_device_create_file(vdev, &class_device_attr_colour);*/
> ret = video_device_create_file(vdev, &class_device_attr_hflip);
> ret = video_device_create_file(vdev, &class_device_attr_vflip);
> ret = video_device_create_file(vdev, &class_device_attr_auto_exposure);

> @@ -867,9 +931,10 @@ int sn9c20x_create_sysfs_files(struct video_device *vdev)

> ret = video_device_create_file(vdev, &dev_attr_exposure);
> ret = video_device_create_file(vdev, &dev_attr_gain);
> ret = video_device_create_file(vdev, &dev_attr_contrast);
> + ret = video_device_create_file(vdev, &dev_attr_saturation);
> + ret = video_device_create_file(vdev, &dev_attr_hue);
> ret = video_device_create_file(vdev, &dev_attr_gamma);
> ret = video_device_create_file(vdev, &dev_attr_sharpness);
> - /*ret = video_device_create_file(vdev, &dev_attr_colour);*/
> ret = video_device_create_file(vdev, &dev_attr_hflip);
> ret = video_device_create_file(vdev, &dev_attr_vflip);
> ret = video_device_create_file(vdev, &dev_attr_auto_exposure);

> @@ -883,9 +948,10 @@ int sn9c20x_create_sysfs_files(struct video_device *vdev)

> ret = device_create_file(&vdev->dev, &dev_attr_exposure);
> ret = device_create_file(&vdev->dev, &dev_attr_gain);
> ret = device_create_file(&vdev->dev, &dev_attr_contrast);
> + ret = device_create_file(&vdev->dev, &dev_attr_saturation);
> + ret = device_create_file(&vdev->dev, &dev_attr_hue);
> ret = device_create_file(&vdev->dev, &dev_attr_gamma);
> ret = device_create_file(&vdev->dev, &dev_attr_sharpness);
> - /*ret = video_device_create_file(vdev, &dev_attr_colour);*/
> ret = device_create_file(&vdev->dev, &dev_attr_hflip);
> ret = device_create_file(&vdev->dev, &dev_attr_vflip);
> ret = device_create_file(&vdev->dev, &dev_attr_auto_exposure);

> @@ -915,9 +981,10 @@ void sn9c20x_remove_sysfs_files(struct video_device *vdev)

> video_device_remove_file(vdev, &class_device_attr_exposure);
> video_device_remove_file(vdev, &class_device_attr_gain);
> video_device_remove_file(vdev, &class_device_attr_contrast);
> + video_device_remove_file(vdev, &class_device_attr_saturation);
> + video_device_remove_file(vdev, &class_device_attr_hue);
> video_device_remove_file(vdev, &class_device_attr_gamma);
> video_device_remove_file(vdev, &class_device_attr_sharpness);
> - /*video_device_remove_file(vdev, &class_device_attr_colour);*/
> video_device_remove_file(vdev, &class_device_attr_hflip);
> video_device_remove_file(vdev, &class_device_attr_vflip);
> video_device_remove_file(vdev, &class_device_attr_auto_exposure);

> @@ -931,9 +998,10 @@ void sn9c20x_remove_sysfs_files(struct video_device *vdev)

> video_device_remove_file(vdev, &dev_attr_exposure);
> video_device_remove_file(vdev, &dev_attr_gain);
> video_device_remove_file(vdev, &dev_attr_contrast);
> + video_device_remove_file(vdev, &dev_attr_saturation);
> + video_device_remove_file(vdev, &dev_attr_hue);
> video_device_remove_file(vdev, &dev_attr_gamma);
> video_device_remove_file(vdev, &dev_attr_sharpness);
> - /*video_device_remove_file(vdev, &dev_attr_colour);*/
> video_device_remove_file(vdev, &dev_attr_hflip);
> video_device_remove_file(vdev, &dev_attr_vflip);
> video_device_remove_file(vdev, &dev_attr_auto_exposure);

> @@ -947,9 +1015,10 @@ void sn9c20x_remove_sysfs_files(struct video_device *vdev)

> device_remove_file(&vdev->dev, &dev_attr_exposure);
> device_remove_file(&vdev->dev, &dev_attr_gain);
> device_remove_file(&vdev->dev, &dev_attr_contrast);
> + device_remove_file(&vdev->dev, &dev_attr_saturation);
> + device_remove_file(&vdev->dev, &dev_attr_hue);
> device_remove_file(&vdev->dev, &dev_attr_gamma);
> device_remove_file(&vdev->dev, &dev_attr_sharpness);
> - /*video_device_remove_file(vdev, &dev_attr_colour);*/
> device_remove_file(&vdev->dev, &dev_attr_hflip);
> device_remove_file(&vdev->dev, &dev_attr_vflip);
> device_remove_file(&vdev->dev, &dev_attr_auto_exposure);
> diff --git a/sn9c20x-usb.c b/sn9c20x-usb.c

> index 17d8c2e..ece50cf 100644

> --- a/sn9c20x-usb.c
> +++ b/sn9c20x-usb.c
> @@ -96,6 +96,18 @@ static __u16 brightness = SN9C20X_PERCENT(50, 0xFF);
> static __u16 gamma = SN9C20X_PERCENT(20, 0xFF);
>
> /**
> + * @var saturation
> + * Module parameter to set the gamma
> + */
> +static __u16 saturation = SN9C20X_PERCENT(50, 0xFF);
> +
> +/**
> + * @var hue
> + * Module parameter to set the gamma
> + */

> +static __s16 hue;

> index b762678..cd84c33 100644

> --- a/sn9c20x-v4l2.c
> +++ b/sn9c20x-v4l2.c
> @@ -68,7 +68,7 @@ static struct v4l2_queryctrl sn9c20x_controls[] = {
> .maximum = 0xff,
> .step = 1,
> },
> -/*
> +
> {
> .id = V4L2_CID_SATURATION,
> .type = V4L2_CTRL_TYPE_INTEGER,
> @@ -77,7 +77,16 @@ static struct v4l2_queryctrl sn9c20x_controls[] = {
> .maximum = 0xff,
> .step = 1,
> },
> -*/
> +
> + {
> + .id = V4L2_CID_HUE,
> + .type = V4L2_CTRL_TYPE_INTEGER,
> + .name = "Hue",

> + .minimum = -180,
> + .maximum = 180,

hue_sat_range.patch

Boris Borisov

unread,

Mar 19, 2009, 3:52:36 PM3/19/09

to micr...@googlegroups.com

Exactly the function is not call via V4L for negative hue. And I not
found the reason for missing call.

Brian Johnson

unread,

Mar 19, 2009, 5:40:27 PM3/19/09

to micr...@googlegroups.com

So you are saying the that when you use v4lucp to adjust the hue
control below 0 that he hue does not actually change? And that the
function sn9c20x_set_optical_parameters is never called?

2009/3/19 Boris Borisov <lz1...@gmail.com>:

Boris Borisov

unread,

Mar 20, 2009, 3:07:39 AM3/20/09

to micr...@googlegroups.com

No v4l2ucp is do correctly control of hue and working with negative hue. I spoke about amsn is not support correctly negative hue. In this case maybe some other applications cold be unsupported negative parameters for control of picture. Really the correct value is between -180 degree to +180 degree. But this is relative angle from some point named 0 degree, In this case maybe some applications is not support negative values. I understand this is the bug into amsn not in the driver and V4l2 standard is damaged by amsn. But really for final user that not mean who write correct code he ask only to work correctly the camera with exactly application. Also the end user is see only the not working control. In this case maybe is more good idea to apply the control in positive value only until some userland applications fix the bugs in V4L2 protocol.
Else some users who is migrated from Windows to Linux is say " the camera is not working under windows i just install the driver and all is OK". Also the depends from libv4l2 is not installed automatically on each distribution and for typical user all 'magic' about installation and tuning is not understandable.
I'm just a user friendly oriented and test each my code with different camera clients.
That is just a my point of view.
Also we test the hue/set/bright/cotr control only on 2 types of camera my and Stefan's (we have 6270) and your camera.
For me all pathes (also Josua) seems OK.

Boris Borisov

unread,

Mar 20, 2009, 3:31:06 AM3/20/09

to micr...@googlegroups.com

I forget! Also kopette is not support correctly hue tuning

Brian Johnson

unread,

Mar 20, 2009, 5:05:06 AM3/20/09

to micr...@googlegroups.com

Ok just took a quick look at amsn source and yes the code for setting
an attribute is definitely broken it checks the value tht you are
setting for a range of 0 - 65535. this is definitely wrong as av4l2
control value is first of all signed and second of all it should be
using its defined min/max values anyways. I attached a quick patch
that fixes this issue with amsn in case anyone is interested.

Also kopete seems to have two issues one it appears it also doesn't
bother to consider that a control value can be negative and kopete
actually natively knows bayer format whcih unless i use somethign like
libv4l it will use by default. and none of the controls that use our
bridges color matrix (hue/sat/contrast/brightness) work when using
bayer since it is not being converted to YUV. this is one reason you
should not use bayer unless there is no other choice.

Also I'm of the mind that we should not modify our driver in order to
fix broken applications. The more correct thing will be to patch the
applications to properly follow v4l2 specs. In this case allow
controls with negative values.

2009/3/20 Boris Borisov <lz1...@gmail.com>:

fix_set_attribute.patch

Boris Borisov

unread,

Mar 20, 2009, 6:14:47 AM3/20/09

to micr...@googlegroups.com

VLC also have the same problem with hue. Maybe is more capable for other video applications to inform developers for mistakes in application and after he fix of userland application to switch to negative values of hue.
For more users is not important the exactly value of hue he ask only to work him cameras with all video applications.

Brian Johnson wrote:

:



snip

Boris Borisov

unread,

Mar 20, 2009, 6:35:13 AM3/20/09

to micr...@googlegroups.com

Brian,
Can you report for this bug into VLC amsn and kopette. As principal I agree with your opinion about following of standards.

Josua Grawitter

unread,

Mar 20, 2009, 4:22:04 PM3/20/09

to micr...@googlegroups.com

Am Freitag 20 März 2009 10:05:06 schrieb Brian Johnson:
> Ok just took a quick look at amsn source and yes the code for setting
> an attribute is definitely broken it checks the value tht you are
> setting for a range of 0 - 65535. this is definitely wrong as av4l2
> control value is first of all signed and second of all it should be
> using its defined min/max values anyways. I attached a quick patch
> that fixes this issue with amsn in case anyone is interested.
>
> Also kopete seems to have two issues one it appears it also doesn't
> bother to consider that a control value can be negative and kopete
> actually natively knows bayer format whcih unless i use somethign like
> libv4l it will use by default. and none of the controls that use our
> bridges color matrix (hue/sat/contrast/brightness) work when using
> bayer since it is not being converted to YUV. this is one reason you
> should not use bayer unless there is no other choice.
>
> Also I'm of the mind that we should not modify our driver in order to
> fix broken applications. The more correct thing will be to patch the
> applications to properly follow v4l2 specs. In this case allow
> controls with negative values.
>

I just did some testing:
1. Somehow green/blue gain are still locked if AWB is enabled. I thought my
patch fixed that but apparently it didn't.
2. Saturation, hue, etc work as expected, however ...
3. If saturation is set to less than circa 40 my computer locks up. I suspect
the framerate drops and the buffer is unhappy but I can't tell exactly because
there is no kernel-oops and dmesg looks fine after I reboot.
4. Why do we keep a wrapper function which calls wrapper functions through a
set of pointers which in the end all end up with the same functions - for hue,
saturation, blue/red gain, brightness, contrast and gamma there is only the
bridge functions so why not call them directly? And why not call
"sn9c20x_set_optical_parameters()" directly?
I think clearing that mess will make the module and the struct usb_sn9c20x at
least a bit smaller and also the code clearer. I'll post a patch once the
style isses are fixed.

GWater

signature.asc

Brian Johnson

unread,

Mar 21, 2009, 3:23:26 AM3/21/09

to micr...@googlegroups.com

Ok both i've submitted a patch for both vlc and amsn to their
respective devel mailing lists in regards to this issue i'll do kde's
tomorrow sometime.

The reason i originally did tie AWB to the red/blue gains is that
logically that is how you would expect it to work. If i have the
driver managing my white balance i would also expect the color gains
to be unavailable. Even if yes technically because one is using the
sensor and the other the bridge you can modify the color gains even
when AWB is in effect. It would also be conceivable since we get AWB
window data back from the bridge to write a similar function to the
software based ae function to implement whitebalanace calculations for
sensors that don't support whitebalance such as the micron sensors
without an IFP. If we ever did this then at least for some sensors the
AWB and color gains would be connected.

Hmm i don't get a lockup on my system when hue drops below 40. does it
still happen if you set it using sysfs to less then 40?

As for the wrapper functions yes we should probably be able to get rid
of them just fine. The only real reason i could think of to keep them
is that a function name like set_contrast is somewhat more obvious
then set_optical_parameters, but i don't think that by itself should
justify keeping the them either.

Boris Borisov

unread,

Mar 21, 2009, 3:24:49 AM3/21/09

to micr...@googlegroups.com

Josua Grawitter wrote:

Good morning (depend of my local time ;-) )
I follow the style of programming for function call. Before to send as patch I call directly the function "sn9c20x_set_optical_parameters()". The freezing of computer is happened if function '..set_optical..' is too slow. I lost too much time to solve this problem. The 'freezing' is happened always if circle array with parameters declared as local for function '..set_optical..'.
I have idea for some improvement of function but I wait for push of new patches for hue,set,bright... and your for camera button before do anything. The volume of patches is too hi to keep in local.

Brian Johnson

unread,

Mar 21, 2009, 8:16:03 PM3/21/09

to micr...@googlegroups.com

Alright a patch has been submitted to kopete for the issue with
negative v4l2 control values now.

Boris Borisov

unread,

Mar 22, 2009, 3:49:32 AM3/22/09

to micr...@googlegroups.com

It is ready to push in git?

Josua Grawitter

unread,

Mar 22, 2009, 7:06:26 AM3/22/09

to micr...@googlegroups.com

> --~--~---------~--~----~------------~-------~--~----~
> Lets make microdia webcams plug'n play, (currently plug'n pray)
> To post to this group, send email to micr...@googlegroups.com
> Visit us online https://groups.google.com/group/microdia
> -~----------~----~----~----~------~----~------~--~---

yes, the computer locks up whether I set saturation via v4l-ioctl or sysfs.

Here's what dmesg tells me:
usb 1-4: new high speed USB device using ehci_hcd and address 4
usb 1-4: configuration #1 chosen from 1 choice
usb 1-4: New USB device found, idVendor=0c45, idProduct=624e
usb 1-4: New USB device strings: Mfr=0, Product=1, SerialNumber=0
usb 1-4: Product: USB20 Camera
sn9c20x: SN9C20X USB 2.0 Webcam - 0C45:624E plugged-in.
sn9c20x: Detected SOI968 Sensor.
sn9c20x: Webcam device 0C45:624E is now controlling video device /dev/video0
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Using yuv420 output format
usbcore: registered new interface driver sn9c20x
sn9c20x: SN9C20x USB 2.0 Webcam Driver v2009.01 loaded
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180

Note: I set saturation only once. Still it seems optical_parameters was called
5 times.

GWater

signature.asc

Boris Borisov

unread,

Mar 22, 2009, 9:34:08 AM3/22/09

to micr...@googlegroups.com

Josua Grawitter wrote:

yes, the computer locks up whether I set saturation via v4l-ioctl or sysfs.

Here's what dmesg tells me:
usb 1-4: new high speed USB device using ehci_hcd and address 4                                                                             
usb 1-4: configuration #1 chosen from 1 choice                                                                                              
usb 1-4: New USB device found, idVendor=0c45, idProduct=624e                                                                                
usb 1-4: New USB device strings: Mfr=0, Product=1, SerialNumber=0                                                                           
usb 1-4: Product: USB20 Camera                                                                                                              
sn9c20x: SN9C20X USB 2.0 Webcam - 0C45:624E plugged-in.                                                                                     
sn9c20x: Detected SOI968 Sensor.                                                                                                            
sn9c20x: Webcam device 0C45:624E is now controlling video device /dev/video0                                                                
sn9c20x: Hue Value: 180                                                                                                                     
sn9c20x: Hue vsettings.hue: 180                                                                                                             
sn9c20x: Hue Value: 180                                                                                                                     
sn9c20x: Hue vsettings.hue: 180                                                                                                             
sn9c20x: Hue Value: 180                                                                                                                     
sn9c20x: Hue vsettings.hue: 180                                                                                                             
sn9c20x: Hue Value: 180                                                                                                                     
sn9c20x: Hue vsettings.hue: 180                                                                                                             
sn9c20x: Using yuv420 output format
usbcore: registered new interface driver sn9c20x
sn9c20x: SN9C20x USB 2.0 Webcam Driver v2009.01 loaded
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180
sn9c20x: Hue Value: 180
sn9c20x: Hue vsettings.hue: 180

Note: I set saturation only once. Still it seems optical_parameters was called 
5 times.

GWater

Maybe the problem is in userland program. With amsn is call not immediately after slide the sliders - that provoke a little delay on camera reaction.

Josua Grawitter

unread,

Mar 22, 2009, 3:37:29 PM3/22/09

to micr...@googlegroups.com

> --~--~---------~--~----~------------~-------~--~----~
> Lets make microdia webcams plug'n play, (currently plug'n pray)
> To post to this group, send email to micr...@googlegroups.com
> Visit us online https://groups.google.com/group/microdia
> -~----------~----~----~----~------~----~------~--~---

My Konsole doesn't delay - I used sysfs.

GWater

signature.asc

Brian Johnson

unread,

Mar 22, 2009, 8:55:46 PM3/22/09

to micr...@googlegroups.com

So its calling set=optical five times when you change saturation via
sysfs? I haven:t been able to get it to do that for me, wonder whats
different.

Also boris do you have a cleanedup version of the atch that has the
checkpatch.pl errors fixed?

On Sun, Mar 22, 2009 at 3:37 PM, Josua Grawitter

Boris Borisov

unread,

Mar 23, 2009, 12:58:12 PM3/23/09

to micr...@googlegroups.com

Maybe I lost coup of old mail from group and can't understand what you mind "checkpatch.pl errors fixed "
I have only the my and you last patch. Based on current (reverted without saturation) git. If I clone the git and apply the patch (my for sat/hue/b/c, and you for negative value of hue) file name hue_set_range.patch all seems ok.
Have your permission to do I commitment or you will do.
For me is very hart to do any improvement if I not have variant for fast reverting.

Brian Johnson wrote:

So its calling set=optical five times when you change saturation via
sysfs? I haven:t been able to get it to do that for me, wonder whats
different.

Also boris do you have a cleanedup version of the atch that has the
checkpatch.pl errors fixed?

On Sun, Mar 22, 2009 at 3:37 PM, Josua Grawitter
<grew...@googlemail.com> wrote:

Am Sonntag 22 MГ¤rz 2009 14:34:08 schrieb Boris Borisov:

Josua Grawitter wrote:

Am Samstag 21 MРґrz 2009 08:23:26 schrieb Brian Johnson:

Ok both i've submitted a patch for both vlc and amsn to their
respective devel mailing lists in regards to this issue i'll do kde's
tomorrow sometime.

The reason i originally did tie AWB to the red/blue gains is that
logically that is how you would expect it to work. If i have the
driver managing my white balance i would also expect the color gains
to be unavailable. Even if yes technically because one is using the
sensor and the other the bridge you can modify the color gains even
when AWB is in effect. В It would also be conceivable since we get AWB
window data back from the bridge to write a similar function to the
software based ae function to implement whitebalanace calculations for
sensors that don't support whitebalance such as the micron sensors
without an IFP. If we ever did this then at least for some sensors the
AWB and color gains would be connected.

Hmm i don't get a lockup on my system when hue drops below 40. does it
still happen if you set it using sysfs to less then 40?

As for the wrapper functions yes we should probably be able to get rid
of them just fine. The only real reason i could think of to keep them
is that a function name like set_contrast is somewhat more obvious
then set_optical_parameters, but i don't think that by itself should
justify keeping the them either.

On Fri, Mar 20, 2009 at 4:22 PM, Josua Grawitter

<grew...@googlemail.com> wrote:

GWater

Brian Johnson

unread,

Mar 23, 2009, 2:00:13 PM3/23/09

to micr...@googlegroups.com

checkpatch.pl is a script included with the linux kernel that will
check a patch to make sure it uses the proper style according to the
linux kernel coding style guidlines. you should run it against the
latest version of the hue_sat_range.patch and make sure you fix any
errors it reports. its located in the scripts directory of the kernel
sources and is run like the following

# checkpatch.pl --no-tree <path/to/patch>

2009/3/23 Boris Borisov <lz1...@gmail.com>:

Boris Borisov

unread,

Mar 23, 2009, 2:22:05 PM3/23/09

to micr...@googlegroups.com

Thanks a lot Brian. That is new for me. I found this pl script and will be clear all warnings.

Brian Johnson wrote:

 wrote:


Am Sonntag 22 MР“В¤rz 2009 14:34:08 schrieb Boris Borisov:


Josua Grawitter wrote:


Am Samstag 21 MР Т‘rz 2009 08:23:26 schrieb Brian Johnson:


Ok both i've submitted a patch for both vlc and amsn to their
respective devel mailing lists in regards to this issue i'll do kde's
tomorrow sometime.

The reason i originally did tie AWB to the red/blue gains is that
logically that is how you would expect it to work. If i have the
driver managing my white balance i would also expect the color gains
to be unavailable. Even if yes technically because one is using the
sensor and the other the bridge you can modify the color gains even
when AWB is in effect. Р’В It would also be conceivable since we get AWB
window data back from the bridge to write a similar function to the
software based ae function to implement whitebalanace calculations for
sensors that don't support whitebalance such as the micron sensors
without an IFP. If we ever did this then at least for some sensors the
AWB and color gains would be connected.

Hmm i don't get a lockup on my system when hue drops below 40. does it
still happen if you set it using sysfs to less then 40?

As for the wrapper functions yes we should probably be able to get rid
of them just fine. The only real reason i could think of to keep them
is that a function name like set_contrast is somewhat more obvious
then set_optical_parameters, but i don't think that by itself should
justify keeping the them either.

On Fri, Mar 20, 2009 at 4:22 PM, Josua Grawitter

Boris Borisov

unread,

Mar 23, 2009, 4:00:18 PM3/23/09

to micr...@googlegroups.com

Brian,
Find in attached file cleared patch. Check again because I have 1 err who is not speak me anything, also some warning but perhaps is normal i think. The messages is:
WARNING: LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
#622: FILE: sn9c20x-sysfs.c:475:

+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)

WARNING: LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
#654: FILE: sn9c20x-sysfs.c:501:

+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)

WARNING: LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
#691: FILE: sn9c20x-sysfs.c:538:

+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)

WARNING: LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged
#718: FILE: sn9c20x-sysfs.c:564:

+#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 24)

WARNING: consider using strict_strtoul in preference to simple_strtoul
#734: FILE: sn9c20x-sysfs.c:580:

+ value = simple_strtoul(buf, &endp, 16);

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 5 warnings, 909 lines checked

Brian Johnson wrote:

 wrote:


Am Sonntag 22 MР“В¤rz 2009 14:34:08 schrieb Boris Borisov:


Josua Grawitter wrote:


Am Samstag 21 MР Т‘rz 2009 08:23:26 schrieb Brian Johnson:


Ok both i've submitted a patch for both vlc and amsn to their
respective devel mailing lists in regards to this issue i'll do kde's
tomorrow sometime.

The reason i originally did tie AWB to the red/blue gains is that
logically that is how you would expect it to work. If i have the
driver managing my white balance i would also expect the color gains
to be unavailable. Even if yes technically because one is using the
sensor and the other the bridge you can modify the color gains even
when AWB is in effect. Р’В It would also be conceivable since we get AWB
window data back from the bridge to write a similar function to the
software based ae function to implement whitebalanace calculations for
sensors that don't support whitebalance such as the micron sensors
without an IFP. If we ever did this then at least for some sensors the
AWB and color gains would be connected.

Hmm i don't get a lockup on my system when hue drops below 40. does it
still happen if you set it using sysfs to less then 40?

As for the wrapper functions yes we should probably be able to get rid
of them just fine. The only real reason i could think of to keep them
is that a function name like set_contrast is somewhat more obvious
then set_optical_parameters, but i don't think that by itself should
justify keeping the them either.

On Fri, Mar 20, 2009 at 4:22 PM, Josua Grawitter

hue_sat_negativerange.patch

Reply all

Reply to author

Forward