Hi Guennadi,
On Monday, 11 December 2017 23:44:09 EET Guennadi Liakhovetski wrote:
> On Mon, 11 Dec 2017, Laurent Pinchart wrote:
> > On Monday, 11 December 2017 22:16:23 EET Laurent Pinchart wrote:
> >> On Wednesday, 6 December 2017 17:15:40 EET Guennadi Liakhovetski wrote:
> >>> From: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxx>
> >>>
> >>> Some UVC video cameras contain metadata in their payload headers. This
> >>> patch extracts that data, adding more clock synchronisation
> >>> information, on both bulk and isochronous endpoints and makes it
> >>> available to the user space on a separate video node, using the
> >>> V4L2_CAP_META_CAPTURE capability and the V4L2_BUF_TYPE_META_CAPTURE
> >>> buffer queue type. By default, only the V4L2_META_FMT_UVC pixel format
> >>> is available from those nodes. However, cameras can be added to the
> >>> device ID table to additionally specify their own metadata format, in
> >>> which case that format will also become available from the metadata
> >>> node.
> >>>
> >>> Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxx>
> >>> ---
> >>>
> >>> v8: addressed comments and integrated changes from Laurent, thanks
> >>> again, e.g.:
> >>>
> >>> - multiple stylistic changes
> >>> - remove the UVC_DEV_FLAG_METADATA_NODE flag / quirk: nodes are now
> >>> created unconditionally
> >>> - reuse uvc_ioctl_querycap()
> >>> - reuse code in uvc_register_video()
> >>> - set an error flag when the metadata buffer overflows
> >>>
> >>> drivers/media/usb/uvc/Makefile | 2 +-
> >>> drivers/media/usb/uvc/uvc_driver.c | 15 ++-
> >>> drivers/media/usb/uvc/uvc_isight.c | 2 +-
> >>> drivers/media/usb/uvc/uvc_metadata.c | 179 ++++++++++++++++++++++++++++
> >>> drivers/media/usb/uvc/uvc_queue.c | 44 +++++++--
> >>> drivers/media/usb/uvc/uvc_video.c | 132 ++++++++++++++++++++++++--
> >>> drivers/media/usb/uvc/uvcvideo.h | 16 +++-
> >>> include/uapi/linux/uvcvideo.h | 26 +++++
> >>> 8 files changed, 394 insertions(+), 22 deletions(-)
> >>> create mode 100644 drivers/media/usb/uvc/uvc_metadata.c
> >>
> >> [snip]
> >>
> >> > diff --git a/drivers/media/usb/uvc/uvc_video.c
> >> > b/drivers/media/usb/uvc/uvc_video.c index 13f459e..2fc0bf2 100644
> >> > --- a/drivers/media/usb/uvc/uvc_video.c
> >> > +++ b/drivers/media/usb/uvc/uvc_video.c
> >>
> >> [snip]
> >>
> >>> +static void uvc_video_decode_meta(struct uvc_streaming *stream,
> >>> + struct uvc_buffer *meta_buf,
> >>> + const u8 *mem, unsigned int length)
> >>> +{
> >>> + struct uvc_meta_buf *meta;
> >>> + size_t len_std = 2;
> >>> + bool has_pts, has_scr;
> >>> + unsigned long flags;
> >>> + ktime_t time;
> >>> + const u8 *scr;
> >>> +
> >>> + if (!meta_buf || length == 2)
> >>> + return;
> >>> +
> >>> + if (meta_buf->length - meta_buf->bytesused <
> >>> + length + sizeof(meta->ns) + sizeof(meta->sof)) {
> >>> + meta_buf->error = 1;
> >>> + return;
> >>> + }
> >>> +
> >>> + has_pts = mem[1] & UVC_STREAM_PTS;
> >>> + has_scr = mem[1] & UVC_STREAM_SCR;
> >>> +
> >>> + if (has_pts) {
> >>> + len_std += 4;
> >>> + scr = mem + 6;
> >>> + } else {
> >>> + scr = mem + 2;
> >>> + }
> >>> +
> >>> + if (has_scr)
> >>> + len_std += 6;
> >>> +
> >>> + if (stream->meta.format == V4L2_META_FMT_UVC)
> >>> + length = len_std;
> >>> +
> >>> + if (length == len_std && (!has_scr ||
> >>> + !memcmp(scr, stream->clock.last_scr, 6)))
> >>> + return;
> >>> +
> >>> + meta = (struct uvc_meta_buf *)((u8 *)meta_buf->mem +
> >>> meta_buf->bytesused); + local_irq_save(flags);
> >>> + time = uvc_video_get_time();
> >>> + meta->sof = usb_get_current_frame_number(stream->dev->udev);
> >>
> >> You need a put_unaligned here too. If you're fine with the patch below
> >> there's no need to resubmit, and
> >
> > One more thing, __put_unaligned_cpu16() and __put_unaligned_cpu64() don't
> > compile on x86_64 with v4.12 (using media_build.git). I propose replacing
> > them with put_unaligned() which compiles and should do the right thing.
> Sure, thanks for catching! Shall I fix and resubmit?
If you're fine with
git://linuxtv.org/pinchartl/media.git uvc/next
there's no need to resubmit.
By the way, could you please review "uvcvideo: Factor out video device
registration to a function" and "uvcvideo: Report V4L2 device caps through the
video_device structure" ?
I will send a pull request after testing the code and getting those two
patches reviewed.
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> >>
> >> Would you mind sending me your test tool patch ?
>
> Will send it offline.
>
> Thanks
> Guennadi
>
> >> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/
> >> uvc_video.c
> >> index 2fc0bf2221db..02e4997a32bb 100644
> >> --- a/drivers/media/usb/uvc/uvc_video.c
> >> +++ b/drivers/media/usb/uvc/uvc_video.c
> >> @@ -1142,6 +1142,7 @@ static void uvc_video_decode_meta(struct
> >> uvc_streaming *stream,
> >> size_t len_std = 2;
> >> bool has_pts, has_scr;
> >> unsigned long flags;
> >> + unsigned int sof;
> >> ktime_t time;
> >> const u8 *scr;
> >>
> >> @@ -1177,9 +1178,10 @@ static void uvc_video_decode_meta(struct
> >> uvc_streaming *stream,
> >> meta = (struct uvc_meta_buf *)((u8 *)meta_buf->mem +
> >> meta_buf->bytesused);
> >>
> >> local_irq_save(flags);
> >> time = uvc_video_get_time();
> >> - meta->sof = usb_get_current_frame_number(stream->dev->udev);
> >> + sof = usb_get_current_frame_number(stream->dev->udev);
> >> local_irq_restore(flags);
> >> __put_unaligned_cpu64(ktime_to_ns(time), &meta->ns);
> >> + __put_unaligned_cpu16(sof, &meta->sof);
> >>
> >> if (has_scr)
> >> memcpy(stream->clock.last_scr, scr, 6);
> >>
> >>> + local_irq_restore(flags);
> >>> + __put_unaligned_cpu64(ktime_to_ns(time), &meta->ns);
> >>> +
> >>> + if (has_scr)
> >>> + memcpy(stream->clock.last_scr, scr, 6);
> >>> +
> >>> + memcpy(&meta->length, mem, length);
> >>> + meta_buf->bytesused += length + sizeof(meta->ns) +
> >>> sizeof(meta->sof);
> >>> +
> >>> + uvc_trace(UVC_TRACE_FRAME,
> >>> + "%s(): t-sys %lluns, SOF %u, len %u, flags 0x%x, PTS %u, STC %u
> >>> frame
> >>> SOF %u\n", + __func__, time, meta->sof, meta->length, meta-
>flags,
> >>> + has_pts ? *(u32 *)meta->buf : 0,
> >>> + has_scr ? *(u32 *)scr : 0,
> >>> + has_scr ? *(u32 *)(scr + 4) & 0x7ff : 0);
> >>> +}
> >>
> >> [snip]
--
Regards,
Laurent Pinchart
