[{"id":2319,"web_url":"https://patchwork.libcamera.org/comment/2319/","msgid":"<20190805173727.GE13149@pendragon.ideasonboard.com>","date":"2019-08-05T17:37:27","subject":"Re: [libcamera-devel] [PATCH 3/4] tests: v4l2_videodevice:\n\tPropagate media bus format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Aug 05, 2019 at 05:51:32PM +0200, Niklas Söderlund wrote:\n> Most of the video device tests are based on vimc and Linux commit\n> 85ab1aa1fac17bcd (\"media: vimc: deb: fix default sink bayer format\")\n> changes the default media bus format for the debayer subdevices. This\n> leads to a -EPIPE error when trying to use the raw capture video device\n> nodes.\n> \n> Fix this by propagating the media bus format used on the debayer\n> subdevcie to the sensor. This allows the same code to function on\n\ns/subdevcie/subdevice/\n\nAnd as for patch 2/4, I would propagate it the other way around, from\nthe sensor to the debayering subdev.\n\n> kernels previous to the change and after it.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> .../v4l2_videodevice_test.cpp | 22 +++++++++++++++++++\n> test/v4l2_videodevice/v4l2_videodevice_test.h | 7 +++++-\n> 2 files changed, 28 insertions(+), 1 deletion(-)\n> \n> diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> index b26d06ad45197c8f..b1d4eaf6e9e4a4b1 100644\n> --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> @@ -62,6 +62,26 @@ int V4L2VideoDeviceTest::init()\n> \tif (ret)\n> \t\treturn TestFail;\n> \n> +\tif (driver_ == \"vimc\") {\n\nA bit hack-ish, but for tests I think we can live with this.\n\n> +\t\tV4L2SubdeviceFormat subformat = {};\n> +\t\tsubformat.size.width = 640;\n> +\t\tsubformat.size.height = 480;\n> +\n> +\t\tsensor_ = new CameraSensor(media_->getEntityByName(\"Sensor A\"));\n> +\t\tif (sensor_->init())\n> +\t\t\treturn TestFail;\n> +\n> +\t\tdebayer_ = new V4L2Subdevice(media_->getEntityByName(\"Debayer A\"));\n> +\t\tif (debayer_->open())\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (debayer_->setFormat(0, &subformat))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (sensor_->setFormat(&subformat))\n> +\t\t\treturn TestFail;\n> +\t}\n> +\n> \tif (capture_->open())\n> \t\treturn TestFail;\n> \n> @@ -83,5 +103,7 @@ void V4L2VideoDeviceTest::cleanup()\n> \tcapture_->releaseBuffers();\n> \tcapture_->close();\n> \n> +\tdelete debayer_;\n> +\tdelete sensor_;\n> \tdelete capture_;\n> };\n> diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> index 3321b5a4f98fdb1d..34dd231c6d9d108d 100644\n> --- a/test/v4l2_videodevice/v4l2_videodevice_test.h\n> +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> @@ -13,8 +13,10 @@\n> \n> #include \"test.h\"\n> \n> +#include \"camera_sensor.h\"\n> #include \"device_enumerator.h\"\n> #include \"media_device.h\"\n> +#include \"v4l2_subdevice.h\"\n> #include \"v4l2_videodevice.h\"\n> \n> using namespace libcamera;\n> @@ -23,7 +25,8 @@ class V4L2VideoDeviceTest : public Test\n> {\n> public:\n> \tV4L2VideoDeviceTest(const char *driver, const char *entity)\n> -\t\t: driver_(driver), entity_(entity), capture_(nullptr)\n> +\t\t: driver_(driver), entity_(entity), sensor_(nullptr),\n> +\t\t debayer_(nullptr), capture_(nullptr)\n> \t{\n> \t}\n> \n> @@ -35,6 +38,8 @@ protected:\n> \tstd::string entity_;\n> \tstd::unique_ptr enumerator_;\n> \tstd::shared_ptr media_;\n> +\tCameraSensor *sensor_;\n> +\tV4L2Subdevice *debayer_;\n\nDo you need to store these internally, can't they be local variables in\nV4L2VideoDeviceTest::init() ?\n\n> \tV4L2VideoDevice *capture_;\n> \tBufferPool pool_;\n> };","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 403F260E33\n\tfor ;\n\tMon, 5 Aug 2019 19:37:30 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AE8E42F9;\n\tMon, 5 Aug 2019 19:37:29 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1565026649;\n\tbh=oLsRWYO0/N3Ao2/Gp6aeFSJTuDANc7DPnl5nJAOQV/E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WMW4nn5zYkPmCVc00w9IYsJXBqE3OKFj9axsHXXN3Qbu/h6CBmAqTXUz/4p8b0rPk\n\t/yMOAdRtyvgVb18PdW5vklEXSgg09sl+k8IbhAwk7OjDCg21hpb7UZD+BvBJN5oEA2\n\tTKuE767JUMP+U78fweiJBjdA9ncNK4UKMaS7mifk=","Date":"Mon, 5 Aug 2019 20:37:27 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190805173727.GE13149@pendragon.ideasonboard.com>","References":"<20190805155133.11335-1-niklas.soderlund@ragnatech.se>\n\t<20190805155133.11335-4-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190805155133.11335-4-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/4] tests: v4l2_videodevice:\n\tPropagate media bus format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 05 Aug 2019 17:37:30 -0000"}},{"id":2334,"web_url":"https://patchwork.libcamera.org/comment/2334/","msgid":"<20190807204712.GI3186@bigcity.dyn.berto.se>","date":"2019-08-07T20:47:12","subject":"Re: [libcamera-devel] [PATCH 3/4] tests: v4l2_videodevice:\n\tPropagate media bus format","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-08-05 20:37:27 +0300, Laurent Pinchart wrote:\n> > @@ -35,6 +38,8 @@ protected:\n> > \tstd::string entity_;\n> > \tstd::unique_ptr enumerator_;\n> > \tstd::shared_ptr media_;\n> > +\tCameraSensor *sensor_;\n> > +\tV4L2Subdevice *debayer_;\n> \n> Do you need to store these internally, can't they be local variables in\n> V4L2VideoDeviceTest::init() ?\n\nIf I store them locally in V4L2VideoDeviceTest::init() I would need to \nhandle all error cases and delete them if I don't want to leak memory \nfor valgrind, so I went for this option. I also fear they might be \nneeded outside init() soon enough once this problem is sorted out \nupstream. I have keep this for v2.","headers":{"Return-Path":"","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2BE936161A\n\tfor ;\n\tWed, 7 Aug 2019 22:47:14 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id a30so1992019lfk.12\n\tfor ;\n\tWed, 07 Aug 2019 13:47:14 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tp13sm18599553ljc.39.2019.08.07.13.47.12\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tWed, 07 Aug 2019 13:47:12 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=BVP7kV36x0mFYS9FRJUicrLds+bLYzunp2g2jJwDTR0=;\n\tb=dRqvgoVf3bH7AjjuYkbsaCWWTKij2pksJ8cY8vUYwZ+3hyc9Pkum9loMPst5TGZTTJ\n\t7IwfkCxtFD1C/HKN9/mka5H73W7uHzVcmNa1rXRe+d+nMqKD4/lyR2MwoR4b2ySgeQFr\n\tj9eWmJ1vGPWKo4LoSVg9cwsse5d7rb89qeLbkQFPMzGz1WtraOCArQL0FedKUT1PV4dl\n\tusG/sPoAmpMi/9nU5TI4gc6cFp/5MZnkDO694uHouKV3Ep8zxyaUMLdtUGJiPnkxO5lq\n\t0iSgvJpmNWb93YFQwsxjTrPcuU87jE/f4Vl1FR2gmiTUqmm3H6YdkIES0M+IoTYO+27P\n\toMxg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=BVP7kV36x0mFYS9FRJUicrLds+bLYzunp2g2jJwDTR0=;\n\tb=sXEKJ0O3GPtsHfLZ1eT3jiAL43/s85ukMVeuzYp/tUs/skxWnsb4LhS0WFQVXTy8dj\n\tTBrofhj+2Je+OigIH1gLgwSe8b8IiLAlpuxeeWJugJO1cP29nIO5rRBez2Lj4PUmz/QQ\n\t3dfg4V9rPI5U2aMoMSgTIXRwkoFiJFRxo4UwbiXS6WCc75xL2QuCr7nYCn/nR7zcl/Rp\n\tfcpg1/bPps63Ckqx2eZKTdToYBZqd5YGplK0ZUNQ69shXJ/EzJRowUO0W99hvr41BVYU\n\tLrYUZ/PV9AVujiUx3yzNKlLTXnJPA6sXEbJJWpU5xkF2YKdEI5mRfJhLZJ/84Db+z3tl\n\tFYOw==","X-Gm-Message-State":"APjAAAVJWWYYfCnOjpSrbLuvIUKMdHJY0sycqHkI67Ob6eyao/JlO2xm\n\tkO8mu+YmZkMv7X6c5ynO1lnmPevudno=","X-Google-Smtp-Source":"APXvYqyI94nabKg8J3lcoYbr6l8iufiO4CWb0a1gAl+3AJzq7CqHMjpJtzlCe1rEphyuMF6pafRIdA==","X-Received":"by 2002:a19:5503:: with SMTP id n3mr6307565lfe.168.1565210833635;\n\tWed, 07 Aug 2019 13:47:13 -0700 (PDT)","Date":"Wed, 7 Aug 2019 22:47:12 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190807204712.GI3186@bigcity.dyn.berto.se>","References":"<20190805155133.11335-1-niklas.soderlund@ragnatech.se>\n\t<20190805155133.11335-4-niklas.soderlund@ragnatech.se>\n\t<20190805173727.GE13149@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190805173727.GE13149@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 3/4] tests: v4l2_videodevice:\n\tPropagate media bus format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 07 Aug 2019 20:47:14 -0000"}},{"id":2338,"web_url":"https://patchwork.libcamera.org/comment/2338/","msgid":"<20190807212150.GE20497@pendragon.ideasonboard.com>","date":"2019-08-07T21:21:50","subject":"Re: [libcamera-devel] [PATCH 3/4] tests: v4l2_videodevice:\n\tPropagate media bus format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Aug 07, 2019 at 10:47:12PM +0200, Niklas Söderlund wrote:\n> Hi Laurent,\n> \n> Thanks for your feedback.\n> \n> On 2019-08-05 20:37:27 +0300, Laurent Pinchart wrote:\n> > > @@ -35,6 +38,8 @@ protected:\n> > > \tstd::string entity_;\n> > > \tstd::unique_ptr enumerator_;\n> > > \tstd::shared_ptr media_;\n> > > +\tCameraSensor *sensor_;\n> > > +\tV4L2Subdevice *debayer_;\n> > \n> > Do you need to store these internally, can't they be local variables in\n> > V4L2VideoDeviceTest::init() ?\n> \n> If I store them locally in V4L2VideoDeviceTest::init() I would need to \n> handle all error cases and delete them if I don't want to leak memory \n> for valgrind, so I went for this option. I also fear they might be \n> needed outside init() soon enough once this problem is sorted out \n> upstream. I have keep this for v2.\n\nOK, please disregard my comment on v2 then.","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C2C76161A\n\tfor ;\n\tWed, 7 Aug 2019 23:21:52 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3194A67;\n\tWed, 7 Aug 2019 23:21:52 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1565212912;\n\tbh=UFwvQvt7HzNyJxHV2uGHg6Roq2oQJ6jvuazSLk2Kp/o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IqS3gJVMEMFIm3agMWBq1q78yAdlp5qkrmmMFsuMqANdChlAX5v9m9N/oL9WkdOox\n\tr2P8+2xeYt/9FubOK/J/Sgch5JeRDtoKqrCLRRXo0hU+3P4tj9lrEJHIW4Mfn80HC/\n\t2Kebro/IZ4gOXaLBoLnMEtRan5AW+BhLUaMbvMsI=","Date":"Thu, 8 Aug 2019 00:21:50 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190807212150.GE20497@pendragon.ideasonboard.com>","References":"<20190805155133.11335-1-niklas.soderlund@ragnatech.se>\n\t<20190805155133.11335-4-niklas.soderlund@ragnatech.se>\n\t<20190805173727.GE13149@pendragon.ideasonboard.com>\n\t<20190807204712.GI3186@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190807204712.GI3186@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/4] tests: v4l2_videodevice:\n\tPropagate media bus format","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 07 Aug 2019 21:21:52 -0000"}}]