Re: [RFCv2 PATCH 08/11] v4l2-ctrls: simplify event subscription.

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

 



Hi Hans,

Thanks for the patch.

On Wednesday 25 May 2011 15:33:52 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
>  drivers/media/video/v4l2-ctrls.c |   31 +++++++++++++++++++++++++++++++
>  include/media/v4l2-ctrls.h       |   25 +++++++++++++++++++++++++
>  2 files changed, 56 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-ctrls.c
> b/drivers/media/video/v4l2-ctrls.c index e2a7ac7..9807a20 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -831,6 +831,22 @@ int v4l2_ctrl_handler_init(struct v4l2_ctrl_handler
> *hdl, }
>  EXPORT_SYMBOL(v4l2_ctrl_handler_init);
> 
> +/* Count the number of controls */
> +unsigned v4l2_ctrl_handler_cnt(struct v4l2_ctrl_handler *hdl)
> +{
> +	struct v4l2_ctrl_ref *ref;
> +	unsigned cnt = 0;
> +
> +	if (hdl == NULL)
> +		return 0;
> +	mutex_lock(&hdl->lock);
> +	list_for_each_entry(ref, &hdl->ctrl_refs, node)
> +		cnt++;

As you don't use the entry, you can replace list_for_each_entry with 
list_for_each.

Should the handler keep a controls count ? In that case you wouldn't need this 
function.

> +	mutex_unlock(&hdl->lock);
> +	return cnt;
> +}
> +EXPORT_SYMBOL(v4l2_ctrl_handler_cnt);
> +
>  /* Free all controls and control refs */
>  void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl)
>  {
> @@ -1999,3 +2015,18 @@ void v4l2_ctrl_del_fh(struct v4l2_ctrl *ctrl, struct
> v4l2_fh *fh) v4l2_ctrl_unlock(ctrl);
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_del_fh);
> +
> +int v4l2_ctrl_sub_fh(struct v4l2_fh *fh, struct v4l2_event_subscription
> *sub, +		     unsigned n)

I would rename this to v4l2_ctrl_subscribe_fh(). I had trouble understanding 
what v4l2_ctrl_sub_fh() before reading the documentation. sub makes me think 
about sub-devices and subtract, not subscription.

> +{
> +	int ret = 0;
> +
> +	if (!fh->events)
> +		ret = v4l2_event_init(fh);
> +	if (!ret)
> +		ret = v4l2_event_alloc(fh, n);
> +	if (!ret)
> +		ret = v4l2_event_subscribe(fh, sub);

I tend to return errors when they occur instead of continuing to the end of 
the function. Handling errors on the spot makes code easier to read in my 
opinion, as I expect the main code flow to be the error-free path.

> +	return ret;
> +}

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