[{"id":90,"web_url":"https://patchwork.libcamera.org/comment/90/","msgid":"<20181228033508.GT19796@bigcity.dyn.berto.se>","date":"2018-12-28T03:35:08","subject":"Re: [libcamera-devel] [PATCH v2 0/4] Add MediaDevice and associated\n\tMediaObject","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 work.\n\nOn 2018-12-24 16:37:17 +0100, Jacopo Mondi wrote:\n> Hello,\n> this is v2 of MediaDevice and friends.\n> \n> v1 available for reference here:\n> https://lists.libcamera.org/pipermail/libcamera-devel/2018-December/000120.html\n> \n> While reviewing Kieran's and Niklas' patches I noticed some comments I made\n> on their series actually applied on my v1, so here it is the outcome of\n> a small private self-review. I'm sure I have missed something anyhow...\n> \n> It's mostly about using 'const' a lot more for values returned by reference\n> by getter functions, a more extensive use of 'auto' keyboard to shorten\n> iterators declarations and a few cosmetic changes.\n> \n> Both branches (v1 and v2) are available for comparison\n> (jmondi/master/media_device_v1 and jmondi/master/media_device_v2) and a diff\n> is collected here: https://paste.debian.net/1057251/\n> \n> On top of the first two patches that were included in v1, I have added what\n> I proposed in the review of Niklas' series: a MediaDeviceDesc object and a\n> match function to be added to the MediaDevice. Niklas, what do you think\n> about this? Does this help clarifying the proposal I made in the review\n> comments? (those patches are now broken out to eases discussion and review,\n> but will be ideally squashed on the [02/04] later).\n> \n> I have some private tests for IPU3, and the matching code looks like the\n> following:\n> \n> \tMediaDeviceDesc cio2_desc(\"ipu3-cio2\");\n> \tcio2_desc.addEntity(\"ipu3-csi2 0\");\n> \tcio2_desc.addEntity(\"ipu3-cio2 0\");\n> \tcio2_desc.addEntity(\"ipu3-csi2 1\");\n> \tcio2_desc.addEntity(\"ipu3-cio2 1\");\n> \tcio2_desc.addEntity(\"ipu3-csi2 2\");\n> \tcio2_desc.addEntity(\"ipu3-cio2 2\");\n> \tcio2_desc.addEntity(\"ipu3-csi2 3\");\n> \tcio2_desc.addEntity(\"ipu3-cio2 3\");\n> \tcio2_desc.addEntity(\"ov13858 2-0010\");\n> \tcio2_desc.addEntity(\"ov5670 4-0036\");\n> \n> \tbool match = cio2.match(cio2_desc);\n> \tif (match) {\n> \t\tLOG(Info) << \"CIO2 matched\";\n> \t} else {\n> \t\tLOG(Error) << \"Cannot match CIO2 with the provided desc\";\n> \t\treturn -1;\n> \t}\n> \n> If we adopt something like this, in place of cio2.match() we would have a\n> MediaEnumerator, as proposed by Niklas, but the idea stays the same: is the\n> MediaDevice that matches the MediaDeviceDesc, and the MediaEnumerator receives\n> a MediaDeviceDesc and tries it on all its open MediaDevices.\n> \n> Thanks\n> j\n> \n> v1 -> v2:\n> - diff collected here: https://paste.debian.net/1057251/\n> - patches 3 and 4 are new, broken out for review, to be squashed later.\n> \n> Jacopo Mondi (4):\n> libcamera: Add MediaObject class hierarchy\n> libcamera: Add MediaDevice class\n> libcamera: media device: Add MediaDeviceDesc\n> libcamera: media device: Add 'match()' method\n\nMaybe my client is misbehaving but I'm missing all the patches in this \nseries except 00/04.\n\n> \n> src/libcamera/include/media_device.h | 115 +++++\n> src/libcamera/include/media_object.h | 117 +++++\n> src/libcamera/media_device.cpp | 615 +++++++++++++++++++++++++++\n> src/libcamera/media_object.cpp | 302 +++++++++++++\n> src/libcamera/meson.build | 2 +\n> 5 files changed, 1151 insertions(+)\n> create mode 100644 src/libcamera/include/media_device.h\n> create mode 100644 src/libcamera/include/media_object.h\n> create mode 100644 src/libcamera/media_device.cpp\n> create mode 100644 src/libcamera/media_object.cpp\n> \n> --\n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 65DB1600CC\n\tfor ;\n\tFri, 28 Dec 2018 04:35:10 +0100 (CET)","by mail-lj1-x241.google.com with SMTP id c19-v6so17709018lja.5\n\tfor ;\n\tThu, 27 Dec 2018 19:35:10 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tp19-v6sm8398344ljb.47.2018.12.27.19.35.08\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 27 Dec 2018 19:35:09 -0800 (PST)"],"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=azYXmAqbLdvCOPddMKmpgnow40IiB6/wfPNAuDtefFI=;\n\tb=nyCFl87c2ni9i46rliFEhrItBqAuFA/2VwuJc9+kgd5MX08MnFaY46eP9wQVA0QRAo\n\tm3E/6mmU3VmNXxHtYEaswRKEVL2jTqc3ezlMkD9MA1j8Bo09TaA9+O+YkUuj+5s4PhNv\n\tgVQh31B9QmWTwX1m0XZRQWdg8R26hzeHynXPi3vgOHbRWs8exWMGmRWfTdEMD0RkD6zc\n\tnMlyBvVgjeARZQ/q/i+Dt6uLqle3ufIOpu0x4AJEM0hERWAtYYeKXzfO4wt4RBtDYlG8\n\tzZgZ7rlqGq6DNVm8V269SzgaOXSCKk/+eLuThPVyR4Z/7zBDhW6eLt/uCwp5LCwr87Rk\n\tadLg==","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=azYXmAqbLdvCOPddMKmpgnow40IiB6/wfPNAuDtefFI=;\n\tb=pNfRhm0FNkR28xXP1EOK4Ju20f8xSoTN1chl6ct6eKOpI9sySR9GEeSUoi+OQkzSLx\n\tXwnQut+LdzRdHP7TjNhzAd/W94NAhWVqmtUHiAhJxljhbkdcFMmVBWCGXDeYH/YJxG2K\n\tv3NK7ZpMw/DPCAeOpY9sHKvh4riHh7ypRAbF8DWFZz0cbTNSi5RG4pyGkr2A3Dj3lsz4\n\tAf+c1SNTuUv1/g06O82h0CbHuuoO2ALHqbxb5w7m3drpyOns2/svlvElznW4W+LH9OCN\n\tIm88xTzak1eIdQqmPuh9Ac+mvBN1Fe4A/jY7hGzctMqTs4yPSXoajrPz2gqLH7fjvSki\n\t/fig==","X-Gm-Message-State":"AJcUukfA3wvVY16u027xA+lxrBDzF27epi16pDIgBVXbhP/7yvoNb08v\n\tU9Tbq4q+vgtliKw9p9uAK+YPaKN0X3w=","X-Google-Smtp-Source":"ALg8bN5IJYiVzg2r8FUVVHcQ0gEJb74JUBPJ8plcl3esaHvKBkddHJhgBkXeR+Fdy1Uac1y245Y7SA==","X-Received":"by 2002:a2e:5854:: with SMTP id\n\tx20-v6mr14795079ljd.31.1545968109696; \n\tThu, 27 Dec 2018 19:35:09 -0800 (PST)","Date":"Fri, 28 Dec 2018 04:35:08 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20181228033508.GT19796@bigcity.dyn.berto.se>","References":"<20181224153721.8359-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20181224153721.8359-1-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 0/4] Add MediaDevice and associated\n\tMediaObject","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":"Fri, 28 Dec 2018 03:35:10 -0000"}},{"id":91,"web_url":"https://patchwork.libcamera.org/comment/91/","msgid":"<20181228075552.GB909@uno.localdomain>","date":"2018-12-28T07:55:52","subject":"Re: [libcamera-devel] [PATCH v2 0/4] Add MediaDevice and associated\n\tMediaObject","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Dec 28, 2018 at 04:35:08AM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2018-12-24 16:37:17 +0100, Jacopo Mondi wrote:\n> > Hello,\n> > this is v2 of MediaDevice and friends.\n> >\n> > v1 available for reference here:\n> > https://lists.libcamera.org/pipermail/libcamera-devel/2018-December/000120.html\n> >\n> > While reviewing Kieran's and Niklas' patches I noticed some comments I made\n> > on their series actually applied on my v1, so here it is the outcome of\n> > a small private self-review. I'm sure I have missed something anyhow...\n> >\n> > It's mostly about using 'const' a lot more for values returned by reference\n> > by getter functions, a more extensive use of 'auto' keyboard to shorten\n> > iterators declarations and a few cosmetic changes.\n> >\n> > Both branches (v1 and v2) are available for comparison\n> > (jmondi/master/media_device_v1 and jmondi/master/media_device_v2) and a diff\n> > is collected here: https://paste.debian.net/1057251/\n> >\n> > On top of the first two patches that were included in v1, I have added what\n> > I proposed in the review of Niklas' series: a MediaDeviceDesc object and a\n> > match function to be added to the MediaDevice. Niklas, what do you think\n> > about this? Does this help clarifying the proposal I made in the review\n> > comments? (those patches are now broken out to eases discussion and review,\n> > but will be ideally squashed on the [02/04] later).\n> >\n> > I have some private tests for IPU3, and the matching code looks like the\n> > following:\n> >\n> > \tMediaDeviceDesc cio2_desc(\"ipu3-cio2\");\n> > \tcio2_desc.addEntity(\"ipu3-csi2 0\");\n> > \tcio2_desc.addEntity(\"ipu3-cio2 0\");\n> > \tcio2_desc.addEntity(\"ipu3-csi2 1\");\n> > \tcio2_desc.addEntity(\"ipu3-cio2 1\");\n> > \tcio2_desc.addEntity(\"ipu3-csi2 2\");\n> > \tcio2_desc.addEntity(\"ipu3-cio2 2\");\n> > \tcio2_desc.addEntity(\"ipu3-csi2 3\");\n> > \tcio2_desc.addEntity(\"ipu3-cio2 3\");\n> > \tcio2_desc.addEntity(\"ov13858 2-0010\");\n> > \tcio2_desc.addEntity(\"ov5670 4-0036\");\n> >\n> > \tbool match = cio2.match(cio2_desc);\n> > \tif (match) {\n> > \t\tLOG(Info) << \"CIO2 matched\";\n> > \t} else {\n> > \t\tLOG(Error) << \"Cannot match CIO2 with the provided desc\";\n> > \t\treturn -1;\n> > \t}\n> >\n> > If we adopt something like this, in place of cio2.match() we would have a\n> > MediaEnumerator, as proposed by Niklas, but the idea stays the same: is the\n> > MediaDevice that matches the MediaDeviceDesc, and the MediaEnumerator receives\n> > a MediaDeviceDesc and tries it on all its open MediaDevices.\n> >\n> > Thanks\n> > j\n> >\n> > v1 -> v2:\n> > - diff collected here: https://paste.debian.net/1057251/\n> > - patches 3 and 4 are new, broken out for review, to be squashed later.\n> >\n> > Jacopo Mondi (4):\n> > libcamera: Add MediaObject class hierarchy\n> > libcamera: Add MediaDevice class\n> > libcamera: media device: Add MediaDeviceDesc\n> > libcamera: media device: Add 'match()' method\n>\n> Maybe my client is misbehaving but I'm missing all the patches in this\n> series except 00/04.\n>\n\nOh damn, I actually just sent [00/04]\nSorry about this.\n\nI'll re-send now.\n\n> >\n> > src/libcamera/include/media_device.h | 115 +++++\n> > src/libcamera/include/media_object.h | 117 +++++\n> > src/libcamera/media_device.cpp | 615 +++++++++++++++++++++++++++\n> > src/libcamera/media_object.cpp | 302 +++++++++++++\n> > src/libcamera/meson.build | 2 +\n> > 5 files changed, 1151 insertions(+)\n> > create mode 100644 src/libcamera/include/media_device.h\n> > create mode 100644 src/libcamera/include/media_object.h\n> > create mode 100644 src/libcamera/media_device.cpp\n> > create mode 100644 src/libcamera/media_object.cpp\n> >\n> > --\n> > 2.20.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","Received":["from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 409F6600CC\n\tfor ;\n\tFri, 28 Dec 2018 08:55:52 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 8E1E4240003;\n\tFri, 28 Dec 2018 07:55:51 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 28 Dec 2018 08:55:52 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20181228075552.GB909@uno.localdomain>","References":"<20181224153721.8359-1-jacopo@jmondi.org>\n\t<20181228033508.GT19796@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"bAmEntskrkuBymla\"","Content-Disposition":"inline","In-Reply-To":"<20181228033508.GT19796@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.11.1 (2018-12-01)","Subject":"Re: [libcamera-devel] [PATCH v2 0/4] Add MediaDevice and associated\n\tMediaObject","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":"Fri, 28 Dec 2018 07:55:52 -0000"}}]