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]

 



Hi Guennadi,

On Tuesday 30 August 2011 15:13:25 Guennadi Liakhovetski wrote:
> On Tue, 30 Aug 2011, Laurent Pinchart wrote:
> > 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.

I've also made other comments. I expect at least a v2 that addresses them.

-- 
Regards,

Laurent Pinchart
--
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