Re: [PATCH] media: Add support for arbitrary resolution for the ov5642 camera driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux