[{"id":14984,"web_url":"https://patchwork.libcamera.org/comment/14984/","msgid":"","date":"2021-02-04T20:07:25","subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: camera_sensor: Add\n\tframe length limits to CameraSensorInfo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Fri, Jan 29, 2021 at 11:16:13AM +0000, Naushir Patuck wrote:\n> Sensor frame length is made up of active and inactive (blanking) lines.\n> The minimum and maximum frame length values may be used by pipeline\n> handlers to limit frame durations based on the sensor mode capabilities.\n> \n> Store the minimum and maximum allowable frame length values (in lines)\n> in the CameraSensorInfo structure. These values are computed in\n> CameraSensor::sensorInfo() by querying the sensor subdevice\n> V4L2_CID_VBLANK control limits. This in turn means that V4L2_CID_VBLANK\n> is now a mandatory subdevice control.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Jacopo Mondi \n> ---\n> include/libcamera/internal/camera_sensor.h | 3 ++\n> src/libcamera/camera_sensor.cpp | 43 ++++++++++++++++++++--\n> test/ipa/ipa_wrappers_test.cpp | 2 +\n> 3 files changed, 44 insertions(+), 4 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index fed36bf26e47..5d8c9b1a3121 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -33,6 +33,9 @@ struct CameraSensorInfo {\n> \n> \tuint64_t pixelRate;\n> \tuint32_t lineLength;\n> +\n> +\tuint32_t minFrameLength;\n> +\tuint32_t maxFrameLength;\n> };\n> \n> class CameraSensor : protected Loggable\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index ab315bdc468c..f60d0cc9c6fa 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -113,6 +113,36 @@ LOG_DEFINE_CATEGORY(CameraSensor)\n> * The total line length in pixel clock periods, including blanking.\n> */\n> \n> +/**\n> + * \\var CameraSensorInfo::minFrameLength\n> + * \\brief The minimum allowable frame length in units of lines\n> + *\n> + * The sensor frame length comprises of active output lines and blanking lines\n\ns/comprises of/comprises/ ? If you can let me know which form is\ncorrect, I can fix this when applying.\n\nReviewed-by: Laurent Pinchart \n\n> + * in a frame. The minimum frame length value dictates the minimum allowable\n> + * frame duration of the sensor mode.\n> + *\n> + * To obtain the minimum frame duration:\n> + *\n> + * \\verbatim\n> +\tframeDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> + \\endverbatim\n> + */\n> +\n> +/**\n> + * \\var CameraSensorInfo::maxFrameLength\n> + * \\brief The maximum allowable frame length in units of lines\n> + *\n> + * The sensor frame length comprises of active output lines and blanking lines\n> + * in a frame. The maximum frame length value dictates the maximum allowable\n> + * frame duration of the sensor mode.\n> + *\n> + * To obtain the maximum frame duration:\n> + *\n> + * \\verbatim\n> +\tframeDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> + \\endverbatim\n> + */\n> +\n> /**\n> * \\class CameraSensor\n> * \\brief A camera sensor based on V4L2 subdevices\n> @@ -699,12 +729,13 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \tinfo->outputSize = format.size;\n> \n> \t/*\n> -\t * Retrieve the pixel rate and the line length through V4L2 controls.\n> -\t * Support for the V4L2_CID_PIXEL_RATE and V4L2_CID_HBLANK controls is\n> -\t * mandatory.\n> +\t * Retrieve the pixel rate, line length and minimum/maximum frame\n> +\t * duration through V4L2 controls. Support for the V4L2_CID_PIXEL_RATE,\n> +\t * V4L2_CID_HBLANK and V4L2_CID_VBLANK controls is mandatory.\n> \t */\n> \tControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE,\n> -\t\t\t\t\t\t V4L2_CID_HBLANK });\n> +\t\t\t\t\t\t V4L2_CID_HBLANK,\n> +\t\t\t\t\t\t V4L2_CID_VBLANK });\n> \tif (ctrls.empty()) {\n> \t\tLOG(CameraSensor, Error)\n> \t\t\t<< \"Failed to retrieve camera info controls\";\n> @@ -715,6 +746,10 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> \tinfo->lineLength = info->outputSize.width + hblank;\n> \tinfo->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get();\n> \n> +\tconst ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK);\n> +\tinfo->minFrameLength = info->outputSize.height + vblank.min().get();\n> +\tinfo->maxFrameLength = info->outputSize.height + vblank.max().get();\n> +\n> \treturn 0;\n> }\n> \n> diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\n> index 47533d105d03..eb6d783e8489 100644\n> --- a/test/ipa/ipa_wrappers_test.cpp\n> +++ b/test/ipa/ipa_wrappers_test.cpp\n> @@ -313,6 +313,8 @@ protected:\n> \t\t\t.outputSize = { 2560, 1940 },\n> \t\t\t.pixelRate = 96000000,\n> \t\t\t.lineLength = 2918,\n> +\t\t\t.minFrameLength = 1940,\n> +\t\t\t.maxFrameLength = 2880\n> \t\t};\n> \t\tstd::map config{\n> \t\t\t{ 1, { V4L2_PIX_FMT_YUYV, { 1024, 768 } } },","headers":{"Return-Path":"","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 17D97BD160\n\tfor ;\n\tThu, 4 Feb 2021 20:07:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E0936145D;\n\tThu, 4 Feb 2021 21:07:49 +0100 (CET)","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 85FD261430\n\tfor ;\n\tThu, 4 Feb 2021 21:07:48 +0100 (CET)","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 0AACC45D;\n\tThu, 4 Feb 2021 21:07:47 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JqYq8pCg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612469268;\n\tbh=7EnaTukGp7/4nO399ufmUl8TflVMFDKthZ0CEAEDUgk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JqYq8pCgOx4jVizMJpnijzzS44CoIokXpP5E/yXxbalOIDj2uWpHaeA9qcZ6NT/TS\n\tknDKeDAgO0u+3obPg968dCIR/rD+i8ooP/sTZ+bsAZqufC6ZezripYM5ff24oYmG2m\n\tx2ZIA9UO0NY4jxpV72RdQjYDLQunNKfdIUBL/y6E=","Date":"Thu, 4 Feb 2021 22:07:25 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210129111616.1047483-1-naush@raspberrypi.com>\n\t<20210129111616.1047483-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210129111616.1047483-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: camera_sensor: Add\n\tframe length limits to CameraSensorInfo","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15008,"web_url":"https://patchwork.libcamera.org/comment/15008/","msgid":"","date":"2021-02-04T22:15:58","subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: camera_sensor: Add\n\tframe length limits to CameraSensorInfo","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your review feedback.\n\nOn Thu, 4 Feb 2021 at 20:07, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Fri, Jan 29, 2021 at 11:16:13AM +0000, Naushir Patuck wrote:\n> > Sensor frame length is made up of active and inactive (blanking) lines.\n> > The minimum and maximum frame length values may be used by pipeline\n> > handlers to limit frame durations based on the sensor mode capabilities.\n> >\n> > Store the minimum and maximum allowable frame length values (in lines)\n> > in the CameraSensorInfo structure. These values are computed in\n> > CameraSensor::sensorInfo() by querying the sensor subdevice\n> > V4L2_CID_VBLANK control limits. This in turn means that V4L2_CID_VBLANK\n> > is now a mandatory subdevice control.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: Jacopo Mondi \n> > ---\n> > include/libcamera/internal/camera_sensor.h | 3 ++\n> > src/libcamera/camera_sensor.cpp | 43 ++++++++++++++++++++--\n> > test/ipa/ipa_wrappers_test.cpp | 2 +\n> > 3 files changed, 44 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/camera_sensor.h\n> b/include/libcamera/internal/camera_sensor.h\n> > index fed36bf26e47..5d8c9b1a3121 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -33,6 +33,9 @@ struct CameraSensorInfo {\n> >\n> > uint64_t pixelRate;\n> > uint32_t lineLength;\n> > +\n> > + uint32_t minFrameLength;\n> > + uint32_t maxFrameLength;\n> > };\n> >\n> > class CameraSensor : protected Loggable\n> > diff --git a/src/libcamera/camera_sensor.cpp\n> b/src/libcamera/camera_sensor.cpp\n> > index ab315bdc468c..f60d0cc9c6fa 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -113,6 +113,36 @@ LOG_DEFINE_CATEGORY(CameraSensor)\n> > * The total line length in pixel clock periods, including blanking.\n> > */\n> >\n> > +/**\n> > + * \\var CameraSensorInfo::minFrameLength\n> > + * \\brief The minimum allowable frame length in units of lines\n> > + *\n> > + * The sensor frame length comprises of active output lines and\n> blanking lines\n>\n> s/comprises of/comprises/ ? If you can let me know which form is\n> correct, I can fix this when applying.\n>\n\nI would have said \"comprises of\" sounds more appropriate here. If it's ok\nwith you, I'd leave as-is.\n\nRegards,\nNaush\n\n\n> Reviewed-by: Laurent Pinchart \n>\n> > + * in a frame. The minimum frame length value dictates the minimum\n> allowable\n> > + * frame duration of the sensor mode.\n> > + *\n> > + * To obtain the minimum frame duration:\n> > + *\n> > + * \\verbatim\n> > + frameDuration(s) = minFrameLength(lines) * lineLength(pixels) /\n> pixelRate(pixels per second)\n> > + \\endverbatim\n> > + */\n> > +\n> > +/**\n> > + * \\var CameraSensorInfo::maxFrameLength\n> > + * \\brief The maximum allowable frame length in units of lines\n> > + *\n> > + * The sensor frame length comprises of active output lines and\n> blanking lines\n> > + * in a frame. The maximum frame length value dictates the maximum\n> allowable\n> > + * frame duration of the sensor mode.\n> > + *\n> > + * To obtain the maximum frame duration:\n> > + *\n> > + * \\verbatim\n> > + frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) /\n> pixelRate(pixels per second)\n> > + \\endverbatim\n> > + */\n> > +\n> > /**\n> > * \\class CameraSensor\n> > * \\brief A camera sensor based on V4L2 subdevices\n> > @@ -699,12 +729,13 @@ int CameraSensor::sensorInfo(CameraSensorInfo\n> *info) const\n> > info->outputSize = format.size;\n> >\n> > /*\n> > - * Retrieve the pixel rate and the line length through V4L2\n> controls.\n> > - * Support for the V4L2_CID_PIXEL_RATE and V4L2_CID_HBLANK\n> controls is\n> > - * mandatory.\n> > + * Retrieve the pixel rate, line length and minimum/maximum frame\n> > + * duration through V4L2 controls. Support for the\n> V4L2_CID_PIXEL_RATE,\n> > + * V4L2_CID_HBLANK and V4L2_CID_VBLANK controls is mandatory.\n> > */\n> > ControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE,\n> > - V4L2_CID_HBLANK });\n> > + V4L2_CID_HBLANK,\n> > + V4L2_CID_VBLANK });\n> > if (ctrls.empty()) {\n> > LOG(CameraSensor, Error)\n> > << \"Failed to retrieve camera info controls\";\n> > @@ -715,6 +746,10 @@ int CameraSensor::sensorInfo(CameraSensorInfo\n> *info) const\n> > info->lineLength = info->outputSize.width + hblank;\n> > info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get();\n> >\n> > + const ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK);\n> > + info->minFrameLength = info->outputSize.height +\n> vblank.min().get();\n> > + info->maxFrameLength = info->outputSize.height +\n> vblank.max().get();\n> > +\n> > return 0;\n> > }\n> >\n> > diff --git a/test/ipa/ipa_wrappers_test.cpp\n> b/test/ipa/ipa_wrappers_test.cpp\n> > index 47533d105d03..eb6d783e8489 100644\n> > --- a/test/ipa/ipa_wrappers_test.cpp\n> > +++ b/test/ipa/ipa_wrappers_test.cpp\n> > @@ -313,6 +313,8 @@ protected:\n> > .outputSize = { 2560, 1940 },\n> > .pixelRate = 96000000,\n> > .lineLength = 2918,\n> > + .minFrameLength = 1940,\n> > + .maxFrameLength = 2880\n> > };\n> > std::map config{\n> > { 1, { V4L2_PIX_FMT_YUYV, { 1024, 768 } } },\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","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 B1546BD160\n\tfor ;\n\tThu, 4 Feb 2021 22:16:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40F1C61487;\n\tThu, 4 Feb 2021 23:16:17 +0100 (CET)","from mail-lf1-x134.google.com (mail-lf1-x134.google.com\n\t[IPv6:2a00:1450:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8664861430\n\tfor ;\n\tThu, 4 Feb 2021 23:16:15 +0100 (CET)","by mail-lf1-x134.google.com with SMTP id i187so6951692lfd.4\n\tfor ;\n\tThu, 04 Feb 2021 14:16:15 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"CIm7itSA\"; dkim-atps=neutral","DKIM-Signature":"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=Il68BthvdrK9U9NdMx1KCjw81tpoDS60WTDuslNljbs=;\n\tb=CIm7itSAowT46SLwms9/I6dDmkuA7WEQKDECNkJHrpvXcd7Bd9jWW10/Ck9kemABCz\n\t3QDKMNI13p4FJ9EWsc9akvmKv9d3X4a2V56RhjSM+gH9Qx+tAXihHMU/xx4Cs2eCRgr4\n\tr+gWaftautn2r6TRYDsdPYryJrRgaK8p8VNJEBk6qFsuUYBSRhNBIVXX+qGQitYjG+m7\n\tWf4YUggkHKWUj8RmzzLGYEocJUJld1wVARW9dnm4pMY/bH+R+oe+hzr3w0dpdj3HrOnd\n\tW6qQtZ6nZ5+ak2uWkuvWTJ6Ks30+Tq41361d4rP1vOea/mJ6c8iQr+XPMlJkxb1DpSeY\n\tY5RQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Il68BthvdrK9U9NdMx1KCjw81tpoDS60WTDuslNljbs=;\n\tb=LpdayDzIsAeFCAXqX+bwfg2xuXA8WrbgT6y/+fPEqYWs1NWN5Kb2fEC8xIx1UHuHFd\n\tOEkBYFOBJrc/dWfaBD8+WxZ1iCMwnHhZvMQrrgWPYSlQf+BUKw/5QOEEM9ZMpsPmiBRG\n\tGWrevZaT7RkCVNc0QNSO0uJC+ZAbPf0LYzlwtxtBV5eXMOwwbgFjc3yOCWvDuHp5Enf+\n\t5JijWS51pXNX4hBDAKDcfm0myKemwOh92YZBze4pgQpbg3rrmVJ8Q7Ohndt2OM8pO7GQ\n\t+ZVo6CWF3lchG62e85xqifl89npfzaTtG2m+PwOIk7Pi/wMIEi8cIdSzzG7wAOkE/jwl\n\tguMQ==","X-Gm-Message-State":"AOAM533LkhyxbenD4CRJR2QGcZMGmww/DqmGRdMNZ8HqMaAfipiUY8VP\n\tSxJqGx/VBmJexHVYgjvc2n4tWgMVbekT07y79dT1FQ==","X-Google-Smtp-Source":"ABdhPJw9Cs8Nke8eTus6pxBaUBsMvWCkJXZSzvUF/gm9E66f1rmL22S094YHu6FfF8pO10D0+54lu9G9Oqe827D4zDc=","X-Received":"by 2002:a19:8805:: with SMTP id k5mr757140lfd.567.1612476974886; \n\tThu, 04 Feb 2021 14:16:14 -0800 (PST)","MIME-Version":"1.0","References":"<20210129111616.1047483-1-naush@raspberrypi.com>\n\t<20210129111616.1047483-3-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 4 Feb 2021 22:15:58 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: camera_sensor: Add\n\tframe length limits to CameraSensorInfo","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"multipart/mixed;\n\tboundary=\"===============5079097342239234905==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15016,"web_url":"https://patchwork.libcamera.org/comment/15016/","msgid":"","date":"2021-02-04T23:23:08","subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: camera_sensor: Add\n\tframe length limits to CameraSensorInfo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Thu, Feb 04, 2021 at 10:15:58PM +0000, Naushir Patuck wrote:\n> On Thu, 4 Feb 2021 at 20:07, Laurent Pinchart wrote:\n> > On Fri, Jan 29, 2021 at 11:16:13AM +0000, Naushir Patuck wrote:\n> > > Sensor frame length is made up of active and inactive (blanking) lines.\n> > > The minimum and maximum frame length values may be used by pipeline\n> > > handlers to limit frame durations based on the sensor mode capabilities.\n> > >\n> > > Store the minimum and maximum allowable frame length values (in lines)\n> > > in the CameraSensorInfo structure. These values are computed in\n> > > CameraSensor::sensorInfo() by querying the sensor subdevice\n> > > V4L2_CID_VBLANK control limits. This in turn means that V4L2_CID_VBLANK\n> > > is now a mandatory subdevice control.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > Reviewed-by: Jacopo Mondi \n> > > ---\n> > > include/libcamera/internal/camera_sensor.h | 3 ++\n> > > src/libcamera/camera_sensor.cpp | 43 ++++++++++++++++++++--\n> > > test/ipa/ipa_wrappers_test.cpp | 2 +\n> > > 3 files changed, 44 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> > > index fed36bf26e47..5d8c9b1a3121 100644\n> > > --- a/include/libcamera/internal/camera_sensor.h\n> > > +++ b/include/libcamera/internal/camera_sensor.h\n> > > @@ -33,6 +33,9 @@ struct CameraSensorInfo {\n> > >\n> > > uint64_t pixelRate;\n> > > uint32_t lineLength;\n> > > +\n> > > + uint32_t minFrameLength;\n> > > + uint32_t maxFrameLength;\n> > > };\n> > >\n> > > class CameraSensor : protected Loggable\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index ab315bdc468c..f60d0cc9c6fa 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -113,6 +113,36 @@ LOG_DEFINE_CATEGORY(CameraSensor)\n> > > * The total line length in pixel clock periods, including blanking.\n> > > */\n> > >\n> > > +/**\n> > > + * \\var CameraSensorInfo::minFrameLength\n> > > + * \\brief The minimum allowable frame length in units of lines\n> > > + *\n> > > + * The sensor frame length comprises of active output lines and blanking lines\n> >\n> > s/comprises of/comprises/ ? If you can let me know which form is\n> > correct, I can fix this when applying.\n> \n> I would have said \"comprises of\" sounds more appropriate here. If it's ok\n> with you, I'd leave as-is.\n\nI was looking at https://en.wiktionary.org/wiki/comprise#Usage_notes\nwhen trying to figure out why it sounded weird to me, but I'll trust you\nmore than a webpage and will keep it as-is :-)\n\n> > Reviewed-by: Laurent Pinchart \n> >\n> > > + * in a frame. The minimum frame length value dictates the minimum allowable\n> > > + * frame duration of the sensor mode.\n> > > + *\n> > > + * To obtain the minimum frame duration:\n> > > + *\n> > > + * \\verbatim\n> > > + frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> > > + \\endverbatim\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var CameraSensorInfo::maxFrameLength\n> > > + * \\brief The maximum allowable frame length in units of lines\n> > > + *\n> > > + * The sensor frame length comprises of active output lines and blanking lines\n> > > + * in a frame. The maximum frame length value dictates the maximum allowable\n> > > + * frame duration of the sensor mode.\n> > > + *\n> > > + * To obtain the maximum frame duration:\n> > > + *\n> > > + * \\verbatim\n> > > + frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)\n> > > + \\endverbatim\n> > > + */\n> > > +\n> > > /**\n> > > * \\class CameraSensor\n> > > * \\brief A camera sensor based on V4L2 subdevices\n> > > @@ -699,12 +729,13 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > > info->outputSize = format.size;\n> > >\n> > > /*\n> > > - * Retrieve the pixel rate and the line length through V4L2 controls.\n> > > - * Support for the V4L2_CID_PIXEL_RATE and V4L2_CID_HBLANK controls is\n> > > - * mandatory.\n> > > + * Retrieve the pixel rate, line length and minimum/maximum frame\n> > > + * duration through V4L2 controls. Support for the V4L2_CID_PIXEL_RATE,\n> > > + * V4L2_CID_HBLANK and V4L2_CID_VBLANK controls is mandatory.\n> > > */\n> > > ControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE,\n> > > - V4L2_CID_HBLANK });\n> > > + V4L2_CID_HBLANK,\n> > > + V4L2_CID_VBLANK });\n> > > if (ctrls.empty()) {\n> > > LOG(CameraSensor, Error)\n> > > << \"Failed to retrieve camera info controls\";\n> > > @@ -715,6 +746,10 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const\n> > > info->lineLength = info->outputSize.width + hblank;\n> > > info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get();\n> > >\n> > > + const ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK);\n> > > + info->minFrameLength = info->outputSize.height + vblank.min().get();\n> > > + info->maxFrameLength = info->outputSize.height + vblank.max().get();\n> > > +\n> > > return 0;\n> > > }\n> > >\n> > > diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\n> > > index 47533d105d03..eb6d783e8489 100644\n> > > --- a/test/ipa/ipa_wrappers_test.cpp\n> > > +++ b/test/ipa/ipa_wrappers_test.cpp\n> > > @@ -313,6 +313,8 @@ protected:\n> > > .outputSize = { 2560, 1940 },\n> > > .pixelRate = 96000000,\n> > > .lineLength = 2918,\n> > > + .minFrameLength = 1940,\n> > > + .maxFrameLength = 2880\n> > > };\n> > > std::map config{\n> > > { 1, { V4L2_PIX_FMT_YUYV, { 1024, 768 } } },","headers":{"Return-Path":"","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 27E3EBD160\n\tfor ;\n\tThu, 4 Feb 2021 23:23:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 85D0A61497;\n\tFri, 5 Feb 2021 00:23:32 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7EB47613D4\n\tfor ;\n\tFri, 5 Feb 2021 00:23:31 +0100 (CET)","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 EF68A2C1;\n\tFri, 5 Feb 2021 00:23:30 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WguR+sS9\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1612481011;\n\tbh=q/dNgMH6fPe3Rhnk8df/jnYcdzB+QxzRdHGNm2GB5LU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WguR+sS9p3ATdqNi1m+tjpcYTe+r/311x1g9AglxRzFufe6AUD/wbv+KfgBZdRFL2\n\tb0pR6lTXSLoSYHB27buMxxwbLpzaqUPoLHpjevdO6QrLYRozXMOF20Nk+0XfJZhqyq\n\tV3/RbHgdqoBtF5Ma5FfDSk2JQwER7q8NRqAr7mkc=","Date":"Fri, 5 Feb 2021 01:23:08 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210129111616.1047483-1-naush@raspberrypi.com>\n\t<20210129111616.1047483-3-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 2/5] libcamera: camera_sensor: Add\n\tframe length limits to CameraSensorInfo","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]