[{"id":13324,"web_url":"https://patchwork.libcamera.org/comment/13324/","msgid":"<20201021104258.2vowwthkoucww7j4@uno.localdomain>","date":"2020-10-21T10:42:58","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: pipeline: simple: Set\n\tformat on sink pad during propagation","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Wed, Oct 21, 2020 at 03:24:18AM +0300, Laurent Pinchart wrote:\n> When propagating formats through the media controller pipeline, the\n> simple pipeline handler retrieves the format on sink pads and sets it on\n> source pads. This assumes that subdevs will propagate formats\n> internally, as required by the V4L2 subdev API. However, in some cases,\n> propagation isn't properly handled by the subdev driver.\n>\n\nWorking around a driver issue then.\n\n> When can work around this issue by setting the format on source pads\n> instead of getting it. This will have the effect of trying to propgate\n> the same format through the pipeline, possibly overriding the default\n> format propagated by subdev drivers. It will however not cause the\n> pipeline configuration to be invalid, as subdevs are required to\n> constraint the format set on their sources based on the configuration of\n> the sources, and this requirement is better implemented in kernel driver\n\ns/sources/sinks ? If you are referring to the usual \"formats shall be\npropagated from sinks to sources\" inside a subdev. Or are you\nreferring to cross-link formats ?\n\n> than the format propagation requirement.\n>\n\nI don't see any real drawback in doing this, so:\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 4b6f708e8fee..45ecefc59851 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -386,7 +386,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,\n>\n> \t\tif (source->entity() != sensor_->entity()) {\n> \t\t\tV4L2Subdevice *subdev = pipe->subdev(source->entity());\n> -\t\t\tret = subdev->getFormat(source->index(), format, whence);\n> +\t\t\tret = subdev->setFormat(source->index(), format, whence);\n> \t\t\tif (ret < 0)\n> \t\t\t\treturn ret;\n> \t\t}\n> --\n> Regards,\n>\n> Laurent Pinchart\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 254D8BDB13\n\tfor ;\n\tWed, 21 Oct 2020 10:43:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0224E61D98;\n\tWed, 21 Oct 2020 12:43:02 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8416660353\n\tfor ;\n\tWed, 21 Oct 2020 12:43:00 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 0F16FC0013;\n\tWed, 21 Oct 2020 10:42:59 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Wed, 21 Oct 2020 12:42:58 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20201021104258.2vowwthkoucww7j4@uno.localdomain>","References":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>\n\t<20201021002418.21764-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201021002418.21764-3-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: pipeline: simple: Set\n\tformat on sink pad during propagation","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":13333,"web_url":"https://patchwork.libcamera.org/comment/13333/","msgid":"<20201021113915.GF2092600@oden.dyn.berto.se>","date":"2020-10-21T11:39:15","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: pipeline: simple: Set\n\tformat on sink pad during propagation","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 work.\n\nOn 2020-10-21 03:24:18 +0300, Laurent Pinchart wrote:\n> When propagating formats through the media controller pipeline, the\n> simple pipeline handler retrieves the format on sink pads and sets it on\n> source pads. This assumes that subdevs will propagate formats\n> internally, as required by the V4L2 subdev API. However, in some cases,\n> propagation isn't properly handled by the subdev driver.\n> \n> When can work around this issue by setting the format on source pads\n> instead of getting it. This will have the effect of trying to propgate\n> the same format through the pipeline, possibly overriding the default\n> format propagated by subdev drivers. It will however not cause the\n> pipeline configuration to be invalid, as subdevs are required to\n> constraint the format set on their sources based on the configuration of\n> the sources, and this requirement is better implemented in kernel driver\n> than the format propagation requirement.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Niklas Söderlund \n\n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 4b6f708e8fee..45ecefc59851 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -386,7 +386,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,\n> \n> \t\tif (source->entity() != sensor_->entity()) {\n> \t\t\tV4L2Subdevice *subdev = pipe->subdev(source->entity());\n> -\t\t\tret = subdev->getFormat(source->index(), format, whence);\n> +\t\t\tret = subdev->setFormat(source->index(), format, whence);\n> \t\t\tif (ret < 0)\n> \t\t\t\treturn ret;\n> \t\t}\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 2A06DBDB13\n\tfor ;\n\tWed, 21 Oct 2020 11:39:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC23D61D7F;\n\tWed, 21 Oct 2020 13:39:18 +0200 (CEST)","from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A675760CDD\n\tfor ;\n\tWed, 21 Oct 2020 13:39:17 +0200 (CEST)","by mail-lj1-x244.google.com with SMTP id a5so2209374ljj.11\n\tfor ;\n\tWed, 21 Oct 2020 04:39:17 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\t207sm350723lfi.149.2020.10.21.04.39.16\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 21 Oct 2020 04:39:16 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"X/HOgVtp\"; dkim-atps=neutral","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\tbh=qO/yjGMHF2xT85EJvwcmy4YEdiSQ38kTZDRUAA3YUkU=;\n\tb=X/HOgVtpkF7Jfnesd1tPMotoIdfnnSt/DqIrm1jQwjUntG1eIDAFkQXK08+7KxFwfQ\n\tFcFQ1ZfGtdHe6/82Ug5Wlq1vkPn543j2nrQWmqfDDoo0uJXbeQzYFRDMJw29axvL+Uld\n\tEBNsokvdpHDGxcQvINgiHqaYmJQFR6u1FnYA4dYOCt7zDzPhm2u6CvqfcIsVFO0zdNc9\n\tX79bOhjBUcaWw3Q7RptoshRhKr1BVD8w9LgzIqkG5xr3Fgfu2aAvT74m33bAJ3HMy/fu\n\tXBvQBmOzBfDPBA/udzUpNGjg06jFLJwXR/ADImvyb8ogrLSNOeDawilIKU/K2rLZlz/9\n\tdktA==","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;\n\tbh=qO/yjGMHF2xT85EJvwcmy4YEdiSQ38kTZDRUAA3YUkU=;\n\tb=rOo0e86PTMXdcfrmY0eIRfZYORyystDQ2ahzu2lf7Tvt8K0HVBig8nXDkEcc/Cydu4\n\tDyKuCr1NjMQ7x+tBg80Pqx12lHxm/WhizCL3XL2ZXZkiLepQNlRbVZhLnW4X5zXjgjz3\n\teNp0V//rOdUBrgtGSkQbKNFukjM41fbuW5JlEWg+6UWuiqPvpiYVRNFz4QgUOjVSkge2\n\tUUeIRyVJh9/PsFXPdtfyBVQXGmK8NeFUytKUIIqQn2EO0jbIjcf4xnL2Xh3GeYz/U2kE\n\tyNFu+jk+xRsqO7MOQ/cycO+11jqmUTgmvJNjBjVxOXxWB1Rk7JxsjgcEQWihhQxQeK5G\n\tLodA==","X-Gm-Message-State":"AOAM531gp7cXw2cwJEFMapc7TBVKfIkgQik/m7zQ6rML1apeCXa+ga8p\n\t/e/Ja3ceu2HhoE9PyXP48Rm56w==","X-Google-Smtp-Source":"ABdhPJyoPmxiRt2mAzIsl4+RYm0ukPrcyNJnTNKLMfPUsj3+N53KSWeAA3M1s3+qjpoV8IjS16OL8A==","X-Received":"by 2002:a2e:a310:: with SMTP id l16mr1183305lje.36.1603280357167;\n\tWed, 21 Oct 2020 04:39:17 -0700 (PDT)","Date":"Wed, 21 Oct 2020 13:39:15 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20201021113915.GF2092600@oden.dyn.berto.se>","References":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>\n\t<20201021002418.21764-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201021002418.21764-3-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: pipeline: simple: Set\n\tformat on sink pad during propagation","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":13375,"web_url":"https://patchwork.libcamera.org/comment/13375/","msgid":"<20201021183250.GK3942@pendragon.ideasonboard.com>","date":"2020-10-21T18:32:50","subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: pipeline: simple: Set\n\tformat on sink pad during propagation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Oct 21, 2020 at 12:42:58PM +0200, Jacopo Mondi wrote:\n> On Wed, Oct 21, 2020 at 03:24:18AM +0300, Laurent Pinchart wrote:\n> > When propagating formats through the media controller pipeline, the\n> > simple pipeline handler retrieves the format on sink pads and sets it on\n> > source pads. This assumes that subdevs will propagate formats\n> > internally, as required by the V4L2 subdev API. However, in some cases,\n> > propagation isn't properly handled by the subdev driver.\n> \n> Working around a driver issue then.\n\nYes. In a nutshell, this subdev has multiple inputs and one outputs, and\nselects which output to use based on the enabled links (only one input\nlink may be enabled). This makes propagation of formats more\ncomplicated. I think it should move to the VIDIOC_SUBDEV_S_ROUTING API\n(not merged yet), so the simple pipeline handler will have to be updated.\n\n> > When can work around this issue by setting the format on source pads\n> > instead of getting it. This will have the effect of trying to propgate\n> > the same format through the pipeline, possibly overriding the default\n> > format propagated by subdev drivers. It will however not cause the\n> > pipeline configuration to be invalid, as subdevs are required to\n> > constraint the format set on their sources based on the configuration of\n> > the sources, and this requirement is better implemented in kernel driver\n> \n> s/sources/sinks ? If you are referring to the usual \"formats shall be\n> propagated from sinks to sources\" inside a subdev. Or are you\n> referring to cross-link formats ?\n\nYou're right, I meant sinks. I'll fix it, thanks.\n\n> > than the format propagation requirement.\n> \n> I don't see any real drawback in doing this, so:\n\nThere could be some drawbacks in the future, but I don't see any now\neither. I haven't made up my mind yet though, maybe I'll just fix the\ndriver and drop this patch :-)\n\n> Reviewed-by: Jacopo Mondi \n> \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/pipeline/simple/simple.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 4b6f708e8fee..45ecefc59851 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -386,7 +386,7 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,\n> >\n> > \t\tif (source->entity() != sensor_->entity()) {\n> > \t\t\tV4L2Subdevice *subdev = pipe->subdev(source->entity());\n> > -\t\t\tret = subdev->getFormat(source->index(), format, whence);\n> > +\t\t\tret = subdev->setFormat(source->index(), format, whence);\n> > \t\t\tif (ret < 0)\n> > \t\t\t\treturn ret;\n> > \t\t}","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 5FA93C3D3C\n\tfor ;\n\tWed, 21 Oct 2020 18:33:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EDA1560986;\n\tWed, 21 Oct 2020 20:33:37 +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 4F7B160361\n\tfor ;\n\tWed, 21 Oct 2020 20:33:36 +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 B01F7BB5;\n\tWed, 21 Oct 2020 20:33:35 +0200 (CEST)"],"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=\"rSAseas+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603305215;\n\tbh=883lokbRqGmGEOTkYJTGfSGCPrKEIYM+mFY0m8as4Vs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rSAseas+qY9LwCuSg3WbIvHOXLzj8MCE9zaBqFX2RxSHUkrqoP+emoHdLTMPLJxgj\n\tf5Y1ZNhhwJY4z4mR4U2308AzbfXViAnqzkDRYH4TW6tkZ3aV/R9k33LXou253Xoeej\n\tPZAxryayxmU9fdR+U6IL/Ra7rotf1lhE61gn/3b8=","Date":"Wed, 21 Oct 2020 21:32:50 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20201021183250.GK3942@pendragon.ideasonboard.com>","References":"<20201021002418.21764-1-laurent.pinchart@ideasonboard.com>\n\t<20201021002418.21764-3-laurent.pinchart@ideasonboard.com>\n\t<20201021104258.2vowwthkoucww7j4@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201021104258.2vowwthkoucww7j4@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 2/2] libcamera: pipeline: simple: Set\n\tformat on sink pad during propagation","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\" "}}]