linux-kernel - Re: [PATCH v2] media: uvcvideo: Implement V4L2_EVENT_FRAME

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANiDSCunRRyod3Rg+L9ZsffOZyC7CKbMVTHX3u-n5iuNyZQk0w@mail.gmail.com>
Date:   Wed, 8 Nov 2023 23:46:40 +0100
From:   Ricardo Ribalda <ribalda@...omium.org>
To:     nicolas@...fresne.ca
Cc:     Esker Wong <esker@...gle.com>,
        Kieran Bingham <kieran.bingham@...asonboard.com>,
        Sakari Ailus <sakari.ailus@....fi>,
        Esker Wong <esker@...omium.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] media: uvcvideo: Implement V4L2_EVENT_FRAME_SYNC

Hi Nicolas

On Wed, 8 Nov 2023 at 21:32, <nicolas@...fresne.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
>