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

[Laurent Pinchart]

Hello,

On Wed, Jan 30, 2019 at 06:05:34PM +0100, Niklas Söderlund wrote:
> On 2019-01-30 13:46:17 +0000, Kieran Bingham wrote:
> > Hi Niklas,
> > 
> > When I rebase a series with these patches included - it fails to compile
> > each patch cleanly using the -x command.
> > 
> >  git rebase -i $BASECOMMIT -x "cd build && ninja"
> > 
> > 
> > Now I don't think that's a fault of this series, but a fault of the
> > src/cam/meson.build file not specifying the options.h header as part of
> > the sources. I think that is causing the main.cpp not to get
> > rebuilt/re-linked when there are further changes here to the options
> > file ...

$ ninja -v
ninja: no work to do.
$ touch ../src/cam/options.h
$ ninja -v
[1/3] c++ -Isrc/cam/src at cam@@cam at exe -Isrc/cam -I../src/cam -Iinclude -I../include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++11 -g -Wno-unused-parameter  -MD -MQ 'src/cam/src at cam@@cam at exe/main.cpp.o' -MF 'src/cam/src at cam@@cam at exe/main.cpp.o.d' -o 'src/cam/src at cam@@cam at exe/main.cpp.o' -c ../src/cam/main.cpp
[2/3] c++ -Isrc/cam/src at cam@@cam at exe -Isrc/cam -I../src/cam -Iinclude -I../include -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++11 -g -Wno-unused-parameter  -MD -MQ 'src/cam/src at cam@@cam at exe/options.cpp.o' -MF 'src/cam/src at cam@@cam at exe/options.cpp.o.d' -o 'src/cam/src at cam@@cam at exe/options.cpp.o' -c ../src/cam/options.cpp
[3/3] c++  -o src/cam/cam 'src/cam/src at cam@@cam at exe/event_loop.cpp.o' 'src/cam/src at cam@@cam at exe/main.cpp.o' 'src/cam/src at cam@@cam at exe/options.cpp.o' -Wl,--no-undefined -Wl,--as-needed -Wl,--start-group src/libcamera/libcamera.so -Wl,--end-group '-Wl,-rpath,$ORIGIN/../libcamera' -Wl,-rpath-link,src/libcamera

Am I missing something ?

> > However - I'm surprised, as I would have expected meson to do some sort
> > of include depend checking or generation.
> > 
> > Have you experienced anything similar here? or am I going crazy.
> 
> I have experienced it, and not only with the cam tool but the library as 
> a whole. As I understand it we don't list any header files as sources.  
> We just list them to feed them to Doxygen if I understand it correctly.  
> 
> I had it on my todo list to figure out why, but have not gotten around 
> to do it as other things have been higher on my list and my workaround 
> of using ccache and
> 
>     git rebase .. --exec "cd build && ninja clean && ninja"
> 
> allowed me to move forward :-)
> 
> > 
> > Now I think about it - the options.cpp changes too - so it should still
> > rebuild. Ugh.. so I'm not sure what the underlying fault is yet.
> 
> How do you think we should solve this? Looking around it seems other 
> people are including there header files in the source file list, should 
> we do the same?
> 
> > 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