[{"id":10900,"web_url":"https://patchwork.libcamera.org/comment/10900/","msgid":"<20200627101715.l56dtukycuhmckqh@uno.localdomain>","date":"2020-06-27T10:17:15","subject":"Re: [libcamera-devel] [PATCH 00/13] libcamera: ipu3: Refactoring of\n\tImgU","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\n I only have very small comments on the series, nice clean up !\n\nFor the whole series\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\nOn Sat, Jun 27, 2020 at 05:00:30AM +0200, Niklas Söderlund wrote:\n> Hi,\n>\n> This series do some cleanup of the IPU3 pipeline. It removes dead or\n> almost dead code, replacing it with something more coherent to what is\n> done elsewhere in the pipeline. It breaks out the ImgU to a separate cpp\n> and h file.\n>\n> It reworks a bit how the streams are identified and handled inside the\n> pipeline. When the pipeline was first developed only the output and\n> viewfinder streams where available, now we have a RAW stream in addition\n> to this. Some of the abstraction around streams and which hardware\n> resource they are backed by made sens in the earlier context but not so\n> much once RAW is added to the mix. The biggest difference in this rework\n> is that the ImgUDevice gets explicit functions to configure each of its\n> sink instead of having a generic one which depends which pointer is\n> passed to it. This makes reading code where RAW and ImgU stremas are\n> mixed much nicer IMHO.\n>\n> Lastly some assumptions that buffers must be allocated at video nodes\n> that are not involved in the capture session are being challenged. This\n> was true a year ago but not any more it seems. Chancing this simplifies\n> the driver enormously and saves on memory that otherwise would be\n> wasted. I have really tried to force the end result to failed by\n> resetting the hardware between each test so that no video node\n> configuration from a previous sessions saves the day.\n>\n> I have for both sensors reset the hardware and then tested the following\n> capture combinations successfully. After the first capture session for\n> each one in the list that was done after a power cycle all the other\n> captures where tried in a semi random order and always succeeded.\n>\n> cam -c 1 -s role=viewfinder -C\n> cam -c 1 -s role=still -C\n> cam -c 1 -s role=still -s role=viewfinder -C\n> cam -c 1 -s role=still -s role=viewfinder -s role=stillraw -C\n> cam -c 1 -s role=stillraw -C\n>\n> I have also played a bit with qcam and it works as expected and the\n> resulting normal och raw frames are generated and contains the expected\n> data.\n>\n> Niklas Söderlund (13):\n> libcamera: ipu3: Remove unused name_ filed from IPU3Stream\n> libcamera: ipu3: Import instead of allocate statistic buffers\n> libcamera: ipu3: Always import buffers for ImgU sinks\n> libcamera: ipu3: Remove usage of IPU3CameraData from ImgUDevice\n> libcamera: ipu3: imgu: Move the ImgUDevice class to separate files\n> libcamera: ipu3: imgu: Do not cache index\n> libcamera: ipu3: imgu: Mark things that are internal as private\n> libcamera: ipu3: imgu: Use specific functions to configure each sink\n> libcamera: ipu3: Do not duplicate data in IPU3Stream\n> libcamera: ipu3: Remove the active flag from IPU3Stream\n> libcamera: ipu3: Remove IPU3Stream\n> libcamera: ipu3: Align how CIO2 and ImgU are stored in CameraData\n> libcamera: ipu3: imgu: Remove ImgUOutput\n>\n> src/libcamera/pipeline/ipu3/imgu.cpp | 351 ++++++++++++++\n> src/libcamera/pipeline/ipu3/imgu.h | 87 ++++\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 581 ++++--------------------\n> src/libcamera/pipeline/ipu3/meson.build | 1 +\n> 4 files changed, 515 insertions(+), 505 deletions(-)\n> create mode 100644 src/libcamera/pipeline/ipu3/imgu.cpp\n> create mode 100644 src/libcamera/pipeline/ipu3/imgu.h\n>\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 2B8C1C2E69\n\tfor ;\n\tSat, 27 Jun 2020 10:13:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04BED609DD;\n\tSat, 27 Jun 2020 12:13:45 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5961A609C2\n\tfor ;\n\tSat, 27 Jun 2020 12:13:44 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id DA537240003;\n\tSat, 27 Jun 2020 10:13:43 +0000 (UTC)"],"Date":"Sat, 27 Jun 2020 12:17:15 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200627101715.l56dtukycuhmckqh@uno.localdomain>","References":"<20200627030043.2088585-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200627030043.2088585-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 00/13] libcamera: ipu3: Refactoring of\n\tImgU","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]