Re: [PATCH] ov772x: add support S_CROP operation.
On Fri, 30 Jan 2009, morimoto.kunin...@renesas.com wrote: On my opinion, not only calling set_bus_param but also try_fmt is important for tw9910. Because tw9910 needs INTERLACE mode, and sh_mobile_ceu sets is_interlace flag in try_fmt. Ooh, this is wrong. As we discussed before - try_fmt shall not perform any configuration, it only tries, i.e., tests, whether the specified configuration is possible. This has to be moved to S_FMT. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- 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] ov772x: add support S_CROP operation.
On Fri, 30 Jan 2009, morimoto.kunin...@renesas.com wrote: Dear Guennadi On my opinion, not only calling set_bus_param but also try_fmt is important for tw9910. Because tw9910 needs INTERLACE mode, and sh_mobile_ceu sets is_interlace flag in try_fmt. Ooh, this is wrong. As we discussed before - try_fmt shall not perform any configuration, it only tries, i.e., tests, whether the specified configuration is possible. This has to be moved to S_FMT. Indeed. But set_fmt doesn't called with filed now. Can I fix it ? Hm, ok, I was thinking about this several times, to use struct v4l2_format *fmt instead of __u32 pixfmt as a second parameter to set_fmt in both host and device structs. The disadvantage of this is, that then the information in the third parameter struct v4l2_rect *rect becomes redundant in case of S_FMT... I think, it might be better to finally separate S_FMT and S_CROP, i.e., add new set_crop methods to both host and device structs and switch all drivers to use them... Looks like this would be a cleaner solution than keeping them together and struggling to differentiate between the two... Specific drivers can then decide to implement them using the same function internally, like, e.g., mt9m001 which doesn't use pixfmt at all, or mt9v022, which only uses it to check validity. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- 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] ov772x: add support S_CROP operation.
On Tue, 27 Jan 2009 08:53:23 +0100 (CET) Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Guennadi, I'm understanding that you're reviewing this patch and other ones for soc_camera and will send me a PULL request after reviewing those stuff. I've updated patchwork to reflect this. Thanks, Mauro On Tue, 27 Jan 2009, morimoto.kunin...@renesas.com wrote: Dear Guennadi what is the best way to us ??? or do I miss understanding ??? Fix behaviour if no S_FMT is done. I attached stupid 4 patches. I would like to hear your opinion. please check it. I wonder is there any soc_camera that works without calling S_FMT though set_bus_param is not called ? Don't know, never tested that way. Might well be they don't, in which case they need to be fixed. If soc_camera works without calling S_FMT, s_crop should call try_fmt_vid_cap and set_bus_param like s_fmt_vid_cap I think. And I think current_fmt is better than 0 to set_fmt if user wants only geometry changes on s_crop. it mean keep format. These patches works well on my local environment. ov772x and tw9910 work even if without -f option on capture_example. If you can agree with this idea, I will send these as formal patch. Thanks for the patches, please, give me a couple of days for review. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- 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 Cheers, Mauro -- 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] ov772x: add support S_CROP operation.
On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote: On Tue, 27 Jan 2009 08:53:23 +0100 (CET) Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Guennadi, I'm understanding that you're reviewing this patch and other ones for soc_camera and will send me a PULL request after reviewing those stuff. Yes, I'm (going to be) reviewing them, as soon as I find some time. Then I'll send you two pull requests - fixes for 2.6.29 and 2.6.30 material. AFAIK, unfortunately, mercurial doesn't support branches, so, I probably will end up first sending you a pull request with fixes, and after some time I'll also add 2.6.30 further development to the same tree and send another pull request. No idea what I do, if after that more 2.6.29 fixes come... I've updated patchwork to reflect this. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- 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] ov772x: add support S_CROP operation.
On Thu, 29 Jan 2009 11:00:41 +0100 (CET) Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: On Thu, 29 Jan 2009, Mauro Carvalho Chehab wrote: On Tue, 27 Jan 2009 08:53:23 +0100 (CET) Guennadi Liakhovetski g.liakhovet...@gmx.de wrote: Hi Guennadi, I'm understanding that you're reviewing this patch and other ones for soc_camera and will send me a PULL request after reviewing those stuff. Yes, I'm (going to be) reviewing them, as soon as I find some time. Then I'll send you two pull requests - fixes for 2.6.29 and 2.6.30 material. AFAIK, unfortunately, mercurial doesn't support branches, so, I probably will end up first sending you a pull request with fixes, and after some time I'll also add 2.6.30 further development to the same tree and send another pull request. No idea what I do, if after that more 2.6.29 fixes come... Yes, this is another drawback of hg. For fixes, please add: Priority: high At the body of the description. My scripts will understand this as fix patches. Cheers, Mauro -- 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] ov772x: add support S_CROP operation.
Dear Guennadi I attached stupid 4 patches. (snip) Thanks for the patches, please, give me a couple of days for review. (snip) Yes, I'm (going to be) reviewing them, as soon as I find some time. If you have not reviewed now, please use attached one. It use more clever way I think. 0001-soc_camera-s_crop-call-s_fmt_vid_cap-directly.patch Description: Binary data Best regards -- Kuninori Morimoto
Re: [PATCH] ov772x: add support S_CROP operation.
Hi Morimoto-san, On Fri, 30 Jan 2009, morimoto.kunin...@renesas.com wrote: I attached stupid 4 patches. (snip) Thanks for the patches, please, give me a couple of days for review. (snip) Yes, I'm (going to be) reviewing them, as soon as I find some time. If you have not reviewed now, please use attached one. It use more clever way I think. I'll have to think about it more, but the first impression is - this wouldn't work. At the moment we use the same soc-camera API call set_fmt for both S_FMT and S_CROP calls. But it you look in various instance drivers - host and camera - you will see, that almost all of them have a test if (pixfmt), i.e., they have to differentiate between the two cases. And not only because with pixfmt == 0 they cannot configure the stack completely, but because the meaning of these two calls even just with respect to geometry is different: S_FMT specifies scaling, whereas S_CROP preserves the current scaling and only specifies a window using the current scaled coordinates. So, you have to be able to differentiate. The original mt9m001 and mt9v022 drivers didn't support scaling, so, for they just used cropping for both, but the recently added mt9t031 does support scaling, so, it is indeed important. Not sure about mt9m111, and yours ov772x and tw9910. Further, calling set_bus_param() from (or soc_camera_s_fmt_vid_cap()) from S_CROP is not enough too. This lets the capture.c example run, but, I think, we should be able to run with no configuration at all - even without a S_CROP. So, some default configuration has to be set up on open() or on STREAMON if none is set yet (current_fmt == NULL). So, you can either wit until I find time to address this, or try to do something along these lines yourself. But I'm not sure if I manage to propose anything meaningful before FOSDEM (next weekend), so, this might take up to two weeks. But, I think, we have a bit of time before the 2.6.30 merge window:-) Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- 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] ov772x: add support S_CROP operation.
Dear Guennadi This is probably because you don't setup a default mode in sh_mobile_ceu_camera.c or ov772x.c if S_FMT is never called. This should be fixed. I'm not sure if other drivers would deliver anything meaningful without a call to S_FMT - has to be tested / fixed... (snip) In this case, shoud ov772x use defaul color format when S_CROP order ? The thing is that older versions of capture.c always called S_FMT, newer ones don't unless forced per -f switch. Yes. Now, sh_mobile_ceu :: set_bus_param must be called to use camera. We need -f option. Thank you. sorry. wrong understanding. ov772x and tw9910 should works even if -f is not used. is this correct ? -- 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] ov772x: add support S_CROP operation.
On Mon, 26 Jan 2009, morimoto.kunin...@renesas.com wrote: Dear Guennadi This is probably because you don't setup a default mode in sh_mobile_ceu_camera.c or ov772x.c if S_FMT is never called. This should be fixed. I'm not sure if other drivers would deliver anything meaningful without a call to S_FMT - has to be tested / fixed... (snip) In this case, shoud ov772x use defaul color format when S_CROP order ? The thing is that older versions of capture.c always called S_FMT, newer ones don't unless forced per -f switch. Yes. Now, sh_mobile_ceu :: set_bus_param must be called to use camera. We need -f option. Thank you. sorry. wrong understanding. ov772x and tw9910 should works even if -f is not used. is this correct ? Yes. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer -- 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] ov772x: add support S_CROP operation.
ov772x_set_fmt had returned NULL when pixfmt is 0,
although it mean only geometry change.
This patch modify this problem.
Signed-off-by: Kuninori Morimoto morimoto.kunin...@renesas.com
---
drivers/media/video/ov772x.c | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/media/video/ov772x.c b/drivers/media/video/ov772x.c
index 681a11b..30eb80e 100644
--- a/drivers/media/video/ov772x.c
+++ b/drivers/media/video/ov772x.c
@@ -792,12 +792,15 @@ static int ov772x_set_fmt(struct soc_camera_device *icd,
/*
* select format
+* when pixfmt is 0, only geometry change
*/
- priv-fmt = NULL;
- for (i = 0; i ARRAY_SIZE(ov772x_cfmts); i++) {
- if (pixfmt == ov772x_cfmts[i].fourcc) {
- priv-fmt = ov772x_cfmts + i;
- break;
+ if (pixfmt) {
+ priv-fmt = NULL;
+ for (i = 0; i ARRAY_SIZE(ov772x_cfmts); i++) {
+ if (pixfmt == ov772x_cfmts[i].fourcc) {
+ priv-fmt = ov772x_cfmts + i;
+ break;
+ }
}
}
if (!priv-fmt)
--
1.5.6.3
--
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
