Re: [PATCH 1/4] v4l: add support for selection api

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

 



On Mon, Oct 17, 2011 at 03:31:17PM +0200, Tomasz Stanislawski wrote:
> Hi Sakari,
> 
> On 10/14/2011 07:19 PM, Sakari Ailus wrote:
> >On Wed, Oct 12, 2011 at 05:08:20PM +0200, Tomasz Stanislawski wrote:
> >>On 10/12/2011 01:48 PM, Sakari Ailus wrote:
> >>>Hi Tomasz,
> >>>
> >>>On Wed, Aug 31, 2011 at 02:28:20PM +0200, Tomasz Stanislawski wrote:
> >>>...
> >>>>diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> >>>>index fca24cc..b7471fe 100644
> >>>>--- a/include/linux/videodev2.h
> >>>>+++ b/include/linux/videodev2.h
> >>>>@@ -738,6 +738,48 @@ struct v4l2_crop {
> >>>>  	struct v4l2_rect        c;
> >>>>  };
> >>>>
> >>>>+/* Hints for adjustments of selection rectangle */
> >>>>+#define V4L2_SEL_SIZE_GE	0x00000001
> >>>>+#define V4L2_SEL_SIZE_LE	0x00000002
> >>>
> >>>A minor comment. If the patches have not been pulled yet, how about adding
> >>>FLAG_ to the flag names? I.e. V4L2_SEL_FLAG_SIZE_GE and
> >>>V4L2_SEL_FLAG_SIZE_LE.
> 
> I thought that it may be worth to drop _SEL_. The constraint flags
> could be reused in future ioctls? I mean S_FRAMESIZES or extensions
> to control API. What do you think?
> 
> >>
> >>Hi Sakari,
> >>
> >>The idea is good. I preferred to avoid using long names if possible.
> >>I agree that using _FLAGS_ produce more informative name.
> >>I'll fix it in the new version of selection API.
> >
> >Hi Tomasz,
> >
> >I'd also have the same comment on the selection targets.
> >V4L2_SEL_TGT_CROP_ACTIVE, for example?
> 
> It is logical to use _TGT_, it is sad that the names becomes so long :/

It's not _that_ long --- we have longer ones e.g. in include/linux/media.h.
They only need to be just descriptive enough...

Cheers,

-- 
Sakari Ailus
e-mail: sakari.ailus@xxxxxx	jabber/XMPP/Gmail: 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