Re: [PATCH] v4l: mt9v032: Fix Bayer pattern
On Sat, Jul 16, 2011 at 11:40:23PM +0200, Guennadi Liakhovetski wrote: On Sat, 16 Jul 2011, Laurent Pinchart wrote: Hi Guennadi, On Saturday 16 July 2011 01:11:28 Guennadi Liakhovetski wrote: On Fri, 15 Jul 2011, Laurent Pinchart wrote: Compute crop rectangle boundaries to ensure a GRBG Bayer pattern. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/video/mt9v032.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) If there's no comment I'll send a pull request for this patch in a couple of days. Hm, I might have a comment: why?... Isn't it natural to accept the fact, that different sensors put a different Bayer pixel at their sensor matrix origin? Isn't that why we have all possible Bayer formats? Maybe you just have to choose a different output format? That's the other solution. The driver currently claims the device outputs SGRBG, but configures it to output SGBGR. This is clearly a bug. Is it better to modify the format than the crop rectangle location ? I would definitely crop the rectangle location than change the v4l2_mbus_framefmt.code; cropping should have no such side effects. If the user wants a different format, (s)he will use VIDIOC_SUBDEV_S_FMT ioctl to change it. Otherwise much of the pipeline configuration would require changes to it. What might still be important would be that the size of the image would not change as a result of adjusting due to the colour pattern. That should be avoided if possible, as alignment requirements exist and the user may well have requested a size which fits to the requirements of other blocks in the pipeline. Actually, it is interesting. I just looked (again) in the mt9v032 and some other Aptina Bayer sensor datasheets, and they actually have _odd_ numbers of rows and columns. So, mt9v032 actually has 753x481 active pixels. And that extra pixel is explicitly provided to adjust the origin colour. Ok, they write, it is for uniformity with the mirrored image, but who believes them?;-) So, maybe you should adjust your max values to the above ones, then taking one pixel out of your image will not reduce your useful image size. Some sensors I've seen are only able to produce just one colour pattern, even when using cropping. -- Sakari Ailus sakari.ai...@iki.fi -- 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] v4l: mt9v032: Fix Bayer pattern
On Monday 18 July 2011 00:30:52 Guennadi Liakhovetski wrote: On Mon, 18 Jul 2011, Laurent Pinchart wrote: On Monday 18 July 2011 00:14:21 Guennadi Liakhovetski wrote: On Sun, 17 Jul 2011, Laurent Pinchart wrote: Hi Guennadi, On Saturday 16 July 2011 23:40:23 Guennadi Liakhovetski wrote: On Sat, 16 Jul 2011, Laurent Pinchart wrote: On Saturday 16 July 2011 01:11:28 Guennadi Liakhovetski wrote: On Fri, 15 Jul 2011, Laurent Pinchart wrote: Compute crop rectangle boundaries to ensure a GRBG Bayer pattern. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/video/mt9v032.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) If there's no comment I'll send a pull request for this patch in a couple of days. Hm, I might have a comment: why?... Isn't it natural to accept the fact, that different sensors put a different Bayer pixel at their sensor matrix origin? Isn't that why we have all possible Bayer formats? Maybe you just have to choose a different output format? That's the other solution. The driver currently claims the device outputs SGRBG, but configures it to output SGBGR. This is clearly a bug. Is it better to modify the format than the crop rectangle location ? Actually, it is interesting. I just looked (again) in the mt9v032 and some other Aptina Bayer sensor datasheets, and they actually have _odd_ numbers of rows and columns. So, mt9v032 actually has 753x481 active pixels. And that extra pixel is explicitly provided to adjust the origin colour. Ok, they write, it is for uniformity with the mirrored image, but who believes them?;-) So, maybe you should adjust your max values to the above ones, then taking one pixel out of your image will not reduce your useful image size. I'm not sure what you mean. Even though the pixel array is bigger than that, the maximum output width/height are 752x480 according to the datasheet. Have a look at the Pixel array structure (p.10 in my version) section. I've seen that, but the sensor is still unable to output an image bigger than 752x480. See registers 3 and 4 maximum values on page 24 (in my version :-)). Right, sorry, what I mean, is, that even when you use one pixel to adjust your image origin, you don't actually lose the size. So, you can output 752 pixels in a row whether you begin at 0 or 1. That's why I suggested to set max width to 753, but then make sure it's always actually even. That way a configuration offset = 1, width = 752 will also be valid. offset = 1, width = 752 is valid. That's the default configuration with this patch applied. -- Regards, Laurent Pinchart -- 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] v4l: mt9v032: Fix Bayer pattern
On Monday 18 July 2011 00:14:21 Guennadi Liakhovetski wrote: On Sun, 17 Jul 2011, Laurent Pinchart wrote: Hi Guennadi, On Saturday 16 July 2011 23:40:23 Guennadi Liakhovetski wrote: On Sat, 16 Jul 2011, Laurent Pinchart wrote: On Saturday 16 July 2011 01:11:28 Guennadi Liakhovetski wrote: On Fri, 15 Jul 2011, Laurent Pinchart wrote: Compute crop rectangle boundaries to ensure a GRBG Bayer pattern. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/video/mt9v032.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) If there's no comment I'll send a pull request for this patch in a couple of days. Hm, I might have a comment: why?... Isn't it natural to accept the fact, that different sensors put a different Bayer pixel at their sensor matrix origin? Isn't that why we have all possible Bayer formats? Maybe you just have to choose a different output format? That's the other solution. The driver currently claims the device outputs SGRBG, but configures it to output SGBGR. This is clearly a bug. Is it better to modify the format than the crop rectangle location ? Actually, it is interesting. I just looked (again) in the mt9v032 and some other Aptina Bayer sensor datasheets, and they actually have _odd_ numbers of rows and columns. So, mt9v032 actually has 753x481 active pixels. And that extra pixel is explicitly provided to adjust the origin colour. Ok, they write, it is for uniformity with the mirrored image, but who believes them?;-) So, maybe you should adjust your max values to the above ones, then taking one pixel out of your image will not reduce your useful image size. I'm not sure what you mean. Even though the pixel array is bigger than that, the maximum output width/height are 752x480 according to the datasheet. Have a look at the Pixel array structure (p.10 in my version) section. I've seen that, but the sensor is still unable to output an image bigger than 752x480. See registers 3 and 4 maximum values on page 24 (in my version :-)). -- Regards, Laurent Pinchart -- 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] v4l: mt9v032: Fix Bayer pattern
On Mon, 18 Jul 2011, Laurent Pinchart wrote: On Monday 18 July 2011 00:14:21 Guennadi Liakhovetski wrote: On Sun, 17 Jul 2011, Laurent Pinchart wrote: Hi Guennadi, On Saturday 16 July 2011 23:40:23 Guennadi Liakhovetski wrote: On Sat, 16 Jul 2011, Laurent Pinchart wrote: On Saturday 16 July 2011 01:11:28 Guennadi Liakhovetski wrote: On Fri, 15 Jul 2011, Laurent Pinchart wrote: Compute crop rectangle boundaries to ensure a GRBG Bayer pattern. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/video/mt9v032.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) If there's no comment I'll send a pull request for this patch in a couple of days. Hm, I might have a comment: why?... Isn't it natural to accept the fact, that different sensors put a different Bayer pixel at their sensor matrix origin? Isn't that why we have all possible Bayer formats? Maybe you just have to choose a different output format? That's the other solution. The driver currently claims the device outputs SGRBG, but configures it to output SGBGR. This is clearly a bug. Is it better to modify the format than the crop rectangle location ? Actually, it is interesting. I just looked (again) in the mt9v032 and some other Aptina Bayer sensor datasheets, and they actually have _odd_ numbers of rows and columns. So, mt9v032 actually has 753x481 active pixels. And that extra pixel is explicitly provided to adjust the origin colour. Ok, they write, it is for uniformity with the mirrored image, but who believes them?;-) So, maybe you should adjust your max values to the above ones, then taking one pixel out of your image will not reduce your useful image size. I'm not sure what you mean. Even though the pixel array is bigger than that, the maximum output width/height are 752x480 according to the datasheet. Have a look at the Pixel array structure (p.10 in my version) section. I've seen that, but the sensor is still unable to output an image bigger than 752x480. See registers 3 and 4 maximum values on page 24 (in my version :-)). Right, sorry, what I mean, is, that even when you use one pixel to adjust your image origin, you don't actually lose the size. So, you can output 752 pixels in a row whether you begin at 0 or 1. That's why I suggested to set max width to 753, but then make sure it's always actually even. That way a configuration offset = 1, width = 752 will also be valid. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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] v4l: mt9v032: Fix Bayer pattern
Hi Guennadi, On Saturday 16 July 2011 01:11:28 Guennadi Liakhovetski wrote: On Fri, 15 Jul 2011, Laurent Pinchart wrote: Compute crop rectangle boundaries to ensure a GRBG Bayer pattern. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/video/mt9v032.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) If there's no comment I'll send a pull request for this patch in a couple of days. Hm, I might have a comment: why?... Isn't it natural to accept the fact, that different sensors put a different Bayer pixel at their sensor matrix origin? Isn't that why we have all possible Bayer formats? Maybe you just have to choose a different output format? That's the other solution. The driver currently claims the device outputs SGRBG, but configures it to output SGBGR. This is clearly a bug. Is it better to modify the format than the crop rectangle location ? The OMAP3 ISP supports all Bayer formats, but the driver configures itself for SGRBG by default. Using another pattern currently requires userspace software to change several hardware-dependent parameters (including matrices and tables). This should eventually be fixed in the OMAP3 ISP driver, but for the time being application developers will have an easier life if the sensor outputs SGRBG instead of SGBRG. -- Regards, Laurent Pinchart -- 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] v4l: mt9v032: Fix Bayer pattern
On Sat, 16 Jul 2011, Laurent Pinchart wrote: Hi Guennadi, On Saturday 16 July 2011 01:11:28 Guennadi Liakhovetski wrote: On Fri, 15 Jul 2011, Laurent Pinchart wrote: Compute crop rectangle boundaries to ensure a GRBG Bayer pattern. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/video/mt9v032.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) If there's no comment I'll send a pull request for this patch in a couple of days. Hm, I might have a comment: why?... Isn't it natural to accept the fact, that different sensors put a different Bayer pixel at their sensor matrix origin? Isn't that why we have all possible Bayer formats? Maybe you just have to choose a different output format? That's the other solution. The driver currently claims the device outputs SGRBG, but configures it to output SGBGR. This is clearly a bug. Is it better to modify the format than the crop rectangle location ? Actually, it is interesting. I just looked (again) in the mt9v032 and some other Aptina Bayer sensor datasheets, and they actually have _odd_ numbers of rows and columns. So, mt9v032 actually has 753x481 active pixels. And that extra pixel is explicitly provided to adjust the origin colour. Ok, they write, it is for uniformity with the mirrored image, but who believes them?;-) So, maybe you should adjust your max values to the above ones, then taking one pixel out of your image will not reduce your useful image size. Thanks Guennadi The OMAP3 ISP supports all Bayer formats, but the driver configures itself for SGRBG by default. Using another pattern currently requires userspace software to change several hardware-dependent parameters (including matrices and tables). This should eventually be fixed in the OMAP3 ISP driver, but for the time being application developers will have an easier life if the sensor outputs SGRBG instead of SGBRG. -- Regards, Laurent Pinchart --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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] v4l: mt9v032: Fix Bayer pattern
Compute crop rectangle boundaries to ensure a GRBG Bayer pattern. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/video/mt9v032.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) If there's no comment I'll send a pull request for this patch in a couple of days. diff --git a/drivers/media/video/mt9v032.c b/drivers/media/video/mt9v032.c index 1319c2c..c64e1dc 100644 --- a/drivers/media/video/mt9v032.c +++ b/drivers/media/video/mt9v032.c @@ -31,14 +31,14 @@ #define MT9V032_CHIP_VERSION 0x00 #defineMT9V032_CHIP_ID_REV10x1311 #defineMT9V032_CHIP_ID_REV30x1313 -#define MT9V032_ROW_START 0x01 -#defineMT9V032_ROW_START_MIN 4 -#defineMT9V032_ROW_START_DEF 10 -#defineMT9V032_ROW_START_MAX 482 -#define MT9V032_COLUMN_START 0x02 +#define MT9V032_COLUMN_START 0x01 #defineMT9V032_COLUMN_START_MIN1 -#defineMT9V032_COLUMN_START_DEF2 +#defineMT9V032_COLUMN_START_DEF1 #defineMT9V032_COLUMN_START_MAX752 +#define MT9V032_ROW_START 0x02 +#defineMT9V032_ROW_START_MIN 4 +#defineMT9V032_ROW_START_DEF 5 +#defineMT9V032_ROW_START_MAX 482 #define MT9V032_WINDOW_HEIGHT 0x03 #defineMT9V032_WINDOW_HEIGHT_MIN 1 #defineMT9V032_WINDOW_HEIGHT_DEF 480 @@ -420,13 +420,13 @@ static int mt9v032_set_crop(struct v4l2_subdev *subdev, struct v4l2_rect *__crop; struct v4l2_rect rect; - /* Clamp the crop rectangle boundaries and align them to a multiple of 2 -* pixels. + /* Clamp the crop rectangle boundaries and align them to a non multiple +* of 2 pixels to ensure a GRBG Bayer pattern. */ - rect.left = clamp(ALIGN(crop-rect.left, 2), + rect.left = clamp(ALIGN(crop-rect.left + 1, 2) - 1, MT9V032_COLUMN_START_MIN, MT9V032_COLUMN_START_MAX); - rect.top = clamp(ALIGN(crop-rect.top, 2), + rect.top = clamp(ALIGN(crop-rect.top + 1, 2) - 1, MT9V032_ROW_START_MIN, MT9V032_ROW_START_MAX); rect.width = clamp(ALIGN(crop-rect.width, 2), -- Regards, Laurent Pinchart -- 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] v4l: mt9v032: Fix Bayer pattern
On Fri, 15 Jul 2011, Laurent Pinchart wrote: Compute crop rectangle boundaries to ensure a GRBG Bayer pattern. Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com --- drivers/media/video/mt9v032.c | 20 ++-- 1 files changed, 10 insertions(+), 10 deletions(-) If there's no comment I'll send a pull request for this patch in a couple of days. Hm, I might have a comment: why?... Isn't it natural to accept the fact, that different sensors put a different Bayer pixel at their sensor matrix origin? Isn't that why we have all possible Bayer formats? Maybe you just have to choose a different output format? Thanks Guennadi diff --git a/drivers/media/video/mt9v032.c b/drivers/media/video/mt9v032.c index 1319c2c..c64e1dc 100644 --- a/drivers/media/video/mt9v032.c +++ b/drivers/media/video/mt9v032.c @@ -31,14 +31,14 @@ #define MT9V032_CHIP_VERSION 0x00 #define MT9V032_CHIP_ID_REV10x1311 #define MT9V032_CHIP_ID_REV30x1313 -#define MT9V032_ROW_START0x01 -#define MT9V032_ROW_START_MIN 4 -#define MT9V032_ROW_START_DEF 10 -#define MT9V032_ROW_START_MAX 482 -#define MT9V032_COLUMN_START 0x02 +#define MT9V032_COLUMN_START 0x01 #define MT9V032_COLUMN_START_MIN1 -#define MT9V032_COLUMN_START_DEF2 +#define MT9V032_COLUMN_START_DEF1 #define MT9V032_COLUMN_START_MAX752 +#define MT9V032_ROW_START0x02 +#define MT9V032_ROW_START_MIN 4 +#define MT9V032_ROW_START_DEF 5 +#define MT9V032_ROW_START_MAX 482 #define MT9V032_WINDOW_HEIGHT0x03 #define MT9V032_WINDOW_HEIGHT_MIN 1 #define MT9V032_WINDOW_HEIGHT_DEF 480 @@ -420,13 +420,13 @@ static int mt9v032_set_crop(struct v4l2_subdev *subdev, struct v4l2_rect *__crop; struct v4l2_rect rect; - /* Clamp the crop rectangle boundaries and align them to a multiple of 2 - * pixels. + /* Clamp the crop rectangle boundaries and align them to a non multiple + * of 2 pixels to ensure a GRBG Bayer pattern. */ - rect.left = clamp(ALIGN(crop-rect.left, 2), + rect.left = clamp(ALIGN(crop-rect.left + 1, 2) - 1, MT9V032_COLUMN_START_MIN, MT9V032_COLUMN_START_MAX); - rect.top = clamp(ALIGN(crop-rect.top, 2), + rect.top = clamp(ALIGN(crop-rect.top + 1, 2) - 1, MT9V032_ROW_START_MIN, MT9V032_ROW_START_MAX); rect.width = clamp(ALIGN(crop-rect.width, 2), -- Regards, Laurent Pinchart -- 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 --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
