Re: [PATCH 4/5] [media] v4l: fix copying ioctl results on failure

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

 



Hi Tomasz,

On Monday 29 August 2011 13:55:31 Tomasz Stanislawski wrote:
> On 08/29/2011 10:56 AM, Laurent Pinchart wrote:
> > On Monday 29 August 2011 10:01:58 Tomasz Stanislawski wrote:
> >> On 08/26/2011 05:09 PM, Laurent Pinchart wrote:
> >>> On Friday 26 August 2011 15:06:06 Tomasz Stanislawski wrote:
> >>>> This patch fix the handling of data passed to V4L2 ioctls.  The
> >>>> content of the structures is not copied if the ioctl fails.  It
> >>>> blocks ability to obtain any information about occurred error other
> >>>> then errno code. This patch fix this issue.
> >>> 
> >>> Does the V4L2 spec say anything on this topic ? We might have
> >>> applications that rely on the ioctl argument structure not being
> >>> touched when a failure occurs.
> >> 
> >> Ups.. I missed something. It looks that modifying ioctl content is
> >> illegal if ioctl fails. The spec says:
> >> "When an ioctl that takes an output or read/write parameter fails, the
> >> parameter remains unmodified." (v4l2 ioctl section)
> >> However, there is probably a bug already present in V4L2 framework.
> >> There are some ioctls that takes a pointer to an array as a field in the
> >> argument struct.
> >> The examples are all VIDIOC_*_EXT_CTRLS and VIDIOC_{QUERY/DQ/Q}_BUF
> >> family. The content of such an auxiliary arays is copied even if ioctl
> >> fails. Please take a look to video_usercopy function in v4l2-ioctl.c.
> >> Therefore I think that the spec is already violated. What is your
> >> opinion about this problem?
> > 
> > I think it was a bad idea to state that a parameter remains unmodified
> > when the ioctl fails in the first place. I'm fine with not following
> > that for new ioctls, but applications might rely on it for existing
> > ioctls.
> > 
> >> Now back to selection case.
> >> This patch was added as proposition of fix to VIDIOC_S_SELECTION, to
> >> return the best-hit rectangle if constraints could not be satisfied. The
> >> ioctl return -ERANGE in this case. Using those return values the
> >> application gets some feedback on loosing constraints.
> > 
> > Shouldn't that always be the case ? :-) VIDIOC_S_SELECTION should adjust
> > the rectangle up or down depending on the constraints and always return
> > the best match without any error.
> 
> ok.. but what to do if constraints could not be satisfied?
> The configuration should not be applied to the hardware in such a case,
> because it is not what the application desired.
> Therefore the ioctl must fail ... somehow.
> If the ioctl always succeed then the constraint flags becomes actually
> the hints.

That's what currently happens with S_FMT. Is that behaviour too limited in 
your opinion ?

> We may need TRY_SELECTION to test rectangles without applying it.
> 
> I thought that returning the best-hit rectangle by S_SELECTION might be
> useful because it gives the application some feedback on what the driver
> would accept.

If we indeed need to fail with -ERANGE, I agree with you, the behaviour is 
useful. It would make sense in that case to only update the parameter for a 
specific set of ioctls. The V4L2 documentation will also need to be fixed.

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