[{"id":639,"web_url":"https://patchwork.libcamera.org/comment/639/","msgid":"<20190127160426.GA5934@pendragon.ideasonboard.com>","date":"2019-01-27T16:04:26","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","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 Sun, Jan 27, 2019 at 01:52:06AM +0100, Niklas Söderlund wrote:\n> If for any reason a default video device is not found in the media graph\n> the creation of the UVC pipeline silently failed. This is not optimal\n> when debugging problems with the pipeline, add a warning to notify the\n> user of the issue.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/libcamera/pipeline/uvcvideo.cpp | 6 ++++++\n>  1 file changed, 6 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index c51e8bc1f2c2bf25..4ae179d24709c856 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -8,12 +8,15 @@\n>  #include <libcamera/camera.h>\n>  \n>  #include \"device_enumerator.h\"\n> +#include \"log.h\"\n>  #include \"media_device.h\"\n>  #include \"pipeline_handler.h\"\n>  #include \"v4l2_device.h\"\n>  \n>  namespace libcamera {\n>  \n> +LOG_DEFINE_CATEGORY(UVC)\n> +\n>  class PipelineHandlerUVC : public PipelineHandler\n>  {\n>  public:\n> @@ -60,6 +63,9 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>  \t}\n>  \n>  \tif (!video_ || video_->open()) {\n> +\t\tif (!video_)\n> +\t\t\tLOG(UVC, Warning) << \"Could not find a default video device\";\n\nI'd make it an error, as it's quite fatal. I think we should also log a\nmessage in the video_->open() failure case, as that's equally fatal (the\npermissions denied case is especially important).\n\n> +\n>  \t\tmedia_->release();\n>  \t\treturn false;\n>  \t}","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D7CDD60DB4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 27 Jan 2019 17:04:30 +0100 (CET)","from pendragon.ideasonboard.com (85-76-23-203-nat.elisa-mobile.fi\n\t[85.76.23.203])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 69D56564;\n\tSun, 27 Jan 2019 17:04:29 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548605070;\n\tbh=EOtrZLFT0LRAhCLs6//ZVKzch7vh+juazCKaKn14KKI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nD2lpqDxxARK/V5O0jW+x9n80+l6Ec9LoP2k55VappwXPM2uWR/17VEZl3FCe4ZvO\n\tmnE7gg1OBXytboC5jG8+iokZyPIPMh13aWo/qqI5R48jpHX94lqzQKF1H1EE9pgz8y\n\taiFYmnK9xLlGt0l68EDH+hkyddD02jfIhKSYBYsM=","Date":"Sun, 27 Jan 2019 18:04:26 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190127160426.GA5934@pendragon.ideasonboard.com>","References":"<20190127005206.20901-1-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":"<20190127005206.20901-1-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","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, 27 Jan 2019 16:04:31 -0000"}},{"id":641,"web_url":"https://patchwork.libcamera.org/comment/641/","msgid":"<20190127161452.GB27958@bigcity.dyn.berto.se>","date":"2019-01-27T16:14:52","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","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-01-27 18:04:26 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Sun, Jan 27, 2019 at 01:52:06AM +0100, Niklas Söderlund wrote:\n> > If for any reason a default video device is not found in the media graph\n> > the creation of the UVC pipeline silently failed. This is not optimal\n> > when debugging problems with the pipeline, add a warning to notify the\n> > user of the issue.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/libcamera/pipeline/uvcvideo.cpp | 6 ++++++\n> >  1 file changed, 6 insertions(+)\n> > \n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index c51e8bc1f2c2bf25..4ae179d24709c856 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -8,12 +8,15 @@\n> >  #include <libcamera/camera.h>\n> >  \n> >  #include \"device_enumerator.h\"\n> > +#include \"log.h\"\n> >  #include \"media_device.h\"\n> >  #include \"pipeline_handler.h\"\n> >  #include \"v4l2_device.h\"\n> >  \n> >  namespace libcamera {\n> >  \n> > +LOG_DEFINE_CATEGORY(UVC)\n> > +\n> >  class PipelineHandlerUVC : public PipelineHandler\n> >  {\n> >  public:\n> > @@ -60,6 +63,9 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >  \t}\n> >  \n> >  \tif (!video_ || video_->open()) {\n> > +\t\tif (!video_)\n> > +\t\t\tLOG(UVC, Warning) << \"Could not find a default video device\";\n> \n> I'd make it an error, as it's quite fatal. I think we should also log a\n> message in the video_->open() failure case, as that's equally fatal (the\n> permissions denied case is especially important).\n\nI will make it an error.\n\nRegarding logging the failure of video_->open() this already happens in \nV4L2Device::open(). I'm open to a discussion of adding logging in each \npipeline handler for the open call, my initial position is however that \nit would add little of value as such an error is already logged. What is \nthe rest of the groups view?\n\n> \n> > +\n> >  \t\tmedia_->release();\n> >  \t\treturn false;\n> >  \t}\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 E0B1660DB4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 27 Jan 2019 17:14:54 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id b20so10096084lfa.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 27 Jan 2019 08:14:54 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tq3sm2464162lff.42.2019.01.27.08.14.52\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 27 Jan 2019 08:14:52 -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=RrD2GRv0ioje/R61GXTFb0SmrfuEiJ2Pr0xOHv0wvKk=;\n\tb=nn8NJBcJ7rsuHJH1w3JTH4pww1WxuS/uGLIJq+XwU+rj2bCLwKOFOmJN2iyzi9z3le\n\tPGPCc/pojmo8B8TgPb1FgwcgfhGykgyQNP6nCsTZzA2a5FXCI8OC6H9ZnT+0nynesYZ3\n\tHAeRob/PyCPFJFmTvYoD7zhyvfWUqOWQGGybXbA3P7L1ft4SLkTJ2IPSQZyz2/0Yrp6w\n\tC7qwWg+wdrnJq7nnni9BfB4dK110y70zIJNpOwNTrJsGgSWUsSL0C/HYzjdFDSQYUtN/\n\tdemWuG11sVAtJ65FGHrA5WpoL0wd30mtfZyAWYYUs79STc4ytzIio26EaQTk+11JjQHa\n\tDQGQ==","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=RrD2GRv0ioje/R61GXTFb0SmrfuEiJ2Pr0xOHv0wvKk=;\n\tb=Y2TtIWnkRfg6+UAHc1sJkaVija8ozsDephWqXM/9jeeFVkwZ/2oM0hb0x6YHkIJvUK\n\tQDZbrm+igABlJ4DAHNlA2KrntlqoCvGCuFPtgS0PBUjS6LBxo3gVrIOQ0EaWUdzfexqN\n\tHhClGUr4wyVM4U2RWizEApdYtMLsHWxswN76YG48j0dggp3Uv0Uc9r98sKVDuhN8UNLe\n\tYAM1b65nXlDbjuGxQttKv70Jilo9EAYhthkMAI/fsEYl20fHl27FTZzcgWPjwrdvd+ku\n\tvHoRqdgG6F0O3hunh7p4kNChxdUa19CmVJhyK+llKQ0TsYgclG0e6RK3y/eIbrp4eBo/\n\tEQVA==","X-Gm-Message-State":"AJcUukeCPcNbHGPRpx6yqw3Eej46hGLfR8rpJZCWT2bx/VBVYBbZuAYB\n\tBF+DCXyrOJvy0QhTekW43hTFiA==","X-Google-Smtp-Source":"ALg8bN7SKpbzUIgbVBcp0w6Xlr7aAp/BmCWNQTDWIr1pQqrmTulpOQNJiT1oFe0eCXSZReP3xQm4jQ==","X-Received":"by 2002:a19:6806:: with SMTP id d6mr13864841lfc.48.1548605693879;\n\tSun, 27 Jan 2019 08:14:53 -0800 (PST)","Date":"Sun, 27 Jan 2019 17:14:52 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190127161452.GB27958@bigcity.dyn.berto.se>","References":"<20190127005206.20901-1-niklas.soderlund@ragnatech.se>\n\t<20190127160426.GA5934@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":"<20190127160426.GA5934@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","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, 27 Jan 2019 16:14:55 -0000"}},{"id":642,"web_url":"https://patchwork.libcamera.org/comment/642/","msgid":"<20190127202159.GA4323@pendragon.ideasonboard.com>","date":"2019-01-27T20:21:59","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Sun, Jan 27, 2019 at 05:14:52PM +0100, Niklas Söderlund wrote:\n> On 2019-01-27 18:04:26 +0200, Laurent Pinchart wrote:\n> > On Sun, Jan 27, 2019 at 01:52:06AM +0100, Niklas Söderlund wrote:\n> >> If for any reason a default video device is not found in the media graph\n> >> the creation of the UVC pipeline silently failed. This is not optimal\n> >> when debugging problems with the pipeline, add a warning to notify the\n> >> user of the issue.\n> >> \n> >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >> ---\n> >>  src/libcamera/pipeline/uvcvideo.cpp | 6 ++++++\n> >>  1 file changed, 6 insertions(+)\n> >> \n> >> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> >> index c51e8bc1f2c2bf25..4ae179d24709c856 100644\n> >> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> >> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> >> @@ -8,12 +8,15 @@\n> >>  #include <libcamera/camera.h>\n> >>  \n> >>  #include \"device_enumerator.h\"\n> >> +#include \"log.h\"\n> >>  #include \"media_device.h\"\n> >>  #include \"pipeline_handler.h\"\n> >>  #include \"v4l2_device.h\"\n> >>  \n> >>  namespace libcamera {\n> >>  \n> >> +LOG_DEFINE_CATEGORY(UVC)\n> >> +\n> >>  class PipelineHandlerUVC : public PipelineHandler\n> >>  {\n> >>  public:\n> >> @@ -60,6 +63,9 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >>  \t}\n> >>  \n> >>  \tif (!video_ || video_->open()) {\n> >> +\t\tif (!video_)\n> >> +\t\t\tLOG(UVC, Warning) << \"Could not find a default video device\";\n> > \n> > I'd make it an error, as it's quite fatal. I think we should also log a\n> > message in the video_->open() failure case, as that's equally fatal (the\n> > permissions denied case is especially important).\n> \n> I will make it an error.\n> \n> Regarding logging the failure of video_->open() this already happens in \n> V4L2Device::open(). I'm open to a discussion of adding logging in each \n> pipeline handler for the open call, my initial position is however that \n> it would add little of value as such an error is already logged. What is \n> the rest of the groups view?\n\nI agree with you, it's best to handle as much as possible in the\nlibcamera core to minimize the potential issues in the pipeline\nhandlers.\n\n> >> +\n> >>  \t\tmedia_->release();\n> >>  \t\treturn false;\n> >>  \t}","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3AF7A60DB4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 27 Jan 2019 21:22:04 +0100 (CET)","from pendragon.ideasonboard.com (85-76-73-155-nat.elisa-mobile.fi\n\t[85.76.73.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 055DD85;\n\tSun, 27 Jan 2019 21:22:01 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548620523;\n\tbh=WYX1lqvgbi3PtqqkuDVpB56+oNjEw7Vk9kN5tonYP1w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EJOch6XHLG2HOM6Rj/rD/rsHpisgZiekM9Aa/ElCqMSbe1Va6RqHx5TRnaWBO8saI\n\tYPxjUwQVOujPpTlqMr+YSuQj4a2263EPAVnkeMvQzexpLLBvhzz1PPPBYupgGHCXEI\n\tf9llbk3QZ3vjACJefEhRdU/qjlWVorpiPUgQs7B4=","Date":"Sun, 27 Jan 2019 22:21:59 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190127202159.GA4323@pendragon.ideasonboard.com>","References":"<20190127005206.20901-1-niklas.soderlund@ragnatech.se>\n\t<20190127160426.GA5934@pendragon.ideasonboard.com>\n\t<20190127161452.GB27958@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":"<20190127161452.GB27958@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","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, 27 Jan 2019 20:22:04 -0000"}},{"id":651,"web_url":"https://patchwork.libcamera.org/comment/651/","msgid":"<ae1551e2-d217-da8c-f399-fbd4096bc0a4@ideasonboard.com>","date":"2019-01-28T09:21:30","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Heya,\n\nOn 27/01/2019 20:21, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Sun, Jan 27, 2019 at 05:14:52PM +0100, Niklas Söderlund wrote:\n>> On 2019-01-27 18:04:26 +0200, Laurent Pinchart wrote:\n>>> On Sun, Jan 27, 2019 at 01:52:06AM +0100, Niklas Söderlund wrote:\n>>>> If for any reason a default video device is not found in the media graph\n>>>> the creation of the UVC pipeline silently failed. This is not optimal\n>>>> when debugging problems with the pipeline, add a warning to notify the\n>>>> user of the issue.\n>>>>\n>>>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>>>> ---\n>>>>  src/libcamera/pipeline/uvcvideo.cpp | 6 ++++++\n>>>>  1 file changed, 6 insertions(+)\n>>>>\n>>>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n>>>> index c51e8bc1f2c2bf25..4ae179d24709c856 100644\n>>>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n>>>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n>>>> @@ -8,12 +8,15 @@\n>>>>  #include <libcamera/camera.h>\n>>>>  \n>>>>  #include \"device_enumerator.h\"\n>>>> +#include \"log.h\"\n>>>>  #include \"media_device.h\"\n>>>>  #include \"pipeline_handler.h\"\n>>>>  #include \"v4l2_device.h\"\n>>>>  \n>>>>  namespace libcamera {\n>>>>  \n>>>> +LOG_DEFINE_CATEGORY(UVC)\n>>>> +\n>>>>  class PipelineHandlerUVC : public PipelineHandler\n>>>>  {\n>>>>  public:\n>>>> @@ -60,6 +63,9 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>>>>  \t}\n>>>>  \n>>>>  \tif (!video_ || video_->open()) {\n>>>> +\t\tif (!video_)\n>>>> +\t\t\tLOG(UVC, Warning) << \"Could not find a default video device\";\n>>>\n>>> I'd make it an error, as it's quite fatal. I think we should also log a\n>>> message in the video_->open() failure case, as that's equally fatal (the\n>>> permissions denied case is especially important).\n>>\n>> I will make it an error.\n>>\n>> Regarding logging the failure of video_->open() this already happens in \n>> V4L2Device::open(). I'm open to a discussion of adding logging in each \n>> pipeline handler for the open call, my initial position is however that \n>> it would add little of value as such an error is already logged. What is \n>> the rest of the groups view?\n> \n> I agree with you, it's best to handle as much as possible in the\n> libcamera core to minimize the potential issues in the pipeline\n> handlers.\n\n\nYes, failures and errors should be reported as early as possible, and as\nclose to the root component as possible.\n\nIn this case - at the point of the Open call.\n\nIt does however present an possible issue with filtering.\nIf an error occurs - and someone sets something like:\n\nLIBCAMERA_LOG_LEVELS=pipeline:Debug\n\n- (and nothing else) Would that then /hide/ the V4L2 errors? (or does\neach category stay at it's default severity level unless explicitly changed)\n\nAs long as the V4L2 category remains unchanged, and will still print\nthen that's fine.\n\n\n>>>> +\n>>>>  \t\tmedia_->release();\n>>>>  \t\treturn false;\n>>>>  \t}\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","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 503C660B1B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Jan 2019 10:21:33 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AEB3985;\n\tMon, 28 Jan 2019 10:21:32 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548667292;\n\tbh=eshKdo4QMuKTHFT96wG79Z4DHKm+t8Jo6lyGU/RFLTw=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=iwvdFv2RZloYyPNLyMTY2PQ3HFYYd8DHcurnLyV3z98TIhMuTVlQzWtS6NwWTGtQs\n\tzOHztkZCYnSz5MDarJoD2sISkZDIJAKvgXxjRqxttjJBq7GNII8P2uzGi1wtd5QmoK\n\tS6aZnn6jE1ObzYGiXDvmXwXfUR3HkXA6xVuxW/8Y=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>, =?utf-8?q?Niklas?=\n\t=?utf-8?q?_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20190127005206.20901-1-niklas.soderlund@ragnatech.se>\n\t<20190127160426.GA5934@pendragon.ideasonboard.com>\n\t<20190127161452.GB27958@bigcity.dyn.berto.se>\n\t<20190127202159.GA4323@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<ae1551e2-d217-da8c-f399-fbd4096bc0a4@ideasonboard.com>","Date":"Mon, 28 Jan 2019 09:21:30 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190127202159.GA4323@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","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":"Mon, 28 Jan 2019 09:21:33 -0000"}},{"id":665,"web_url":"https://patchwork.libcamera.org/comment/665/","msgid":"<20190128225752.GD4332@pendragon.ideasonboard.com>","date":"2019-01-28T22:57:52","subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Jan 28, 2019 at 09:21:30AM +0000, Kieran Bingham wrote:\n> On 27/01/2019 20:21, Laurent Pinchart wrote:\n> > On Sun, Jan 27, 2019 at 05:14:52PM +0100, Niklas Söderlund wrote:\n> >> On 2019-01-27 18:04:26 +0200, Laurent Pinchart wrote:\n> >>> On Sun, Jan 27, 2019 at 01:52:06AM +0100, Niklas Söderlund wrote:\n> >>>> If for any reason a default video device is not found in the media graph\n> >>>> the creation of the UVC pipeline silently failed. This is not optimal\n> >>>> when debugging problems with the pipeline, add a warning to notify the\n> >>>> user of the issue.\n> >>>>\n> >>>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >>>> ---\n> >>>>  src/libcamera/pipeline/uvcvideo.cpp | 6 ++++++\n> >>>>  1 file changed, 6 insertions(+)\n> >>>>\n> >>>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> >>>> index c51e8bc1f2c2bf25..4ae179d24709c856 100644\n> >>>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> >>>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> >>>> @@ -8,12 +8,15 @@\n> >>>>  #include <libcamera/camera.h>\n> >>>>  \n> >>>>  #include \"device_enumerator.h\"\n> >>>> +#include \"log.h\"\n> >>>>  #include \"media_device.h\"\n> >>>>  #include \"pipeline_handler.h\"\n> >>>>  #include \"v4l2_device.h\"\n> >>>>  \n> >>>>  namespace libcamera {\n> >>>>  \n> >>>> +LOG_DEFINE_CATEGORY(UVC)\n> >>>> +\n> >>>>  class PipelineHandlerUVC : public PipelineHandler\n> >>>>  {\n> >>>>  public:\n> >>>> @@ -60,6 +63,9 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >>>>  \t}\n> >>>>  \n> >>>>  \tif (!video_ || video_->open()) {\n> >>>> +\t\tif (!video_)\n> >>>> +\t\t\tLOG(UVC, Warning) << \"Could not find a default video device\";\n> >>>\n> >>> I'd make it an error, as it's quite fatal. I think we should also log a\n> >>> message in the video_->open() failure case, as that's equally fatal (the\n> >>> permissions denied case is especially important).\n> >>\n> >> I will make it an error.\n> >>\n> >> Regarding logging the failure of video_->open() this already happens in \n> >> V4L2Device::open(). I'm open to a discussion of adding logging in each \n> >> pipeline handler for the open call, my initial position is however that \n> >> it would add little of value as such an error is already logged. What is \n> >> the rest of the groups view?\n> > \n> > I agree with you, it's best to handle as much as possible in the\n> > libcamera core to minimize the potential issues in the pipeline\n> > handlers.\n> \n> Yes, failures and errors should be reported as early as possible, and as\n> close to the root component as possible.\n> \n> In this case - at the point of the Open call.\n> \n> It does however present an possible issue with filtering.\n> If an error occurs - and someone sets something like:\n> \n> LIBCAMERA_LOG_LEVELS=pipeline:Debug\n> \n> - (and nothing else) Would that then /hide/ the V4L2 errors? (or does\n> each category stay at it's default severity level unless explicitly changed)\n> \n> As long as the V4L2 category remains unchanged, and will still print\n> then that's fine.\n\nCategories not explicitly specified keep their default log level, so\nit's fine.\n\n> >>>> +\n> >>>>  \t\tmedia_->release();\n> >>>>  \t\treturn false;\n> >>>>  \t}","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 88D0E60B1B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Jan 2019 23:57:54 +0100 (CET)","from pendragon.ideasonboard.com (85-76-73-159-nat.elisa-mobile.fi\n\t[85.76.73.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8348D85;\n\tMon, 28 Jan 2019 23:57:53 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548716274;\n\tbh=6Jjx0iDk0bAOrX603hzhOG3pb8ZIdz2GKerQe0B8PC8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iQoa3nZ+CQ5bNUUtlDsmOyapRB2MrppuPWG31NO7XU7wRx85oKVlUYl49SHERvIpv\n\tYbpYjNY1TdVOHBCm2tRlnl/SWVcjaL1N9PytmUqzUEhV8gd9UfkjnQwKPcwQ49low3\n\t4BvZXD1UrRk+1MsOq5zUVDhPTEkcNpGOanXGitoA=","Date":"Tue, 29 Jan 2019 00:57:52 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190128225752.GD4332@pendragon.ideasonboard.com>","References":"<20190127005206.20901-1-niklas.soderlund@ragnatech.se>\n\t<20190127160426.GA5934@pendragon.ideasonboard.com>\n\t<20190127161452.GB27958@bigcity.dyn.berto.se>\n\t<20190127202159.GA4323@pendragon.ideasonboard.com>\n\t<ae1551e2-d217-da8c-f399-fbd4096bc0a4@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<ae1551e2-d217-da8c-f399-fbd4096bc0a4@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH] libcamera: pipeline: uvcvideo: add\n\twarning if no default video device is found","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":"Mon, 28 Jan 2019 22:57:54 -0000"}}]