| Message ID | 20200929164000.15429-7-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thanks for your patch. On 2020-09-29 17:40:00 +0100, David Plowman wrote: > This allows the user to request digital zoom by adding, for example, > "-z 0.25,0.25,0.5,0.5" which would give a 2x zoom, still centred in > the middle of the image. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> If I understand the cover letter correctly this is just a rebase so I understand not all comments from v2 are addressed. But just to reiterate my comments from v2. > --- > src/cam/capture.cpp | 25 +++++++++++++++++++++++-- > src/cam/capture.h | 2 +- > src/cam/main.cpp | 3 +++ > src/cam/main.h | 1 + > 4 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > index 5510c009..3b50c591 100644 > --- a/src/cam/capture.cpp > +++ b/src/cam/capture.cpp > @@ -10,6 +10,9 @@ > #include <limits.h> > #include <sstream> > > +#include <libcamera/control_ids.h> > +#include <libcamera/property_ids.h> > + > #include "capture.h" > #include "main.h" > > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options) > > FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_); > > - ret = capture(allocator); > + ret = capture(allocator, options); > > if (options.isSet(OptFile)) { > delete writer_; > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options) > return ret; > } > > -int Capture::capture(FrameBufferAllocator *allocator) > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options) > { > int ret; > > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator) > requests.push_back(request); > } > > + /* Set up digital zoom if it was requested on the command line. */ > + if (options.isSet(OptZoom)) { > + std::string zoom = options[OptZoom].toString(); > + float x, y, width, height; > + > + if (sscanf(zoom.c_str(), "%f,%f,%f,%f", &x, &y, &width, &height) == 4) { This is the wrong place to parse the input from the user. If the zoom options are global something similar like StreamKeyValueParser (ZoomKeyValueParser?) should be created. If it is to be moved a stream level StreamKeyValueParser should be extended. > + Size sensorArea = camera_->properties().get(properties::SensorOutputSize); > + Rectangle crop(x * sensorArea.width, > + y * sensorArea.height, > + width * sensorArea.width, > + height * sensorArea.height); > + > + requests.front()->controls().set(controls::IspCrop, crop); Do we wish to expose the ISP in the public API? If we do is the ISP (from an application point of view) the correct level to operate on? Would not setting this on a stream level cover more hardware? I'm thinking of an ISP that can apply different crop parameters to two (or more) source pads from a single sink pad. Having a control that sets a global crop on the ISP would that not limit such hardware or am I missing something? > + } else { > + std::cout << "Invalid zoom values - ignoring" << std::endl; > + } > + } > + > ret = camera_->start(); > if (ret) { > std::cout << "Failed to start capture" << std::endl; > diff --git a/src/cam/capture.h b/src/cam/capture.h > index 0aebdac9..52806164 100644 > --- a/src/cam/capture.h > +++ b/src/cam/capture.h > @@ -29,7 +29,7 @@ public: > > int run(const OptionsParser::Options &options); > private: > - int capture(libcamera::FrameBufferAllocator *allocator); > + int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options); > > void requestComplete(libcamera::Request *request); > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index 244720b4..b871d049 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[]) > parser.addOption(OptStrictFormats, OptionNone, > "Do not allow requested stream format(s) to be adjusted", > "strict-formats"); > + parser.addOption(OptZoom, OptionString, > + "Zoom in on the image portion described by the x,y,width,height rectangle (fractions of the maximum FOV) e.g. 0.25,0.25,0.5,0.5", > + "zoom", ArgumentRequired, "zoom"); > > options_ = parser.parse(argc, argv); > if (!options_.valid()) > diff --git a/src/cam/main.h b/src/cam/main.h > index ea8dfd33..a1fd899f 100644 > --- a/src/cam/main.h > +++ b/src/cam/main.h > @@ -17,6 +17,7 @@ enum { > OptListProperties = 'p', > OptMonitor = 'm', > OptStream = 's', > + OptZoom = 'z', > OptListControls = 256, > OptStrictFormats = 257, > }; > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas Thanks for the comments, and sorry for not addressing them sooner! On Tue, 29 Sep 2020 at 23:33, Niklas Söderlund <niklas.soderlund@ragnatech.se> wrote: > > Hi David, > > Thanks for your patch. > > On 2020-09-29 17:40:00 +0100, David Plowman wrote: > > This allows the user to request digital zoom by adding, for example, > > "-z 0.25,0.25,0.5,0.5" which would give a 2x zoom, still centred in > > the middle of the image. > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > If I understand the cover letter correctly this is just a rebase so I Correct, apart from a couple of minor mods (though one rather important one!) > understand not all comments from v2 are addressed. But just to reiterate > my comments from v2. > > > --- > > src/cam/capture.cpp | 25 +++++++++++++++++++++++-- > > src/cam/capture.h | 2 +- > > src/cam/main.cpp | 3 +++ > > src/cam/main.h | 1 + > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > > index 5510c009..3b50c591 100644 > > --- a/src/cam/capture.cpp > > +++ b/src/cam/capture.cpp > > @@ -10,6 +10,9 @@ > > #include <limits.h> > > #include <sstream> > > > > +#include <libcamera/control_ids.h> > > +#include <libcamera/property_ids.h> > > + > > #include "capture.h" > > #include "main.h" > > > > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options) > > > > FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_); > > > > - ret = capture(allocator); > > + ret = capture(allocator, options); > > > > if (options.isSet(OptFile)) { > > delete writer_; > > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options) > > return ret; > > } > > > > -int Capture::capture(FrameBufferAllocator *allocator) > > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options) > > { > > int ret; > > > > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator) > > requests.push_back(request); > > } > > > > + /* Set up digital zoom if it was requested on the command line. */ > > + if (options.isSet(OptZoom)) { > > + std::string zoom = options[OptZoom].toString(); > > + float x, y, width, height; > > + > > + if (sscanf(zoom.c_str(), "%f,%f,%f,%f", &x, &y, &width, &height) == 4) { > > This is the wrong place to parse the input from the user. If the zoom > options are global something similar like StreamKeyValueParser > (ZoomKeyValueParser?) should be created. If it is to be moved a stream > level StreamKeyValueParser should be extended. Yes, I was kind of hoping not to get too involved with all that! How about this: 1. Add an OptionFloat type 2. Change the "isArray" option field to "size" or something, and make it an unsigned int. We'd make its default value 1, meaning "not an array" and edit any existing usage that sets it to False to use 1 instead. 3. It seems to me that the current semantics for an "array" option is that they're variable length, i.e. it accepts as many as you type in. Perhaps we could use zero size for such an array, and otherwise insist that we get exactly the number that were wanted? 4. Then in my case I could supply a size of 4, and get an array of 4 floats. 5. It's true there is still some checking that needs to be done on the values to be sure they're sensible - my view here is that pipeline handlers should deal with garbage as applications can easily submit garbage, and here we have a way of checking that the pipeline handler actually does this correctly! > > > + Size sensorArea = camera_->properties().get(properties::SensorOutputSize); > > + Rectangle crop(x * sensorArea.width, > > + y * sensorArea.height, > > + width * sensorArea.width, > > + height * sensorArea.height); > > + > > + requests.front()->controls().set(controls::IspCrop, crop); > > Do we wish to expose the ISP in the public API? If we do is the ISP > (from an application point of view) the correct level to operate on? > Would not setting this on a stream level cover more hardware? > > I'm thinking of an ISP that can apply different crop parameters to two > (or more) source pads from a single sink pad. Having a control that sets > a global crop on the ISP would that not limit such hardware or am I > missing something? Indeed you're right, there is a bit of a dilemma here. There was some discussion of this a while back, I think with Jacopo, unfortunately I don't have any email references to hand. I think the feeling was that in all the real use cases that we can imagine, you generally want the same field of view on all your outputs. I think situations where something else would be useful are certainly less "mainstream". Secondly, some ISPs will let you crop outputs differently, and some won't. That's an awkward thing to handle - how would you know what the underlying hardware does, and then would you write different code for each case? The pragmatic answer may just be to implement the "usual" case here, and it becomes a question for the future whether we get per-stream crop controls, or whether a StreamConfiguration gets crop rectangles... Maybe Jacopo can comment if I've got anything wrong there! Thanks again and best regards David > > > + } else { > > + std::cout << "Invalid zoom values - ignoring" << std::endl; > > + } > > + } > > + > > ret = camera_->start(); > > if (ret) { > > std::cout << "Failed to start capture" << std::endl; > > diff --git a/src/cam/capture.h b/src/cam/capture.h > > index 0aebdac9..52806164 100644 > > --- a/src/cam/capture.h > > +++ b/src/cam/capture.h > > @@ -29,7 +29,7 @@ public: > > > > int run(const OptionsParser::Options &options); > > private: > > - int capture(libcamera::FrameBufferAllocator *allocator); > > + int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options); > > > > void requestComplete(libcamera::Request *request); > > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index 244720b4..b871d049 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[]) > > parser.addOption(OptStrictFormats, OptionNone, > > "Do not allow requested stream format(s) to be adjusted", > > "strict-formats"); > > + parser.addOption(OptZoom, OptionString, > > + "Zoom in on the image portion described by the x,y,width,height rectangle (fractions of the maximum FOV) e.g. 0.25,0.25,0.5,0.5", > > + "zoom", ArgumentRequired, "zoom"); > > > > options_ = parser.parse(argc, argv); > > if (!options_.valid()) > > diff --git a/src/cam/main.h b/src/cam/main.h > > index ea8dfd33..a1fd899f 100644 > > --- a/src/cam/main.h > > +++ b/src/cam/main.h > > @@ -17,6 +17,7 @@ enum { > > OptListProperties = 'p', > > OptMonitor = 'm', > > OptStream = 's', > > + OptZoom = 'z', > > OptListControls = 256, > > OptStrictFormats = 257, > > }; > > -- > > 2.20.1 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards, > Niklas Söderlund
Hi David, On Wed, Sep 30, 2020 at 08:00:18AM +0100, David Plowman wrote: > Hi Niklas > > Thanks for the comments, and sorry for not addressing them sooner! > > On Tue, 29 Sep 2020 at 23:33, Niklas Söderlund > <niklas.soderlund@ragnatech.se> wrote: > > > > Hi David, > > > > Thanks for your patch. > > > > On 2020-09-29 17:40:00 +0100, David Plowman wrote: > > > This allows the user to request digital zoom by adding, for example, > > > "-z 0.25,0.25,0.5,0.5" which would give a 2x zoom, still centred in > > > the middle of the image. > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > If I understand the cover letter correctly this is just a rebase so I > > Correct, apart from a couple of minor mods (though one rather important one!) > > > understand not all comments from v2 are addressed. But just to reiterate > > my comments from v2. > > > > > --- > > > src/cam/capture.cpp | 25 +++++++++++++++++++++++-- > > > src/cam/capture.h | 2 +- > > > src/cam/main.cpp | 3 +++ > > > src/cam/main.h | 1 + > > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > > > index 5510c009..3b50c591 100644 > > > --- a/src/cam/capture.cpp > > > +++ b/src/cam/capture.cpp > > > @@ -10,6 +10,9 @@ > > > #include <limits.h> > > > #include <sstream> > > > > > > +#include <libcamera/control_ids.h> > > > +#include <libcamera/property_ids.h> > > > + > > > #include "capture.h" > > > #include "main.h" > > > > > > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options) > > > > > > FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_); > > > > > > - ret = capture(allocator); > > > + ret = capture(allocator, options); > > > > > > if (options.isSet(OptFile)) { > > > delete writer_; > > > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options) > > > return ret; > > > } > > > > > > -int Capture::capture(FrameBufferAllocator *allocator) > > > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options) > > > { > > > int ret; > > > > > > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator) > > > requests.push_back(request); > > > } > > > > > > + /* Set up digital zoom if it was requested on the command line. */ > > > + if (options.isSet(OptZoom)) { > > > + std::string zoom = options[OptZoom].toString(); > > > + float x, y, width, height; > > > + > > > + if (sscanf(zoom.c_str(), "%f,%f,%f,%f", &x, &y, &width, &height) == 4) { > > > > This is the wrong place to parse the input from the user. If the zoom > > options are global something similar like StreamKeyValueParser > > (ZoomKeyValueParser?) should be created. If it is to be moved a stream > > level StreamKeyValueParser should be extended. > > Yes, I was kind of hoping not to get too involved with all that! How about this: > > 1. Add an OptionFloat type > > 2. Change the "isArray" option field to "size" or something, and make > it an unsigned int. We'd make its default value 1, meaning "not an > array" and edit any existing usage that sets it to False to use 1 > instead. > > 3. It seems to me that the current semantics for an "array" option is > that they're variable length, i.e. it accepts as many as you type in. > Perhaps we could use zero size for such an array, and otherwise insist > that we get exactly the number that were wanted? > > 4. Then in my case I could supply a size of 4, and get an array of 4 floats. > > 5. It's true there is still some checking that needs to be done on the > values to be sure they're sensible - my view here is that pipeline > handlers should deal with garbage as applications can easily submit > garbage, and here we have a way of checking that the pipeline handler > actually does this correctly! > > > > > > + Size sensorArea = camera_->properties().get(properties::SensorOutputSize); > > > + Rectangle crop(x * sensorArea.width, > > > + y * sensorArea.height, > > > + width * sensorArea.width, > > > + height * sensorArea.height); > > > + > > > + requests.front()->controls().set(controls::IspCrop, crop); > > > > Do we wish to expose the ISP in the public API? If we do is the ISP > > (from an application point of view) the correct level to operate on? > > Would not setting this on a stream level cover more hardware? > > > > I'm thinking of an ISP that can apply different crop parameters to two > > (or more) source pads from a single sink pad. Having a control that sets > > a global crop on the ISP would that not limit such hardware or am I > > missing something? > > Indeed you're right, there is a bit of a dilemma here. There was some > discussion of this a while back, I think with Jacopo, unfortunately I > don't have any email references to hand. > > I think the feeling was that in all the real use cases that we can > imagine, you generally want the same field of view on all your > outputs. I think situations where something else would be useful are > certainly less "mainstream". > > Secondly, some ISPs will let you crop outputs differently, and some > won't. That's an awkward thing to handle - how would you know what the > underlying hardware does, and then would you write different code for > each case? The pragmatic answer may just be to implement the "usual" > case here, and it becomes a question for the future whether we get > per-stream crop controls, or whether a StreamConfiguration gets crop > rectangles... > > Maybe Jacopo can comment if I've got anything wrong there! Yes, we've gone through that, and I think my intereptation was that you're better positioned than me, having a large user base and knowing what they expect, to decide if it was worth blocking this feature until we have decided how to implement per-Stream controls and how to handle the situation you have described here when a platform supports such a feature at the Camera level or at the Stream level. My opinion is that digital zoom it's a feature it's worth having, even if in sub-optimal shape (same applies to the SensorOutputSize property, which ideally should be reported as part of the CameraConfiguration). The alternative is to post-pone this until we have augmented CameraConfiguration with properties (relatively easy) and implemented per-Stream control (less easy if you ask me). I know Laurent wanted to have a look here, so let's see what his opinion is. Thanks j > > Thanks again and best regards > David > > > > > > + } else { > > > + std::cout << "Invalid zoom values - ignoring" << std::endl; > > > + } > > > + } > > > + > > > ret = camera_->start(); > > > if (ret) { > > > std::cout << "Failed to start capture" << std::endl; > > > diff --git a/src/cam/capture.h b/src/cam/capture.h > > > index 0aebdac9..52806164 100644 > > > --- a/src/cam/capture.h > > > +++ b/src/cam/capture.h > > > @@ -29,7 +29,7 @@ public: > > > > > > int run(const OptionsParser::Options &options); > > > private: > > > - int capture(libcamera::FrameBufferAllocator *allocator); > > > + int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options); > > > > > > void requestComplete(libcamera::Request *request); > > > > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > > index 244720b4..b871d049 100644 > > > --- a/src/cam/main.cpp > > > +++ b/src/cam/main.cpp > > > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[]) > > > parser.addOption(OptStrictFormats, OptionNone, > > > "Do not allow requested stream format(s) to be adjusted", > > > "strict-formats"); > > > + parser.addOption(OptZoom, OptionString, > > > + "Zoom in on the image portion described by the x,y,width,height rectangle (fractions of the maximum FOV) e.g. 0.25,0.25,0.5,0.5", > > > + "zoom", ArgumentRequired, "zoom"); > > > > > > options_ = parser.parse(argc, argv); > > > if (!options_.valid()) > > > diff --git a/src/cam/main.h b/src/cam/main.h > > > index ea8dfd33..a1fd899f 100644 > > > --- a/src/cam/main.h > > > +++ b/src/cam/main.h > > > @@ -17,6 +17,7 @@ enum { > > > OptListProperties = 'p', > > > OptMonitor = 'm', > > > OptStream = 's', > > > + OptZoom = 'z', > > > OptListControls = 256, > > > OptStrictFormats = 257, > > > }; > > > -- > > > 2.20.1 > > > > > > _______________________________________________ > > > libcamera-devel mailing list > > > libcamera-devel@lists.libcamera.org > > > https://lists.libcamera.org/listinfo/libcamera-devel > > > > -- > > Regards, > > Niklas Söderlund > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi David, On 2020-09-30 08:00:18 +0100, David Plowman wrote: > Hi Niklas > > Thanks for the comments, and sorry for not addressing them sooner! > > On Tue, 29 Sep 2020 at 23:33, Niklas Söderlund > <niklas.soderlund@ragnatech.se> wrote: > > > > Hi David, > > > > Thanks for your patch. > > > > On 2020-09-29 17:40:00 +0100, David Plowman wrote: > > > This allows the user to request digital zoom by adding, for example, > > > "-z 0.25,0.25,0.5,0.5" which would give a 2x zoom, still centred in > > > the middle of the image. > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > If I understand the cover letter correctly this is just a rebase so I > > Correct, apart from a couple of minor mods (though one rather important one!) > > > understand not all comments from v2 are addressed. But just to reiterate > > my comments from v2. > > > > > --- > > > src/cam/capture.cpp | 25 +++++++++++++++++++++++-- > > > src/cam/capture.h | 2 +- > > > src/cam/main.cpp | 3 +++ > > > src/cam/main.h | 1 + > > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > > > index 5510c009..3b50c591 100644 > > > --- a/src/cam/capture.cpp > > > +++ b/src/cam/capture.cpp > > > @@ -10,6 +10,9 @@ > > > #include <limits.h> > > > #include <sstream> > > > > > > +#include <libcamera/control_ids.h> > > > +#include <libcamera/property_ids.h> > > > + > > > #include "capture.h" > > > #include "main.h" > > > > > > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options) > > > > > > FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_); > > > > > > - ret = capture(allocator); > > > + ret = capture(allocator, options); > > > > > > if (options.isSet(OptFile)) { > > > delete writer_; > > > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options) > > > return ret; > > > } > > > > > > -int Capture::capture(FrameBufferAllocator *allocator) > > > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options) > > > { > > > int ret; > > > > > > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator) > > > requests.push_back(request); > > > } > > > > > > + /* Set up digital zoom if it was requested on the command line. */ > > > + if (options.isSet(OptZoom)) { > > > + std::string zoom = options[OptZoom].toString(); > > > + float x, y, width, height; > > > + > > > + if (sscanf(zoom.c_str(), "%f,%f,%f,%f", &x, &y, &width, &height) == 4) { > > > > This is the wrong place to parse the input from the user. If the zoom > > options are global something similar like StreamKeyValueParser > > (ZoomKeyValueParser?) should be created. If it is to be moved a stream > > level StreamKeyValueParser should be extended. > > Yes, I was kind of hoping not to get too involved with all that! :-) > How about this: > > 1. Add an OptionFloat type > > 2. Change the "isArray" option field to "size" or something, and make > it an unsigned int. We'd make its default value 1, meaning "not an > array" and edit any existing usage that sets it to False to use 1 > instead. > > 3. It seems to me that the current semantics for an "array" option is > that they're variable length, i.e. it accepts as many as you type in. > Perhaps we could use zero size for such an array, and otherwise insist > that we get exactly the number that were wanted? > > 4. Then in my case I could supply a size of 4, and get an array of 4 floats. > > 5. It's true there is still some checking that needs to be done on the > values to be sure they're sensible - my view here is that pipeline > handlers should deal with garbage as applications can easily submit > garbage, and here we have a way of checking that the pipeline handler > actually does this correctly! I think cam is the wrong tool to test error injection into pipelines. I also understand you wish to avoid hacking on the input parsers. I on the other hand enjoy that work so I would be more then happy to supply you with a patch you could squash into this patch. Only thing blocking me from writing one now is that I'm not sure this should be a new --zoom option (ZoomKeyValueParser) or added to the existing --stream option (extend StreamKeyValueParser). This is effect what we are discussing below so I will postpone until we have resolved this. > > > > > > + Size sensorArea = camera_->properties().get(properties::SensorOutputSize); > > > + Rectangle crop(x * sensorArea.width, > > > + y * sensorArea.height, > > > + width * sensorArea.width, > > > + height * sensorArea.height); > > > + > > > + requests.front()->controls().set(controls::IspCrop, crop); > > > > Do we wish to expose the ISP in the public API? If we do is the ISP > > (from an application point of view) the correct level to operate on? > > Would not setting this on a stream level cover more hardware? > > > > I'm thinking of an ISP that can apply different crop parameters to two > > (or more) source pads from a single sink pad. Having a control that sets > > a global crop on the ISP would that not limit such hardware or am I > > missing something? > > Indeed you're right, there is a bit of a dilemma here. There was some > discussion of this a while back, I think with Jacopo, unfortunately I > don't have any email references to hand. > > I think the feeling was that in all the real use cases that we can > imagine, you generally want the same field of view on all your > outputs. I think situations where something else would be useful are > certainly less "mainstream". I'm sure there are use cases for different fields of view. I have seen video conferencing gear that in the participant list zoom in on the center of the screen while the speakers video is a larger FoV. I thought this was Google HO but a quick test proves me wrong. In any case I'm sure this is done in software without any support from the camera, but I think there are use-cases for this. I also think the flavor of a zoom control is in the same area as a rotation control. And rotation is something we sure wish to have per stream. So I think no matter which controls go first we are reaching the point we need to add support for per stream controls. > > Secondly, some ISPs will let you crop outputs differently, and some > won't. That's an awkward thing to handle - how would you know what the > underlying hardware does, and then would you write different code for > each case? Applications must be able to enumerate the features of the camera and then enable or disable certain use-cases based on the enumerated features. Of course this might lead to the applications rejecting a camera as it don't support the features it needs (A raw capture application would likely reject a camera that can't capture in a raw format it knows how to process). > The pragmatic answer may just be to implement the "usual" > case here, and it becomes a question for the future whether we get > per-stream crop controls, or whether a StreamConfiguration gets crop > rectangles... I think we need to implement both the usual and special cases, but we need to do so in a user-friendly way. One way to do that is to provide helpers for applications. I'm thinking as we know controls like digital zoom and rotation depending on the hardware capabilities can apply to individual or all streams the low-level API must allow for the most complex setup, controls on the stream level. But as you say their are use-cases where an application just wish to zoom or rotate all streams and then we could add helpers for that, maybe something simple like this would work? Request::setControlAllStreams(Control, Value); Then applications who want to rotate or zoom all streams use such helpers while special case applications will enumerate the camera features and detect if it's possible to rotate or zoom on indivudual streams and if so do it. As Jacopo points out, we know Laurent is working in this area so I'm sure he has more comments or at least a different viewpoint then mine ;-) > > Maybe Jacopo can comment if I've got anything wrong there! > > Thanks again and best regards > David > > > > > > + } else { > > > + std::cout << "Invalid zoom values - ignoring" << std::endl; > > > + } > > > + } > > > + > > > ret = camera_->start(); > > > if (ret) { > > > std::cout << "Failed to start capture" << std::endl; > > > diff --git a/src/cam/capture.h b/src/cam/capture.h > > > index 0aebdac9..52806164 100644 > > > --- a/src/cam/capture.h > > > +++ b/src/cam/capture.h > > > @@ -29,7 +29,7 @@ public: > > > > > > int run(const OptionsParser::Options &options); > > > private: > > > - int capture(libcamera::FrameBufferAllocator *allocator); > > > + int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options); > > > > > > void requestComplete(libcamera::Request *request); > > > > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > > index 244720b4..b871d049 100644 > > > --- a/src/cam/main.cpp > > > +++ b/src/cam/main.cpp > > > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[]) > > > parser.addOption(OptStrictFormats, OptionNone, > > > "Do not allow requested stream format(s) to be adjusted", > > > "strict-formats"); > > > + parser.addOption(OptZoom, OptionString, > > > + "Zoom in on the image portion described by the x,y,width,height rectangle (fractions of the maximum FOV) e.g. 0.25,0.25,0.5,0.5", > > > + "zoom", ArgumentRequired, "zoom"); > > > > > > options_ = parser.parse(argc, argv); > > > if (!options_.valid()) > > > diff --git a/src/cam/main.h b/src/cam/main.h > > > index ea8dfd33..a1fd899f 100644 > > > --- a/src/cam/main.h > > > +++ b/src/cam/main.h > > > @@ -17,6 +17,7 @@ enum { > > > OptListProperties = 'p', > > > OptMonitor = 'm', > > > OptStream = 's', > > > + OptZoom = 'z', > > > OptListControls = 256, > > > OptStrictFormats = 257, > > > }; > > > -- > > > 2.20.1 > > > > > > _______________________________________________ > > > libcamera-devel mailing list > > > libcamera-devel@lists.libcamera.org > > > https://lists.libcamera.org/listinfo/libcamera-devel > > > > -- > > Regards, > > Niklas Söderlund
Hi Niklas Thanks for your comments. On Wed, 30 Sep 2020 at 12:32, Niklas Söderlund <niklas.soderlund@ragnatech.se> wrote: > > Hi David, > > On 2020-09-30 08:00:18 +0100, David Plowman wrote: > > Hi Niklas > > > > Thanks for the comments, and sorry for not addressing them sooner! > > > > On Tue, 29 Sep 2020 at 23:33, Niklas Söderlund > > <niklas.soderlund@ragnatech.se> wrote: > > > > > > Hi David, > > > > > > Thanks for your patch. > > > > > > On 2020-09-29 17:40:00 +0100, David Plowman wrote: > > > > This allows the user to request digital zoom by adding, for example, > > > > "-z 0.25,0.25,0.5,0.5" which would give a 2x zoom, still centred in > > > > the middle of the image. > > > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > > > If I understand the cover letter correctly this is just a rebase so I > > > > Correct, apart from a couple of minor mods (though one rather important one!) > > > > > understand not all comments from v2 are addressed. But just to reiterate > > > my comments from v2. > > > > > > > --- > > > > src/cam/capture.cpp | 25 +++++++++++++++++++++++-- > > > > src/cam/capture.h | 2 +- > > > > src/cam/main.cpp | 3 +++ > > > > src/cam/main.h | 1 + > > > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > > > > index 5510c009..3b50c591 100644 > > > > --- a/src/cam/capture.cpp > > > > +++ b/src/cam/capture.cpp > > > > @@ -10,6 +10,9 @@ > > > > #include <limits.h> > > > > #include <sstream> > > > > > > > > +#include <libcamera/control_ids.h> > > > > +#include <libcamera/property_ids.h> > > > > + > > > > #include "capture.h" > > > > #include "main.h" > > > > > > > > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options) > > > > > > > > FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_); > > > > > > > > - ret = capture(allocator); > > > > + ret = capture(allocator, options); > > > > > > > > if (options.isSet(OptFile)) { > > > > delete writer_; > > > > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options) > > > > return ret; > > > > } > > > > > > > > -int Capture::capture(FrameBufferAllocator *allocator) > > > > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options) > > > > { > > > > int ret; > > > > > > > > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator) > > > > requests.push_back(request); > > > > } > > > > > > > > + /* Set up digital zoom if it was requested on the command line. */ > > > > + if (options.isSet(OptZoom)) { > > > > + std::string zoom = options[OptZoom].toString(); > > > > + float x, y, width, height; > > > > + > > > > + if (sscanf(zoom.c_str(), "%f,%f,%f,%f", &x, &y, &width, &height) == 4) { > > > > > > This is the wrong place to parse the input from the user. If the zoom > > > options are global something similar like StreamKeyValueParser > > > (ZoomKeyValueParser?) should be created. If it is to be moved a stream > > > level StreamKeyValueParser should be extended. > > > > Yes, I was kind of hoping not to get too involved with all that! > > :-) > > > How about this: > > > > 1. Add an OptionFloat type > > > > 2. Change the "isArray" option field to "size" or something, and make > > it an unsigned int. We'd make its default value 1, meaning "not an > > array" and edit any existing usage that sets it to False to use 1 > > instead. > > > > 3. It seems to me that the current semantics for an "array" option is > > that they're variable length, i.e. it accepts as many as you type in. > > Perhaps we could use zero size for such an array, and otherwise insist > > that we get exactly the number that were wanted? > > > > 4. Then in my case I could supply a size of 4, and get an array of 4 floats. > > > > 5. It's true there is still some checking that needs to be done on the > > values to be sure they're sensible - my view here is that pipeline > > handlers should deal with garbage as applications can easily submit > > garbage, and here we have a way of checking that the pipeline handler > > actually does this correctly! > > I think cam is the wrong tool to test error injection into pipelines. I > also understand you wish to avoid hacking on the input parsers. I on the > other hand enjoy that work so I would be more then happy to supply you > with a patch you could squash into this patch. > > Only thing blocking me from writing one now is that I'm not sure this > should be a new --zoom option (ZoomKeyValueParser) or added to the > existing --stream option (extend StreamKeyValueParser). This is effect > what we are discussing below so I will postpone until we have resolved > this. Well, any help is always appreciated! As I said initially, I'm very happy to drop this patch, and we could let another one replace it. > > > > > > > > > > + Size sensorArea = camera_->properties().get(properties::SensorOutputSize); > > > > + Rectangle crop(x * sensorArea.width, > > > > + y * sensorArea.height, > > > > + width * sensorArea.width, > > > > + height * sensorArea.height); > > > > + > > > > + requests.front()->controls().set(controls::IspCrop, crop); > > > > > > Do we wish to expose the ISP in the public API? If we do is the ISP > > > (from an application point of view) the correct level to operate on? > > > Would not setting this on a stream level cover more hardware? > > > > > > I'm thinking of an ISP that can apply different crop parameters to two > > > (or more) source pads from a single sink pad. Having a control that sets > > > a global crop on the ISP would that not limit such hardware or am I > > > missing something? > > > > Indeed you're right, there is a bit of a dilemma here. There was some > > discussion of this a while back, I think with Jacopo, unfortunately I > > don't have any email references to hand. > > > > I think the feeling was that in all the real use cases that we can > > imagine, you generally want the same field of view on all your > > outputs. I think situations where something else would be useful are > > certainly less "mainstream". > > I'm sure there are use cases for different fields of view. I have seen > video conferencing gear that in the participant list zoom in on the > center of the screen while the speakers video is a larger FoV. I thought > this was Google HO but a quick test proves me wrong. In any case I'm > sure this is done in software without any support from the camera, but I > think there are use-cases for this. So yes, I agree there are use cases, and indeed I've seen this kind of use case before. But I've seen it done using software too, which kind of underlines the whole problem. Namely, are applications going to write different versions of code for underlying hardware with different features? Unless the developer is targeting particular hardware known to have this feature they will try to avoid it. But in any case, we have to expose what we regard as our minimum expected hardware features, which would probably be a single zoom control, and make that convenient to use. > > I also think the flavor of a zoom control is in the same area as a > rotation control. And rotation is something we sure wish to have per > stream. So I think no matter which controls go first we are reaching the > point we need to add support for per stream controls. Agreed, rotation (or transform) control is a case in point too. To be fair, we've followed a similar procedure there and allowed for a single per-camera transform (the CameraConfiguration has a field to request this transform). The question of per-stream transforms is tricky - many ISPs won't support it which limits its usefulness, others may support them only at startup, others per-frame, and so that whole question is deferred for the moment. > > > > > Secondly, some ISPs will let you crop outputs differently, and some > > won't. That's an awkward thing to handle - how would you know what the > > underlying hardware does, and then would you write different code for > > each case? > > Applications must be able to enumerate the features of the camera and > then enable or disable certain use-cases based on the enumerated > features. Of course this might lead to the applications rejecting a > camera as it don't support the features it needs (A raw capture > application would likely reject a camera that can't capture in a raw > format it knows how to process). > > > The pragmatic answer may just be to implement the "usual" > > case here, and it becomes a question for the future whether we get > > per-stream crop controls, or whether a StreamConfiguration gets crop > > rectangles... > > I think we need to implement both the usual and special cases, but we > need to do so in a user-friendly way. One way to do that is to provide > helpers for applications. > > I'm thinking as we know controls like digital zoom and rotation > depending on the hardware capabilities can apply to individual or all > streams the low-level API must allow for the most complex setup, > controls on the stream level. But as you say their are use-cases where > an application just wish to zoom or rotate all streams and then we could > add helpers for that, maybe something simple like this would work? > > Request::setControlAllStreams(Control, Value); > > Then applications who want to rotate or zoom all streams use such > helpers while special case applications will enumerate the camera > features and detect if it's possible to rotate or zoom on indivudual > streams and if so do it. Well, I'm happy to go with this approach too. For us, digital zoom is an important feature, so if we could come to some consensus that would be very helpful! Best regards David > > As Jacopo points out, we know Laurent is working in this area so I'm > sure he has more comments or at least a different viewpoint then mine > ;-) > > > > > Maybe Jacopo can comment if I've got anything wrong there! > > > > Thanks again and best regards > > David > > > > > > > > > + } else { > > > > + std::cout << "Invalid zoom values - ignoring" << std::endl; > > > > + } > > > > + } > > > > + > > > > ret = camera_->start(); > > > > if (ret) { > > > > std::cout << "Failed to start capture" << std::endl; > > > > diff --git a/src/cam/capture.h b/src/cam/capture.h > > > > index 0aebdac9..52806164 100644 > > > > --- a/src/cam/capture.h > > > > +++ b/src/cam/capture.h > > > > @@ -29,7 +29,7 @@ public: > > > > > > > > int run(const OptionsParser::Options &options); > > > > private: > > > > - int capture(libcamera::FrameBufferAllocator *allocator); > > > > + int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options); > > > > > > > > void requestComplete(libcamera::Request *request); > > > > > > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > > > index 244720b4..b871d049 100644 > > > > --- a/src/cam/main.cpp > > > > +++ b/src/cam/main.cpp > > > > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[]) > > > > parser.addOption(OptStrictFormats, OptionNone, > > > > "Do not allow requested stream format(s) to be adjusted", > > > > "strict-formats"); > > > > + parser.addOption(OptZoom, OptionString, > > > > + "Zoom in on the image portion described by the x,y,width,height rectangle (fractions of the maximum FOV) e.g. 0.25,0.25,0.5,0.5", > > > > + "zoom", ArgumentRequired, "zoom"); > > > > > > > > options_ = parser.parse(argc, argv); > > > > if (!options_.valid()) > > > > diff --git a/src/cam/main.h b/src/cam/main.h > > > > index ea8dfd33..a1fd899f 100644 > > > > --- a/src/cam/main.h > > > > +++ b/src/cam/main.h > > > > @@ -17,6 +17,7 @@ enum { > > > > OptListProperties = 'p', > > > > OptMonitor = 'm', > > > > OptStream = 's', > > > > + OptZoom = 'z', > > > > OptListControls = 256, > > > > OptStrictFormats = 257, > > > > }; > > > > -- > > > > 2.20.1 > > > > > > > > _______________________________________________ > > > > libcamera-devel mailing list > > > > libcamera-devel@lists.libcamera.org > > > > https://lists.libcamera.org/listinfo/libcamera-devel > > > > > > -- > > > Regards, > > > Niklas Söderlund > > -- > Regards, > Niklas Söderlund
Hi Niklas, On Wed, Sep 30, 2020 at 12:33:56AM +0200, Niklas Söderlund wrote: > On 2020-09-29 17:40:00 +0100, David Plowman wrote: > > This allows the user to request digital zoom by adding, for example, > > "-z 0.25,0.25,0.5,0.5" which would give a 2x zoom, still centred in > > the middle of the image. > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > If I understand the cover letter correctly this is just a rebase so I > understand not all comments from v2 are addressed. But just to reiterate > my comments from v2. > > > --- > > src/cam/capture.cpp | 25 +++++++++++++++++++++++-- > > src/cam/capture.h | 2 +- > > src/cam/main.cpp | 3 +++ > > src/cam/main.h | 1 + > > 4 files changed, 28 insertions(+), 3 deletions(-) > > > > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp > > index 5510c009..3b50c591 100644 > > --- a/src/cam/capture.cpp > > +++ b/src/cam/capture.cpp > > @@ -10,6 +10,9 @@ > > #include <limits.h> > > #include <sstream> > > > > +#include <libcamera/control_ids.h> > > +#include <libcamera/property_ids.h> > > + > > #include "capture.h" > > #include "main.h" > > > > @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options) > > > > FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_); > > > > - ret = capture(allocator); > > + ret = capture(allocator, options); > > > > if (options.isSet(OptFile)) { > > delete writer_; > > @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options) > > return ret; > > } > > > > -int Capture::capture(FrameBufferAllocator *allocator) > > +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options) > > { > > int ret; > > > > @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator) > > requests.push_back(request); > > } > > > > + /* Set up digital zoom if it was requested on the command line. */ > > + if (options.isSet(OptZoom)) { > > + std::string zoom = options[OptZoom].toString(); > > + float x, y, width, height; > > + > > + if (sscanf(zoom.c_str(), "%f,%f,%f,%f", &x, &y, &width, &height) == 4) { > > This is the wrong place to parse the input from the user. If the zoom > options are global something similar like StreamKeyValueParser > (ZoomKeyValueParser?) should be created. If it is to be moved a stream > level StreamKeyValueParser should be extended. > > > + Size sensorArea = camera_->properties().get(properties::SensorOutputSize); > > + Rectangle crop(x * sensorArea.width, > > + y * sensorArea.height, > > + width * sensorArea.width, > > + height * sensorArea.height); > > + > > + requests.front()->controls().set(controls::IspCrop, crop); > > Do we wish to expose the ISP in the public API? If we do is the ISP > (from an application point of view) the correct level to operate on? > Would not setting this on a stream level cover more hardware? > > I'm thinking of an ISP that can apply different crop parameters to two > (or more) source pads from a single sink pad. Having a control that sets > a global crop on the ISP would that not limit such hardware or am I > missing something? (Copied from a previous version, to centralize all discussions in this series) This is a very good question, and a very fundamental design decision we need to make. I've started thinking that we need to define and document an explicit abstract pipeline model for cameras, striking the right balance between flexibility and adaptability to different hardware architectures, and ease of implementation for pipeline handlers and of use for applications. In this context, David mentioned in a previous version of this series that, while different streams could indeed have different zoom levels, use cases for this feature were not immediately apparent. We could thus leave this out of our pipeline model. If we ever want to support this feature in the future, I think we will be able to do so with the help of per-stream controls in requests, using the same IspCrop control, so it appears we would have a way forward. > > + } else { > > + std::cout << "Invalid zoom values - ignoring" << std::endl; > > + } > > + } > > + > > ret = camera_->start(); > > if (ret) { > > std::cout << "Failed to start capture" << std::endl; > > diff --git a/src/cam/capture.h b/src/cam/capture.h > > index 0aebdac9..52806164 100644 > > --- a/src/cam/capture.h > > +++ b/src/cam/capture.h > > @@ -29,7 +29,7 @@ public: > > > > int run(const OptionsParser::Options &options); > > private: > > - int capture(libcamera::FrameBufferAllocator *allocator); > > + int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options); > > > > void requestComplete(libcamera::Request *request); > > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index 244720b4..b871d049 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[]) > > parser.addOption(OptStrictFormats, OptionNone, > > "Do not allow requested stream format(s) to be adjusted", > > "strict-formats"); > > + parser.addOption(OptZoom, OptionString, > > + "Zoom in on the image portion described by the x,y,width,height rectangle (fractions of the maximum FOV) e.g. 0.25,0.25,0.5,0.5", > > + "zoom", ArgumentRequired, "zoom"); > > > > options_ = parser.parse(argc, argv); > > if (!options_.valid()) > > diff --git a/src/cam/main.h b/src/cam/main.h > > index ea8dfd33..a1fd899f 100644 > > --- a/src/cam/main.h > > +++ b/src/cam/main.h > > @@ -17,6 +17,7 @@ enum { > > OptListProperties = 'p', > > OptMonitor = 'm', > > OptStream = 's', > > + OptZoom = 'z', > > OptListControls = 256, > > OptStrictFormats = 257, > > };
diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 5510c009..3b50c591 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -10,6 +10,9 @@ #include <limits.h> #include <sstream> +#include <libcamera/control_ids.h> +#include <libcamera/property_ids.h> + #include "capture.h" #include "main.h" @@ -58,7 +61,7 @@ int Capture::run(const OptionsParser::Options &options) FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_); - ret = capture(allocator); + ret = capture(allocator, options); if (options.isSet(OptFile)) { delete writer_; @@ -70,7 +73,7 @@ int Capture::run(const OptionsParser::Options &options) return ret; } -int Capture::capture(FrameBufferAllocator *allocator) +int Capture::capture(FrameBufferAllocator *allocator, const OptionsParser::Options &options) { int ret; @@ -120,6 +123,24 @@ int Capture::capture(FrameBufferAllocator *allocator) requests.push_back(request); } + /* Set up digital zoom if it was requested on the command line. */ + if (options.isSet(OptZoom)) { + std::string zoom = options[OptZoom].toString(); + float x, y, width, height; + + if (sscanf(zoom.c_str(), "%f,%f,%f,%f", &x, &y, &width, &height) == 4) { + Size sensorArea = camera_->properties().get(properties::SensorOutputSize); + Rectangle crop(x * sensorArea.width, + y * sensorArea.height, + width * sensorArea.width, + height * sensorArea.height); + + requests.front()->controls().set(controls::IspCrop, crop); + } else { + std::cout << "Invalid zoom values - ignoring" << std::endl; + } + } + ret = camera_->start(); if (ret) { std::cout << "Failed to start capture" << std::endl; diff --git a/src/cam/capture.h b/src/cam/capture.h index 0aebdac9..52806164 100644 --- a/src/cam/capture.h +++ b/src/cam/capture.h @@ -29,7 +29,7 @@ public: int run(const OptionsParser::Options &options); private: - int capture(libcamera::FrameBufferAllocator *allocator); + int capture(libcamera::FrameBufferAllocator *allocator, const OptionsParser::Options &options); void requestComplete(libcamera::Request *request); diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 244720b4..b871d049 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -201,6 +201,9 @@ int CamApp::parseOptions(int argc, char *argv[]) parser.addOption(OptStrictFormats, OptionNone, "Do not allow requested stream format(s) to be adjusted", "strict-formats"); + parser.addOption(OptZoom, OptionString, + "Zoom in on the image portion described by the x,y,width,height rectangle (fractions of the maximum FOV) e.g. 0.25,0.25,0.5,0.5", + "zoom", ArgumentRequired, "zoom"); options_ = parser.parse(argc, argv); if (!options_.valid()) diff --git a/src/cam/main.h b/src/cam/main.h index ea8dfd33..a1fd899f 100644 --- a/src/cam/main.h +++ b/src/cam/main.h @@ -17,6 +17,7 @@ enum { OptListProperties = 'p', OptMonitor = 'm', OptStream = 's', + OptZoom = 'z', OptListControls = 256, OptStrictFormats = 257, };
This allows the user to request digital zoom by adding, for example, "-z 0.25,0.25,0.5,0.5" which would give a 2x zoom, still centred in the middle of the image. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- src/cam/capture.cpp | 25 +++++++++++++++++++++++-- src/cam/capture.h | 2 +- src/cam/main.cpp | 3 +++ src/cam/main.h | 1 + 4 files changed, 28 insertions(+), 3 deletions(-)