Re: [PATCH 1/2] v4l2 subdevs: replace get/set_crop by get/set_selection

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

 



Hi Laurent,

On Mon, Dec 08, 2014 at 02:17:11AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wednesday 03 December 2014 13:06:00 Sakari Ailus wrote:
> > On Tue, Dec 02, 2014 at 01:21:40PM +0100, Hans Verkuil wrote:
> > > From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> > > 
> > > The crop and selection pad ops are duplicates. Replace all uses of
> > > get/set_crop by get/set_selection. This will make it possible to drop
> > > get/set_crop altogether.
> > > 
> > > Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> > > Cc: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> > > Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > Cc: Prabhakar Lad <prabhakar.csengg@xxxxxxxxx>
> > > Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > 
> > For both:
> > 
> > Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > 
> > Another point I'd like to draw attention to are the reserved fields --- some
> > drivers appear to zero them whereas some pay no attention. Shouldn't we
> > check in the sub-device IOCTL handler that the user has zeroed them, or
> > zero them for the user? I think this has probably been discussed before on
> > V4L2. Both have their advantages, probably zeroing them in the framework
> > would be the best option. What do you think?
> 
> I think we should at least be consistent across drivers. Duplicating checks 
> across drivers being more error-prone it would likely be better to implement 
> them in core code. The question that remains to be answered is whether we can 
> consider that bridge drivers will correctly zero reserved fields when using 
> the API internally. If not, we'll need wrapper functions around subdev 
> operations to zero reserved fields, or possibly just to output a WARN_ON (as a 
> bridge driver bug should be fixed instead of silently ignored).

I'd simply check these fields in v4l2-subdev.c ioctl wrappers.

There are over 300 instances of v4l2_subdev_call(). It's at least possible
to audit them. I'd favour that over adding a wrapper to each op, and then
paying attention to the topic in reviews. It's not only a matter of that
interface.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@xxxxxx	XMPP: sailus@xxxxxxxxxxxxxx
--
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