Message ID | 20231106-uvc-event-v2-1-7d8e36f0df16@chromium.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:8f47:0:b0:403:3b70:6f57 with SMTP id j7csp2571683vqu; Mon, 6 Nov 2023 02:52:58 -0800 (PST) X-Google-Smtp-Source: AGHT+IHkqc1vkcAySX5+gojguawWFevw4TMj5r7yelTNgcOXipPleRiPXzejPT02hg12ebJgGiCL X-Received: by 2002:a05:6808:d49:b0:3b2:e0f0:e53d with SMTP id w9-20020a0568080d4900b003b2e0f0e53dmr34416834oik.37.1699267978489; Mon, 06 Nov 2023 02:52:58 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699267978; cv=none; d=google.com; s=arc-20160816; b=DdvU/Jat6JkURDN4x5w+4YBkPQhZuhEALl0BhhNwakTy6ypDtlhDgKsyZihhNQWcNF P/ohxhyQfCa1gQefn2LNJV0e3aB1gd9JyfUJ1tYQ4vxXg6eF6g0ZYdLpBvIxkcYw/m9S Knku+55K48GY3DXMrKp9m8xrWMgDCmS0Ho/OmSvrm/9XwR3GYY/GWCsYdeCmm8yIUM7d oG0VOct6QlU72Ij3MvyN1Hqp28JEhlfcB52/t7rhPgGze+u84i7Vl9Je+qtaqd3ViHYm fFOquU9Nmb87sbIv+YeGAcoCyB42TOcNPmGzaYieBoErD/MqwCDeYyV7pPWI4U9C7IJ4 9k4g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:message-id:content-transfer-encoding :mime-version:subject:date:from:dkim-signature; bh=OQ4NhUhZrOjrN1RWsT6V7ekYOQJfHj+sCfHy/yyUHn4=; fh=juUYT8HK8LGpjZXsXX23mnx6oe4bv7mQPkb8s5QbVrc=; b=i1QL57HDveYAWsQbbQRgkaJhW4dqdJjsg9BTQPKu2PGkcXF9dXxNVPKbPvgjdPO9r+ 9JwdQ03eqzlzrMU6llytTETZU6pMgdmVKpQGceQzX5v9qzg/TSwjI0tqn8uVvf+Ymc6g 0xfK0FbOpHtTp0ic0oZ9riMtoh0nW/NanzyiTktyzjh6Zrc9hjHgJYqnkHw1gS4+axyz w8Bm/H6zz/qkb7TOXsfLn0Y8EAO+TYqDTkCiTE2nyTOnFj8FBi0InW4xUwIWFAk4xL1Y V8z6uoFRz+Zv3+84qKLKdnrERfostveBKi22ovS4erhK6c0HkKpU52gCbNEdrtqPZ1Yc GE4A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=kbeuRVoW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from agentk.vger.email (agentk.vger.email. [23.128.96.32]) by mx.google.com with ESMTPS id i62-20020a636d41000000b005b98cb7b465si7781390pgc.637.2023.11.06.02.52.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Nov 2023 02:52:58 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=kbeuRVoW; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by agentk.vger.email (Postfix) with ESMTP id 3B7AD80407D4; Mon, 6 Nov 2023 02:52:56 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231433AbjKFKwk (ORCPT <rfc822;jaysivo@gmail.com> + 36 others); Mon, 6 Nov 2023 05:52:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51292 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231176AbjKFKwi (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 6 Nov 2023 05:52:38 -0500 Received: from mail-qv1-xf2d.google.com (mail-qv1-xf2d.google.com [IPv6:2607:f8b0:4864:20::f2d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5BB6CC6 for <linux-kernel@vger.kernel.org>; Mon, 6 Nov 2023 02:52:35 -0800 (PST) Received: by mail-qv1-xf2d.google.com with SMTP id 6a1803df08f44-66d13ac2796so28123966d6.2 for <linux-kernel@vger.kernel.org>; Mon, 06 Nov 2023 02:52:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1699267954; x=1699872754; darn=vger.kernel.org; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:from:to:cc:subject:date:message-id:reply-to; bh=OQ4NhUhZrOjrN1RWsT6V7ekYOQJfHj+sCfHy/yyUHn4=; b=kbeuRVoW4U1oqDn7Smk5Qg/g5FpIWIR//okTtHhFdql8FttLvijH/ewXKmH9ruXRac W/Cu0YNq5SJaj+MZ+sgIUWKUR/xRsWnyn+ys7YGqY52EWPupV+hI562jo1qIzwjcVU/h wbYMdPDjVXQzRYQzhs21K1QS4mt5+ibR9V6wo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699267954; x=1699872754; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=OQ4NhUhZrOjrN1RWsT6V7ekYOQJfHj+sCfHy/yyUHn4=; b=OZkUdv+jr81WUsNn5yelRQeyVFag15wPLQq4rPVAg34pyvNsv9Oht9f5SfdIMkjp3g Lf1cnrjVxtoIk8RuP5VQ2T1eB1NjfzqdvfhXmdBjgZIPxSzurhCENmL071B4/R1X1pgJ 8bjqPeUwZJVyyqjs6u6oRiKxRa6XXFJxu+pvJDuO43VDjUn4Xt3+iRNNL6fMb9qdxO3F Os/xeIQ/7U3rovXxVG5cHoF1G23xXOqyJqLQb3BFChQvc7zA9YnnBTPZBd7bSNi8cuPN meAiMp6aHg0v6hDkM5jeGjsjYky/Q3/QCcVExMK+zJk/M6A7wmP58TEOhSv/XuyLfYM0 eyxg== X-Gm-Message-State: AOJu0Yy/DEhcVVI9z/tnX4BIyY/xqR1JHJIrQDV6wYqw/zIDcrZcXUzi TJx7qft88BAdOMzSTnGYtF0iiA== X-Received: by 2002:a05:6214:20e6:b0:672:96a7:4614 with SMTP id 6-20020a05621420e600b0067296a74614mr26316553qvk.17.1699267954477; Mon, 06 Nov 2023 02:52:34 -0800 (PST) Received: from denia.c.googlers.com (112.49.199.35.bc.googleusercontent.com. [35.199.49.112]) by smtp.gmail.com with ESMTPSA id z10-20020a0cfc0a000000b0066d0f35554asm3314339qvo.79.2023.11.06.02.52.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Nov 2023 02:52:34 -0800 (PST) From: Ricardo Ribalda <ribalda@chromium.org> Date: Mon, 06 Nov 2023 10:52:27 +0000 Subject: [PATCH v2] media: uvcvideo: Implement V4L2_EVENT_FRAME_SYNC MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20231106-uvc-event-v2-1-7d8e36f0df16@chromium.org> X-B4-Tracking: v=1; b=H4sIAGvFSGUC/22NTQ6DIBBGr2JYdxrAtIld9R6NCwZGmUShASU2x ruXuu7yvXw/u8iUmLJ4NLtIVDhzDBX0pRHWmzASsKsstNStklrCWixQobCAa51CRGdRD6Lm0WQ CTCZYXxthnaYq34kG3s6DV1/Zc15i+px/Rf3sv+miQEGLxkjqhnt300/rU5x5na8xjaI/juMLp IC7lroAAAA= To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, Ricardo Ribalda <ribalda@chromium.org> Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, Esker Wong <esker@chromium.org> X-Mailer: b4 0.12.3 X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Mon, 06 Nov 2023 02:52:56 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780255692686881900 X-GMAIL-MSGID: 1781811619553430943 |
Series |
[v2] media: uvcvideo: Implement V4L2_EVENT_FRAME_SYNC
|
|
Commit Message
Ricardo Ribalda
Nov. 6, 2023, 10:52 a.m. UTC
Add support for the frame_sync event, so user-space can become aware earlier of new frames. Suggested-by: Esker Wong <esker@chromium.org> Tested-by: Esker Wong <esker@chromium.org> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- We have measured a latency of around 30msecs between frame sync and dqbuf. --- Changes in v2: - Suggested by Laurent. Split sequence++ and event init. - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org --- drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ drivers/media/usb/uvc/uvc_video.c | 7 +++++++ 2 files changed, 9 insertions(+) --- base-commit: ce55c22ec8b223a90ff3e084d842f73cfba35588 change-id: 20231020-uvc-event-d3d1bbbdcb2f Best regards,
Comments
Hi Ricardo, On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > Add support for the frame_sync event, so user-space can become aware > earlier of new frames. > > Suggested-by: Esker Wong <esker@chromium.org> > Tested-by: Esker Wong <esker@chromium.org> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > We have measured a latency of around 30msecs between frame sync > and dqbuf. > --- > Changes in v2: > - Suggested by Laurent. Split sequence++ and event init. > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > --- > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index f4988f03640a..9f3fb5fd2375 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > switch (sub->type) { > case V4L2_EVENT_CTRL: > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > + case V4L2_EVENT_FRAME_SYNC: > + return v4l2_event_subscribe(fh, sub, 0, NULL); > default: > return -EINVAL; > } > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > index 28dde08ec6c5..4f3a510ca4fe 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > * that discontinuous sequence numbers always indicate lost frames. > */ > if (stream->last_fid != fid) { > + struct v4l2_event event = { > + .type = V4L2_EVENT_FRAME_SYNC, > + }; > + > stream->sequence++; > if (stream->sequence) > uvc_video_stats_update(stream); > + > + event.u.frame_sync.frame_sequence = stream->sequence, > + v4l2_event_queue(&stream->vdev, &event); uvc_video_decode_start() is called when the reception of the entire frame has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC says that the event is "Triggered immediately when the reception of a frame has begun.". The functionality here doesn't seem to fit to this patch. Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a concept of vertical sync in the case of USB? That event doesn't have the sequence though but I guess it's not an issue at least if your case. Another technically correct option could be to create a new event for this but I'm not sure it's worth it. > } > > uvc_video_clock_decode(stream, buf, data, len); >
Hi Sakari On Mon, 6 Nov 2023 at 12:06, Sakari Ailus <sakari.ailus@iki.fi> wrote: > > Hi Ricardo, > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > Add support for the frame_sync event, so user-space can become aware > > earlier of new frames. > > > > Suggested-by: Esker Wong <esker@chromium.org> > > Tested-by: Esker Wong <esker@chromium.org> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > We have measured a latency of around 30msecs between frame sync > > and dqbuf. > > --- > > Changes in v2: > > - Suggested by Laurent. Split sequence++ and event init. > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > --- > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index f4988f03640a..9f3fb5fd2375 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > switch (sub->type) { > > case V4L2_EVENT_CTRL: > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > + case V4L2_EVENT_FRAME_SYNC: > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > default: > > return -EINVAL; > > } > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > --- a/drivers/media/usb/uvc/uvc_video.c > > +++ b/drivers/media/usb/uvc/uvc_video.c > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > * that discontinuous sequence numbers always indicate lost frames. > > */ > > if (stream->last_fid != fid) { > > + struct v4l2_event event = { > > + .type = V4L2_EVENT_FRAME_SYNC, > > + }; > > + > > stream->sequence++; > > if (stream->sequence) > > uvc_video_stats_update(stream); > > + > > + event.u.frame_sync.frame_sequence = stream->sequence, > > + v4l2_event_queue(&stream->vdev, &event); > > uvc_video_decode_start() is called when the reception of the entire frame > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > says that the event is "Triggered immediately when the reception of a frame > has begun.". The functionality here doesn't seem to fit to this patch. If I understand correctly, there are two types of transfers, BULK and ISOC. For bulk frames are typically divided in multiple packages and the event is generated in the first package. So I guess we fit the description For isoc we receive the whole frame at once and we trigger the event before we process it... It is maybe a bit more borderline implementation of the event description, but not very far away. > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > concept of vertical sync in the case of USB? That event doesn't have the > sequence though but I guess it's not an issue at least if your case. All that said, If Esker does not need the sequence number I have no issue converting it. @Esker Wong WDYT? > > Another technically correct option could be to create a new event for this > but I'm not sure it's worth it. > > > } > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > -- > Regards, > > Sakari Ailus
[send again in text mode] Hi Sakari, Sequence number is important to us. We need it to measure the latency from this event to the time we display the frame. Regards, Esker On Mon, Nov 6, 2023 at 7:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote: > > Hi Ricardo, > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > Add support for the frame_sync event, so user-space can become aware > > earlier of new frames. > > > > Suggested-by: Esker Wong <esker@chromium.org> > > Tested-by: Esker Wong <esker@chromium.org> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > We have measured a latency of around 30msecs between frame sync > > and dqbuf. > > --- > > Changes in v2: > > - Suggested by Laurent. Split sequence++ and event init. > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > --- > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index f4988f03640a..9f3fb5fd2375 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > switch (sub->type) { > > case V4L2_EVENT_CTRL: > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > + case V4L2_EVENT_FRAME_SYNC: > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > default: > > return -EINVAL; > > } > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > --- a/drivers/media/usb/uvc/uvc_video.c > > +++ b/drivers/media/usb/uvc/uvc_video.c > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > * that discontinuous sequence numbers always indicate lost frames. > > */ > > if (stream->last_fid != fid) { > > + struct v4l2_event event = { > > + .type = V4L2_EVENT_FRAME_SYNC, > > + }; > > + > > stream->sequence++; > > if (stream->sequence) > > uvc_video_stats_update(stream); > > + > > + event.u.frame_sync.frame_sequence = stream->sequence, > > + v4l2_event_queue(&stream->vdev, &event); > > uvc_video_decode_start() is called when the reception of the entire frame > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > says that the event is "Triggered immediately when the reception of a frame > has begun.". The functionality here doesn't seem to fit to this patch. > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > concept of vertical sync in the case of USB? That event doesn't have the > sequence though but I guess it's not an issue at least if your case. > > Another technically correct option could be to create a new event for this > but I'm not sure it's worth it. > > > } > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > -- > Regards, > > Sakari Ailus
Hi Esker, On Tue, Nov 07, 2023 at 01:06:20PM +0800, Esker Wong wrote: > [send again in text mode] > Hi Sakari, > > Sequence number is important to us. We need it to measure the latency > from this event to the time we display the frame. I think we could also add the sequence number to V4L2_EVENT_VSYNC. Cc Hans. > On Mon, Nov 6, 2023 at 7:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote: > > > > Hi Ricardo, > > > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > > Add support for the frame_sync event, so user-space can become aware > > > earlier of new frames. > > > > > > Suggested-by: Esker Wong <esker@chromium.org> > > > Tested-by: Esker Wong <esker@chromium.org> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > --- > > > We have measured a latency of around 30msecs between frame sync > > > and dqbuf. > > > --- > > > Changes in v2: > > > - Suggested by Laurent. Split sequence++ and event init. > > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > > --- > > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > > 2 files changed, 9 insertions(+) > > > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > > index f4988f03640a..9f3fb5fd2375 100644 > > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > > switch (sub->type) { > > > case V4L2_EVENT_CTRL: > > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > > + case V4L2_EVENT_FRAME_SYNC: > > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > > default: > > > return -EINVAL; > > > } > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > > --- a/drivers/media/usb/uvc/uvc_video.c > > > +++ b/drivers/media/usb/uvc/uvc_video.c > > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > > * that discontinuous sequence numbers always indicate lost frames. > > > */ > > > if (stream->last_fid != fid) { > > > + struct v4l2_event event = { > > > + .type = V4L2_EVENT_FRAME_SYNC, > > > + }; > > > + > > > stream->sequence++; > > > if (stream->sequence) > > > uvc_video_stats_update(stream); > > > + > > > + event.u.frame_sync.frame_sequence = stream->sequence, > > > + v4l2_event_queue(&stream->vdev, &event); > > > > uvc_video_decode_start() is called when the reception of the entire frame > > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > > says that the event is "Triggered immediately when the reception of a frame > > has begun.". The functionality here doesn't seem to fit to this patch. > > > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > > concept of vertical sync in the case of USB? That event doesn't have the > > sequence though but I guess it's not an issue at least if your case. > > > > Another technically correct option could be to create a new event for this > > but I'm not sure it's worth it. > > > > > } > > > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > > > > -- > > Regards, > > > > Sakari Ailus
Hi Sakari On Tue, 7 Nov 2023 at 08:37, Sakari Ailus <sakari.ailus@iki.fi> wrote: > > Hi Esker, > > On Tue, Nov 07, 2023 at 01:06:20PM +0800, Esker Wong wrote: > > [send again in text mode] > > Hi Sakari, > > > > Sequence number is important to us. We need it to measure the latency > > from this event to the time we display the frame. > > I think we could also add the sequence number to V4L2_EVENT_VSYNC. > > Cc Hans. I think you forgot to add him :) @Hans What do you prefer? - Use V4L2_EVENT_FRAME_SYNC because we trigger it as soon as the driver knows about the frame - Use V4L2_EVENT_VSYNC, but extending it to support the frame sequence - Use a new EVENT Thanks! > > > On Mon, Nov 6, 2023 at 7:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote: > > > > > > Hi Ricardo, > > > > > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > > > Add support for the frame_sync event, so user-space can become aware > > > > earlier of new frames. > > > > > > > > Suggested-by: Esker Wong <esker@chromium.org> > > > > Tested-by: Esker Wong <esker@chromium.org> > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > > --- > > > > We have measured a latency of around 30msecs between frame sync > > > > and dqbuf. > > > > --- > > > > Changes in v2: > > > > - Suggested by Laurent. Split sequence++ and event init. > > > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > > > --- > > > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > > > 2 files changed, 9 insertions(+) > > > > > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > > > index f4988f03640a..9f3fb5fd2375 100644 > > > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > > > switch (sub->type) { > > > > case V4L2_EVENT_CTRL: > > > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > > > + case V4L2_EVENT_FRAME_SYNC: > > > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > > > default: > > > > return -EINVAL; > > > > } > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > > > --- a/drivers/media/usb/uvc/uvc_video.c > > > > +++ b/drivers/media/usb/uvc/uvc_video.c > > > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > > > * that discontinuous sequence numbers always indicate lost frames. > > > > */ > > > > if (stream->last_fid != fid) { > > > > + struct v4l2_event event = { > > > > + .type = V4L2_EVENT_FRAME_SYNC, > > > > + }; > > > > + > > > > stream->sequence++; > > > > if (stream->sequence) > > > > uvc_video_stats_update(stream); > > > > + > > > > + event.u.frame_sync.frame_sequence = stream->sequence, > > > > + v4l2_event_queue(&stream->vdev, &event); > > > > > > uvc_video_decode_start() is called when the reception of the entire frame > > > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > > > says that the event is "Triggered immediately when the reception of a frame > > > has begun.". The functionality here doesn't seem to fit to this patch. > > > > > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > > > concept of vertical sync in the case of USB? That event doesn't have the > > > sequence though but I guess it's not an issue at least if your case. > > > > > > Another technically correct option could be to create a new event for this > > > but I'm not sure it's worth it. > > > > > > > } > > > > > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > > > > > > > -- > > > Regards, > > > > > > Sakari Ailus > > -- > Regards, > > Sakari Ailus
Hi, Le mardi 07 novembre 2023 à 13:06 +0800, Esker Wong a écrit : > [send again in text mode] > Hi Sakari, > > Sequence number is important to us. We need it to measure the latency > from this event to the time we display the frame. how much precision do you expect, because as described, this number will be completely false for bulk. Aren't UVC timestamp support to allow measuring latency properly ? Nicolas > > Regards, > Esker > > > On Mon, Nov 6, 2023 at 7:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote: > > > > Hi Ricardo, > > > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > > Add support for the frame_sync event, so user-space can become aware > > > earlier of new frames. > > > > > > Suggested-by: Esker Wong <esker@chromium.org> > > > Tested-by: Esker Wong <esker@chromium.org> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > --- > > > We have measured a latency of around 30msecs between frame sync > > > and dqbuf. > > > --- > > > Changes in v2: > > > - Suggested by Laurent. Split sequence++ and event init. > > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > > --- > > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > > 2 files changed, 9 insertions(+) > > > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > > index f4988f03640a..9f3fb5fd2375 100644 > > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > > switch (sub->type) { > > > case V4L2_EVENT_CTRL: > > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > > + case V4L2_EVENT_FRAME_SYNC: > > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > > default: > > > return -EINVAL; > > > } > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > > --- a/drivers/media/usb/uvc/uvc_video.c > > > +++ b/drivers/media/usb/uvc/uvc_video.c > > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > > * that discontinuous sequence numbers always indicate lost frames. > > > */ > > > if (stream->last_fid != fid) { > > > + struct v4l2_event event = { > > > + .type = V4L2_EVENT_FRAME_SYNC, > > > + }; > > > + > > > stream->sequence++; > > > if (stream->sequence) > > > uvc_video_stats_update(stream); > > > + > > > + event.u.frame_sync.frame_sequence = stream->sequence, > > > + v4l2_event_queue(&stream->vdev, &event); > > > > uvc_video_decode_start() is called when the reception of the entire frame > > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > > says that the event is "Triggered immediately when the reception of a frame > > has begun.". The functionality here doesn't seem to fit to this patch. > > > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > > concept of vertical sync in the case of USB? That event doesn't have the > > sequence though but I guess it's not an issue at least if your case. > > > > Another technically correct option could be to create a new event for this > > but I'm not sure it's worth it. > > > > > } > > > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > > > > -- > > Regards, > > > > Sakari Ailus
Hi Nicholas and Sakari, We need it as precise as possible. Currently the earliest time of a frame we can have in userspace is the dqbuf. And for UVC timestamp, it is somewhat awkward for us to use. Since other functions in our stacks do not necessarily contain such timestamps. So we want some event to be trigger and we can get the system time directly. If the V4L2_EVENT_FRAME_SYNC will be earlier then V4L2_EVENT_VSYNC, then it has value. We would want to know the delay of a frame being captured to the time it is displayed. I'm not sure for bulk is the V4L2_EVENT_VSYNC more accurate? Esker On Wed, Nov 8, 2023 at 3:27 AM <nicolas@ndufresne.ca> wrote: > > Hi, > > Le mardi 07 novembre 2023 à 13:06 +0800, Esker Wong a écrit : > > [send again in text mode] > > Hi Sakari, > > > > Sequence number is important to us. We need it to measure the latency > > from this event to the time we display the frame. > > how much precision do you expect, because as described, this number > will be completely false for bulk. > > Aren't UVC timestamp support to allow measuring latency properly ? > > Nicolas > > > > > Regards, > > Esker > > > > > > On Mon, Nov 6, 2023 at 7:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote: > > > > > > Hi Ricardo, > > > > > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > > > Add support for the frame_sync event, so user-space can become aware > > > > earlier of new frames. > > > > > > > > Suggested-by: Esker Wong <esker@chromium.org> > > > > Tested-by: Esker Wong <esker@chromium.org> > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > > --- > > > > We have measured a latency of around 30msecs between frame sync > > > > and dqbuf. > > > > --- > > > > Changes in v2: > > > > - Suggested by Laurent. Split sequence++ and event init. > > > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > > > --- > > > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > > > 2 files changed, 9 insertions(+) > > > > > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > > > index f4988f03640a..9f3fb5fd2375 100644 > > > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > > > switch (sub->type) { > > > > case V4L2_EVENT_CTRL: > > > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > > > + case V4L2_EVENT_FRAME_SYNC: > > > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > > > default: > > > > return -EINVAL; > > > > } > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > > > --- a/drivers/media/usb/uvc/uvc_video.c > > > > +++ b/drivers/media/usb/uvc/uvc_video.c > > > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > > > * that discontinuous sequence numbers always indicate lost frames. > > > > */ > > > > if (stream->last_fid != fid) { > > > > + struct v4l2_event event = { > > > > + .type = V4L2_EVENT_FRAME_SYNC, > > > > + }; > > > > + > > > > stream->sequence++; > > > > if (stream->sequence) > > > > uvc_video_stats_update(stream); > > > > + > > > > + event.u.frame_sync.frame_sequence = stream->sequence, > > > > + v4l2_event_queue(&stream->vdev, &event); > > > > > > uvc_video_decode_start() is called when the reception of the entire frame > > > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > > > says that the event is "Triggered immediately when the reception of a frame > > > has begun.". The functionality here doesn't seem to fit to this patch. > > > > > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > > > concept of vertical sync in the case of USB? That event doesn't have the > > > sequence though but I guess it's not an issue at least if your case. > > > > > > Another technically correct option could be to create a new event for this > > > but I'm not sure it's worth it. > > > > > > > } > > > > > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > > > > > > > -- > > > Regards, > > > > > > Sakari Ailus >
Hi Esker On Wed, 8 Nov 2023 at 07:54, Esker Wong <esker@google.com> wrote: > > Hi Nicholas and Sakari, > > We need it as precise as possible. Currently the earliest time of a > frame we can have in userspace is the dqbuf. > > And for UVC timestamp, it is somewhat awkward for us to use. Since > other functions in our stacks do not necessarily contain such > timestamps. So we want some event to be trigger and we can get the > system time directly. Not to mention that the UVC timestamping requires a bit of love. @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > If the V4L2_EVENT_FRAME_SYNC will be earlier then V4L2_EVENT_VSYNC, > then it has value. We would want to know the delay of a frame being > captured to the time it is displayed. > > I'm not sure for bulk is the V4L2_EVENT_VSYNC more accurate? V4L2_EVENT_VSYNC wont be more accurate than V4L2_EVENT_FRAME_SYNC. My understanding is that Sakari thinks that the description of V4L2_EVENT_FRAME_SYNC https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-dqevent.html#description does not match the current implementation, and suggests using V4L2_EVENT_VSYNC instead. > > Esker > > > On Wed, Nov 8, 2023 at 3:27 AM <nicolas@ndufresne.ca> wrote: > > > > Hi, > > > > Le mardi 07 novembre 2023 à 13:06 +0800, Esker Wong a écrit : > > > [send again in text mode] > > > Hi Sakari, > > > > > > Sequence number is important to us. We need it to measure the latency > > > from this event to the time we display the frame. > > > > how much precision do you expect, because as described, this number > > will be completely false for bulk. > > > > Aren't UVC timestamp support to allow measuring latency properly ? > > > > Nicolas > > > > > > > > Regards, > > > Esker > > > > > > > > > On Mon, Nov 6, 2023 at 7:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote: > > > > > > > > Hi Ricardo, > > > > > > > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > > > > Add support for the frame_sync event, so user-space can become aware > > > > > earlier of new frames. > > > > > > > > > > Suggested-by: Esker Wong <esker@chromium.org> > > > > > Tested-by: Esker Wong <esker@chromium.org> > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > > > --- > > > > > We have measured a latency of around 30msecs between frame sync > > > > > and dqbuf. > > > > > --- > > > > > Changes in v2: > > > > > - Suggested by Laurent. Split sequence++ and event init. > > > > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > > > > --- > > > > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > > > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > > > > 2 files changed, 9 insertions(+) > > > > > > > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > > > > index f4988f03640a..9f3fb5fd2375 100644 > > > > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > > > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > > > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > > > > switch (sub->type) { > > > > > case V4L2_EVENT_CTRL: > > > > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > > > > + case V4L2_EVENT_FRAME_SYNC: > > > > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > > > > default: > > > > > return -EINVAL; > > > > > } > > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > > > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > > > > --- a/drivers/media/usb/uvc/uvc_video.c > > > > > +++ b/drivers/media/usb/uvc/uvc_video.c > > > > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > > > > * that discontinuous sequence numbers always indicate lost frames. > > > > > */ > > > > > if (stream->last_fid != fid) { > > > > > + struct v4l2_event event = { > > > > > + .type = V4L2_EVENT_FRAME_SYNC, > > > > > + }; > > > > > + > > > > > stream->sequence++; > > > > > if (stream->sequence) > > > > > uvc_video_stats_update(stream); > > > > > + > > > > > + event.u.frame_sync.frame_sequence = stream->sequence, > > > > > + v4l2_event_queue(&stream->vdev, &event); > > > > > > > > uvc_video_decode_start() is called when the reception of the entire frame > > > > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > > > > says that the event is "Triggered immediately when the reception of a frame > > > > has begun.". The functionality here doesn't seem to fit to this patch. > > > > > > > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > > > > concept of vertical sync in the case of USB? That event doesn't have the > > > > sequence though but I guess it's not an issue at least if your case. > > > > > > > > Another technically correct option could be to create a new event for this > > > > but I'm not sure it's worth it. > > > > > > > > > } > > > > > > > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > > > > > > > > > > -- > > > > Regards, > > > > > > > > Sakari Ailus > >
Le mercredi 08 novembre 2023 à 08:04 +0100, Ricardo Ribalda a écrit : > Hi Esker > > On Wed, 8 Nov 2023 at 07:54, Esker Wong <esker@google.com> wrote: > > > > Hi Nicholas and Sakari, > > > > We need it as precise as possible. Currently the earliest time of a > > frame we can have in userspace is the dqbuf. > > > > And for UVC timestamp, it is somewhat awkward for us to use. Since > > other functions in our stacks do not necessarily contain such > > timestamps. So we want some event to be trigger and we can get the > > system time directly. The fact that you interpret the time from FRAME_SYNC to DQBUF (well the READ IO notification) as the actual latency is yours of course. It assumes that the camera on the other end does not introduce other source of latency (or that these are negligible). You are also going to introduce a lot of jitter, since it relies on when the OS decides to wake up your process. I think my opinion resides in if you can accurately *enough* implement what the spec says for FRAME_SYNC then do it, otherwise just don't lie. I think for ISO, "after the first chunk" i a small lie, but acceptable. But for BULK, the way it was explained is that it will be always very close to DQBUF time. and it should not emit FRAME_SYNC for this type of UVC device. If it fits other events fine of course, I'm just making a judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. In term of accuracy, if timestamp was passed with the FRAME_SYNC event, it would not matter how fast your process the event anymore and greatly improve accuracy. > > Not to mention that the UVC timestamping requires a bit of love. > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 Thanks for working on this by the way, hope someone will find the time to review this. The timestamps should in theory provide a jitter free measurement of the delay Esker is trying to measure, and if it wasn't of bugs (and crazy complexity) it would in the worst case match the transfer time. Nicolas > Esker > > > > > > If the V4L2_EVENT_FRAME_SYNC will be earlier then V4L2_EVENT_VSYNC, > > then it has value. We would want to know the delay of a frame being > > captured to the time it is displayed. > > > > I'm not sure for bulk is the V4L2_EVENT_VSYNC more accurate? > > V4L2_EVENT_VSYNC wont be more accurate than V4L2_EVENT_FRAME_SYNC. > > My understanding is that Sakari thinks that the description of > V4L2_EVENT_FRAME_SYNC > https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-dqevent.html#description > does not match the current implementation, and suggests using > V4L2_EVENT_VSYNC instead. > > > > > > Esker > > > > > > On Wed, Nov 8, 2023 at 3:27 AM <nicolas@ndufresne.ca> wrote: > > > > > > Hi, > > > > > > Le mardi 07 novembre 2023 à 13:06 +0800, Esker Wong a écrit : > > > > [send again in text mode] > > > > Hi Sakari, > > > > > > > > Sequence number is important to us. We need it to measure the latency > > > > from this event to the time we display the frame. > > > > > > how much precision do you expect, because as described, this number > > > will be completely false for bulk. > > > > > > Aren't UVC timestamp support to allow measuring latency properly ? > > > > > > Nicolas > > > > > > > > > > > Regards, > > > > Esker > > > > > > > > > > > > On Mon, Nov 6, 2023 at 7:06 PM Sakari Ailus <sakari.ailus@iki.fi> wrote: > > > > > > > > > > Hi Ricardo, > > > > > > > > > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > > > > > Add support for the frame_sync event, so user-space can become aware > > > > > > earlier of new frames. > > > > > > > > > > > > Suggested-by: Esker Wong <esker@chromium.org> > > > > > > Tested-by: Esker Wong <esker@chromium.org> > > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > > > > --- > > > > > > We have measured a latency of around 30msecs between frame sync > > > > > > and dqbuf. > > > > > > --- > > > > > > Changes in v2: > > > > > > - Suggested by Laurent. Split sequence++ and event init. > > > > > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > > > > > --- > > > > > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > > > > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > > > > > 2 files changed, 9 insertions(+) > > > > > > > > > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > > > > > index f4988f03640a..9f3fb5fd2375 100644 > > > > > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > > > > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > > > > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > > > > > switch (sub->type) { > > > > > > case V4L2_EVENT_CTRL: > > > > > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > > > > > + case V4L2_EVENT_FRAME_SYNC: > > > > > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > > > > > default: > > > > > > return -EINVAL; > > > > > > } > > > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > > > > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > > > > > --- a/drivers/media/usb/uvc/uvc_video.c > > > > > > +++ b/drivers/media/usb/uvc/uvc_video.c > > > > > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > > > > > * that discontinuous sequence numbers always indicate lost frames. > > > > > > */ > > > > > > if (stream->last_fid != fid) { > > > > > > + struct v4l2_event event = { > > > > > > + .type = V4L2_EVENT_FRAME_SYNC, > > > > > > + }; > > > > > > + > > > > > > stream->sequence++; > > > > > > if (stream->sequence) > > > > > > uvc_video_stats_update(stream); > > > > > > + > > > > > > + event.u.frame_sync.frame_sequence = stream->sequence, > > > > > > + v4l2_event_queue(&stream->vdev, &event); > > > > > > > > > > uvc_video_decode_start() is called when the reception of the entire frame > > > > > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > > > > > says that the event is "Triggered immediately when the reception of a frame > > > > > has begun.". The functionality here doesn't seem to fit to this patch. > > > > > > > > > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > > > > > concept of vertical sync in the case of USB? That event doesn't have the > > > > > sequence though but I guess it's not an issue at least if your case. > > > > > > > > > > Another technically correct option could be to create a new event for this > > > > > but I'm not sure it's worth it. > > > > > > > > > > > } > > > > > > > > > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > > > > > > > > > > > > > -- > > > > > Regards, > > > > > > > > > > Sakari Ailus > > > > > >
Hello, On Wed, Nov 08, 2023 at 03:32:23PM -0500, nicolas@ndufresne.ca wrote: > Le mercredi 08 novembre 2023 à 08:04 +0100, Ricardo Ribalda a écrit : > > On Wed, 8 Nov 2023 at 07:54, Esker Wong wrote: > > > > > > Hi Nicholas and Sakari, > > > > > > We need it as precise as possible. Currently the earliest time of a > > > frame we can have in userspace is the dqbuf. > > > > > > And for UVC timestamp, it is somewhat awkward for us to use. Since > > > other functions in our stacks do not necessarily contain such > > > timestamps. So we want some event to be trigger and we can get the > > > system time directly. > > The fact that you interpret the time from FRAME_SYNC to DQBUF (well the > READ IO notification) as the actual latency is yours of course. It > assumes that the camera on the other end does not introduce other > source of latency (or that these are negligible). You are also going to > introduce a lot of jitter, since it relies on when the OS decides to > wake up your process. > > I think my opinion resides in if you can accurately *enough* implement > what the spec says for FRAME_SYNC then do it, otherwise just don't lie. > I think for ISO, "after the first chunk" i a small lie, but acceptable. > But for BULK, the way it was explained is that it will be always very > close to DQBUF time. and it should not emit FRAME_SYNC for this type of > UVC device. If it fits other events fine of course, I'm just making a > judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. I agree. V4L2_EVENT_FRAME_SYNC should be fine for isoc-based devices as it should be "close enough" to the start of frame. For bulk it woul dbe too much of a lie, so I would not emit it for bulk-based devices. > In term of accuracy, if timestamp was passed with the FRAME_SYNC event, > it would not matter how fast your process the event anymore and greatly > improve accuracy. > > > Not to mention that the UVC timestamping requires a bit of love. > > > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > Thanks for working on this by the way, hope someone will find the time > to review this. The timestamps should in theory provide a jitter free > measurement of the delay Esker is trying to measure, and if it wasn't > of bugs (and crazy complexity) it would in the worst case match the > transfer time. Assuming the device firmware isn't too buggy, the UVC timestamps should indeed provide much better accuracy than when V4L2_EVENT_FRAME_SYNC could give. I think the biggest problem will be to figure out if a particular device can be trusted. > > > If the V4L2_EVENT_FRAME_SYNC will be earlier then V4L2_EVENT_VSYNC, > > > then it has value. We would want to know the delay of a frame being > > > captured to the time it is displayed. > > > > > > I'm not sure for bulk is the V4L2_EVENT_VSYNC more accurate? > > > > V4L2_EVENT_VSYNC wont be more accurate than V4L2_EVENT_FRAME_SYNC. > > > > My understanding is that Sakari thinks that the description of > > V4L2_EVENT_FRAME_SYNC > > https://www.kernel.org/doc/html/v4.9/media/uapi/v4l/vidioc-dqevent.html#description > > does not match the current implementation, and suggests using > > V4L2_EVENT_VSYNC instead. > > > > > On Wed, Nov 8, 2023 at 3:27 AM <nicolas@ndufresne.ca> wrote: > > > > Le mardi 07 novembre 2023 à 13:06 +0800, Esker Wong a écrit : > > > > > [send again in text mode] > > > > > Hi Sakari, > > > > > > > > > > Sequence number is important to us. We need it to measure the latency > > > > > from this event to the time we display the frame. > > > > > > > > how much precision do you expect, because as described, this number > > > > will be completely false for bulk. > > > > > > > > Aren't UVC timestamp support to allow measuring latency properly ? > > > > > > > > > On Mon, Nov 6, 2023 at 7:06 PM Sakari Ailus wrote: > > > > > > On Mon, Nov 06, 2023 at 10:52:27AM +0000, Ricardo Ribalda wrote: > > > > > > > Add support for the frame_sync event, so user-space can become aware > > > > > > > earlier of new frames. > > > > > > > > > > > > > > Suggested-by: Esker Wong <esker@chromium.org> > > > > > > > Tested-by: Esker Wong <esker@chromium.org> > > > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > > > > > --- > > > > > > > We have measured a latency of around 30msecs between frame sync > > > > > > > and dqbuf. > > > > > > > --- > > > > > > > Changes in v2: > > > > > > > - Suggested by Laurent. Split sequence++ and event init. > > > > > > > - Link to v1: https://lore.kernel.org/r/20231020-uvc-event-v1-1-3baa0e9f6952@chromium.org > > > > > > > --- > > > > > > > drivers/media/usb/uvc/uvc_v4l2.c | 2 ++ > > > > > > > drivers/media/usb/uvc/uvc_video.c | 7 +++++++ > > > > > > > 2 files changed, 9 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > > > > > > index f4988f03640a..9f3fb5fd2375 100644 > > > > > > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > > > > > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > > > > > > @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, > > > > > > > switch (sub->type) { > > > > > > > case V4L2_EVENT_CTRL: > > > > > > > return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); > > > > > > > + case V4L2_EVENT_FRAME_SYNC: > > > > > > > + return v4l2_event_subscribe(fh, sub, 0, NULL); > > > > > > > default: > > > > > > > return -EINVAL; > > > > > > > } > > > > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > > > > > > index 28dde08ec6c5..4f3a510ca4fe 100644 > > > > > > > --- a/drivers/media/usb/uvc/uvc_video.c > > > > > > > +++ b/drivers/media/usb/uvc/uvc_video.c > > > > > > > @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, > > > > > > > * that discontinuous sequence numbers always indicate lost frames. > > > > > > > */ > > > > > > > if (stream->last_fid != fid) { > > > > > > > + struct v4l2_event event = { > > > > > > > + .type = V4L2_EVENT_FRAME_SYNC, > > > > > > > + }; > > > > > > > + > > > > > > > stream->sequence++; > > > > > > > if (stream->sequence) > > > > > > > uvc_video_stats_update(stream); > > > > > > > + > > > > > > > + event.u.frame_sync.frame_sequence = stream->sequence, > > > > > > > + v4l2_event_queue(&stream->vdev, &event); > > > > > > > > > > > > uvc_video_decode_start() is called when the reception of the entire frame > > > > > > has been completed. However, the documentation for V4L2_EVENT_FRAME_SYNC > > > > > > says that the event is "Triggered immediately when the reception of a frame > > > > > > has begun.". The functionality here doesn't seem to fit to this patch. > > > > > > > > > > > > Wouldn't V4L2_EVENT_VSYNC be a better fit, even if we don't really have a > > > > > > concept of vertical sync in the case of USB? That event doesn't have the > > > > > > sequence though but I guess it's not an issue at least if your case. > > > > > > > > > > > > Another technically correct option could be to create a new event for this > > > > > > but I'm not sure it's worth it. > > > > > > > > > > > > > } > > > > > > > > > > > > > > uvc_video_clock_decode(stream, buf, data, len); > > > > > > >
Hi Nicolas On Wed, 8 Nov 2023 at 21:32, <nicolas@ndufresne.ca> wrote: > > The fact that you interpret the time from FRAME_SYNC to DQBUF (well the > READ IO notification) as the actual latency is yours of course. It > assumes that the camera on the other end does not introduce other We want to use this signal to measure how much power is used since we start receiving the frame until we can use it. I agree with you that the latency between capture and dqbuf should be measured using the timestamp. That is not our use case here. > source of latency (or that these are negligible). You are also going to > introduce a lot of jitter, since it relies on when the OS decides to > wake up your process. We have measured a jitter of around 2.5 msec, which is acceptable for our needs. > > I think my opinion resides in if you can accurately *enough* implement > what the spec says for FRAME_SYNC then do it, otherwise just don't lie. What the specs says is: ``` Triggered immediately when the reception of a frame has begun ``` In my opinion, that is true for usb devices, we are triggering it as soon as the transfer has started to the eyes of the driver. We cannot trigger earlier than that. > I think for ISO, "after the first chunk" i a small lie, but acceptable. > But for BULK, the way it was explained is that it will be always very > close to DQBUF time. and it should not emit FRAME_SYNC for this type of > UVC device. If it fits other events fine of course, I'm just making a > judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. nit: I believe that you have swapped iso and bulk on this description > > In term of accuracy, if timestamp was passed with the FRAME_SYNC event, > it would not matter how fast your process the event anymore and greatly > improve accuracy. +1 to that. If we could easily change the uAPI for FRAME_SYNC that should definitely be implemented. > > > > > Not to mention that the UVC timestamping requires a bit of love. > > > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > Thanks for working on this by the way, hope someone will find the time > to review this. The timestamps should in theory provide a jitter free It already has a couple of Reviewed-by stamped in.... ;) > measurement of the delay Esker is trying to measure, and if it wasn't > of bugs (and crazy complexity) it would in the worst case match the > transfer time. Sorry to repeat myself, but just to avoid the confusion: Esker needs to know how much power is used since we start receiving a frame until it is available for dqbuf, not de frame latency. Regards! > > Nicolas >
Hi Ricardo, On Wed, Nov 08, 2023 at 11:46:40PM +0100, Ricardo Ribalda wrote: > On Wed, 8 Nov 2023 at 21:32, <nicolas@ndufresne.ca> wrote: > > > > The fact that you interpret the time from FRAME_SYNC to DQBUF (well the > > READ IO notification) as the actual latency is yours of course. It > > assumes that the camera on the other end does not introduce other > > We want to use this signal to measure how much power is used since we > start receiving the frame until we can use it. > I agree with you that the latency between capture and dqbuf should be > measured using the timestamp. That is not our use case here. > > > source of latency (or that these are negligible). You are also going to > > introduce a lot of jitter, since it relies on when the OS decides to > > wake up your process. > > We have measured a jitter of around 2.5 msec, which is acceptable for our needs. > > > I think my opinion resides in if you can accurately *enough* implement > > what the spec says for FRAME_SYNC then do it, otherwise just don't lie. > > What the specs says is: > ``` > Triggered immediately when the reception of a frame has begun > ``` > In my opinion, that is true for usb devices, we are triggering it as > soon as the transfer has started to the eyes of the driver. We cannot > trigger earlier than that. > > > > I think for ISO, "after the first chunk" i a small lie, but acceptable. > > But for BULK, the way it was explained is that it will be always very > > close to DQBUF time. and it should not emit FRAME_SYNC for this type of > > UVC device. If it fits other events fine of course, I'm just making a > > judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. > > nit: I believe that you have swapped iso and bulk on this description I've confused the USB packet size and the UVC payload size. The latter is typically much bigger for bulk devices than isoc devices, but the former will be in similar order of magnitudes in a large number of cases, but not all cases. The URB size is the result of the USB packet size and number of packets per URB. The uvcvideo driver currently sets the number of packets per URB to 32 at most (and lowers it if the frame size is small, or if not enough memory can be allocated). This could be increased or made dynamic in the future, as higher speeds typically benefit from larger URB sizes. The packet size differs between bulk and isoc endpoints. For bulk, the packet size can be up to 512 bytes for USB 2.0 and 1024 bytes for USB 3.0, and the device can select a smaller size. The largest URB size (again based on the current implementation of the uvcvideo driver) is thus 32 KiB. For isochronous the situation is more complicated. The term "packet" as used in the uvcvideo driver actually means all the data transferred in one service interval, thus made of multiple isoc packets. It is heavily dependent on the USB speed, and the device can advertise different supported sizes (which translate directly to the reserved bandwidth for the transfer), with the driver picking the smallest bandwidth large enough for the data rate required by the resolution and frame rate. The theoretical worst case is 1024 bytes per isoc packet * 16 isoc packets per burst * 6 burst per interval * 32 "packets" per URB, equal to 3 MiB. Even with the largest URB size you have witnessed of ~1 MiB, we will end up lying quite a bit if we consider the URB completion callback for the first URB of the frame as indicating the start of reception. > > In term of accuracy, if timestamp was passed with the FRAME_SYNC event, > > it would not matter how fast your process the event anymore and greatly > > improve accuracy. > > +1 to that. If we could easily change the uAPI for FRAME_SYNC that > should definitely be implemented. > > > > Not to mention that the UVC timestamping requires a bit of love. > > > > > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > > > Thanks for working on this by the way, hope someone will find the time > > to review this. The timestamps should in theory provide a jitter free > > It already has a couple of Reviewed-by stamped in.... ;) > > > measurement of the delay Esker is trying to measure, and if it wasn't > > of bugs (and crazy complexity) it would in the worst case match the > > transfer time. > > Sorry to repeat myself, but just to avoid the confusion: Esker needs > to know how much power is used since we start receiving a frame until > it is available for dqbuf, not de frame latency. As I think everybody is aware, the earliest notification you get on the CPU side is the *end* of reception of the first URB, which can possibly be significantly later than the start of reception of the frame. Based on what I understand, the goal is to measure the CPU power consumption related to CPU processing of the frame. If that's the case, there's good and bad news. The good news is that the CPU doesn't process the frame at all until the URB has been received (if you were to measure the power consumption of the USB host controller too, it would be a different story), so the delay shouldn't be a problem. The bad news is that I don't see how the information you're trying to get will help you, as there's plenty of other things unrelated to the uvcvideo driver that can take CPU time while a frame is being received. That may not be any of my business, but from the point of view of the uvcvideo driver, I'm less inclined to accept a possibly significant V4L2_EVENT_FRAME_SYNC lie if the use case ends up making little sense :-)
Hi Laurent On Thu, 9 Nov 2023 at 01:03, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Ricardo, > > On Wed, Nov 08, 2023 at 11:46:40PM +0100, Ricardo Ribalda wrote: > > On Wed, 8 Nov 2023 at 21:32, <nicolas@ndufresne.ca> wrote: > > > > > > The fact that you interpret the time from FRAME_SYNC to DQBUF (well the > > > READ IO notification) as the actual latency is yours of course. It > > > assumes that the camera on the other end does not introduce other > > > > We want to use this signal to measure how much power is used since we > > start receiving the frame until we can use it. > > I agree with you that the latency between capture and dqbuf should be > > measured using the timestamp. That is not our use case here. > > > > > source of latency (or that these are negligible). You are also going to > > > introduce a lot of jitter, since it relies on when the OS decides to > > > wake up your process. > > > > We have measured a jitter of around 2.5 msec, which is acceptable for our needs. > > > > > I think my opinion resides in if you can accurately *enough* implement > > > what the spec says for FRAME_SYNC then do it, otherwise just don't lie. > > > > What the specs says is: > > ``` > > Triggered immediately when the reception of a frame has begun > > ``` > > In my opinion, that is true for usb devices, we are triggering it as > > soon as the transfer has started to the eyes of the driver. We cannot > > trigger earlier than that. > > > > > > > I think for ISO, "after the first chunk" i a small lie, but acceptable. > > > But for BULK, the way it was explained is that it will be always very > > > close to DQBUF time. and it should not emit FRAME_SYNC for this type of > > > UVC device. If it fits other events fine of course, I'm just making a > > > judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. > > > > nit: I believe that you have swapped iso and bulk on this description > > I've confused the USB packet size and the UVC payload size. The latter > is typically much bigger for bulk devices than isoc devices, but the > former will be in similar order of magnitudes in a large number of > cases, but not all cases. > > The URB size is the result of the USB packet size and number of packets > per URB. The uvcvideo driver currently sets the number of packets per > URB to 32 at most (and lowers it if the frame size is small, or if not > enough memory can be allocated). This could be increased or made dynamic > in the future, as higher speeds typically benefit from larger URB sizes. > The packet size differs between bulk and isoc endpoints. > > For bulk, the packet size can be up to 512 bytes for USB 2.0 and 1024 > bytes for USB 3.0, and the device can select a smaller size. The largest > URB size (again based on the current implementation of the uvcvideo > driver) is thus 32 KiB. > > For isochronous the situation is more complicated. The term "packet" as > used in the uvcvideo driver actually means all the data transferred in > one service interval, thus made of multiple isoc packets. It is heavily > dependent on the USB speed, and the device can advertise different > supported sizes (which translate directly to the reserved bandwidth for > the transfer), with the driver picking the smallest bandwidth large > enough for the data rate required by the resolution and frame rate. The > theoretical worst case is 1024 bytes per isoc packet * 16 isoc packets > per burst * 6 burst per interval * 32 "packets" per URB, equal to 3 MiB. > > Even with the largest URB size you have witnessed of ~1 MiB, we will end > up lying quite a bit if we consider the URB completion callback for the > first URB of the frame as indicating the start of reception. > > > > In term of accuracy, if timestamp was passed with the FRAME_SYNC event, > > > it would not matter how fast your process the event anymore and greatly > > > improve accuracy. > > > > +1 to that. If we could easily change the uAPI for FRAME_SYNC that > > should definitely be implemented. > > > > > > Not to mention that the UVC timestamping requires a bit of love. > > > > > > > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > > > > > Thanks for working on this by the way, hope someone will find the time > > > to review this. The timestamps should in theory provide a jitter free > > > > It already has a couple of Reviewed-by stamped in.... ;) > > > > > measurement of the delay Esker is trying to measure, and if it wasn't > > > of bugs (and crazy complexity) it would in the worst case match the > > > transfer time. > > > > Sorry to repeat myself, but just to avoid the confusion: Esker needs > > to know how much power is used since we start receiving a frame until > > it is available for dqbuf, not de frame latency. > > As I think everybody is aware, the earliest notification you get on the > CPU side is the *end* of reception of the first URB, which can possibly > be significantly later than the start of reception of the frame. > > Based on what I understand, the goal is to measure the CPU power > consumption related to CPU processing of the frame. If that's the case, > there's good and bad news. The good news is that the CPU doesn't process > the frame at all until the URB has been received (if you were to measure > the power consumption of the USB host controller too, it would be a > different story), so the delay shouldn't be a problem. The bad news is > that I don't see how the information you're trying to get will help you, > as there's plenty of other things unrelated to the uvcvideo driver that > can take CPU time while a frame is being received. That may not be any > of my business, but from the point of view of the uvcvideo driver, I'm > less inclined to accept a possibly significant V4L2_EVENT_FRAME_SYNC lie > if the use case ends up making little sense :-) Just to add some numbers to add some context: V4L2_EVENT_FRAME_SYNC for BULK and ISO will be delayed from: ``` Triggered immediately when the reception of a frame has begun ``` one urb. For bulk devices this is a maximum of 0.05 msec (32KiB/600MBps) For 1MiB transfer isoc devices (which is the biggest we have seen), that is 1.8 msec. In both cases, this is smaller than the jitter to process the event itself by userspace. The time from V4L2_EVENT_FRAME_SYNC to DQBUF is around 30 msec. I do not know how much delay is considered acceptable... but if we take the delay argument to the extreme, we could say that V4L2_EVENT_FRAME_SYNC can never be implemented, because the event will always be delayed by something. Regards > > -- > Regards, > > Laurent Pinchart
Hi Laurent, The use case here we want is actually the latency. We want to know what is the most accurate delay the user will feel when the camera capture the frame to the frame get displayed. So anytime earlier then the dqbuf should be an improvement here. It would help us know better of a camera quality. Regards, Esker On Thu, Nov 9, 2023 at 8:03 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Ricardo, > > On Wed, Nov 08, 2023 at 11:46:40PM +0100, Ricardo Ribalda wrote: > > On Wed, 8 Nov 2023 at 21:32, <nicolas@ndufresne.ca> wrote: > > > > > > The fact that you interpret the time from FRAME_SYNC to DQBUF (well the > > > READ IO notification) as the actual latency is yours of course. It > > > assumes that the camera on the other end does not introduce other > > > > We want to use this signal to measure how much power is used since we > > start receiving the frame until we can use it. > > I agree with you that the latency between capture and dqbuf should be > > measured using the timestamp. That is not our use case here. > > > > > source of latency (or that these are negligible). You are also going to > > > introduce a lot of jitter, since it relies on when the OS decides to > > > wake up your process. > > > > We have measured a jitter of around 2.5 msec, which is acceptable for our needs. > > > > > I think my opinion resides in if you can accurately *enough* implement > > > what the spec says for FRAME_SYNC then do it, otherwise just don't lie. > > > > What the specs says is: > > ``` > > Triggered immediately when the reception of a frame has begun > > ``` > > In my opinion, that is true for usb devices, we are triggering it as > > soon as the transfer has started to the eyes of the driver. We cannot > > trigger earlier than that. > > > > > > > I think for ISO, "after the first chunk" i a small lie, but acceptable. > > > But for BULK, the way it was explained is that it will be always very > > > close to DQBUF time. and it should not emit FRAME_SYNC for this type of > > > UVC device. If it fits other events fine of course, I'm just making a > > > judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. > > > > nit: I believe that you have swapped iso and bulk on this description > > I've confused the USB packet size and the UVC payload size. The latter > is typically much bigger for bulk devices than isoc devices, but the > former will be in similar order of magnitudes in a large number of > cases, but not all cases. > > The URB size is the result of the USB packet size and number of packets > per URB. The uvcvideo driver currently sets the number of packets per > URB to 32 at most (and lowers it if the frame size is small, or if not > enough memory can be allocated). This could be increased or made dynamic > in the future, as higher speeds typically benefit from larger URB sizes. > The packet size differs between bulk and isoc endpoints. > > For bulk, the packet size can be up to 512 bytes for USB 2.0 and 1024 > bytes for USB 3.0, and the device can select a smaller size. The largest > URB size (again based on the current implementation of the uvcvideo > driver) is thus 32 KiB. > > For isochronous the situation is more complicated. The term "packet" as > used in the uvcvideo driver actually means all the data transferred in > one service interval, thus made of multiple isoc packets. It is heavily > dependent on the USB speed, and the device can advertise different > supported sizes (which translate directly to the reserved bandwidth for > the transfer), with the driver picking the smallest bandwidth large > enough for the data rate required by the resolution and frame rate. The > theoretical worst case is 1024 bytes per isoc packet * 16 isoc packets > per burst * 6 burst per interval * 32 "packets" per URB, equal to 3 MiB. > > Even with the largest URB size you have witnessed of ~1 MiB, we will end > up lying quite a bit if we consider the URB completion callback for the > first URB of the frame as indicating the start of reception. > > > > In term of accuracy, if timestamp was passed with the FRAME_SYNC event, > > > it would not matter how fast your process the event anymore and greatly > > > improve accuracy. > > > > +1 to that. If we could easily change the uAPI for FRAME_SYNC that > > should definitely be implemented. > > > > > > Not to mention that the UVC timestamping requires a bit of love. > > > > > > > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > > > > > Thanks for working on this by the way, hope someone will find the time > > > to review this. The timestamps should in theory provide a jitter free > > > > It already has a couple of Reviewed-by stamped in.... ;) > > > > > measurement of the delay Esker is trying to measure, and if it wasn't > > > of bugs (and crazy complexity) it would in the worst case match the > > > transfer time. > > > > Sorry to repeat myself, but just to avoid the confusion: Esker needs > > to know how much power is used since we start receiving a frame until > > it is available for dqbuf, not de frame latency. > > As I think everybody is aware, the earliest notification you get on the > CPU side is the *end* of reception of the first URB, which can possibly > be significantly later than the start of reception of the frame. > > Based on what I understand, the goal is to measure the CPU power > consumption related to CPU processing of the frame. If that's the case, > there's good and bad news. The good news is that the CPU doesn't process > the frame at all until the URB has been received (if you were to measure > the power consumption of the USB host controller too, it would be a > different story), so the delay shouldn't be a problem. The bad news is > that I don't see how the information you're trying to get will help you, > as there's plenty of other things unrelated to the uvcvideo driver that > can take CPU time while a frame is being received. That may not be any > of my business, but from the point of view of the uvcvideo driver, I'm > less inclined to accept a possibly significant V4L2_EVENT_FRAME_SYNC lie > if the use case ends up making little sense :-) > > -- > Regards, > > Laurent Pinchart
Hi Esker, On Thu, Nov 09, 2023 at 08:59:13AM +0800, Esker Wong wrote: > Hi Laurent, > > The use case here we want is actually the latency. We want to know > what is the most accurate delay the user will feel when the camera > capture the frame to the frame get displayed. So anytime earlier then > the dqbuf should be an improvement here. It would help us know better > of a camera quality. If it's the latency you're after, wouldn't it be best to use the timestamp produced by the camera ? That's the most accurate information. It is expressed relative to the camera internal clock, but UVC transmits information that enables conversion of the value to a system timestamp. > On Thu, Nov 9, 2023 at 8:03 AM Laurent Pinchart wrote: > > On Wed, Nov 08, 2023 at 11:46:40PM +0100, Ricardo Ribalda wrote: > > > On Wed, 8 Nov 2023 at 21:32, <nicolas@ndufresne.ca> wrote: > > > > > > > > The fact that you interpret the time from FRAME_SYNC to DQBUF (well the > > > > READ IO notification) as the actual latency is yours of course. It > > > > assumes that the camera on the other end does not introduce other > > > > > > We want to use this signal to measure how much power is used since we > > > start receiving the frame until we can use it. > > > I agree with you that the latency between capture and dqbuf should be > > > measured using the timestamp. That is not our use case here. > > > > > > > source of latency (or that these are negligible). You are also going to > > > > introduce a lot of jitter, since it relies on when the OS decides to > > > > wake up your process. > > > > > > We have measured a jitter of around 2.5 msec, which is acceptable for our needs. > > > > > > > I think my opinion resides in if you can accurately *enough* implement > > > > what the spec says for FRAME_SYNC then do it, otherwise just don't lie. > > > > > > What the specs says is: > > > ``` > > > Triggered immediately when the reception of a frame has begun > > > ``` > > > In my opinion, that is true for usb devices, we are triggering it as > > > soon as the transfer has started to the eyes of the driver. We cannot > > > trigger earlier than that. > > > > > > > > > > I think for ISO, "after the first chunk" i a small lie, but acceptable. > > > > But for BULK, the way it was explained is that it will be always very > > > > close to DQBUF time. and it should not emit FRAME_SYNC for this type of > > > > UVC device. If it fits other events fine of course, I'm just making a > > > > judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. > > > > > > nit: I believe that you have swapped iso and bulk on this description > > > > I've confused the USB packet size and the UVC payload size. The latter > > is typically much bigger for bulk devices than isoc devices, but the > > former will be in similar order of magnitudes in a large number of > > cases, but not all cases. > > > > The URB size is the result of the USB packet size and number of packets > > per URB. The uvcvideo driver currently sets the number of packets per > > URB to 32 at most (and lowers it if the frame size is small, or if not > > enough memory can be allocated). This could be increased or made dynamic > > in the future, as higher speeds typically benefit from larger URB sizes. > > The packet size differs between bulk and isoc endpoints. > > > > For bulk, the packet size can be up to 512 bytes for USB 2.0 and 1024 > > bytes for USB 3.0, and the device can select a smaller size. The largest > > URB size (again based on the current implementation of the uvcvideo > > driver) is thus 32 KiB. > > > > For isochronous the situation is more complicated. The term "packet" as > > used in the uvcvideo driver actually means all the data transferred in > > one service interval, thus made of multiple isoc packets. It is heavily > > dependent on the USB speed, and the device can advertise different > > supported sizes (which translate directly to the reserved bandwidth for > > the transfer), with the driver picking the smallest bandwidth large > > enough for the data rate required by the resolution and frame rate. The > > theoretical worst case is 1024 bytes per isoc packet * 16 isoc packets > > per burst * 6 burst per interval * 32 "packets" per URB, equal to 3 MiB. > > > > Even with the largest URB size you have witnessed of ~1 MiB, we will end > > up lying quite a bit if we consider the URB completion callback for the > > first URB of the frame as indicating the start of reception. > > > > > > In term of accuracy, if timestamp was passed with the FRAME_SYNC event, > > > > it would not matter how fast your process the event anymore and greatly > > > > improve accuracy. > > > > > > +1 to that. If we could easily change the uAPI for FRAME_SYNC that > > > should definitely be implemented. > > > > > > > > Not to mention that the UVC timestamping requires a bit of love. > > > > > > > > > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > > > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > > > > > > > Thanks for working on this by the way, hope someone will find the time > > > > to review this. The timestamps should in theory provide a jitter free > > > > > > It already has a couple of Reviewed-by stamped in.... ;) > > > > > > > measurement of the delay Esker is trying to measure, and if it wasn't > > > > of bugs (and crazy complexity) it would in the worst case match the > > > > transfer time. > > > > > > Sorry to repeat myself, but just to avoid the confusion: Esker needs > > > to know how much power is used since we start receiving a frame until > > > it is available for dqbuf, not de frame latency. > > > > As I think everybody is aware, the earliest notification you get on the > > CPU side is the *end* of reception of the first URB, which can possibly > > be significantly later than the start of reception of the frame. > > > > Based on what I understand, the goal is to measure the CPU power > > consumption related to CPU processing of the frame. If that's the case, > > there's good and bad news. The good news is that the CPU doesn't process > > the frame at all until the URB has been received (if you were to measure > > the power consumption of the USB host controller too, it would be a > > different story), so the delay shouldn't be a problem. The bad news is > > that I don't see how the information you're trying to get will help you, > > as there's plenty of other things unrelated to the uvcvideo driver that > > can take CPU time while a frame is being received. That may not be any > > of my business, but from the point of view of the uvcvideo driver, I'm > > less inclined to accept a possibly significant V4L2_EVENT_FRAME_SYNC lie > > if the use case ends up making little sense :-)
Hi Laurent, We not only need t(dqbuf) - t(exposure), we are also interested in t(dqbuf) - t(frame_recieved) to know more detail inside the camera for analysis. Thanks, Esker On Thu, Nov 9, 2023 at 7:34 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Esker, > > On Thu, Nov 09, 2023 at 08:59:13AM +0800, Esker Wong wrote: > > Hi Laurent, > > > > The use case here we want is actually the latency. We want to know > > what is the most accurate delay the user will feel when the camera > > capture the frame to the frame get displayed. So anytime earlier then > > the dqbuf should be an improvement here. It would help us know better > > of a camera quality. > > If it's the latency you're after, wouldn't it be best to use the > timestamp produced by the camera ? That's the most accurate information. > It is expressed relative to the camera internal clock, but UVC transmits > information that enables conversion of the value to a system timestamp. > > > On Thu, Nov 9, 2023 at 8:03 AM Laurent Pinchart wrote: > > > On Wed, Nov 08, 2023 at 11:46:40PM +0100, Ricardo Ribalda wrote: > > > > On Wed, 8 Nov 2023 at 21:32, <nicolas@ndufresne.ca> wrote: > > > > > > > > > > The fact that you interpret the time from FRAME_SYNC to DQBUF (well the > > > > > READ IO notification) as the actual latency is yours of course. It > > > > > assumes that the camera on the other end does not introduce other > > > > > > > > We want to use this signal to measure how much power is used since we > > > > start receiving the frame until we can use it. > > > > I agree with you that the latency between capture and dqbuf should be > > > > measured using the timestamp. That is not our use case here. > > > > > > > > > source of latency (or that these are negligible). You are also going to > > > > > introduce a lot of jitter, since it relies on when the OS decides to > > > > > wake up your process. > > > > > > > > We have measured a jitter of around 2.5 msec, which is acceptable for our needs. > > > > > > > > > I think my opinion resides in if you can accurately *enough* implement > > > > > what the spec says for FRAME_SYNC then do it, otherwise just don't lie. > > > > > > > > What the specs says is: > > > > ``` > > > > Triggered immediately when the reception of a frame has begun > > > > ``` > > > > In my opinion, that is true for usb devices, we are triggering it as > > > > soon as the transfer has started to the eyes of the driver. We cannot > > > > trigger earlier than that. > > > > > > > > > > > > > I think for ISO, "after the first chunk" i a small lie, but acceptable. > > > > > But for BULK, the way it was explained is that it will be always very > > > > > close to DQBUF time. and it should not emit FRAME_SYNC for this type of > > > > > UVC device. If it fits other events fine of course, I'm just making a > > > > > judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. > > > > > > > > nit: I believe that you have swapped iso and bulk on this description > > > > > > I've confused the USB packet size and the UVC payload size. The latter > > > is typically much bigger for bulk devices than isoc devices, but the > > > former will be in similar order of magnitudes in a large number of > > > cases, but not all cases. > > > > > > The URB size is the result of the USB packet size and number of packets > > > per URB. The uvcvideo driver currently sets the number of packets per > > > URB to 32 at most (and lowers it if the frame size is small, or if not > > > enough memory can be allocated). This could be increased or made dynamic > > > in the future, as higher speeds typically benefit from larger URB sizes. > > > The packet size differs between bulk and isoc endpoints. > > > > > > For bulk, the packet size can be up to 512 bytes for USB 2.0 and 1024 > > > bytes for USB 3.0, and the device can select a smaller size. The largest > > > URB size (again based on the current implementation of the uvcvideo > > > driver) is thus 32 KiB. > > > > > > For isochronous the situation is more complicated. The term "packet" as > > > used in the uvcvideo driver actually means all the data transferred in > > > one service interval, thus made of multiple isoc packets. It is heavily > > > dependent on the USB speed, and the device can advertise different > > > supported sizes (which translate directly to the reserved bandwidth for > > > the transfer), with the driver picking the smallest bandwidth large > > > enough for the data rate required by the resolution and frame rate. The > > > theoretical worst case is 1024 bytes per isoc packet * 16 isoc packets > > > per burst * 6 burst per interval * 32 "packets" per URB, equal to 3 MiB. > > > > > > Even with the largest URB size you have witnessed of ~1 MiB, we will end > > > up lying quite a bit if we consider the URB completion callback for the > > > first URB of the frame as indicating the start of reception. > > > > > > > > In term of accuracy, if timestamp was passed with the FRAME_SYNC event, > > > > > it would not matter how fast your process the event anymore and greatly > > > > > improve accuracy. > > > > > > > > +1 to that. If we could easily change the uAPI for FRAME_SYNC that > > > > should definitely be implemented. > > > > > > > > > > Not to mention that the UVC timestamping requires a bit of love. > > > > > > > > > > > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > > > > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > > > > > > > > > Thanks for working on this by the way, hope someone will find the time > > > > > to review this. The timestamps should in theory provide a jitter free > > > > > > > > It already has a couple of Reviewed-by stamped in.... ;) > > > > > > > > > measurement of the delay Esker is trying to measure, and if it wasn't > > > > > of bugs (and crazy complexity) it would in the worst case match the > > > > > transfer time. > > > > > > > > Sorry to repeat myself, but just to avoid the confusion: Esker needs > > > > to know how much power is used since we start receiving a frame until > > > > it is available for dqbuf, not de frame latency. > > > > > > As I think everybody is aware, the earliest notification you get on the > > > CPU side is the *end* of reception of the first URB, which can possibly > > > be significantly later than the start of reception of the frame. > > > > > > Based on what I understand, the goal is to measure the CPU power > > > consumption related to CPU processing of the frame. If that's the case, > > > there's good and bad news. The good news is that the CPU doesn't process > > > the frame at all until the URB has been received (if you were to measure > > > the power consumption of the USB host controller too, it would be a > > > different story), so the delay shouldn't be a problem. The bad news is > > > that I don't see how the information you're trying to get will help you, > > > as there's plenty of other things unrelated to the uvcvideo driver that > > > can take CPU time while a frame is being received. That may not be any > > > of my business, but from the point of view of the uvcvideo driver, I'm > > > less inclined to accept a possibly significant V4L2_EVENT_FRAME_SYNC lie > > > if the use case ends up making little sense :-) > > -- > Regards, > > Laurent Pinchart
Le jeudi 09 novembre 2023 à 01:27 +0100, Ricardo Ribalda a écrit : > Hi Laurent > > On Thu, 9 Nov 2023 at 01:03, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > Hi Ricardo, > > > > On Wed, Nov 08, 2023 at 11:46:40PM +0100, Ricardo Ribalda wrote: > > > On Wed, 8 Nov 2023 at 21:32, <nicolas@ndufresne.ca> wrote: > > > > > > > > The fact that you interpret the time from FRAME_SYNC to DQBUF (well the > > > > READ IO notification) as the actual latency is yours of course. It > > > > assumes that the camera on the other end does not introduce other > > > > > > We want to use this signal to measure how much power is used since we > > > start receiving the frame until we can use it. > > > I agree with you that the latency between capture and dqbuf should be > > > measured using the timestamp. That is not our use case here. > > > > > > > source of latency (or that these are negligible). You are also going to > > > > introduce a lot of jitter, since it relies on when the OS decides to > > > > wake up your process. > > > > > > We have measured a jitter of around 2.5 msec, which is acceptable for our needs. > > > > > > > I think my opinion resides in if you can accurately *enough* implement > > > > what the spec says for FRAME_SYNC then do it, otherwise just don't lie. > > > > > > What the specs says is: > > > ``` > > > Triggered immediately when the reception of a frame has begun > > > ``` > > > In my opinion, that is true for usb devices, we are triggering it as > > > soon as the transfer has started to the eyes of the driver. We cannot > > > trigger earlier than that. > > > > > > > > > > I think for ISO, "after the first chunk" i a small lie, but acceptable. > > > > But for BULK, the way it was explained is that it will be always very > > > > close to DQBUF time. and it should not emit FRAME_SYNC for this type of > > > > UVC device. If it fits other events fine of course, I'm just making a > > > > judgment on if its fits V4L2_EVENT_FRAME_SYNC or not. > > > > > > nit: I believe that you have swapped iso and bulk on this description > > > > I've confused the USB packet size and the UVC payload size. The latter > > is typically much bigger for bulk devices than isoc devices, but the > > former will be in similar order of magnitudes in a large number of > > cases, but not all cases. > > > > The URB size is the result of the USB packet size and number of packets > > per URB. The uvcvideo driver currently sets the number of packets per > > URB to 32 at most (and lowers it if the frame size is small, or if not > > enough memory can be allocated). This could be increased or made dynamic > > in the future, as higher speeds typically benefit from larger URB sizes. > > The packet size differs between bulk and isoc endpoints. > > > > For bulk, the packet size can be up to 512 bytes for USB 2.0 and 1024 > > bytes for USB 3.0, and the device can select a smaller size. The largest > > URB size (again based on the current implementation of the uvcvideo > > driver) is thus 32 KiB. > > > > For isochronous the situation is more complicated. The term "packet" as > > used in the uvcvideo driver actually means all the data transferred in > > one service interval, thus made of multiple isoc packets. It is heavily > > dependent on the USB speed, and the device can advertise different > > supported sizes (which translate directly to the reserved bandwidth for > > the transfer), with the driver picking the smallest bandwidth large > > enough for the data rate required by the resolution and frame rate. The > > theoretical worst case is 1024 bytes per isoc packet * 16 isoc packets > > per burst * 6 burst per interval * 32 "packets" per URB, equal to 3 MiB. > > > > Even with the largest URB size you have witnessed of ~1 MiB, we will end > > up lying quite a bit if we consider the URB completion callback for the > > first URB of the frame as indicating the start of reception. > > > > > > In term of accuracy, if timestamp was passed with the FRAME_SYNC event, > > > > it would not matter how fast your process the event anymore and greatly > > > > improve accuracy. > > > > > > +1 to that. If we could easily change the uAPI for FRAME_SYNC that > > > should definitely be implemented. > > > > > > > > Not to mention that the UVC timestamping requires a bit of love. > > > > > > > > > > @Laurent Pinchart, @Kieran Bingham any progress reviewing :P : > > > > > https://patchwork.linuxtv.org/project/linux-media/list/?series=10083 > > > > > > > > Thanks for working on this by the way, hope someone will find the time > > > > to review this. The timestamps should in theory provide a jitter free > > > > > > It already has a couple of Reviewed-by stamped in.... ;) > > > > > > > measurement of the delay Esker is trying to measure, and if it wasn't > > > > of bugs (and crazy complexity) it would in the worst case match the > > > > transfer time. > > > > > > Sorry to repeat myself, but just to avoid the confusion: Esker needs > > > to know how much power is used since we start receiving a frame until > > > it is available for dqbuf, not de frame latency. > > > > As I think everybody is aware, the earliest notification you get on the > > CPU side is the *end* of reception of the first URB, which can possibly > > be significantly later than the start of reception of the frame. > > > > Based on what I understand, the goal is to measure the CPU power > > consumption related to CPU processing of the frame. If that's the case, > > there's good and bad news. The good news is that the CPU doesn't process > > the frame at all until the URB has been received (if you were to measure > > the power consumption of the USB host controller too, it would be a > > different story), so the delay shouldn't be a problem. The bad news is > > that I don't see how the information you're trying to get will help you, > > as there's plenty of other things unrelated to the uvcvideo driver that > > can take CPU time while a frame is being received. That may not be any > > of my business, but from the point of view of the uvcvideo driver, I'm > > less inclined to accept a possibly significant V4L2_EVENT_FRAME_SYNC lie > > if the use case ends up making little sense :-) > > Just to add some numbers to add some context: > > V4L2_EVENT_FRAME_SYNC for BULK and ISO will be delayed from: > ``` > Triggered immediately when the reception of a frame has begun > ``` > one urb. > > For bulk devices this is a maximum of 0.05 msec (32KiB/600MBps) I lack a bit of knowledge on how to scale this to different devices, with different speed/framesize. My only bulk device is: https://inogeni.com/product/4k2usb3/ Which is USB 3.0, and have raw (NV12) resolution from 640x480 (max 60pfs) to 4K (max 30fps). What would the error look like with that ? > For 1MiB transfer isoc devices (which is the biggest we have seen), > that is 1.8 msec. > In both cases, this is smaller than the jitter to process the event > itself by userspace. > > The time from V4L2_EVENT_FRAME_SYNC to DQBUF is around 30 msec. > > I do not know how much delay is considered acceptable... but if we > take the delay argument to the extreme, we could say that > V4L2_EVENT_FRAME_SYNC can never be implemented, because the event will > always be delayed by something. We have v4l2_event.timestamp for all events, so the jitter to process the event by userpace can be removed reliably already. Nicolas p.s. missed it earlier > > > > > -- > > Regards, > > > > Laurent Pinchart > > >
> > > > For bulk devices this is a maximum of 0.05 msec (32KiB/600MBps) > > I lack a bit of knowledge on how to scale this to different devices, with > different speed/framesize. My only bulk device is: > > https://inogeni.com/product/4k2usb3/ > > Which is USB 3.0, and have raw (NV12) resolution from 640x480 (max 60pfs) to 4K > (max 30fps). What would the error look like with that ? For bulk devices the maximum delay from packing multiple packets into a urb is 0.05 msec (32KiB/600MBps) Laurent's message <20231109000327.GE21616@pendragon.ideasonboard.com> explains where those numbers come from :). Regards! > > > For 1MiB transfer isoc devices (which is the biggest we have seen), > > that is 1.8 msec. > > In both cases, this is smaller than the jitter to process the event > > itself by userspace. > > > > The time from V4L2_EVENT_FRAME_SYNC to DQBUF is around 30 msec. > > > > I do not know how much delay is considered acceptable... but if we > > take the delay argument to the extreme, we could say that > > V4L2_EVENT_FRAME_SYNC can never be implemented, because the event will > > always be delayed by something. > > We have v4l2_event.timestamp for all events, so the jitter to process the event > by userpace can be removed reliably already. > > Nicolas > > p.s. missed it earlier > > > > > > > > > -- > > > Regards, > > > > > > Laurent Pinchart > > > > > > > -- Ricardo Ribalda
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index f4988f03640a..9f3fb5fd2375 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -1352,6 +1352,8 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh, switch (sub->type) { case V4L2_EVENT_CTRL: return v4l2_event_subscribe(fh, sub, 0, &uvc_ctrl_sub_ev_ops); + case V4L2_EVENT_FRAME_SYNC: + return v4l2_event_subscribe(fh, sub, 0, NULL); default: return -EINVAL; } diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index 28dde08ec6c5..4f3a510ca4fe 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -1073,9 +1073,16 @@ static int uvc_video_decode_start(struct uvc_streaming *stream, * that discontinuous sequence numbers always indicate lost frames. */ if (stream->last_fid != fid) { + struct v4l2_event event = { + .type = V4L2_EVENT_FRAME_SYNC, + }; + stream->sequence++; if (stream->sequence) uvc_video_stats_update(stream); + + event.u.frame_sync.frame_sequence = stream->sequence, + v4l2_event_queue(&stream->vdev, &event); } uvc_video_clock_decode(stream, buf, data, len);