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]

 



On Tue, 30 Aug 2011, Hans Verkuil wrote:

> On Tuesday, August 30, 2011 16:24:55 Guennadi Liakhovetski wrote:
> > Hi Hans
> > 
> > On Tue, 30 Aug 2011, Hans Verkuil wrote:

[snip]

> > > The problem with S_FMT changing the crop rectangle (and I assume we are not
> > > talking about small pixel tweaks to make the hardware happy) is that the
> > > crop operation actually removes part of the frame. That's not something you
> > > would expect S_FMT to do, ever. Such an operation has to be explicitly
> > > requested by the user.
> > > 
> > > It's also why properly written applications (e.g. capture-example.c) has
> > > code like this to reset the crop rectangle before starting streaming:
> > > 
> > >         if (0 == xioctl(fd, VIDIOC_CROPCAP, &cropcap)) {
> > >                 crop.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > >                 crop.c = cropcap.defrect; /* reset to default */
> > > 
> > >                 if (-1 == xioctl(fd, VIDIOC_S_CROP, &crop)) {
> > >                         switch (errno) {
> > >                         case EINVAL:
> > >                                 /* Cropping not supported. */
> > >                                 break;
> > >                         default:
> > >                                 /* Errors ignored. */
> > >                                 break;
> > >                         }
> > >                 }
> > >         }
> > > 
> > > (Hmm, capture-example.c should also test for ENOTTY since we changed the
> > > error code).
> > 
> > I agree, that preserving input rectangle == output rectangle in reply to 
> > S_FMT is not nice, and should be avoided, wherever possible. Still, I 
> > prefer this to sticking with just one fixed output geometry, especially 
> > since (1) the spec doesn't prohibit this behaviour,
> 
> Hmm, I think it should be prohibited. Few drivers actually implement crop,
> and fewer applications use it. So I'm not surprised the spec doesn't go into 
> much detail.
> 
> > (2) there are already 
> > precedents in the mainline.
> 
> Which precedents? My guess is that any driver that does this was either not
> (or poorly) reviewed, or everyone just missed it.

My first two sensor drivers mt9m001 and mt9v022 do this, but, I suspect, I 
didn't invent it at that time, I think, I copied it from somewhere, cannot 
say for sure though anymore.

> > Maybe, a bit of hardware background would help: the sensor is actually 
> > supposed to be able to both crop and scale, and we did try to implement 
> > scales other than 1:1, but the chip just refused to produce anything 
> > meaningful.
> 
> I still don't see any reason why S_FMT would suddenly crop on such a sensor.
> It's completely unexpected and the user does not get what he expects.

Good, let's make it simple for all (except Bastian) then: Bastian, sorry 
for having misguided you, please, switch to .s_crop().

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