Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1227708

Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval

Show all headers | View raw

On 09/18/15 11:09, Arnd Bergmann wrote:
> On Friday 18 September 2015 10:05:06 Hans Verkuil wrote:
>> On 09/17/15 23:19, Arnd Bergmann wrote:
>>> The v4l2 API uses a 'struct timeval' to communicate time stamps to user
>>> space. This is broken on 32-bit architectures as soon as we have a C library
>>> that defines time_t as 64 bit, which then changes the structure layout of
>>> struct v4l2_buffer.
>>>
>>> Since existing user space source code relies on the type to be 'struct
>>> timeva' and we want to preserve compile-time compatibility when moving
>>
>> s/timeva/timeval/
> 
> Fixed
> 
>>> to a new libc, we cannot make user-visible changes to the header file.
>>>
>>> In this patch, we change the type of the timestamp to 'struct v4l2_timeval'
>>
>> Don't we need a kernel-wide timeval64? Rather than adding a v4l2-specific
>> struct?
> 
> I still hope to avoid doing that. All in-kernel users should be changed to
> use timespec64 or ktime_t, which are always more efficient and accurate.
> 
> For the system call interface, all timeval APIs are deprecated and have
> replacements using timespec64 (e.g. clock_gettime() replaces gettimeofday).
> 
> Only a handful of ioctls pass timeval, and so far my impression is that
> we are better off handling each one separately. The total amount of code
> we need to add this way should be less than if we have to duplicate all
> common code functions that today operate on timeval and can eventually
> get removed.

Ah, OK. Got it.

I think this is dependent on the upcoming media workshop next month. If we
decide to redesign v4l2_buffer anyway, then we can avoid timeval completely.
And the only place where we would need to convert it in the compat code
hidden in the v4l2 core (likely v4l2-ioctl.c).

I am not really keen on having v4l2_timeval in all these drivers. I would
have to check them anyway since I suspect that in several drivers the local
timeval variable can be avoided by rewriting that part of the driver.

Personally I am in favor of a redesigned v4l2_buffer: it's awkward to use
with multiplanar formats, there is cruft in there that can be removed (timecode),
and there is little space for additions (HW-specific timecodes, more buffer
meta data, etc).

We'll see.

Regards,

	Hans

> 
>>> diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
>>> index 295fde5fdb75..df5daac6d099 100644
>>> --- a/drivers/media/platform/vim2m.c
>>> +++ b/drivers/media/platform/vim2m.c
>>> @@ -235,7 +235,7 @@ static int device_process(struct vim2m_ctx *ctx,
>>>  	in_vb->v4l2_buf.sequence = q_data->sequence++;
>>>  	memcpy(&out_vb->v4l2_buf.timestamp,
>>>  			&in_vb->v4l2_buf.timestamp,
>>> -			sizeof(struct timeval));
>>> +			sizeof(struct v4l2_timeval));
>>>  	if (in_vb->v4l2_buf.flags & V4L2_BUF_FLAG_TIMECODE)
>>>  		memcpy(&out_vb->v4l2_buf.timecode, &in_vb->v4l2_buf.timecode,
>>>  			sizeof(struct v4l2_timecode));
>>
>> See https://patchwork.linuxtv.org/patch/31405/
>>
>> I'll merge that one for 4.4 very soon.
> 
> Ok.
> 
> 	Arnd
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2 0/9] [media] y2038 conversion for subsystem Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
  [PATCH v2 2/9] [media] dvb: remove unused systime() function Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
  [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
    Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Hans Verkuil <hverkuil@xs4all.nl> - 2015-09-18 10:10 +0200
      Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Arnd Bergmann <arnd@arndb.de> - 2015-09-18 11:20 +0200
        Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Hans Verkuil <hverkuil@xs4all.nl> - 2015-09-18 11:30 +0200
          Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Arnd Bergmann <arnd@arndb.de> - 2015-09-18 11:50 +0200
            Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Hans Verkuil <hverkuil@xs4all.nl> - 2015-09-18 12:00 +0200
              Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Arnd Bergmann <arnd@arndb.de> - 2015-09-18 12:10 +0200
                Re: [PATCH v2 7/9] [media] v4l2: introduce v4l2_timeval Hans Verkuil <hverkuil@xs4all.nl> - 2015-09-18 12:30 +0200
  [PATCH v2 9/9] [media] omap3isp: support 64-bit version of omap3isp_stat_data Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
  [PATCH v2 5/9] [media] make VIDIOC_DQEVENT work with 64-bit time_t Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
  [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
    Re: [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Hans Verkuil <hverkuil@xs4all.nl> - 2015-09-18 09:30 +0200
      Re: [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Arnd Bergmann <arnd@arndb.de> - 2015-09-18 11:30 +0200
        Re: [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Hans Verkuil <hverkuil@xs4all.nl> - 2015-09-18 12:00 +0200
          Re: [PATCH v2 8/9] [media] handle 64-bit time_t in v4l2_buffer Arnd Bergmann <arnd@arndb.de> - 2015-09-18 12:20 +0200
  [PATCH v2 3/9] [media] dvb: don't use 'time_t' in event ioctl Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
  [PATCH v2 6/9] [media] use v4l2_get_timestamp where possible Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
  [PATCH v2 1/9] [media] dvb: use ktime_t for internal timeout Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200
  [PATCH v2 4/9] [media] exynos4-is: use monotonic timestamps as advertized Arnd Bergmann <arnd@arndb.de> - 2015-09-17 23:30 +0200

csiph-web