[{"id":23400,"web_url":"https://patchwork.libcamera.org/comment/23400/","msgid":"<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>","date":"2022-06-13T10:52:47","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":27,"url":"https://patchwork.libcamera.org/api/people/27/","name":"Dave Stevenson","email":"dave.stevenson@raspberrypi.com"},"content":"Hi Kieran\n\nOn Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel\n<libcamera-devel@lists.libcamera.org> wrote:\n>\n> V4L2 Video devices should always start streaming from index zero.\n> Identify and report a warning if they don't, and correct for any offset.\n\n[1] sequence\n\"The count starts at zero and includes dropped or repeated frames\"\n\nSo if the sensor driver has set a value via subdev sensor ops\ng_skip_frames[2], shouldn't it include that count? Or is skipping\nconsidered different from dropping?\n\n(I probably ought to add support for g_skip_frames to the Unicam\ndriver at some point).\n\n  Dave\n\n[1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer\n[2] https://www.kernel.org/doc/html/latest/driver-api/media/v4l2-subdev.html#c.v4l2_subdev_sensor_ops\n\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>\n> I'm not yet sure if the Warning print is perhaps just unhelpful.\n> It doesn't really affect us if we have this work around in - so we can\n> be silent, and I'm not yet sure if V4L2 has any 'requirement' that it\n> should start at 0 at every stream on.\n>\n>  include/libcamera/internal/v4l2_videodevice.h |  1 +\n>  src/libcamera/v4l2_videodevice.cpp            | 15 +++++++++++++++\n>  2 files changed, 16 insertions(+)\n>\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 23f37f83f8e2..8525acbc558d 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -276,6 +276,7 @@ private:\n>         EventNotifier *fdBufferNotifier_;\n>\n>         State state_;\n> +       std::optional<unsigned int> firstFrame_;\n>\n>         Timer watchdog_;\n>         utils::Duration watchdogDuration_;\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 430715afd554..63911339f96e 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -1771,6 +1771,19 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n>         if (V4L2_TYPE_IS_OUTPUT(buf.type))\n>                 return buffer;\n>\n> +       /*\n> +        * Detect kernel drivers which do not reset the sequence number to zero\n> +        * on stream start.\n> +        */\n> +       if (!firstFrame_) {\n> +               if (buf.sequence)\n> +                       LOG(V4L2, Warning)\n> +                               << \"Zero sequence expected for first frame (got \"\n> +                               << buf.sequence << \")\";\n> +               firstFrame_ = buf.sequence;\n> +       }\n> +       buffer->metadata_.sequence -= firstFrame_.value();\n> +\n>         unsigned int numV4l2Planes = multiPlanar ? buf.length : 1;\n>         FrameMetadata &metadata = buffer->metadata_;\n>\n> @@ -1847,6 +1860,8 @@ int V4L2VideoDevice::streamOn()\n>  {\n>         int ret;\n>\n> +       firstFrame_.reset();\n> +\n>         ret = ioctl(VIDIOC_STREAMON, &bufferType_);\n>         if (ret < 0) {\n>                 LOG(V4L2, Error)\n> --\n> 2.25.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 319C1BD161\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 13 Jun 2022 10:53:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8326465635;\n\tMon, 13 Jun 2022 12:53:04 +0200 (CEST)","from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com\n\t[IPv6:2a00:1450:4864:20::52c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F497600F1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 13 Jun 2022 12:53:03 +0200 (CEST)","by mail-ed1-x52c.google.com with SMTP id x5so6577423edi.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 13 Jun 2022 03:53:03 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655117584;\n\tbh=zFJSLcQi1Y77c7khxuL7BR//TXt4RNpEeljc2vU8Kjw=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=qBTQ1qBOPyPvfFalArsbxbgt6PPX/oziAa8B253i4x3ChEqPmCVpkNgAqspcqrBzZ\n\tNv3B1iH9+Eb1YZrV6tgv4qzhJm0uTW0G4RMOPuzSyX9PWhFCdCtfoa9JgeK2yXMVC0\n\taB+fbFq2VLgjduMQxj+WzZcpX1j5ChiCNWhFvxkV4LQMLd78fFwznVbE3a+KaRHsBD\n\tz61NdZbLpodDlvQfLeZJ4rf6lBegWsySpfrxWlARu/b7Fp7uJFmEXuotW6wZ1cS0fa\n\tLtJgrc9DHrjtraFN9PTXpVX8lBytEeu1KtTZxuCbnfUEN9OhWqoT+6TIQwP0X4ZgOj\n\te1b0ll75DBXhg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=xm0h4LvlSOVL5+KjRhLsVRUedobRC5ydEA4R7bBHS5I=;\n\tb=RbQH6HlvVAYAjn4sqWqRoZfWp0x/0srYhBY/XmiR4ZWGWz6sZ2mwHgwJJDgUEm8t4Y\n\tctQJgM1gDcmL++4j8iD/KZW5gyf9VGqCO9QbbJ3PnfLMhoLC0NWWGA2I5hqSYl+fhbrt\n\tCgJXYqHrsFI2vB3PdguZlHNmk9npMhkUngKMc9FjW1WSU5k55+LT6oQxz5WzI7TrcxUL\n\tjpDS+hjH97v3maVsPRSEM+VeR2LIF+g9fUDUu0zaEkr3PfDuMcJtTaXF6eYf2Q3xoTL8\n\thosbHZUUE5Rxtc7OWTCebTlBqXccaJK5SegPZpIcFFQVMpvflb9j95I6Q0o4NidEwmrR\n\tvjgg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"RbQH6Hlv\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=xm0h4LvlSOVL5+KjRhLsVRUedobRC5ydEA4R7bBHS5I=;\n\tb=dRxDldDFFzuWC4KQcsi8pXph6VViEHzFhIJxHfCY0rpHeYHUl/S0jCXQEFt7Rk2asp\n\tnEuggV1hKDbN7dpNlmR17SKgSkf/nhFetFMQAfYZerICM4abRD6fX9CyMgm7+L4QyX2L\n\taG4GCYCggS0OBdKhg5bk8CxjHZphezHvT9DK6nnbgeuHgYB8A/JKtSEriRCf5r7t5P8q\n\tud3gJPyMtdM50hB/b3+OZAHFKWDX+7AfTkGP7VWek8yNOmP8isL9ZqAVSvFFhtdfYEs0\n\tyDfOayX2hLHmYwa6aJNOeYtG6CxUHfv84kYpBgohfObw1xBAfbSG3lWTOEQJgVSysdFO\n\tIIQg==","X-Gm-Message-State":"AOAM531hfZq6+6dUbxfmVaWGY/02PswwsdfuAdvmCld3gZLDRPpx0cm2\n\tZjsTOXGZCiptu3oSE9CKEU4FMeRtpsYDKOjyVezk4I5FGaxsWHqG","X-Google-Smtp-Source":"ABdhPJyNIB8oxinhdzlxv34t+pvN9uBRrbAC59HzaNNNfg2gGfMwFgGXES2TlU+3xjj4MbDlDP/faDfFJLBQSoX6xng=","X-Received":"by 2002:a05:6402:524d:b0:42e:332:dd04 with SMTP id\n\tt13-20020a056402524d00b0042e0332dd04mr64249855edd.258.1655117583086;\n\tMon, 13 Jun 2022 03:53:03 -0700 (PDT)","MIME-Version":"1.0","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>","In-Reply-To":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>","Date":"Mon, 13 Jun 2022 11:52:47 +0100","Message-ID":"<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Dave Stevenson via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Dave Stevenson <dave.stevenson@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23401,"web_url":"https://patchwork.libcamera.org/comment/23401/","msgid":"<165512125453.1793421.15923714185258018579@Monstersaurus>","date":"2022-06-13T11:54:14","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Dave Stevenson (2022-06-13 11:52:47)\n> Hi Kieran\n> \n> On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel\n> <libcamera-devel@lists.libcamera.org> wrote:\n> >\n> > V4L2 Video devices should always start streaming from index zero.\n> > Identify and report a warning if they don't, and correct for any offset.\n> \n> [1] sequence\n> \"The count starts at zero and includes dropped or repeated frames\"\n> \n> So if the sensor driver has set a value via subdev sensor ops\n> g_skip_frames[2], shouldn't it include that count? Or is skipping\n> considered different from dropping?\n\nWell - from my perspective, if the frames are skipped at the beginning\nof the stream 'consistently', and didn't even require a buffer - that's\nlike they never existed, so I would say they are excluded.\n\nI also presume though that this would mean userspace has no ability to\nset controls during those frames (as there are no SoF events?). And if\nso - then I think it's right to exclude.\n\nOf course then to counter-argument myself, could knowing those two frames are\nskipped - provide time to set controls in advance? Or do we then assume\nthat controls set from before starting the stream are always fully\nactive at the 'first' received real frame?\n\n\n> \n> (I probably ought to add support for g_skip_frames to the Unicam\n> driver at some point).\n> \n>   Dave\n> \n> [1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer\n\nAha thanks, I'm sure I looked there, and somehow must have missed that\ntext. I think I saw the 'In V4L2_FIELD_ALTERNATE_MODE ...' and believed\nthe following text was irrelevant.\n\n\n\n> [2] https://www.kernel.org/doc/html/latest/driver-api/media/v4l2-subdev.html#c.v4l2_subdev_sensor_ops\n> \n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >\n> > I'm not yet sure if the Warning print is perhaps just unhelpful.\n> > It doesn't really affect us if we have this work around in - so we can\n> > be silent, and I'm not yet sure if V4L2 has any 'requirement' that it\n> > should start at 0 at every stream on.\n\nSounds like this print is actually a valid warning and should stay to\nreport kernel 'bugs'.\n\n--\nKieran\n\n> >\n> >  include/libcamera/internal/v4l2_videodevice.h |  1 +\n> >  src/libcamera/v4l2_videodevice.cpp            | 15 +++++++++++++++\n> >  2 files changed, 16 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> > index 23f37f83f8e2..8525acbc558d 100644\n> > --- a/include/libcamera/internal/v4l2_videodevice.h\n> > +++ b/include/libcamera/internal/v4l2_videodevice.h\n> > @@ -276,6 +276,7 @@ private:\n> >         EventNotifier *fdBufferNotifier_;\n> >\n> >         State state_;\n> > +       std::optional<unsigned int> firstFrame_;\n> >\n> >         Timer watchdog_;\n> >         utils::Duration watchdogDuration_;\n> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> > index 430715afd554..63911339f96e 100644\n> > --- a/src/libcamera/v4l2_videodevice.cpp\n> > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > @@ -1771,6 +1771,19 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n> >         if (V4L2_TYPE_IS_OUTPUT(buf.type))\n> >                 return buffer;\n> >\n> > +       /*\n> > +        * Detect kernel drivers which do not reset the sequence number to zero\n> > +        * on stream start.\n> > +        */\n> > +       if (!firstFrame_) {\n> > +               if (buf.sequence)\n> > +                       LOG(V4L2, Warning)\n> > +                               << \"Zero sequence expected for first frame (got \"\n> > +                               << buf.sequence << \")\";\n> > +               firstFrame_ = buf.sequence;\n> > +       }\n> > +       buffer->metadata_.sequence -= firstFrame_.value();\n> > +\n> >         unsigned int numV4l2Planes = multiPlanar ? buf.length : 1;\n> >         FrameMetadata &metadata = buffer->metadata_;\n> >\n> > @@ -1847,6 +1860,8 @@ int V4L2VideoDevice::streamOn()\n> >  {\n> >         int ret;\n> >\n> > +       firstFrame_.reset();\n> > +\n> >         ret = ioctl(VIDIOC_STREAMON, &bufferType_);\n> >         if (ret < 0) {\n> >                 LOG(V4L2, Error)\n> > --\n> > 2.25.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8236BBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 13 Jun 2022 11:54:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9FF8365631;\n\tMon, 13 Jun 2022 13:54:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0FB1A600F1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 13 Jun 2022 13:54:17 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B1045305;\n\tMon, 13 Jun 2022 13:54:16 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655121258;\n\tbh=cS3UG9tSS7JnWZZD6UHLHGylgMYFpvKXJFflAPyQ6l4=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=q2XF0b453hfHd6qqe8jN5nT60mWBfbx/FHSHqIbn+s4C6xWbtQaQGJKwfqJMI1UKl\n\tbwRUdiXp4Bxt+N4jQUPjJmzIIl+TmCyDh92ciZHPFrijArQ3WN++xVowfP5OfuCiTR\n\taCQLKEHKagnWaPsGH5Nml4iCJiuoBhz4zD5qjYQ6auv0H2b44qlR+O6bcZOXUu/0Gu\n\t4gE1i4jvZoD3SVxZTJrHLAlefQfSGTF3bp0K0o6QU2Zx4UFBTmHUNq8GD+rn/kSM+s\n\tUyUIwFmfXWX3RB8w4DuBLIKX1spxyectxDUsVOQBQjcIO08jVw5fddgp+1qGMXgw66\n\tB/Z6wx/U8Lxnw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655121256;\n\tbh=cS3UG9tSS7JnWZZD6UHLHGylgMYFpvKXJFflAPyQ6l4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=bI9FMTXcqHezt5SH0W85/o5UWVMQ8T07oQFJGl6GBOfz4CZgogTR3L9iXe6CaWec9\n\tp4yDow1xDpRjMOzkZ29jBhbNRDbjzeY37cJL3HPbcbX1Kw5UKOZahI2q4Ru23Tn7g7\n\t9UNh/hP2K4ZUl2uz/D9YbHL7CUQs9bk6RGnLhUbA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"bI9FMTXc\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>","To":"Dave Stevenson <dave.stevenson@raspberrypi.com>","Date":"Mon, 13 Jun 2022 12:54:14 +0100","Message-ID":"<165512125453.1793421.15923714185258018579@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23479,"web_url":"https://patchwork.libcamera.org/comment/23479/","msgid":"<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>","date":"2022-06-20T08:59:39","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\n(CC'ing Sakari)\n\nOn Mon, Jun 13, 2022 at 12:54:14PM +0100, Kieran Bingham via libcamera-devel wrote:\n> Quoting Dave Stevenson (2022-06-13 11:52:47)\n> > On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel wrote:\n> > >\n> > > V4L2 Video devices should always start streaming from index zero.\n> > > Identify and report a warning if they don't, and correct for any offset.\n> > \n> > [1] sequence\n> > \"The count starts at zero and includes dropped or repeated frames\"\n> > \n> > So if the sensor driver has set a value via subdev sensor ops\n> > g_skip_frames[2], shouldn't it include that count? Or is skipping\n> > considered different from dropping?\n> \n> Well - from my perspective, if the frames are skipped at the beginning\n> of the stream 'consistently', and didn't even require a buffer - that's\n> like they never existed, so I would say they are excluded.\n> \n> I also presume though that this would mean userspace has no ability to\n> set controls during those frames (as there are no SoF events?). And if\n> so - then I think it's right to exclude.\n> \n> Of course then to counter-argument myself, could knowing those two frames are\n> skipped - provide time to set controls in advance? Or do we then assume\n> that controls set from before starting the stream are always fully\n> active at the 'first' received real frame?\n\n.g_skip_frames() is an internal kernel API, so I think its result should\nnot be visible to userspace when it comes to the sequence number.\nOtherwise that would effectively say that the sequence number must\nalways start at zero except when it can start at a different value :-)\n\nI'd go one step further and argue that .g_frame_skip() should be\nconsidered legacy, it's better handled in userspace these days.\n\n> > (I probably ought to add support for g_skip_frames to the Unicam\n> > driver at some point).\n> > \n> > [1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer\n> \n> Aha thanks, I'm sure I looked there, and somehow must have missed that\n> text. I think I saw the 'In V4L2_FIELD_ALTERNATE_MODE ...' and believed\n> the following text was irrelevant.\n> \n> > [2] https://www.kernel.org/doc/html/latest/driver-api/media/v4l2-subdev.html#c.v4l2_subdev_sensor_ops\n> > \n> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > ---\n> > >\n> > > I'm not yet sure if the Warning print is perhaps just unhelpful.\n> > > It doesn't really affect us if we have this work around in - so we can\n> > > be silent, and I'm not yet sure if V4L2 has any 'requirement' that it\n> > > should start at 0 at every stream on.\n> \n> Sounds like this print is actually a valid warning and should stay to\n> report kernel 'bugs'.\n\nAny progress updating the kernel documentation to make this requirement\nexplicit ?\n\n> > >  include/libcamera/internal/v4l2_videodevice.h |  1 +\n> > >  src/libcamera/v4l2_videodevice.cpp            | 15 +++++++++++++++\n> > >  2 files changed, 16 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> > > index 23f37f83f8e2..8525acbc558d 100644\n> > > --- a/include/libcamera/internal/v4l2_videodevice.h\n> > > +++ b/include/libcamera/internal/v4l2_videodevice.h\n> > > @@ -276,6 +276,7 @@ private:\n> > >         EventNotifier *fdBufferNotifier_;\n> > >\n> > >         State state_;\n> > > +       std::optional<unsigned int> firstFrame_;\n> > >\n> > >         Timer watchdog_;\n> > >         utils::Duration watchdogDuration_;\n> > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> > > index 430715afd554..63911339f96e 100644\n> > > --- a/src/libcamera/v4l2_videodevice.cpp\n> > > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > > @@ -1771,6 +1771,19 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n> > >         if (V4L2_TYPE_IS_OUTPUT(buf.type))\n> > >                 return buffer;\n> > >\n> > > +       /*\n> > > +        * Detect kernel drivers which do not reset the sequence number to zero\n> > > +        * on stream start.\n> > > +        */\n> > > +       if (!firstFrame_) {\n> > > +               if (buf.sequence)\n> > > +                       LOG(V4L2, Warning)\n> > > +                               << \"Zero sequence expected for first frame (got \"\n> > > +                               << buf.sequence << \")\";\n> > > +               firstFrame_ = buf.sequence;\n> > > +       }\n\nI'd add a blank line here.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> > > +       buffer->metadata_.sequence -= firstFrame_.value();\n> > > +\n> > >         unsigned int numV4l2Planes = multiPlanar ? buf.length : 1;\n> > >         FrameMetadata &metadata = buffer->metadata_;\n> > >\n> > > @@ -1847,6 +1860,8 @@ int V4L2VideoDevice::streamOn()\n> > >  {\n> > >         int ret;\n> > >\n> > > +       firstFrame_.reset();\n> > > +\n> > >         ret = ioctl(VIDIOC_STREAMON, &bufferType_);\n> > >         if (ret < 0) {\n> > >                 LOG(V4L2, Error)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4B4EABD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Jun 2022 08:59:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8930A65635;\n\tMon, 20 Jun 2022 10:59:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F52C60473\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jun 2022 10:59:55 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A824883;\n\tMon, 20 Jun 2022 10:59:54 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655715596;\n\tbh=s1tAkPIqVNx4oaFfl5DbWWLI07tKPbGg6hM9oVtCfn8=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=dsHvFb/977zaHi+kcx6ldYNjEPlx/F3UmmJScu/YkygE7aLt5/6olt+OhlHePBujp\n\tElK0/8JkLMJB7+tQ245ONk+eF/LwjG46pWXn2zhmduZm7tbtAl9evWiW8d2QO1u9Lr\n\t7Zf6wHQyE0Gflb9PtJPr/H70a112E1tSsV3/UqCFYgguw/Sn+6diVKC47qT/xkkuUD\n\tekKlqEQwoO0BhRAs2OBcAv1pKH9RJuL89L9nU+t9R4VBSGUH6MwJ4600YfMl5JM1zj\n\t3x2Jj1qsebxYNqLaWfab/4kR90Ek4XESlCFOoGz2wmclaTLOdkac9eFzltGQvooMhz\n\t0NwGaJRTBqSCQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655715594;\n\tbh=s1tAkPIqVNx4oaFfl5DbWWLI07tKPbGg6hM9oVtCfn8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uHqg90DpL9vnMCLLP+uV0JNPojDxpe8AmroCGh+V0WxsEusRDdgqCABGjpDqDm+3p\n\tA8njif0Ka0JwCa2Pa9mgfJfcHVVR+I9vcL1cMWb/PyFz84LvYzShT2UKZhvAyYluIQ\n\tmqTdScnzDSIVBbVjrb7fCJ1mKFLUBv6TkI3MYmiw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uHqg90Dp\"; dkim-atps=neutral","Date":"Mon, 20 Jun 2022 11:59:39 +0300","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>\n\t<165512125453.1793421.15923714185258018579@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<165512125453.1793421.15923714185258018579@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>,\n\tSakari Ailus <sakari.ailus@iki.fi>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23510,"web_url":"https://patchwork.libcamera.org/comment/23510/","msgid":"<20220621154232.ycuuwyvtposmxqvv@uno.localdomain>","date":"2022-06-21T15:42:32","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Mon, Jun 13, 2022 at 12:35:28PM +0200, Kieran Bingham via libcamera-devel wrote:\n> V4L2 Video devices should always start streaming from index zero.\n> Identify and report a warning if they don't, and correct for any offset.\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>\n> I'm not yet sure if the Warning print is perhaps just unhelpful.\n> It doesn't really affect us if we have this work around in - so we can\n> be silent, and I'm not yet sure if V4L2 has any 'requirement' that it\n> should start at 0 at every stream on.\n>\n\nLet's keep the warning! As the requirement is documented, as reported\nby Dave, I wonder if we should not \"fix your kernel driver\" as we do\nfor missing sensor driver features.\n\nAnyway, the patch is good\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n>  include/libcamera/internal/v4l2_videodevice.h |  1 +\n>  src/libcamera/v4l2_videodevice.cpp            | 15 +++++++++++++++\n>  2 files changed, 16 insertions(+)\n>\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index 23f37f83f8e2..8525acbc558d 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -276,6 +276,7 @@ private:\n>  \tEventNotifier *fdBufferNotifier_;\n>\n>  \tState state_;\n> +\tstd::optional<unsigned int> firstFrame_;\n>\n>  \tTimer watchdog_;\n>  \tutils::Duration watchdogDuration_;\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index 430715afd554..63911339f96e 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -1771,6 +1771,19 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()\n>  \tif (V4L2_TYPE_IS_OUTPUT(buf.type))\n>  \t\treturn buffer;\n>\n> +\t/*\n> +\t * Detect kernel drivers which do not reset the sequence number to zero\n> +\t * on stream start.\n> +\t */\n> +\tif (!firstFrame_) {\n> +\t\tif (buf.sequence)\n> +\t\t\tLOG(V4L2, Warning)\n> +\t\t\t\t<< \"Zero sequence expected for first frame (got \"\n> +\t\t\t\t<< buf.sequence << \")\";\n> +\t\tfirstFrame_ = buf.sequence;\n> +\t}\n> +\tbuffer->metadata_.sequence -= firstFrame_.value();\n> +\n>  \tunsigned int numV4l2Planes = multiPlanar ? buf.length : 1;\n>  \tFrameMetadata &metadata = buffer->metadata_;\n>\n> @@ -1847,6 +1860,8 @@ int V4L2VideoDevice::streamOn()\n>  {\n>  \tint ret;\n>\n> +\tfirstFrame_.reset();\n> +\n>  \tret = ioctl(VIDIOC_STREAMON, &bufferType_);\n>  \tif (ret < 0) {\n>  \t\tLOG(V4L2, Error)\n> --\n> 2.25.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A3D87BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 21 Jun 2022 15:42:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E84D465634;\n\tTue, 21 Jun 2022 17:42:37 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8853B60473\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 Jun 2022 17:42:36 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 4503CFF80E;\n\tTue, 21 Jun 2022 15:42:34 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655826158;\n\tbh=xjJpsu7FRWs/YvnDTGcdVzPXOJFHEh79QzvEtjcbW6w=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=4A+77AsIOQREd8kqox6UXej+g17EpcnaxTmA+0P9orax3ZrJy8xcmTA6ExhHboIQ6\n\tml4EVmIc3t+f5793khLq598Wwp60S7kfMtPR+MRSoq4qQlXEpDdgFW2jCs9gwWX13B\n\tsERLWfsiboNmqHE0NrRNjSQChjZGMiEhM2EG8o7kfMGG8jgrkN4Ojtb2c+UTNRVjBF\n\tMZ/hFiWcEvTwZbYO0TzJczIUgLqK+q2PPhfBvZ/qXApkAkFpKIcmVS1DthBrjgbItc\n\tMkH+LWomQyU6f9Fce97NohNoTil+nvBK+XZch0oy8OTW9F3QdUe0sPSZnegRXuOBWy\n\tVRtBEN/70qkTQ==","Date":"Tue, 21 Jun 2022 17:42:32 +0200","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20220621154232.ycuuwyvtposmxqvv@uno.localdomain>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23860,"web_url":"https://patchwork.libcamera.org/comment/23860/","msgid":"<Ys8E5lhq8nMeyNIr@valkosipuli.retiisi.eu>","date":"2022-07-13T17:46:14","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":32,"url":"https://patchwork.libcamera.org/api/people/32/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Laurent,\n\nOn Mon, Jun 20, 2022 at 11:59:39AM +0300, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> (CC'ing Sakari)\n> \n> On Mon, Jun 13, 2022 at 12:54:14PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > Quoting Dave Stevenson (2022-06-13 11:52:47)\n> > > On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel wrote:\n> > > >\n> > > > V4L2 Video devices should always start streaming from index zero.\n> > > > Identify and report a warning if they don't, and correct for any offset.\n> > > \n> > > [1] sequence\n> > > \"The count starts at zero and includes dropped or repeated frames\"\n> > > \n> > > So if the sensor driver has set a value via subdev sensor ops\n> > > g_skip_frames[2], shouldn't it include that count? Or is skipping\n> > > considered different from dropping?\n> > \n> > Well - from my perspective, if the frames are skipped at the beginning\n> > of the stream 'consistently', and didn't even require a buffer - that's\n> > like they never existed, so I would say they are excluded.\n> > \n> > I also presume though that this would mean userspace has no ability to\n> > set controls during those frames (as there are no SoF events?). And if\n> > so - then I think it's right to exclude.\n> > \n> > Of course then to counter-argument myself, could knowing those two frames are\n> > skipped - provide time to set controls in advance? Or do we then assume\n> > that controls set from before starting the stream are always fully\n> > active at the 'first' received real frame?\n> \n> .g_skip_frames() is an internal kernel API, so I think its result should\n> not be visible to userspace when it comes to the sequence number.\n> Otherwise that would effectively say that the sequence number must\n> always start at zero except when it can start at a different value :-)\n> \n> I'd go one step further and argue that .g_frame_skip() should be\n> considered legacy, it's better handled in userspace these days.\n\nHow many sensors really need that nowadays? I think some ten years ago it\nwas thought to be mainly a property of old, old stuff. Well, surprises\nalways tend to happen when it comes to hardware.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0504BBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 13 Jul 2022 17:46:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74EFD63312;\n\tWed, 13 Jul 2022 19:46:17 +0200 (CEST)","from lahtoruutu.iki.fi (lahtoruutu.iki.fi [IPv6:2a0b:5c81:1c1::37])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2AA686274E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 13 Jul 2022 19:46:16 +0200 (CEST)","from hillosipuli.retiisi.eu\n\t(dkwl20tj04snw15cjtflt-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:4493:6f40:fec3:d72a:e447:8113])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: sailus)\n\tby lahtoruutu.iki.fi (Postfix) with ESMTPSA id D88F81B00340;\n\tWed, 13 Jul 2022 20:46:14 +0300 (EEST)","from valkosipuli.retiisi.eu (valkosipuli.localdomain [192.168.4.2])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby hillosipuli.retiisi.eu (Postfix) with ESMTPS id 68CF2634D5F;\n\tWed, 13 Jul 2022 20:46:14 +0300 (EEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657734377;\n\tbh=fAE5mNuNLIQ/SKXboxvosgb/Gido6GPnWzlvzvQIkjE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ZAWfjO/FXj9q/gT30DRx8gnikTu75XnZHHmOAk5rhxsrlY0wnb+GIdbXKF8gnioKG\n\tTXV5I5Itrhc7q0/Tv64GKwtUkGI6H/ZXkAFuHvnQF6i2iW8p2juVxTzfx6siVa6VmE\n\tWhw5CbFh7XccKxyscs+8MMsNqyUi6ljR2fmbnQft12CimBtShW+X35BaxMTGGX82FS\n\t8LWCdO8E30RtZ/PDChBuMwdsLlmVRP+MDDjaJE9YxSS304vSlzLqSSeB9bxO7sIfiH\n\t8wk5ibxnBoy8loGNUryWArJu0ixJ2MVzeNbWU1RSHMvq29hFcz1rlNymIAZqd6sNTF\n\tiUjcjO8nT7csw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu;\n\tt=1657734374;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=0mBci0tR1zah/EU/kA3UUh0ljUVvC6UfebPIOSFeQ+8=;\n\tb=YjtVz9/7f9tDIOb+hVOvxJpnoTK+4GtoPOmDipztcbvfEbqPaBK0fW+XNlybsL/Y4LAgFO\n\tm5pDVAw9SAgbHmLlGajdervqr8jaSvex3UeDBvOEhlh2KSJWIFUr9cUXxKUcBVUpNau3uZ\n\tu68yeCo/2s/EyudQDGjXxdohWVuzyQUcjsiQ6KMcaruA0KK+yJF+FDbVNGlWFeg6eLUmuy\n\tbHOwG3wJm2JUHn8PzpA6fPy/irl2ZUrPRHSs3vNVAl5tomJp21UMKh7oGuAe5AP8AZNZw4\n\t8++hJs6VrQN2X4gsLgtBOhgU0jNhasAi8a671hDvL7SO3HMCIh0H3C9YtQEtVQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=iki.fi header.i=@iki.fi\n\theader.b=\"YjtVz9/7\"; dkim-atps=neutral","Date":"Wed, 13 Jul 2022 20:46:14 +0300","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<Ys8E5lhq8nMeyNIr@valkosipuli.retiisi.eu>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>\n\t<165512125453.1793421.15923714185258018579@Monstersaurus>\n\t<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>","ARC-Seal":"i=1; s=lahtoruutu; d=iki.fi; t=1657734374; a=rsa-sha256; cv=none; \n\tb=ufwHqc3CHvzX/+gMBO6Q9naCpLITZYow5+yjrvLQYvM4Qdk6udz18UhHA2Hkjk1D9ZiuxV\n\tuvSQaSNwc/TKq+K8s6X7HCon/UWckRv8LOFybauxjW1e5Nq4CyozPO2vw0DAbkTDwCNYwu\n\tq2fL+5jIcqFduFQRu5dXEqmuGrOkkmtn2gnz7RfBMbJHVE6BxQB5pNsWTnJTNB7/4Dtrbx\n\tNmOzyqgRzd8EsEE7w9vU/7Tt2nfE71Hk0F7jS51xF3c1828zxuQdXMlx/OiOZjS0Vn+dmu\n\t/f+lE1cE54pMRP3+OPJRUVRQe0bMjPW8dhXA7RpW8RjSHOjjBDFNXnhmPDeNMw==","ARC-Authentication-Results":"i=1; ORIGINATING;\n\tauth=pass smtp.auth=sailus smtp.mailfrom=sakari.ailus@iki.fi","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi;\n\ts=lahtoruutu; t=1657734374;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=0mBci0tR1zah/EU/kA3UUh0ljUVvC6UfebPIOSFeQ+8=;\n\tb=aG2pv2rUJ6Hb9CxJfS+tRwHt4xkoNdMIS0E+kiLAfZBcgPcU/94tgyV+NnRK0WwWrS88Ih\n\t9+oDgxQHcORRvx14rTPHwE2cF6pVvZrlwRadcZk7DP4m6yjUYrB8/3zIKJcSVmHsiSlMbe\n\tzAHl0DjKRP/AbQUlm1xMhX8KkOIthDRjDjnOjXJOlm4RgaxZXFSLhHGzzlBR++M7ojwnR5\n\tRibGXMCUn9Q2rZgSMJucGKQ9DHgcSoEd0cCqQFQHm0IM883KJ3BJA6TJtJ0XzcYbPGFcLW\n\t535cNVqiKJK1MM9qc0pC1x7Go1yCUeaj6PpaaTWt9aAib6yitWmEnUZFCpPyKA==","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Sakari Ailus via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Sakari Ailus <sakari.ailus@iki.fi>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23861,"web_url":"https://patchwork.libcamera.org/comment/23861/","msgid":"<Ys8GavkRV9QAAwRi@pendragon.ideasonboard.com>","date":"2022-07-13T17:52:42","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sakari,\n\nOn Wed, Jul 13, 2022 at 08:46:14PM +0300, Sakari Ailus wrote:\n> On Mon, Jun 20, 2022 at 11:59:39AM +0300, Laurent Pinchart wrote:\n> > On Mon, Jun 13, 2022 at 12:54:14PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > > Quoting Dave Stevenson (2022-06-13 11:52:47)\n> > > > On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel wrote:\n> > > > >\n> > > > > V4L2 Video devices should always start streaming from index zero.\n> > > > > Identify and report a warning if they don't, and correct for any offset.\n> > > > \n> > > > [1] sequence\n> > > > \"The count starts at zero and includes dropped or repeated frames\"\n> > > > \n> > > > So if the sensor driver has set a value via subdev sensor ops\n> > > > g_skip_frames[2], shouldn't it include that count? Or is skipping\n> > > > considered different from dropping?\n> > > \n> > > Well - from my perspective, if the frames are skipped at the beginning\n> > > of the stream 'consistently', and didn't even require a buffer - that's\n> > > like they never existed, so I would say they are excluded.\n> > > \n> > > I also presume though that this would mean userspace has no ability to\n> > > set controls during those frames (as there are no SoF events?). And if\n> > > so - then I think it's right to exclude.\n> > > \n> > > Of course then to counter-argument myself, could knowing those two frames are\n> > > skipped - provide time to set controls in advance? Or do we then assume\n> > > that controls set from before starting the stream are always fully\n> > > active at the 'first' received real frame?\n> > \n> > .g_skip_frames() is an internal kernel API, so I think its result should\n> > not be visible to userspace when it comes to the sequence number.\n> > Otherwise that would effectively say that the sequence number must\n> > always start at zero except when it can start at a different value :-)\n> > \n> > I'd go one step further and argue that .g_frame_skip() should be\n> > considered legacy, it's better handled in userspace these days.\n> \n> How many sensors really need that nowadays? I think some ten years ago it\n> was thought to be mainly a property of old, old stuff. Well, surprises\n> always tend to happen when it comes to hardware.\n\nIt's hard to come up with an exact number. I'm sure there are newer\nsensors whose first images are bad. I also wouldn't be surprised if the\nnumber of bad frames could depend on the sensor configuration in some\ncases. That's why I think the kernel shouldn't be involved here.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EF1A3BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 13 Jul 2022 17:53:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6138B6330E;\n\tWed, 13 Jul 2022 19:53:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7E8606274E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 13 Jul 2022 19:53:13 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C6746305;\n\tWed, 13 Jul 2022 19:53:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657734795;\n\tbh=Got+fDBZ0zXwnTEHtP07X5Csod02hAP8Dc+8AlGwxFU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=bvnZ41JhMKzqcd3MjlCuEBV94gSlYMRIQGuL0L++GZ0eHdXhCNOc5afypu6rH3CLb\n\t274bJ4XhK3dw3uvKJGuIHn9FYke75H+lySALyX7AjVhJFWKH1df5dUPUA/QO/Ny7oS\n\tp7nmcl2zDHzWY3mzw4eZBduUsO5UXsIKnx9yw2x7XzObxOhUatTMC3p4PJa0wm1/qp\n\t9e8hmsBeNwGlRIUFV5bJajxS15axOp1c0oqkish737/3VSSMgv2RLaj52L7R08fv4b\n\tEDSWgxFAdN1jdtKa+Fe1AkPTc5P7I+EHbpgBaIgZZEgGOFCyv2CVw4+Q4migqA4vmQ\n\tP1IX8ZmHJoscg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1657734793;\n\tbh=Got+fDBZ0zXwnTEHtP07X5Csod02hAP8Dc+8AlGwxFU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=v8HHbw5M8d+SjlBvAphr3YzXYQ09bKoiJ5Ec6yT+bw0phLCWLp9VGDgiHMj7a3ca0\n\troUW0LK4HAtWdSkarUpU3ZsSXJt+yUO+TNFz7ofHz+HGwiRxMH+nQcsUAOHCdMyvdA\n\tnx2KmUhUJ6V4KtaEvgDcLihY/cq7xD0Ml5wKeeO4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"v8HHbw5M\"; dkim-atps=neutral","Date":"Wed, 13 Jul 2022 20:52:42 +0300","To":"Sakari Ailus <sakari.ailus@iki.fi>","Message-ID":"<Ys8GavkRV9QAAwRi@pendragon.ideasonboard.com>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>\n\t<165512125453.1793421.15923714185258018579@Monstersaurus>\n\t<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>\n\t<Ys8E5lhq8nMeyNIr@valkosipuli.retiisi.eu>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<Ys8E5lhq8nMeyNIr@valkosipuli.retiisi.eu>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23862,"web_url":"https://patchwork.libcamera.org/comment/23862/","msgid":"<Ys8Jcwc3gLLLnSuo@valkosipuli.retiisi.eu>","date":"2022-07-13T18:05:39","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":32,"url":"https://patchwork.libcamera.org/api/people/32/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Laurent,\n\nOn Wed, Jul 13, 2022 at 08:52:42PM +0300, Laurent Pinchart wrote:\n> On Wed, Jul 13, 2022 at 08:46:14PM +0300, Sakari Ailus wrote:\n> > On Mon, Jun 20, 2022 at 11:59:39AM +0300, Laurent Pinchart wrote:\n> > > On Mon, Jun 13, 2022 at 12:54:14PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > > > Quoting Dave Stevenson (2022-06-13 11:52:47)\n> > > > > On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel wrote:\n> > > > > >\n> > > > > > V4L2 Video devices should always start streaming from index zero.\n> > > > > > Identify and report a warning if they don't, and correct for any offset.\n> > > > > \n> > > > > [1] sequence\n> > > > > \"The count starts at zero and includes dropped or repeated frames\"\n> > > > > \n> > > > > So if the sensor driver has set a value via subdev sensor ops\n> > > > > g_skip_frames[2], shouldn't it include that count? Or is skipping\n> > > > > considered different from dropping?\n> > > > \n> > > > Well - from my perspective, if the frames are skipped at the beginning\n> > > > of the stream 'consistently', and didn't even require a buffer - that's\n> > > > like they never existed, so I would say they are excluded.\n> > > > \n> > > > I also presume though that this would mean userspace has no ability to\n> > > > set controls during those frames (as there are no SoF events?). And if\n> > > > so - then I think it's right to exclude.\n> > > > \n> > > > Of course then to counter-argument myself, could knowing those two frames are\n> > > > skipped - provide time to set controls in advance? Or do we then assume\n> > > > that controls set from before starting the stream are always fully\n> > > > active at the 'first' received real frame?\n> > > \n> > > .g_skip_frames() is an internal kernel API, so I think its result should\n> > > not be visible to userspace when it comes to the sequence number.\n> > > Otherwise that would effectively say that the sequence number must\n> > > always start at zero except when it can start at a different value :-)\n> > > \n> > > I'd go one step further and argue that .g_frame_skip() should be\n> > > considered legacy, it's better handled in userspace these days.\n> > \n> > How many sensors really need that nowadays? I think some ten years ago it\n> > was thought to be mainly a property of old, old stuff. Well, surprises\n> > always tend to happen when it comes to hardware.\n> \n> It's hard to come up with an exact number. I'm sure there are newer\n> sensors whose first images are bad. I also wouldn't be surprised if the\n> number of bad frames could depend on the sensor configuration in some\n> cases. That's why I think the kernel shouldn't be involved here.\n\nWould you store this information outside the kernel, bound to the sensor\nsomehow?\n\nBut I agree the g_skip_frames shouldn't be visible outside the kernel. The\nnumber indicates frames that are really broken, i.e. not just e.g.\nunusually dark (most applies to SoC cameras).","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8C1DCBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 13 Jul 2022 18:05:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B923663312;\n\tWed, 13 Jul 2022 20:05:41 +0200 (CEST)","from lahtoruutu.iki.fi (lahtoruutu.iki.fi [185.185.170.37])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C2716274E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 13 Jul 2022 20:05:40 +0200 (CEST)","from hillosipuli.retiisi.eu\n\t(dkwl20tj04snw15cjtflt-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:4493:6f40:fec3:d72a:e447:8113])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: sailus)\n\tby lahtoruutu.iki.fi (Postfix) with ESMTPSA id C340C1B0029D;\n\tWed, 13 Jul 2022 21:05:39 +0300 (EEST)","from valkosipuli.retiisi.eu (valkosipuli.localdomain [192.168.4.2])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby hillosipuli.retiisi.eu (Postfix) with ESMTPS id 6EC93634D5F;\n\tWed, 13 Jul 2022 21:05:39 +0300 (EEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657735541;\n\tbh=FBCfd/lPPZqc/JCVS+XczOI2Z0whornI60EQT/zfeQU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Wp+7RQElG+yM5XonUbTi6GgH8p0YVfxW11DlMj4bNTQnlpxNpTdP3ZNmkDBSuoajZ\n\t8lwuwoPR3wdYydg6wewDVA9Kh1o9B6YXfQorSKdzQQSHv3iFXru+SwtVuzq0un2vs1\n\tmfkK8rz50dF7+1vNzE4SHwSiDpHNRGolCzbjosvPYutjCWdyFo3U3h7KNkC0xXNMDf\n\tEMo6HGfB8D2jZKeFzwRomIWr9FcJY3GhctPiJ5iTEQeyNREgOLWmxfMrA5aSz9NpYX\n\tBe3rKcAjcsWYIMYkSH5M4046N+sgTFih6XCpbWbp2JNOPiAl6pJZIaGG9/2lWYaySc\n\t3yxMU/dj/lShA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu;\n\tt=1657735539;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=7kUgUsZfGIgyk/TEmt9Wdieja6G4Nmi0e+gvA68OyhI=;\n\tb=S333Hy8CHwC6R6kiYW0E1QayiMmH4r7yHGooxFDIyUNO2QVAwzECKnjoawih9nfbUuT9Kd\n\tksq5aswGRRUXVz2KDQzrazhJVkGNIILbmwepkNoDXT9Bm7fXnAxu57JIRsMdYZMwfE0mHn\n\thtOd6TJn2IFMaOlYhyxpnC6Mv4Tn+3dVmBNWWvQ344ZZGvmNuDc8otts+oXBSlr+mJvWa3\n\tft/joZ9M9VFAGxH+NlZ9Rzm7x3XGUpBjXT/f0SYwUeeL46aNMjLUfs+mwkN3S2CvIsXeBV\n\tX4xnJy2NrZwsrk+zIlnnssyrRnrrF84HCnA9TVwvc6+/w4nPt7gmJsY3Z1TcXQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=iki.fi header.i=@iki.fi\n\theader.b=\"S333Hy8C\"; dkim-atps=neutral","Date":"Wed, 13 Jul 2022 21:05:39 +0300","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<Ys8Jcwc3gLLLnSuo@valkosipuli.retiisi.eu>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>\n\t<165512125453.1793421.15923714185258018579@Monstersaurus>\n\t<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>\n\t<Ys8E5lhq8nMeyNIr@valkosipuli.retiisi.eu>\n\t<Ys8GavkRV9QAAwRi@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<Ys8GavkRV9QAAwRi@pendragon.ideasonboard.com>","ARC-Seal":"i=1; s=lahtoruutu; d=iki.fi; t=1657735539; a=rsa-sha256; cv=none; \n\tb=v11J/qVg7X2UiN23ynAI7ID4ZoUeMXeE0J3izVRbhNGnDRrS3nersvgpEqjvUGs2x+WfDK\n\tiISI5irozzjHVwtQxqoj9Jaam1gzCag4Qw3RAvACWcJ7PtIq0dXR6EVmc/9ZeK4VXpfkgT\n\td1d0psYgrt4OGLUHT3MmTqmQBkRnM8KJLlvVbo7HZDWtYeTMeoGKxz/lyFu1Mws5Qe01YO\n\tM8dmHK6EJOtqSshf3xkXSAmF0AsO/Tj+h3zgnF4EpYzpFWmNj1fJI39agK39G2QGJZIP7Y\n\tC9g8Ql2J5jeUz1PbZMY0o2hMRVVhW7q8lGYllJMgNPM+ujKOEWp6gi1+gzO3Gg==","ARC-Authentication-Results":"i=1; ORIGINATING;\n\tauth=pass smtp.auth=sailus smtp.mailfrom=sakari.ailus@iki.fi","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi;\n\ts=lahtoruutu; t=1657735539;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=7kUgUsZfGIgyk/TEmt9Wdieja6G4Nmi0e+gvA68OyhI=;\n\tb=uTTsszUnkGy7s6AtO8SvYPn3eQ6Begjr9L/YCrYz2HLhNjoWAZtX8aeAZNk5iYQnZ/YWLk\n\tfnedpbBWQMqybbWIksfz1q8JTOn4mSc1TVoHPai/BQmIBXLqznZL3amMqYM1mU6KbbFUNz\n\t78/XEs4RkEXXCKMj+yQHdeQHo6IfQpFaPsLJCLKcPSisGMedZP+vPgQlqsbzwcZc09IbDO\n\t9840/+Y7exlgYWlp+dimJHY9RisbUUSVIvbsiwzthl3q+nlJBLYuJcBSxeYzcuZO47Q0fb\n\tHAMbTgHGfBmdFY8zKrwPtWbWCTBm1xPnOQU3HpLreUVoLt4qrnAng4xGXRfvJA==","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Sakari Ailus via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Sakari Ailus <sakari.ailus@iki.fi>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23863,"web_url":"https://patchwork.libcamera.org/comment/23863/","msgid":"<Ys8NhI0e8xCaPsh3@pendragon.ideasonboard.com>","date":"2022-07-13T18:23:00","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sakari,\n\nOn Wed, Jul 13, 2022 at 09:05:39PM +0300, Sakari Ailus wrote:\n> On Wed, Jul 13, 2022 at 08:52:42PM +0300, Laurent Pinchart wrote:\n> > On Wed, Jul 13, 2022 at 08:46:14PM +0300, Sakari Ailus wrote:\n> > > On Mon, Jun 20, 2022 at 11:59:39AM +0300, Laurent Pinchart wrote:\n> > > > On Mon, Jun 13, 2022 at 12:54:14PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > > > > Quoting Dave Stevenson (2022-06-13 11:52:47)\n> > > > > > On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel wrote:\n> > > > > > >\n> > > > > > > V4L2 Video devices should always start streaming from index zero.\n> > > > > > > Identify and report a warning if they don't, and correct for any offset.\n> > > > > > \n> > > > > > [1] sequence\n> > > > > > \"The count starts at zero and includes dropped or repeated frames\"\n> > > > > > \n> > > > > > So if the sensor driver has set a value via subdev sensor ops\n> > > > > > g_skip_frames[2], shouldn't it include that count? Or is skipping\n> > > > > > considered different from dropping?\n> > > > > \n> > > > > Well - from my perspective, if the frames are skipped at the beginning\n> > > > > of the stream 'consistently', and didn't even require a buffer - that's\n> > > > > like they never existed, so I would say they are excluded.\n> > > > > \n> > > > > I also presume though that this would mean userspace has no ability to\n> > > > > set controls during those frames (as there are no SoF events?). And if\n> > > > > so - then I think it's right to exclude.\n> > > > > \n> > > > > Of course then to counter-argument myself, could knowing those two frames are\n> > > > > skipped - provide time to set controls in advance? Or do we then assume\n> > > > > that controls set from before starting the stream are always fully\n> > > > > active at the 'first' received real frame?\n> > > > \n> > > > .g_skip_frames() is an internal kernel API, so I think its result should\n> > > > not be visible to userspace when it comes to the sequence number.\n> > > > Otherwise that would effectively say that the sequence number must\n> > > > always start at zero except when it can start at a different value :-)\n> > > > \n> > > > I'd go one step further and argue that .g_frame_skip() should be\n> > > > considered legacy, it's better handled in userspace these days.\n> > > \n> > > How many sensors really need that nowadays? I think some ten years ago it\n> > > was thought to be mainly a property of old, old stuff. Well, surprises\n> > > always tend to happen when it comes to hardware.\n> > \n> > It's hard to come up with an exact number. I'm sure there are newer\n> > sensors whose first images are bad. I also wouldn't be surprised if the\n> > number of bad frames could depend on the sensor configuration in some\n> > cases. That's why I think the kernel shouldn't be involved here.\n> \n> Would you store this information outside the kernel, bound to the sensor\n> somehow?\n\nYes, I'd store it in libcamera, and we actually already do for some\nsensors.\n\n> But I agree the g_skip_frames shouldn't be visible outside the kernel. The\n> number indicates frames that are really broken, i.e. not just e.g.\n> unusually dark (most applies to SoC cameras).\n\nJust to make sure we're on the same page, I'd deprecated\n.g_skip_frames() in the kernel, and provide all frames to userspace,\nunconditionally.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B99BDBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 13 Jul 2022 18:23:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B726B63312;\n\tWed, 13 Jul 2022 20:23:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C1A246274E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 13 Jul 2022 20:23:31 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 25A36305;\n\tWed, 13 Jul 2022 20:23:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657736612;\n\tbh=6DRY5Vl4gFwUoTzGpU51TixtXHlWXZry9bzzTLuiRrI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=gcjAPJzHexJy0mOBzmoTnxd6Lg/V/d3Pp7Il+B6BJjLmUi9mT4Pm1+/o1HJL4aoSV\n\tzfI3sMQ7SAOVCloi2vwIHZ/p5Vz0ONpySbf7+9QGrPVte6ZzPCvtXSLdsf93cRX9TU\n\tuZUprKvWRmXcwzfXLcPM0ujuRwWgMgVy0ctLa/fDDcoGOCL6jq5oqcK+d6OIxXI2yS\n\tqAJDhZ9/SS4Ya0NwdZ/U5M56j40SZuuDafQu6cQA4g4YQr2C41xAY/mb1OsjeJN92x\n\txQfb3vrTzxrxvC8WNEcPGGIWH4hPM5uM9K0S0AZjDn6I+tyqXIWBnykgce6XUgPadF\n\th4VMiOcbZO2jw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1657736611;\n\tbh=6DRY5Vl4gFwUoTzGpU51TixtXHlWXZry9bzzTLuiRrI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=X1OGaeMLsk13JAntcYaKtuIwL7SRPLLe6ti3px5VHRt7TX/URX7UBAPGZuRsvfb5U\n\tPaxYtrYlwaXTbR9UhSVdtjjqgZbxZjn7/PTCAQ+cd+SIzT+yIL+k5DNoSRSgYHT9JO\n\tac+pKf2i2cwJtSaaG+a3EjaaD4krMmgAG1gNn168="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"X1OGaeML\"; dkim-atps=neutral","Date":"Wed, 13 Jul 2022 21:23:00 +0300","To":"Sakari Ailus <sakari.ailus@iki.fi>","Message-ID":"<Ys8NhI0e8xCaPsh3@pendragon.ideasonboard.com>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>\n\t<165512125453.1793421.15923714185258018579@Monstersaurus>\n\t<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>\n\t<Ys8E5lhq8nMeyNIr@valkosipuli.retiisi.eu>\n\t<Ys8GavkRV9QAAwRi@pendragon.ideasonboard.com>\n\t<Ys8Jcwc3gLLLnSuo@valkosipuli.retiisi.eu>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<Ys8Jcwc3gLLLnSuo@valkosipuli.retiisi.eu>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23864,"web_url":"https://patchwork.libcamera.org/comment/23864/","msgid":"<Ys8TwNITHmM2OBh0@valkosipuli.retiisi.eu>","date":"2022-07-13T18:49:36","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":32,"url":"https://patchwork.libcamera.org/api/people/32/","name":"Sakari Ailus","email":"sakari.ailus@iki.fi"},"content":"Hi Laurent,\n\nOn Wed, Jul 13, 2022 at 09:23:00PM +0300, Laurent Pinchart wrote:\n> Hi Sakari,\n> \n> On Wed, Jul 13, 2022 at 09:05:39PM +0300, Sakari Ailus wrote:\n> > On Wed, Jul 13, 2022 at 08:52:42PM +0300, Laurent Pinchart wrote:\n> > > On Wed, Jul 13, 2022 at 08:46:14PM +0300, Sakari Ailus wrote:\n> > > > On Mon, Jun 20, 2022 at 11:59:39AM +0300, Laurent Pinchart wrote:\n> > > > > On Mon, Jun 13, 2022 at 12:54:14PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > > > > > Quoting Dave Stevenson (2022-06-13 11:52:47)\n> > > > > > > On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel wrote:\n> > > > > > > >\n> > > > > > > > V4L2 Video devices should always start streaming from index zero.\n> > > > > > > > Identify and report a warning if they don't, and correct for any offset.\n> > > > > > > \n> > > > > > > [1] sequence\n> > > > > > > \"The count starts at zero and includes dropped or repeated frames\"\n> > > > > > > \n> > > > > > > So if the sensor driver has set a value via subdev sensor ops\n> > > > > > > g_skip_frames[2], shouldn't it include that count? Or is skipping\n> > > > > > > considered different from dropping?\n> > > > > > \n> > > > > > Well - from my perspective, if the frames are skipped at the beginning\n> > > > > > of the stream 'consistently', and didn't even require a buffer - that's\n> > > > > > like they never existed, so I would say they are excluded.\n> > > > > > \n> > > > > > I also presume though that this would mean userspace has no ability to\n> > > > > > set controls during those frames (as there are no SoF events?). And if\n> > > > > > so - then I think it's right to exclude.\n> > > > > > \n> > > > > > Of course then to counter-argument myself, could knowing those two frames are\n> > > > > > skipped - provide time to set controls in advance? Or do we then assume\n> > > > > > that controls set from before starting the stream are always fully\n> > > > > > active at the 'first' received real frame?\n> > > > > \n> > > > > .g_skip_frames() is an internal kernel API, so I think its result should\n> > > > > not be visible to userspace when it comes to the sequence number.\n> > > > > Otherwise that would effectively say that the sequence number must\n> > > > > always start at zero except when it can start at a different value :-)\n> > > > > \n> > > > > I'd go one step further and argue that .g_frame_skip() should be\n> > > > > considered legacy, it's better handled in userspace these days.\n> > > > \n> > > > How many sensors really need that nowadays? I think some ten years ago it\n> > > > was thought to be mainly a property of old, old stuff. Well, surprises\n> > > > always tend to happen when it comes to hardware.\n> > > \n> > > It's hard to come up with an exact number. I'm sure there are newer\n> > > sensors whose first images are bad. I also wouldn't be surprised if the\n> > > number of bad frames could depend on the sensor configuration in some\n> > > cases. That's why I think the kernel shouldn't be involved here.\n> > \n> > Would you store this information outside the kernel, bound to the sensor\n> > somehow?\n> \n> Yes, I'd store it in libcamera, and we actually already do for some\n> sensors.\n> \n> > But I agree the g_skip_frames shouldn't be visible outside the kernel. The\n> > number indicates frames that are really broken, i.e. not just e.g.\n> > unusually dark (most applies to SoC cameras).\n> \n> Just to make sure we're on the same page, I'd deprecated\n> .g_skip_frames() in the kernel, and provide all frames to userspace,\n> unconditionally.\n\nWould you do that also for SoC cameras or just raw ones?\n\nThe former may well be used without libcamera and so keeping this\ninformation in the kernel is somehow justified.\n\nI agree moving this out of the kernel has its merits --- whether a frame is\ngoing to be usable could be up to user space, too. g_skip_frames() was\nalways a very coarse way to tell the frame was broken.","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DD431BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 13 Jul 2022 18:49:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 519C26330E;\n\tWed, 13 Jul 2022 20:49:42 +0200 (CEST)","from meesny.iki.fi (meesny.iki.fi [195.140.195.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C80156274E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 13 Jul 2022 20:49:40 +0200 (CEST)","from hillosipuli.retiisi.eu\n\t(dkwl20tj04snw15cjtflt-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:4493:6f40:fec3:d72a:e447:8113])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256)\n\t(No client certificate requested) (Authenticated sender: sailus)\n\tby meesny.iki.fi (Postfix) with ESMTPSA id 84E6B2029B;\n\tWed, 13 Jul 2022 21:49:38 +0300 (EEST)","from valkosipuli.retiisi.eu (valkosipuli.localdomain [192.168.4.2])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby hillosipuli.retiisi.eu (Postfix) with ESMTPS id B300D634C98;\n\tWed, 13 Jul 2022 21:49:36 +0300 (EEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657738182;\n\tbh=ZKyViEd41+9Pzqodb7Ju72DG8pAvdpNjHS4Lq6EBcmk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=IgDS6W/j8bemsGXXfmrvondm4xlDOwisixZsUvA71AbrLJ1GUCPK1FchV3QV2+RH0\n\tDhCkfgEEvQZVnEuG2n5GaKgFQQ7DEDMBlO2zSWXoWrGjP9TXNvU1D/8tb+Q7PGzOwH\n\t2wJ+wkjWnHI2Nk54K4vh2a9evRJyXlVZZUIG7nK0b/KO8Bub/QJdYpTvTZzEZuWCDm\n\tEgsKZte180uSTDvMXeTEwDaSlCdxFAFUaT33N8RAgQS9Mlzdym+QWA73U7A6P7sban\n\tlXNB9vofajc7Ij55EXg8380vFvI7+tIFtzqgwhCB6XzKazMb7UkUqHj4Fzj35KVD7/\n\tsfF5nCqlGj4eg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny;\n\tt=1657738178;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=aEy1q7eNB8SkTfL4Ak29SZTwA8Ps/IytWMJzE1Rz3p0=;\n\tb=q/YE3RfBqKMTsxrqzjW0wHJUxOLdC/EOshY749YWUoP6hE1uOMi9Dg5kyaZw8XaeyHB7Hw\n\tdn/lX+1hQB3SuJ7vajDp4TgiZxKAponGc/QjsNeGG9CosAPSA8j2tt/LxS45QbtNZZ8v2z\n\tm3FmkEU0zrTOtF1sRUFM4n80gowXdf0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=iki.fi header.i=@iki.fi\n\theader.b=\"q/YE3RfB\"; dkim-atps=neutral","Date":"Wed, 13 Jul 2022 21:49:36 +0300","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<Ys8TwNITHmM2OBh0@valkosipuli.retiisi.eu>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>\n\t<165512125453.1793421.15923714185258018579@Monstersaurus>\n\t<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>\n\t<Ys8E5lhq8nMeyNIr@valkosipuli.retiisi.eu>\n\t<Ys8GavkRV9QAAwRi@pendragon.ideasonboard.com>\n\t<Ys8Jcwc3gLLLnSuo@valkosipuli.retiisi.eu>\n\t<Ys8NhI0e8xCaPsh3@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<Ys8NhI0e8xCaPsh3@pendragon.ideasonboard.com>","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi;\n\ts=meesny; t=1657738178;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=aEy1q7eNB8SkTfL4Ak29SZTwA8Ps/IytWMJzE1Rz3p0=;\n\tb=rb8PE9Ikt2lFZCSko2H/JGUbS5Oofe3lcb0kcyYhVu0oV9o74H6dqMD/ZjtK7wiDMA8BeX\n\t6chlX2sG0ymcxC//aYRSFT1PFyMUNQXesJchdwvWjM+rysjRi9PvPvfIFaa9IBLdzftAD1\n\txjLnX1CNIqeTsJnVLMGXZbEeVzE9Pqc=","ARC-Authentication-Results":"i=1; ORIGINATING;\n\tauth=pass smtp.auth=sailus smtp.mailfrom=sakari.ailus@iki.fi","ARC-Seal":"i=1; s=meesny; d=iki.fi; t=1657738178; a=rsa-sha256; cv=none;\n\tb=YV4bw0At5l0eWNS7CuBs7lfMjEiApyc+D3U+owL4NhoISRIbu5zffHkhXIyZLqsA8rCHoU\n\tbD7iAOh/N/CM0OruWebR6YSYZijlBspaIpN9/0aOO7ibImIaJhJEciQNhHx+z/UqxcD6PP\n\tO/2tlf4be8ZuhLHGzMxfVyxhcVKIQJg=","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Sakari Ailus via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Sakari Ailus <sakari.ailus@iki.fi>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":23870,"web_url":"https://patchwork.libcamera.org/comment/23870/","msgid":"<Ys/4MffH2p85VSDS@pendragon.ideasonboard.com>","date":"2022-07-14T11:04:17","subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sakari,\n\nOn Wed, Jul 13, 2022 at 09:49:36PM +0300, Sakari Ailus wrote:\n> On Wed, Jul 13, 2022 at 09:23:00PM +0300, Laurent Pinchart wrote:\n> > On Wed, Jul 13, 2022 at 09:05:39PM +0300, Sakari Ailus wrote:\n> > > On Wed, Jul 13, 2022 at 08:52:42PM +0300, Laurent Pinchart wrote:\n> > > > On Wed, Jul 13, 2022 at 08:46:14PM +0300, Sakari Ailus wrote:\n> > > > > On Mon, Jun 20, 2022 at 11:59:39AM +0300, Laurent Pinchart wrote:\n> > > > > > On Mon, Jun 13, 2022 at 12:54:14PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > > > > > > Quoting Dave Stevenson (2022-06-13 11:52:47)\n> > > > > > > > On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel wrote:\n> > > > > > > > >\n> > > > > > > > > V4L2 Video devices should always start streaming from index zero.\n> > > > > > > > > Identify and report a warning if they don't, and correct for any offset.\n> > > > > > > > \n> > > > > > > > [1] sequence\n> > > > > > > > \"The count starts at zero and includes dropped or repeated frames\"\n> > > > > > > > \n> > > > > > > > So if the sensor driver has set a value via subdev sensor ops\n> > > > > > > > g_skip_frames[2], shouldn't it include that count? Or is skipping\n> > > > > > > > considered different from dropping?\n> > > > > > > \n> > > > > > > Well - from my perspective, if the frames are skipped at the beginning\n> > > > > > > of the stream 'consistently', and didn't even require a buffer - that's\n> > > > > > > like they never existed, so I would say they are excluded.\n> > > > > > > \n> > > > > > > I also presume though that this would mean userspace has no ability to\n> > > > > > > set controls during those frames (as there are no SoF events?). And if\n> > > > > > > so - then I think it's right to exclude.\n> > > > > > > \n> > > > > > > Of course then to counter-argument myself, could knowing those two frames are\n> > > > > > > skipped - provide time to set controls in advance? Or do we then assume\n> > > > > > > that controls set from before starting the stream are always fully\n> > > > > > > active at the 'first' received real frame?\n> > > > > > \n> > > > > > .g_skip_frames() is an internal kernel API, so I think its result should\n> > > > > > not be visible to userspace when it comes to the sequence number.\n> > > > > > Otherwise that would effectively say that the sequence number must\n> > > > > > always start at zero except when it can start at a different value :-)\n> > > > > > \n> > > > > > I'd go one step further and argue that .g_frame_skip() should be\n> > > > > > considered legacy, it's better handled in userspace these days.\n> > > > > \n> > > > > How many sensors really need that nowadays? I think some ten years ago it\n> > > > > was thought to be mainly a property of old, old stuff. Well, surprises\n> > > > > always tend to happen when it comes to hardware.\n> > > > \n> > > > It's hard to come up with an exact number. I'm sure there are newer\n> > > > sensors whose first images are bad. I also wouldn't be surprised if the\n> > > > number of bad frames could depend on the sensor configuration in some\n> > > > cases. That's why I think the kernel shouldn't be involved here.\n> > > \n> > > Would you store this information outside the kernel, bound to the sensor\n> > > somehow?\n> > \n> > Yes, I'd store it in libcamera, and we actually already do for some\n> > sensors.\n> > \n> > > But I agree the g_skip_frames shouldn't be visible outside the kernel. The\n> > > number indicates frames that are really broken, i.e. not just e.g.\n> > > unusually dark (most applies to SoC cameras).\n> > \n> > Just to make sure we're on the same page, I'd deprecated\n> > .g_skip_frames() in the kernel, and provide all frames to userspace,\n> > unconditionally.\n> \n> Would you do that also for SoC cameras or just raw ones?\n\nDespite the removal of soc_camera, the name sticks :-)\n\nI would do it for raw sensors for sure, and I'm tempted to do it for\nsensors with an on-board ISP as well. This operation is a bit\nill-defined I think, and most receiver drivers don't use it.\n\nOn the sensor side it's implemented by adv7180, ccs, ov13858 and ov5670\nonly, where ccs sets it to 1 for JT8EW9 only, and the three other\ndrivers set it to 2. On the receiver side, it's only used by camss,\nomap3isp and omap4iss (I've left atomisp2 out as it's in staging and in\na bad shape). It's thus fairly useless, as on most systems the \"bad\nframes\" will still be provided to userspace. I'd rather drop this on the\nkernel side instead of pretending we handle it correctly.\n\nAnother reason why it's better done in userspace is that it will allow\nbetter control over exposure time and analog gain. Sensors often have an\ninternal delay of one or two frames for those controls, which means\nper-frame control of the exposure time can often not be guaranteed for\nthe first few frames. If we know the first or first couple of frames are\nbad, we can still start setting exposure times to prime the per-frame\ncontrol mechanism and get it ready by the time the first good frame is\ncaptured.\n\n> The former may well be used without libcamera and so keeping this\n> information in the kernel is somehow justified.\n\nI understand your concern there though, but the mechanism is currently\nbroken, so I think we should remove it, or fix it for real and use it in\nall drivers.\n\n> I agree moving this out of the kernel has its merits --- whether a frame is\n> going to be usable could be up to user space, too. g_skip_frames() was\n> always a very coarse way to tell the frame was broken.\n\nFor what it's worth, the OV5640 seems to produce a garbage frame when it\nstarts, and it doesn't implement .g_skip_frames(). I think it's a lost\nbattle :-)","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1DDE2BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 14 Jul 2022 11:04:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4EAC163312;\n\tThu, 14 Jul 2022 13:04:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A5F4E6330D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Jul 2022 13:04:48 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 05C7B383;\n\tThu, 14 Jul 2022 13:04:47 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657796690;\n\tbh=UmQGdJxgBcJWsEhmpThycvqh9fzc9BI7JGTkWYSBGAg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=12ceu4wOMlF4aXKY28GNPsfa4ZSGQkOFQihYeqNF07MU9hGtf2IKXs3XOUyih3ePl\n\tLHy8cbqq0lmtPHoX51NRoTR+zGmTlmeESiyJbRdrLPOAe50vynUbHj5cF9P2Mp157W\n\tCqjN3i8HCuvWSZl56ZVts1aMFIlu+b0eJBpM09zaKHnnXLN8XNE+U5uuHcvGa1ypU7\n\tIs9xuZIo7h08pkUOiFg/oeuQXqzsDj7Qnm6zxFFZn2KgGqWWaOdggSG4+dA5c/krc2\n\t7jMqMcSdxr3cdjxXfrD9MYQmatKLycnZPXURKcDC2KKbs7qieKjCZAh4RYQlvUgZP2\n\tuAWovYK1gHU7Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1657796688;\n\tbh=UmQGdJxgBcJWsEhmpThycvqh9fzc9BI7JGTkWYSBGAg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=j3X7B2RtA7ApxSCQ39I8AOrrbEq2IbMm+MPCSRHam+KQXLYGlb6LOhWnQZ2m+bliq\n\tQhon4VhrwP3CpQqha0QuSG8yCD/JmS54Me0J0HdOtaAinDNsOdULE3lLaIkYzvHwNu\n\tOG87cwZkry8YDXRWEpCAiaWFkn/1wBy7KPZ8j9VE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"j3X7B2Rt\"; dkim-atps=neutral","Date":"Thu, 14 Jul 2022 14:04:17 +0300","To":"Sakari Ailus <sakari.ailus@iki.fi>","Message-ID":"<Ys/4MffH2p85VSDS@pendragon.ideasonboard.com>","References":"<20220613103528.226610-1-kieran.bingham@ideasonboard.com>\n\t<CAPY8ntBZPa7JPZhMjDkK1kV4QWysCs_mqTSDnnAWxaz67OKG0w@mail.gmail.com>\n\t<165512125453.1793421.15923714185258018579@Monstersaurus>\n\t<YrA2++6uDhkGDI4t@pendragon.ideasonboard.com>\n\t<Ys8E5lhq8nMeyNIr@valkosipuli.retiisi.eu>\n\t<Ys8GavkRV9QAAwRi@pendragon.ideasonboard.com>\n\t<Ys8Jcwc3gLLLnSuo@valkosipuli.retiisi.eu>\n\t<Ys8NhI0e8xCaPsh3@pendragon.ideasonboard.com>\n\t<Ys8TwNITHmM2OBh0@valkosipuli.retiisi.eu>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<Ys8TwNITHmM2OBh0@valkosipuli.retiisi.eu>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify\n\tnon-zero stream starts","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]