Re: [RFC 4/4] v4l: Tell user space we're using monotonic timestamps

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sakari Ailus <sakari.ailus@iki.fi>
Cc: linux-media@vger.kernel.org, hverkuil@xs4all.nl
Subject: Re: [RFC 4/4] v4l: Tell user space we're using monotonic timestamps
Date: Thu, 08 Nov 2012 13:18:15 +0100	[thread overview]
Message-ID: <6798781.iK5M8mTgMg@avalon> (raw)
In-Reply-To: <20121105140432.GD25623@valkosipuli.retiisi.org.uk>

Hi Sakari,

On Monday 05 November 2012 16:04:32 Sakari Ailus wrote:
> On Sun, Nov 04, 2012 at 01:07:25PM +0100, Laurent Pinchart wrote:
> > On Wednesday 24 October 2012 21:16:23 Sakari Ailus wrote:
> ...
> 
> > > @@ -367,7 +368,8 @@ static void __fill_v4l2_buffer(struct vb2_buffer
> > > *vb,
> > > struct v4l2_buffer *b) /*
> > > 
> > >  	 * Clear any buffer state related flags.
> > >  	 */
> > > 
> > > -	b->flags &= ~V4L2_BUFFER_STATE_FLAGS;
> > > +	b->flags &= ~V4L2_BUFFER_MASK_FLAGS;
> > > +	b->flags |= V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > 
> > That's an issue. Drivers that use videobuf2 would always be restricted to
> > monotonic timestamps in the future, even if they provide support for a
> > device- specific clock.
> > 
> > Would it instead make sense to pass a v4l2_buffer pointer to
> > v4l2_get_timestamp() and set the monotonic flag there ? Not all callers of
> > v4l2_get_timestamp() might have a v4l2_buffer pointer though.
> 
> For now, this patch assumes that all the VB2 users will use monotonic
> timestamps only. Once we have a good use case for different kind of
> timestamps and have agreed how to implement them, I was thinking of adding a
> similar function to v4l2_get_timestamp() but which would be VB2-aware, with
> one argument being the timestamp type. That function could then get the
> timestamp and apply the relevant flags.
> 
> Do you think it'd be enough to support changeable timestamp type for drivers
> using VB2 only?

Given that there's no reason to use anything else than VB2 in V4L2 drivers, I 
don't see any problem there.

How would that work in practice ? You won't be able to override the timestamp 
type flag unconditionally in __fill_v4l2_buffer() anymore.

> Alternatively we could make v4l2_get_timestamp() v4l2_buffer-aware, and for
> drivers that can't provide the buffer pointer we could just set the pointer
> to NULL, and v4l2_get_timestamp() could ignore it. The driver would then be
> responsible for setting the right flags to the buffer on its own. As far as
> I understand, this is essentially what you proposed.

-- 
Regards,

Laurent Pinchart