(also replying to a similar comment by Sakari) On Tue, 30 Aug 2011, Laurent Pinchart wrote:
> Hi Guennadi, > > On Tuesday 30 August 2011 10:55:08 Guennadi Liakhovetski wrote: > > On Mon, 29 Aug 2011, Laurent Pinchart wrote: > > > On Monday 29 August 2011 14:34:53 Guennadi Liakhovetski wrote: > > > > On Mon, 29 Aug 2011, Laurent Pinchart wrote: > > > > > On Monday 29 August 2011 14:18:50 Guennadi Liakhovetski wrote: > > > > > > On Sun, 28 Aug 2011, Laurent Pinchart wrote: > > > > > > > > > > > > [snip] > > > > > > > > > > > > > > @@ -774,17 +839,27 @@ static int ov5642_s_fmt(struct > > > > > > > > v4l2_subdev *sd, > > > > > > > > > > > > > > > > ov5642_try_fmt(sd, mf); > > > > > > > > > > > > > > > > + priv->out_size.width = mf->width; > > > > > > > > + priv->out_size.height = mf->height; > > > > > > > > > > > > > > It looks like to me (but I may be wrong) that you achieve > > > > > > > different resolutions using cropping, not scaling. If that's > > > > > > > correct you should implement s_crop support and refuse changing > > > > > > > the resolution through s_fmt. > > > > > > > > > > > > As the patch explains (I think) on several occasions, currently > > > > > > only the 1:1 scale is supported, and it was our deliberate choice > > > > > > to implement this using the scaling API > > > > > > > > > > If you implement cropping, you should use the crop API, not the > > > > > scaling API > > > > > > > > > > :-) > > > > > > > > It's changing both - input and output sizes. > > > > > > Sure, but it's still cropping. > > > > Why? Isn't it a matter of the PoV? > > No it isn't. Cropping is cropping, regardless of how you look at it. > > > It changes the output window, i.e., implements S_FMT. And S_FMT is by far > > more important / widely used than S_CROP. Refusing to change the output > > window and always just returning the == crop size wouldn't be polite, IMHO. > > If your sensor has no scaler the output size is equal to the crop rectangle. > There's no way around that, and there's no reason to have the driver behave > otherwise. > > > Don't think many users would guess to use S_CROP. > > Users who want to crop use S_CROP. > > > Standard applications a la mplayer don't use S_CROP at all. > > That's because they don't want to crop. mplayer expects the driver to perform > scaling when it calls S_FMT, and users won't be happy if you crop instead. So, here's my opinion, based on the V4L2 spec. I'm going to base on this and pull this patch into my tree and let Mauro decide, unless he expresses his negative opinion before that. The spec defines S_FMT as an operation to set the output (in case of a capture device) frame format. Which this driver clearly does. The output format should be set, using scaling, however, if the driver or the hardware are unable to preserve the exact same input rectangle to satisfy the request, the driver is also allowed to change the cropping rectangle _as much as necessary_ - S_FMT takes precedence. This has been discussed before, and the conclusion was - of the two geometry calls (S_FMT and S_CROP) the last call overrides previous setting of the opposite geometry. It also defines S_CROP as an operation to set the cropping rectangle. The driver is also allowed to change the output window, if it cannot be preserved. Similarly, the last call wins. Ideally in this situation I would implement both S_CROP and S_FMT and let both change the opposite window as needed, which in this case means set it equal to the one, being configured. Since most applications are primarily interested in the S_FMT call to configure their user interface, I find it a wrong approach to refuse S_FMT and always return the current cropping rectangle. In such a case the application will possibly be stuck with some default output rectangle, because it certainly will _not_ guess to use S_CROP to configure it. Whereas if we implement S_FMT with a constant 1:1 scale the application will get the required UI size. I agree, that changing the view area, while changing the output window, is not exactly what the user expects, but it's better than presenting all applications with a fixed, possibly completely unsuitable, UI window. 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
