[{"id":4786,"web_url":"https://patchwork.libcamera.org/comment/4786/","msgid":"<20200512002837.GH5830@pendragon.ideasonboard.com>","date":"2020-05-12T00:28:37","subject":"Re: [libcamera-devel] [PATCH 0/2] raspberrypi: FPS control","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 patches.\n\nOn Mon, May 11, 2020 at 11:01:48AM +0100, Naushir Patuck wrote:\n> Hi,\n> \n> The following two patches add variable framerate control to a sensor\n> subdevice. This will allow shutter speeds below 33ms during\n> viewfinder (for example).\n> \n> I've currently hardcoded the maximum limit at 30.0fps, but we ought to\n> have a discussion on how a user can request fps limits - perhaps\n> though a Request, or stream configuration parameters?\n\nThat's an API we need indeed. I think the frame rate would need to be\nmodifiable at runtime, so that would require a control. Should we go for\na frame duration, using the same units as the exposure time ?\n\n> Naushir Patuck (2):\n> libcamera: raspberrypi: Add control of sensor vblanking\n> libcamera: raspberrypi: Limit the maximum framerate to 30.0fps\n> \n> src/ipa/raspberrypi/cam_helper.hpp | 1 +\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 23 +++++++++++++++++++\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 23 +++++++++++++++++++\n> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 23 +++++++++++++++++++\n> src/ipa/raspberrypi/raspberrypi.cpp | 14 ++++++++++-\n> .../pipeline/raspberrypi/raspberrypi.cpp | 2 ++\n> 6 files changed, 85 insertions(+), 1 deletion(-)","headers":{"Return-Path":"","Received":["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 3E743603DD\n\tfor ;\n\tTue, 12 May 2020 02:28:44 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A2F9533E;\n\tTue, 12 May 2020 02:28:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"u9e4ImxR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1589243323;\n\tbh=N9joLPra7o5PXHKcnPi965kCcbNQAHfiA0rlhLJncAw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=u9e4ImxR+u4GJpNWtztxGGiN7kST0qUyLtseXBdhh4Ptcs6zTKhvfOYmfDWcVmLn9\n\tEnARY2m/TYThNX9noinfsGEacqXW6HazT2evy3kUzoC4b/EBbrOvJvXZs+tu+BvHSg\n\tjdN33W8Whl8yKYBKntfmiY1V+i6sjXuj7meShx/8=","Date":"Tue, 12 May 2020 03:28:37 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200512002837.GH5830@pendragon.ideasonboard.com>","References":"<20200511100150.5205-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200511100150.5205-1-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 0/2] raspberrypi: FPS control","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","X-List-Received-Date":"Tue, 12 May 2020 00:28:44 -0000"}},{"id":4797,"web_url":"https://patchwork.libcamera.org/comment/4797/","msgid":"","date":"2020-05-12T10:22:39","subject":"Re: [libcamera-devel] [PATCH 0/2] raspberrypi: FPS control","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Tue, 12 May 2020 at 01:28, Laurent Pinchart\n wrote:\n>\n> Hi Naush,\n>\n> Thank you for the patches.\n>\n> On Mon, May 11, 2020 at 11:01:48AM +0100, Naushir Patuck wrote:\n> > Hi,\n> >\n> > The following two patches add variable framerate control to a sensor\n> > subdevice. This will allow shutter speeds below 33ms during\n> > viewfinder (for example).\n> >\n> > I've currently hardcoded the maximum limit at 30.0fps, but we ought to\n> > have a discussion on how a user can request fps limits - perhaps\n> > though a Request, or stream configuration parameters?\n>\n> That's an API we need indeed. I think the frame rate would need to be\n> modifiable at runtime, so that would require a control. Should we go for\n> a frame duration, using the same units as the exposure time ?\n\nI can prepare an array control to provide min/max framerate and send\nout a v2 patch with this. Normally I prefer representing framerate as\n1/t, e.g. 30 fps. No objections to using the same form (t) and units\nas exposure - it's your call.\n\nThanks,\nNaush\n\n\n>\n> > Naushir Patuck (2):\n> > libcamera: raspberrypi: Add control of sensor vblanking\n> > libcamera: raspberrypi: Limit the maximum framerate to 30.0fps\n> >\n> > src/ipa/raspberrypi/cam_helper.hpp | 1 +\n> > src/ipa/raspberrypi/cam_helper_imx219.cpp | 23 +++++++++++++++++++\n> > src/ipa/raspberrypi/cam_helper_imx477.cpp | 23 +++++++++++++++++++\n> > src/ipa/raspberrypi/cam_helper_ov5647.cpp | 23 +++++++++++++++++++\n> > src/ipa/raspberrypi/raspberrypi.cpp | 14 ++++++++++-\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 2 ++\n> > 6 files changed, 85 insertions(+), 1 deletion(-)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 663A9603E0\n\tfor ;\n\tTue, 12 May 2020 12:22:57 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id d21so5295681ljg.9\n\tfor ;\n\tTue, 12 May 2020 03:22:57 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"IjwUVEVn\"; 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=FebKZ75Aqs8E6gI1U8gEdnp305YIKc6TezXuLz68Lu0=;\n\tb=IjwUVEVnUJ2Epjzcg9GJdsYV0WK4cy4GFItKhQZNZ6ACN8/AdSYY4aXn2hnYYLoprl\n\t3CJYLm4gEEDlxLQ5jKXCw1bIIzCA2/pzZonmjuEPnbWyFvGvZh4tq2TgO/tItr2BjaOy\n\tX/FX2DQ+x6MS4ubU4FbfozTxFWG6QhOvMJimHgFjXzPtKLAROoreBSSmUpVT3kvEg+5k\n\tFMnRH/shG0o76bNBkTJOJ8LZiLau/ManJPE91yN3pfERSfuq94OCqIGIMnhgRzjNXXPb\n\tE9mnMfvJsOtbFqE70KO//E0DbkbYfekSYT0PhzdNuL0FCaUbivSOChtkUrc/c50iol/X\n\tSFhQ==","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=FebKZ75Aqs8E6gI1U8gEdnp305YIKc6TezXuLz68Lu0=;\n\tb=FVxJ6yVgTR5/AwtnxK3NFxJwjHhzAJYoe6Hgc5weraoDKwlU7jZdlVHg9/wfgScIua\n\tdQeKRuQhpEy9qaBGunSsVg0p9m5m+I08a/J+kzzu+5vEcIwTLGwVn+z7+2tUyHZFDM6X\n\tmU7xaVTctUkhk6KaPtC+5Sbq7hVzZOQ3VQEKgb8a96xFuEoi8t+2GZppm3JybsYXD0BD\n\t5HH4BVycZLXkcn89HNO53InQOUEndOqLSDHQV/D+fpc6Ha7E/9GRVPjsi1XZrJKCLrN1\n\tmxMk6d6NnwBP5RBBT+wjRVfUFjmQlqjZfHfU77d/cUtoxKtkiEZmnKylkFXaAL/dwcCP\n\tgm4w==","X-Gm-Message-State":"AOAM5332LMDQcfbqmOqiJBKnLfvo52pBcgAIVAxK1KYEHTo+MEdVd1W/\n\t53m/V2bLXFFuGMFOhIfUwZ0g7xy7qGBOMHRgxjXOFDTBQgA=","X-Google-Smtp-Source":"ABdhPJxon/qeFFgN8crIGFL155rqyjeMkXIoj26FlTNnW4rCDCNkOjeXmpadBj9sfOxWa970vLFijWhtY0pkTrTAegk=","X-Received":"by 2002:a2e:9d83:: with SMTP id c3mr13401530ljj.90.1589278976540;\n\tTue, 12 May 2020 03:22:56 -0700 (PDT)","MIME-Version":"1.0","References":"<20200511100150.5205-1-naush@raspberrypi.com>\n\t<20200512002837.GH5830@pendragon.ideasonboard.com>","In-Reply-To":"<20200512002837.GH5830@pendragon.ideasonboard.com>","From":"Naushir Patuck ","Date":"Tue, 12 May 2020 11:22:39 +0100","Message-ID":"","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 0/2] raspberrypi: FPS control","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","X-List-Received-Date":"Tue, 12 May 2020 10:22:58 -0000"}},{"id":4808,"web_url":"https://patchwork.libcamera.org/comment/4808/","msgid":"<20200513003811.GA30336@pendragon.ideasonboard.com>","date":"2020-05-13T00:38:11","subject":"Re: [libcamera-devel] [PATCH 0/2] raspberrypi: FPS control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Tue, May 12, 2020 at 11:22:39AM +0100, Naushir Patuck wrote:\n> On Tue, 12 May 2020 at 01:28, Laurent Pinchart wrote:\n> > On Mon, May 11, 2020 at 11:01:48AM +0100, Naushir Patuck wrote:\n> > > Hi,\n> > >\n> > > The following two patches add variable framerate control to a sensor\n> > > subdevice. This will allow shutter speeds below 33ms during\n> > > viewfinder (for example).\n> > >\n> > > I've currently hardcoded the maximum limit at 30.0fps, but we ought to\n> > > have a discussion on how a user can request fps limits - perhaps\n> > > though a Request, or stream configuration parameters?\n> >\n> > That's an API we need indeed. I think the frame rate would need to be\n> > modifiable at runtime, so that would require a control. Should we go for\n> > a frame duration, using the same units as the exposure time ?\n> \n> I can prepare an array control to provide min/max framerate and send\n> out a v2 patch with this. Normally I prefer representing framerate as\n> 1/t, e.g. 30 fps. No objections to using the same form (t) and units\n> as exposure - it's your call.\n\nI've used both, and they have pros and cons. FPS is generally more\nuser-friendly, but I've also called for deprecating the V4L2 FPS-based\nsubdev API for camera sensor in the most common cases, and frame\nduration is controlled there through the total h/v sizes. One issue with\nfps is that we'll have to deal with rounding issues, with different\npipeline handlers ad IPAs doing so slightly differently. I've had to\ndeal with this in the uvcvideo driver, as UVC is based on frame\ndurations (as multiples of 100ns) and V4L2 on frame rates. 30fps is thus\nexpressed as 333333, which I initially translated to 333333/10000000.\nThat's not user-friendly, so I ended up with the following code (which\nI'm shamefully proud of):\n\n/* Simplify a fraction using a simple continued fraction decomposition. The\n * idea here is to convert fractions such as 333333/10000000 to 1/30 using\n * 32 bit arithmetic only. The algorithm is not perfect and relies upon two\n * arbitrary parameters to remove non-significative terms from the simple\n * continued fraction decomposition. Using 8 and 333 for n_terms and threshold\n * respectively seems to give nice results.\n */\nvoid uvc_simplify_fraction(u32 *numerator, u32 *denominator,\n\t\tunsigned int n_terms, unsigned int threshold)\n{\n\tu32 *an;\n\tu32 x, y, r;\n\tunsigned int i, n;\n\n\tan = kmalloc_array(n_terms, sizeof(*an), GFP_KERNEL);\n\tif (an == NULL)\n\t\treturn;\n\n\t/* Convert the fraction to a simple continued fraction. See\n\t * http://mathforum.org/dr.math/faq/faq.fractions.html\n\t * Stop if the current term is bigger than or equal to the given\n\t * threshold.\n\t */\n\tx = *numerator;\n\ty = *denominator;\n\n\tfor (n = 0; n < n_terms && y != 0; ++n) {\n\t\tan[n] = x / y;\n\t\tif (an[n] >= threshold) {\n\t\t\tif (n < 2)\n\t\t\t\tn++;\n\t\t\tbreak;\n\t\t}\n\n\t\tr = x - an[n] * y;\n\t\tx = y;\n\t\ty = r;\n\t}\n\n\t/* Expand the simple continued fraction back to an integer fraction. */\n\tx = 0;\n\ty = 1;\n\n\tfor (i = n; i > 0; --i) {\n\t\tr = y;\n\t\ty = an[i-1] * y + x;\n\t\tx = r;\n\t}\n\n\t*numerator = y;\n\t*denominator = x;\n\tkfree(an);\n}\n\nThe function is called with 8 and 333 for n_terms and threshold, and\nmanages to simplify 333333/10000000 to 1/30 while not simplifying\n1001/60000 to 1/60 (and while conducting a few tests out of curiosity a\nfew minutes ago, I realized the code simplifies 1001/30000 to 367/10999\n:-S).\n\nWe could also argue that a frame duration doesn't allow expressing NTSC\nframe rates exactly, but given that libcamera deals primarily with\ncamera sensors, all this makes me favour frame durations instead of\nframe rates. Of course, I may be overlooking something, if you think we\ncould come up with nice helpers to ensure that the conversions are\nhandled nicely and consistently by pipeline handlers and IPAs, I would\ncertainly be happy to discuss that.\n\nFrame durations would have the nice benefit of being aligned with the\nAndroid camera HAL API, but that's not a requirement by itself.\n\n> > > Naushir Patuck (2):\n> > > libcamera: raspberrypi: Add control of sensor vblanking\n> > > libcamera: raspberrypi: Limit the maximum framerate to 30.0fps\n> > >\n> > > src/ipa/raspberrypi/cam_helper.hpp | 1 +\n> > > src/ipa/raspberrypi/cam_helper_imx219.cpp | 23 +++++++++++++++++++\n> > > src/ipa/raspberrypi/cam_helper_imx477.cpp | 23 +++++++++++++++++++\n> > > src/ipa/raspberrypi/cam_helper_ov5647.cpp | 23 +++++++++++++++++++\n> > > src/ipa/raspberrypi/raspberrypi.cpp | 14 ++++++++++-\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 2 ++\n> > > 6 files changed, 85 insertions(+), 1 deletion(-)","headers":{"Return-Path":"","Received":["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 06E13603DC\n\tfor ;\n\tWed, 13 May 2020 02:38:18 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6574D51F;\n\tWed, 13 May 2020 02:38:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MqNVxQLx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1589330297;\n\tbh=mAzgOMauGfmWV+d4Z97r141Nr1mjasluPQpsuukchtI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MqNVxQLxTyThLM3TmC0J5rvZ/1J0Z646I1sBJYl/8Fk/aa95cabi7kZNdNDpkIFYw\n\tuRz/pvt6YXhji+5YOX4Ql06dZx+dtDR8Nk+Fh5xiQaT18BDWf3ob1ANCgnEnn7rJQT\n\ttCEX7yeFqaTXCLE68t768S3D52IBZo+E6dNjMkTY=","Date":"Wed, 13 May 2020 03:38:11 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200513003811.GA30336@pendragon.ideasonboard.com>","References":"<20200511100150.5205-1-naush@raspberrypi.com>\n\t<20200512002837.GH5830@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 0/2] raspberrypi: FPS control","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","X-List-Received-Date":"Wed, 13 May 2020 00:38:18 -0000"}}]