[libcamera-devel,RFC,2/6] cam: Add option to capture StillCaptureRaw stream

3111 diff mbox series

Add a role name 'stillraw' to request a StillCaptureRaw stream.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/cam/main.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Hi Niklas,

Thank you for the patch.

On Mon, Mar 16, 2020 at 03:41:42AM +0100, Niklas Söderlund wrote:
> Add a role name 'stillraw' to request a StillCaptureRaw stream.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/cam/main.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index f73e77f381779853..dc86b5fee20ffa00 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -155,7 +155,7 @@ int CamApp::parseOptions(int argc, char *argv[])
>  {
>  	KeyValueParser streamKeyValue;
>  	streamKeyValue.addOption("role", OptionString,
> -				 "Role for the stream (viewfinder, video, still)",
> +				 "Role for the stream (viewfinder, video, still, stillraw)",
>  				 ArgumentRequired);
>  	streamKeyValue.addOption("width", OptionInteger, "Width in pixels",
>  				 ArgumentRequired);
> @@ -217,6 +217,8 @@ int CamApp::prepareConfig()
>  				roles.push_back(StreamRole::VideoRecording);
>  			} else if (opt["role"].toString() == "still") {
>  				roles.push_back(StreamRole::StillCapture);
> +			} else if (opt["role"].toString() == "stillraw") {
> +				roles.push_back(StreamRole::StillCaptureRaw);
>  			} else {
>  				std::cerr << "Unknown stream role "
>  					  << opt["role"].toString() << std::endl;

This looks fine, but maybe it's a good time to optimize this a bit by
avoiding the looking of the role property every time. I've sent "[PATCH]
qcam: main: Cache lookup of role property" to fix this.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Hi Niklas,

On Mon, Mar 23, 2020 at 12:40:04PM +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Mon, Mar 16, 2020 at 03:41:42AM +0100, Niklas Söderlund wrote:
> > Add a role name 'stillraw' to request a StillCaptureRaw stream.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/cam/main.cpp | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index f73e77f381779853..dc86b5fee20ffa00 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -155,7 +155,7 @@ int CamApp::parseOptions(int argc, char *argv[])
> >  {
> >  	KeyValueParser streamKeyValue;
> >  	streamKeyValue.addOption("role", OptionString,
> > -				 "Role for the stream (viewfinder, video, still)",
> > +				 "Role for the stream (viewfinder, video, still, stillraw)",
> >  				 ArgumentRequired);
> >  	streamKeyValue.addOption("width", OptionInteger, "Width in pixels",
> >  				 ArgumentRequired);
> > @@ -217,6 +217,8 @@ int CamApp::prepareConfig()
> >  				roles.push_back(StreamRole::VideoRecording);
> >  			} else if (opt["role"].toString() == "still") {
> >  				roles.push_back(StreamRole::StillCapture);
> > +			} else if (opt["role"].toString() == "stillraw") {

I would have just used 'raw', but that's not a big deal indeed

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> > +				roles.push_back(StreamRole::StillCaptureRaw);
> >  			} else {
> >  				std::cerr << "Unknown stream role "
> >  					  << opt["role"].toString() << std::endl;
>
> This looks fine, but maybe it's a good time to optimize this a bit by
> avoiding the looking of the role property every time. I've sent "[PATCH]
> qcam: main: Cache lookup of role property" to fix this.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel