[libcamera-devel] [PATCH 0/6] cam: add --format option to configure a stream

[Niklas Söderlund]

Hi Laurent,

On 2019-01-31 12:06:35 +0200, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Tue, Jan 29, 2019 at 09:33:32AM +0000, Kieran Bingham wrote:
> > Hi Niklas,
> > 
> > Except for a "we're writing an option parser rather than pulling one in"
> > concern, I don't see anything too wrong with this series and it's a
> > stand-alone tool (which you currently own :D )
> > 
> > So I'd say,
> > 
> > Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> >  <edit: a couple of minor notes on 6/6>
> > 
> > I think creating an opaque object to manage the camera from the
> > application point of view will give us a lot of benefits to hiding the
> > internal implementation and shared-ptrs.
> 
> That's Tomasz's proposal, right ?

To me this is the d-pointer discussion we had in the past and recoded as 
'Research ABI stability with regard to private data' topic. And if I 
understand Tomasz proposal his view is somewhat aligned whit this, but I 
might have misunderstood.

> 
> > Then an application can get a single 'unique' ptr - and it's destruction
> > can handle all clean up on the internal calls.
> 
> Should that be a std::unique_ptr<>, or just a normal pointer ?
> 
> > Anyway - Keeping the tool in use and actively developed will help us
> > develop how the applications will use the library - so I'm all for
> > getting this series in and developing it as we go.
> > 
> > On 28/01/2019 00:41, Niklas Söderlund wrote:
> > > Hi,
> > > 
> > > This series aims to add a --format option to the cam utility so that it 
> > > can set the format of a stream of a specified camera. To do this in an 
> > > efficient manner a new key=value parser is needed to understand the 
> > > arguments given to the new option, example
> > > 
> > >     cam --camera mycam --format width=800,height=600
> > > 
> > > This series adds such a parser and then moves on to adding the new 
> > > operation to the cam utility in 6/6. This series depends on [1] for 6/6 
> > > which needs the stream API in the camera object to actually set the 
> > > requested format.
> > > 
> > > 1. [PATCH v3 0/6] libcamera: add basic support for streams and format 
> > >    configuration
> > > 
> > > Niklas Söderlund (6):
> > >   cam: options: move enum OptionArgument
> > >   cam: options: create a template class for options
> > >   cam: options: return if addOption() succeeds or not
> > >   cam: options: remove OptionsParser::options_
> > >   cam: options: add a key=value parser
> > >   cam: add --format option to configure a stream
> > > 
> > >  src/cam/main.cpp    | 107 ++++++++++++++++++++++----
> > >  src/cam/options.cpp | 178 +++++++++++++++++++++++++++++++++++++++-----
> > >  src/cam/options.h   |  70 +++++++++++++----
> > >  3 files changed, 305 insertions(+), 50 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund