Re: [PATCH v3 04/33] v4l: VIDIOC_SUBDEV_S_SELECTION and VIDIOC_SUBDEV_G_SELECTION IOCTLs

Laurent Pinchart
2012-02-27T00:22:26Z

Hi Sakari,

On Thursday 23 February 2012 08:01:23 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > [snip]
> >
> >> +/* active cropping area */
> >> +#define V4L2_SUBDEV_SEL_TGT_CROP_ACTIVE 0x0000
> >> +/* cropping bounds */
> >> +#define V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS 0x0002
> >> +/* current composing area */
> >> +#define V4L2_SUBDEV_SEL_TGT_COMPOSE_ACTIVE 0x0100
> >> +/* composing bounds */
> >
> > I'm not sure if ACTIVE is a good name here. It sounds confusing as we
> > already have V4L2_SUBDEV_FORMAT_ACTIVE.
>
> We are using V4L2_SEL_TGT_COMPOSE_ACTIVE on V4L2 nodes already --- the
> name I'm using here just mirrors the naming on V4L2 device nodes. If I
> choose a different name here, some of that analogy is lost.
>
> That said, I'm not against changing this but the equivalent change
> should then be made on V4L2 selection API for consistency.

I'm not against changing the V4L2 selection API either :-) Just think about
developers talking about "try crop active" or "active crop bounds". Even
worse, will "active crop" refer to the active target or the active "which" ?
That will be very confusing.

--
Regards,

Laurent Pinchart