On Tue, 30 Aug 2011, Bastian Hecht wrote: > 2011/8/30 Guennadi Liakhovetski <g.liakhovetski@xxxxxx>: > > 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(). > > Sure, no problem. So s_fmt() shall be not available at all then, > right? Instead the cropping rectangle can be changed and the output > rectangle is adjusted accordingly. I would keep .s_fmt() and just return the current configuration from it. 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