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-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
