[{"id":1896,"web_url":"https://patchwork.libcamera.org/comment/1896/","msgid":"<20190613173010.pqvzcxwvcgtqcmzj@uno.localdomain>","date":"2019-06-13T17:30:10","subject":"Re: [libcamera-devel] [PATCH v2 16/16] libcamera: pipeline:\n\tuvcvideo: Add format information and validation","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Jun 12, 2019 at 02:43:59AM +0200, Niklas Söderlund wrote:\n> Extend the uvcvideo pipeline with format information and validation. The\n> format information is gathered by enumerating the v4l2 device. This\n> enumeration approach is valid for UVC as it has a static and simple\n> media graph.\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/libcamera/pipeline/uvcvideo.cpp | 41 ++++++++++++++++++++++-------\n>  1 file changed, 31 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 45260f34c8f5daba..7ca9d0eefe8223e8 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -5,6 +5,8 @@\n>   * uvcvideo.cpp - Pipeline handler for uvcvideo devices\n>   */\n>\n> +#include <algorithm>\n> +\n>  #include <libcamera/camera.h>\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n> @@ -95,18 +97,33 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n>  \t}\n>\n>  \tStreamConfiguration &cfg = config_[0];\n> -\n> -\t/* \\todo: Validate the configuration against the device capabilities. */\n> +\tconst StreamFormats &formats = cfg.formats();\n>  \tconst unsigned int pixelFormat = cfg.pixelFormat;\n>  \tconst Size size = cfg.size;\n>\n> -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> -\tcfg.size = { 640, 480 };\n> +\tconst std::vector<unsigned int> pixelFormats = formats.pixelformats();\n> +\tauto iter = std::find(pixelFormats.begin(), pixelFormats.end(), pixelFormat);\n> +\tif (iter == pixelFormats.end()) {\n> +\t\tcfg.pixelFormat = pixelFormats.front();\n> +\t\tLOG(UVC, Debug)\n> +\t\t\t<< \"Adjusting pixel format from \" << pixelFormat\n> +\t\t\t<< \" to \" << cfg.pixelFormat;\n> +\t\tstatus = Adjusted;\n> +\t}\n> +\n> +\tconst std::vector<Size> &formatSizes = formats.sizes(cfg.pixelFormat);\n> +\tcfg.size = formatSizes.front();\n> +\tfor (const Size &formatsSize : formatSizes) {\n> +\t\tif (formatsSize > size)\n> +\t\t\tbreak;\n> +\n> +\t\tcfg.size = formatsSize;\n> +\t}\n\nEven this selects not the closest size, but the smallest closest one, I\nthink it's fine for now.\n>\n> -\tif (cfg.pixelFormat != pixelFormat || cfg.size != size) {\n> +\tif (cfg.size != size) {\n>  \t\tLOG(UVC, Debug)\n> -\t\t\t<< \"Adjusting configuration from \" << cfg.toString()\n> -\t\t\t<< \" to \" << cfg.size.toString() << \"-YUYV\";\n> +\t\t\t<< \"Adjusting size from \" << size.toString()\n> +\t\t\t<< \" to \" << cfg.size.toString();\n>  \t\tstatus = Adjusted;\n>  \t}\n>\n> @@ -123,14 +140,18 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n>  CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,\n>  \tconst StreamRoles &roles)\n>  {\n> +\tUVCCameraData *data = cameraData(camera);\n>  \tCameraConfiguration *config = new UVCCameraConfiguration();\n>\n>  \tif (roles.empty())\n>  \t\treturn config;\n>\n> -\tStreamConfiguration cfg{};\n> -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> -\tcfg.size = { 640, 480 };\n> +\tImageFormats v4l2formats = data->video_->formats();\n> +\tStreamFormats formats(v4l2formats.data());\n\nDo we want to be able to construct a StreamFormat from an ImageFormat?\n\n\nMinors apart\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n> +\tStreamConfiguration cfg(formats);\n> +\n> +\tcfg.pixelFormat = formats.pixelformats().front();\n> +\tcfg.size = formats.sizes(cfg.pixelFormat).back();\n>  \tcfg.bufferCount = 4;\n>\n>  \tconfig->addConfiguration(cfg);\n> --\n> 2.21.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":"<jacopo@jmondi.org>","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9DD9F63946\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 13 Jun 2019 19:28:57 +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 2CEED240009;\n\tThu, 13 Jun 2019 17:28:56 +0000 (UTC)"],"Date":"Thu, 13 Jun 2019 19:30:10 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190613173010.pqvzcxwvcgtqcmzj@uno.localdomain>","References":"<20190612004359.15772-1-niklas.soderlund@ragnatech.se>\n\t<20190612004359.15772-17-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"qc3pcbbmkyrbb4eh\"","Content-Disposition":"inline","In-Reply-To":"<20190612004359.15772-17-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 16/16] libcamera: pipeline:\n\tuvcvideo: Add format information and validation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 13 Jun 2019 17:28:57 -0000"}},{"id":1898,"web_url":"https://patchwork.libcamera.org/comment/1898/","msgid":"<20190616131723.GC20468@bigcity.dyn.berto.se>","date":"2019-06-16T13:17:23","subject":"Re: [libcamera-devel] [PATCH v2 16/16] libcamera: pipeline:\n\tuvcvideo: Add format information and validation","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your feedback.\n\nOn 2019-06-13 19:30:10 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Wed, Jun 12, 2019 at 02:43:59AM +0200, Niklas Söderlund wrote:\n> > Extend the uvcvideo pipeline with format information and validation. The\n> > format information is gathered by enumerating the v4l2 device. This\n> > enumeration approach is valid for UVC as it has a static and simple\n> > media graph.\n> >\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/libcamera/pipeline/uvcvideo.cpp | 41 ++++++++++++++++++++++-------\n> >  1 file changed, 31 insertions(+), 10 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index 45260f34c8f5daba..7ca9d0eefe8223e8 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -5,6 +5,8 @@\n> >   * uvcvideo.cpp - Pipeline handler for uvcvideo devices\n> >   */\n> >\n> > +#include <algorithm>\n> > +\n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/request.h>\n> >  #include <libcamera/stream.h>\n> > @@ -95,18 +97,33 @@ CameraConfiguration::Status UVCCameraConfiguration::validate()\n> >  \t}\n> >\n> >  \tStreamConfiguration &cfg = config_[0];\n> > -\n> > -\t/* \\todo: Validate the configuration against the device capabilities. */\n> > +\tconst StreamFormats &formats = cfg.formats();\n> >  \tconst unsigned int pixelFormat = cfg.pixelFormat;\n> >  \tconst Size size = cfg.size;\n> >\n> > -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> > -\tcfg.size = { 640, 480 };\n> > +\tconst std::vector<unsigned int> pixelFormats = formats.pixelformats();\n> > +\tauto iter = std::find(pixelFormats.begin(), pixelFormats.end(), pixelFormat);\n> > +\tif (iter == pixelFormats.end()) {\n> > +\t\tcfg.pixelFormat = pixelFormats.front();\n> > +\t\tLOG(UVC, Debug)\n> > +\t\t\t<< \"Adjusting pixel format from \" << pixelFormat\n> > +\t\t\t<< \" to \" << cfg.pixelFormat;\n> > +\t\tstatus = Adjusted;\n> > +\t}\n> > +\n> > +\tconst std::vector<Size> &formatSizes = formats.sizes(cfg.pixelFormat);\n> > +\tcfg.size = formatSizes.front();\n> > +\tfor (const Size &formatsSize : formatSizes) {\n> > +\t\tif (formatsSize > size)\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcfg.size = formatsSize;\n> > +\t}\n> \n> Even this selects not the closest size, but the smallest closest one, I\n> think it's fine for now.\n\nYes, closes smallest one is selected, I think that is fine for now.\n\n> >\n> > -\tif (cfg.pixelFormat != pixelFormat || cfg.size != size) {\n> > +\tif (cfg.size != size) {\n> >  \t\tLOG(UVC, Debug)\n> > -\t\t\t<< \"Adjusting configuration from \" << cfg.toString()\n> > -\t\t\t<< \" to \" << cfg.size.toString() << \"-YUYV\";\n> > +\t\t\t<< \"Adjusting size from \" << size.toString()\n> > +\t\t\t<< \" to \" << cfg.size.toString();\n> >  \t\tstatus = Adjusted;\n> >  \t}\n> >\n> > @@ -123,14 +140,18 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n> >  CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,\n> >  \tconst StreamRoles &roles)\n> >  {\n> > +\tUVCCameraData *data = cameraData(camera);\n> >  \tCameraConfiguration *config = new UVCCameraConfiguration();\n> >\n> >  \tif (roles.empty())\n> >  \t\treturn config;\n> >\n> > -\tStreamConfiguration cfg{};\n> > -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> > -\tcfg.size = { 640, 480 };\n> > +\tImageFormats v4l2formats = data->video_->formats();\n> > +\tStreamFormats formats(v4l2formats.data());\n> \n> Do we want to be able to construct a StreamFormat from an ImageFormat?\n\nI don't think so. I think it's a special case for UVC where the \nStreamFormats will be built directly from the ImageFormats. If I'm wrong \nand this will turn into a common thing I would agree that this would be \nneat.\n\n> \n> \n> Minors apart\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> Thanks\n>   j\n> \n> > +\tStreamConfiguration cfg(formats);\n> > +\n> > +\tcfg.pixelFormat = formats.pixelformats().front();\n> > +\tcfg.size = formats.sizes(cfg.pixelFormat).back();\n> >  \tcfg.bufferCount = 4;\n> >\n> >  \tconfig->addConfiguration(cfg);\n> > --\n> > 2.21.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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 256586473B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 16 Jun 2019 15:17:26 +0200 (CEST)","by mail-lf1-x142.google.com with SMTP id b11so4628134lfa.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 16 Jun 2019 06:17:26 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tz26sm1607462ljz.64.2019.06.16.06.17.23\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tSun, 16 Jun 2019 06:17:24 -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=esPDzcyAaNLh8MJ100roVB7+S5tw8COucqDscxBZibE=;\n\tb=U2Q4VSPmr0YBkh38mACW/6qR91BrxocVGZijxnbDUTUKMUETg4cUy1X7XX03YjDf1I\n\t7HWjBcBr2PBbkrHKENQ01T54ZrO/lmFnm7UdWYDKXK02eTkUYT/8kyGxC1mHKNZxdSSX\n\tIdg5Byy5zXOwQncMmNwi8tlKwyFuRPDu3ERvMkI5NFLv/yfw7I1c/9+nypTa3kpsWx9c\n\tk/FJryXZA0GXzl1Z6+WMXemd6SQ4Co1dB/3YQd81o4FzMXnTKkuIIsZgPsXqAggaV9KI\n\tqoI9f2xw2uJCdLqmxzI9+0cr9RK5/YPlSNm83UXUsC1GzVIhj0tknGAjilCk/PIYELbh\n\trZTQ==","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=esPDzcyAaNLh8MJ100roVB7+S5tw8COucqDscxBZibE=;\n\tb=qoLRizK4402+GzGR7Ex5wBlLDPDyaSDbxI5I2aNRCA8Fns8wmZyGxi5dBkz5pzNxZM\n\tpJGoPaMBhBmuCnBGNYkDAiKvsZ3AQIxnabePSwugWzkPnWJIW89Xq7z0ZyHKpP6TYQRW\n\t/HvbYd7zPQjDQEmJDSu8BniItYpJc4yDoNLb0SfRuuMtmncHWq1JCvKG7l6jEOcLGGYn\n\ty8Y/y6+GN+tQv0rIphV6g3MY7kQschSSWgaiKAV22JlF4pMWi2wmJL/J3N69fEuoxByz\n\tU6uqbTU3f6cwqErdjL9CwNRkIBC+fdquyRjDuWaAxY47bjPMaWEYOfcYavLNHVeV8ajp\n\t9KqQ==","X-Gm-Message-State":"APjAAAXBd9MCYiRxgbyoTTCBP/hM5STobnarm1+7nifekliHY6ZAukvW\n\tEve8vlk4VrSCU6hZoDXZgtq/l0Za0cc=","X-Google-Smtp-Source":"APXvYqxMp2iU0LU4cOJTRj/bbCrV7kYT8yT+DBN3lpyQcxuBl8khMc8eu+0bsNlhU4W4pGSMSe6cSg==","X-Received":"by 2002:ac2:4d17:: with SMTP id\n\tr23mr46574920lfi.130.1560691045487; \n\tSun, 16 Jun 2019 06:17:25 -0700 (PDT)","Date":"Sun, 16 Jun 2019 15:17:23 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190616131723.GC20468@bigcity.dyn.berto.se>","References":"<20190612004359.15772-1-niklas.soderlund@ragnatech.se>\n\t<20190612004359.15772-17-niklas.soderlund@ragnatech.se>\n\t<20190613173010.pqvzcxwvcgtqcmzj@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190613173010.pqvzcxwvcgtqcmzj@uno.localdomain>","User-Agent":"Mutt/1.12.0 (2019-05-25)","Subject":"Re: [libcamera-devel] [PATCH v2 16/16] libcamera: pipeline:\n\tuvcvideo: Add format information and validation","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sun, 16 Jun 2019 13:17:26 -0000"}}]