| Message ID | 20190128004109.25860-7-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On 28/01/2019 00:41, Niklas Söderlund wrote: > Add a option to configure the first stream of a camera from an argument > with options and parse the width, height and pixel format from that > list. > > The pixel format is still specified as a integer which should correspond > to the kernels FOURCC identifiers. Going forward this should be turned > into a string representation and the cam parser should translate between > the two. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/main.cpp | 104 +++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 92 insertions(+), 12 deletions(-) > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index bde47a8f17983912..4b4ce9aa29c80bd1 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -20,7 +20,8 @@ using namespace libcamera; > OptionsParser::Options options; > > enum { > - OptCamera = 'c', > + OptCamera = 'C', > + OptFormat = 'f', > OptHelp = 'h', > OptList = 'l', > }; > @@ -35,10 +36,20 @@ void signalHandler(int signal) > > static int parseOptions(int argc, char *argv[]) > { > + KeyValueParser formatKeyValue; > + formatKeyValue.addOption("width", "Set width in pixels", > + ArgumentRequired, "w"); > + formatKeyValue.addOption("height", "Set height in pixels", > + ArgumentRequired, "h"); > + formatKeyValue.addOption("pixelformat", "Set pixel format", > + ArgumentRequired, "pf"); Hrm - can a short-option have two letters? (if so, should it?) > + > OptionsParser parser; > > parser.addOption(OptCamera, "Specify which camera to operate on", > "camera", ArgumentRequired, "camera"); > + parser.addOption(OptFormat, "Set format of the cameras first stream", > + formatKeyValue, "format"); > parser.addOption(OptHelp, "Display this help message", "help"); > parser.addOption(OptList, "List all cameras", "list"); > > @@ -54,6 +65,42 @@ static int parseOptions(int argc, char *argv[]) > return 0; > } > > +int str2uint(std::string str, unsigned int *dest) > +{ > + if (!dest || str == "" || !sscanf(str.c_str(), "%d", dest)) > + return -EINVAL; > + return 0; > +} > + > +bool configureStreams(Camera *camera, std::vector<Stream> &streams) > +{ > + if (!options.isKeyValue(OptFormat)) > + return false; > + > + KeyValueParser::Options format = options.keyValues(OptFormat); > + > + std::map<unsigned int, StreamConfiguration> config = camera->streamConfiguration(streams); > + unsigned int id = streams.front().id(); > + > + if (format.isSet("width")) > + if (str2uint(format["width"], &config[id].width)) > + return false; > + > + if (format.isSet("height")) > + if (str2uint(format["height"], &config[id].height)) > + return false; > + > + /* TODO: Translate 4CC string to ID. */ > + if (format.isSet("pixelformat")) > + if (str2uint(format["pixelformat"], &config[id].pixelFormat)) > + return false; > + > + if (camera->configureStreams(config)) > + return false; > + > + return true; > +} > + > int main(int argc, char **argv) > { > int ret; > @@ -63,6 +110,8 @@ int main(int argc, char **argv) > return EXIT_FAILURE; > > CameraManager *cm = CameraManager::instance(); > + std::shared_ptr<Camera> camera; > + std::vector<Stream> streams; > > ret = cm->start(); > if (ret) { > @@ -71,31 +120,62 @@ int main(int argc, char **argv) > return EXIT_FAILURE; > } > > + loop = new EventLoop(cm->eventDispatcher()); > + > + struct sigaction sa = {}; > + sa.sa_handler = &signalHandler; > + sigaction(SIGINT, &sa, nullptr); > + > if (options.isSet(OptList)) { > std::cout << "Available cameras:" << std::endl; > - for (const std::shared_ptr<Camera> &camera : cm->cameras()) > - std::cout << "- " << camera->name() << std::endl; > + for (const std::shared_ptr<Camera> &cam : cm->cameras()) > + std::cout << "- " << cam->name() << std::endl; > } > > if (options.isSet(OptCamera)) { > - std::shared_ptr<Camera> cam = cm->get(options[OptCamera]); > - > - if (cam) { > - std::cout << "Using camera " << cam->name() << std::endl; > - } else { > + camera = cm->get(options[OptCamera]); > + if (!camera) { > std::cout << "Camera " << options[OptCamera] > << " not found" << std::endl; > + goto out; > + } > + > + streams = camera->streams(); > + if (streams.size() != 1) { > + std::cout << "Camera have " << streams.size() Camera 'has' N streams... > + << " streams, I only know how to work with 1" > + << std::endl; > + goto out; > + } > + > + if (camera->acquire()) { > + std::cout << "Failed to acquire camera" << std::endl; > + goto out; > } > + > + std::cout << "Using camera " << camera->name() << std::endl; > } > > - loop = new EventLoop(cm->eventDispatcher()); > + if (options.isSet(OptFormat)) { > + if (!camera) { > + std::cout << "Can't configure stream, no camera selected" << std::endl; > + goto out_camera; > + } > > - struct sigaction sa = {}; > - sa.sa_handler = &signalHandler; > - sigaction(SIGINT, &sa, nullptr); > + if (!configureStreams(camera.get(), streams)) { > + std::cout << "Failed to configure camera" << std::endl; > + goto out_camera; > + } > + } > > ret = loop->exec(); > > +out_camera: > + if (camera) { > + camera->release(); > + camera.reset(); Uhm ... first a pointer call '->' then a direct member call ? '.' ? > + } > +out: > delete loop; > > cm->stop(); >
Hi Kieran, Thanks for your comment. On 2019-01-29 09:29:37 +0000, Kieran Bingham wrote: > Hi Niklas, > > On 28/01/2019 00:41, Niklas Söderlund wrote: > > Add a option to configure the first stream of a camera from an argument > > with options and parse the width, height and pixel format from that > > list. > > > > The pixel format is still specified as a integer which should correspond > > to the kernels FOURCC identifiers. Going forward this should be turned > > into a string representation and the cam parser should translate between > > the two. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/cam/main.cpp | 104 +++++++++++++++++++++++++++++++++++++++++------ > > 1 file changed, 92 insertions(+), 12 deletions(-) > > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index bde47a8f17983912..4b4ce9aa29c80bd1 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -20,7 +20,8 @@ using namespace libcamera; > > OptionsParser::Options options; > > > > enum { > > - OptCamera = 'c', > > + OptCamera = 'C', > > + OptFormat = 'f', > > OptHelp = 'h', > > OptList = 'l', > > }; > > @@ -35,10 +36,20 @@ void signalHandler(int signal) > > > > static int parseOptions(int argc, char *argv[]) > > { > > + KeyValueParser formatKeyValue; > > + formatKeyValue.addOption("width", "Set width in pixels", > > + ArgumentRequired, "w"); > > + formatKeyValue.addOption("height", "Set height in pixels", > > + ArgumentRequired, "h"); > > + formatKeyValue.addOption("pixelformat", "Set pixel format", > > + ArgumentRequired, "pf"); > > Hrm - can a short-option have two letters? (if so, should it?) This is not a shot-option :-) It is a descriptive name for the parameter only used for printing the usage. The usage printing for the whole --format options: -f, --format key=value[,key=value,...] Set format of the cameras first stream height=h Set height in pixels pixelformat=pf Set pixel format width=w Set width in pixels And one would use it as --format width=800,pixelformat=42 > > > + > > OptionsParser parser; > > > > parser.addOption(OptCamera, "Specify which camera to operate on", > > "camera", ArgumentRequired, "camera"); > > + parser.addOption(OptFormat, "Set format of the cameras first stream", > > + formatKeyValue, "format"); > > parser.addOption(OptHelp, "Display this help message", "help"); > > parser.addOption(OptList, "List all cameras", "list"); > > > > @@ -54,6 +65,42 @@ static int parseOptions(int argc, char *argv[]) > > return 0; > > } > > > > +int str2uint(std::string str, unsigned int *dest) > > +{ > > + if (!dest || str == "" || !sscanf(str.c_str(), "%d", dest)) > > + return -EINVAL; > > + return 0; > > +} > > + > > +bool configureStreams(Camera *camera, std::vector<Stream> &streams) > > +{ > > + if (!options.isKeyValue(OptFormat)) > > + return false; > > + > > + KeyValueParser::Options format = options.keyValues(OptFormat); > > + > > + std::map<unsigned int, StreamConfiguration> config = camera->streamConfiguration(streams); > > + unsigned int id = streams.front().id(); > > + > > + if (format.isSet("width")) > > + if (str2uint(format["width"], &config[id].width)) > > + return false; > > + > > + if (format.isSet("height")) > > + if (str2uint(format["height"], &config[id].height)) > > + return false; > > + > > + /* TODO: Translate 4CC string to ID. */ > > + if (format.isSet("pixelformat")) > > + if (str2uint(format["pixelformat"], &config[id].pixelFormat)) > > + return false; > > + > > + if (camera->configureStreams(config)) > > + return false; > > + > > + return true; > > +} > > + > > int main(int argc, char **argv) > > { > > int ret; > > @@ -63,6 +110,8 @@ int main(int argc, char **argv) > > return EXIT_FAILURE; > > > > CameraManager *cm = CameraManager::instance(); > > + std::shared_ptr<Camera> camera; > > + std::vector<Stream> streams; > > > > ret = cm->start(); > > if (ret) { > > @@ -71,31 +120,62 @@ int main(int argc, char **argv) > > return EXIT_FAILURE; > > } > > > > + loop = new EventLoop(cm->eventDispatcher()); > > + > > + struct sigaction sa = {}; > > + sa.sa_handler = &signalHandler; > > + sigaction(SIGINT, &sa, nullptr); > > + > > if (options.isSet(OptList)) { > > std::cout << "Available cameras:" << std::endl; > > - for (const std::shared_ptr<Camera> &camera : cm->cameras()) > > - std::cout << "- " << camera->name() << std::endl; > > + for (const std::shared_ptr<Camera> &cam : cm->cameras()) > > + std::cout << "- " << cam->name() << std::endl; > > } > > > > if (options.isSet(OptCamera)) { > > - std::shared_ptr<Camera> cam = cm->get(options[OptCamera]); > > - > > - if (cam) { > > - std::cout << "Using camera " << cam->name() << std::endl; > > - } else { > > + camera = cm->get(options[OptCamera]); > > + if (!camera) { > > std::cout << "Camera " << options[OptCamera] > > << " not found" << std::endl; > > + goto out; > > + } > > + > > + streams = camera->streams(); > > + if (streams.size() != 1) { > > + std::cout << "Camera have " << streams.size() > > Camera 'has' N streams... Thanks :-) > > > + << " streams, I only know how to work with 1" > > + << std::endl; > > + goto out; > > + } > > + > > + if (camera->acquire()) { > > + std::cout << "Failed to acquire camera" << std::endl; > > + goto out; > > } > > + > > + std::cout << "Using camera " << camera->name() << std::endl; > > } > > > > - loop = new EventLoop(cm->eventDispatcher()); > > + if (options.isSet(OptFormat)) { > > + if (!camera) { > > + std::cout << "Can't configure stream, no camera selected" << std::endl; > > + goto out_camera; > > + } > > > > - struct sigaction sa = {}; > > - sa.sa_handler = &signalHandler; > > - sigaction(SIGINT, &sa, nullptr); > > + if (!configureStreams(camera.get(), streams)) { > > + std::cout << "Failed to configure camera" << std::endl; > > + goto out_camera; > > + } > > + } > > > > ret = loop->exec(); > > > > +out_camera: > > + if (camera) { > > + camera->release(); > > + camera.reset(); > > Uhm ... first a pointer call '->' then a direct member call ? '.' ? camera is a shard_ptr<Camera*> so the ->release() access the Camera pointers that the shard_ptr<> holds while .reset() clears the shard_ptr<> of the reference to the Camera hence reducing the refcount by one and potentialy deleting the Camera object. > > > > + } > > +out: > > delete loop; > > > > cm->stop(); > > > > -- > Regards > -- > Kieran
Hi Niklas, On 29/01/2019 13:34, Niklas Söderlund wrote: > Hi Kieran, > > Thanks for your comment. > > On 2019-01-29 09:29:37 +0000, Kieran Bingham wrote: >> Hi Niklas, >> >> On 28/01/2019 00:41, Niklas Söderlund wrote: >>> Add a option to configure the first stream of a camera from an argument >>> with options and parse the width, height and pixel format from that >>> list. >>> >>> The pixel format is still specified as a integer which should correspond >>> to the kernels FOURCC identifiers. Going forward this should be turned >>> into a string representation and the cam parser should translate between >>> the two. >>> >>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> >>> --- >>> src/cam/main.cpp | 104 +++++++++++++++++++++++++++++++++++++++++------ >>> 1 file changed, 92 insertions(+), 12 deletions(-) >>> >>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp >>> index bde47a8f17983912..4b4ce9aa29c80bd1 100644 >>> --- a/src/cam/main.cpp >>> +++ b/src/cam/main.cpp >>> @@ -20,7 +20,8 @@ using namespace libcamera; >>> OptionsParser::Options options; >>> >>> enum { >>> - OptCamera = 'c', >>> + OptCamera = 'C', >>> + OptFormat = 'f', >>> OptHelp = 'h', >>> OptList = 'l', >>> }; >>> @@ -35,10 +36,20 @@ void signalHandler(int signal) >>> >>> static int parseOptions(int argc, char *argv[]) >>> { >>> + KeyValueParser formatKeyValue; >>> + formatKeyValue.addOption("width", "Set width in pixels", >>> + ArgumentRequired, "w"); >>> + formatKeyValue.addOption("height", "Set height in pixels", >>> + ArgumentRequired, "h"); >>> + formatKeyValue.addOption("pixelformat", "Set pixel format", >>> + ArgumentRequired, "pf"); >> >> Hrm - can a short-option have two letters? (if so, should it?) > > This is not a shot-option :-) It is a descriptive name for the parameter > only used for printing the usage. The usage printing for the whole > --format options: > > -f, --format key=value[,key=value,...] Set format of the cameras first stream > height=h Set height in pixels > pixelformat=pf Set pixel format > width=w Set width in pixels > > And one would use it as --format width=800,pixelformat=42 Ah - that's clear - this is fine then. > >> >>> + >>> OptionsParser parser; >>> >>> parser.addOption(OptCamera, "Specify which camera to operate on", >>> "camera", ArgumentRequired, "camera"); >>> + parser.addOption(OptFormat, "Set format of the cameras first stream", >>> + formatKeyValue, "format"); >>> parser.addOption(OptHelp, "Display this help message", "help"); >>> parser.addOption(OptList, "List all cameras", "list"); >>> >>> @@ -54,6 +65,42 @@ static int parseOptions(int argc, char *argv[]) >>> return 0; >>> } >>> >>> +int str2uint(std::string str, unsigned int *dest) >>> +{ >>> + if (!dest || str == "" || !sscanf(str.c_str(), "%d", dest)) >>> + return -EINVAL; >>> + return 0; >>> +} >>> + >>> +bool configureStreams(Camera *camera, std::vector<Stream> &streams) >>> +{ >>> + if (!options.isKeyValue(OptFormat)) >>> + return false; >>> + >>> + KeyValueParser::Options format = options.keyValues(OptFormat); >>> + >>> + std::map<unsigned int, StreamConfiguration> config = camera->streamConfiguration(streams); >>> + unsigned int id = streams.front().id(); >>> + >>> + if (format.isSet("width")) >>> + if (str2uint(format["width"], &config[id].width)) >>> + return false; >>> + >>> + if (format.isSet("height")) >>> + if (str2uint(format["height"], &config[id].height)) >>> + return false; >>> + >>> + /* TODO: Translate 4CC string to ID. */ >>> + if (format.isSet("pixelformat")) >>> + if (str2uint(format["pixelformat"], &config[id].pixelFormat)) >>> + return false; >>> + >>> + if (camera->configureStreams(config)) >>> + return false; >>> + >>> + return true; >>> +} >>> + >>> int main(int argc, char **argv) >>> { >>> int ret; >>> @@ -63,6 +110,8 @@ int main(int argc, char **argv) >>> return EXIT_FAILURE; >>> >>> CameraManager *cm = CameraManager::instance(); >>> + std::shared_ptr<Camera> camera; >>> + std::vector<Stream> streams; >>> >>> ret = cm->start(); >>> if (ret) { >>> @@ -71,31 +120,62 @@ int main(int argc, char **argv) >>> return EXIT_FAILURE; >>> } >>> >>> + loop = new EventLoop(cm->eventDispatcher()); >>> + >>> + struct sigaction sa = {}; >>> + sa.sa_handler = &signalHandler; >>> + sigaction(SIGINT, &sa, nullptr); >>> + >>> if (options.isSet(OptList)) { >>> std::cout << "Available cameras:" << std::endl; >>> - for (const std::shared_ptr<Camera> &camera : cm->cameras()) >>> - std::cout << "- " << camera->name() << std::endl; >>> + for (const std::shared_ptr<Camera> &cam : cm->cameras()) >>> + std::cout << "- " << cam->name() << std::endl; >>> } >>> >>> if (options.isSet(OptCamera)) { >>> - std::shared_ptr<Camera> cam = cm->get(options[OptCamera]); >>> - >>> - if (cam) { >>> - std::cout << "Using camera " << cam->name() << std::endl; >>> - } else { >>> + camera = cm->get(options[OptCamera]); >>> + if (!camera) { >>> std::cout << "Camera " << options[OptCamera] >>> << " not found" << std::endl; >>> + goto out; >>> + } >>> + >>> + streams = camera->streams(); >>> + if (streams.size() != 1) { >>> + std::cout << "Camera have " << streams.size() >> >> Camera 'has' N streams... > > Thanks :-) > >> >>> + << " streams, I only know how to work with 1" >>> + << std::endl; >>> + goto out; >>> + } >>> + >>> + if (camera->acquire()) { >>> + std::cout << "Failed to acquire camera" << std::endl; >>> + goto out; >>> } >>> + >>> + std::cout << "Using camera " << camera->name() << std::endl; >>> } >>> >>> - loop = new EventLoop(cm->eventDispatcher()); >>> + if (options.isSet(OptFormat)) { >>> + if (!camera) { >>> + std::cout << "Can't configure stream, no camera selected" << std::endl; >>> + goto out_camera; >>> + } >>> >>> - struct sigaction sa = {}; >>> - sa.sa_handler = &signalHandler; >>> - sigaction(SIGINT, &sa, nullptr); >>> + if (!configureStreams(camera.get(), streams)) { >>> + std::cout << "Failed to configure camera" << std::endl; >>> + goto out_camera; >>> + } >>> + } >>> >>> ret = loop->exec(); >>> >>> +out_camera: >>> + if (camera) { >>> + camera->release(); >>> + camera.reset(); >> >> Uhm ... first a pointer call '->' then a direct member call ? '.' ? > > camera is a shard_ptr<Camera*> so the ->release() access the Camera > pointers that the shard_ptr<> holds while .reset() clears the > shard_ptr<> of the reference to the Camera hence reducing the refcount > by one and potentialy deleting the Camera object. Of course :) Ok - no issues there either then! -- Kieran > >> >> >>> + } >>> +out: >>> delete loop; >>> >>> cm->stop(); >>> >> >> -- >> Regards >> -- >> Kieran >
Hi Niklas, Thank you for the patch. On Mon, Jan 28, 2019 at 01:41:09AM +0100, Niklas Söderlund wrote: > Add a option to configure the first stream of a camera from an argument s/a option/an option/ > with options and parse the width, height and pixel format from that > list. > > The pixel format is still specified as a integer which should correspond > to the kernels FOURCC identifiers. Going forward this should be turned > into a string representation and the cam parser should translate between > the two. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/main.cpp | 104 +++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 92 insertions(+), 12 deletions(-) > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index bde47a8f17983912..4b4ce9aa29c80bd1 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -20,7 +20,8 @@ using namespace libcamera; > OptionsParser::Options options; > > enum { > - OptCamera = 'c', > + OptCamera = 'C', Why do you rename this option ? > + OptFormat = 'f', > OptHelp = 'h', > OptList = 'l', > }; > @@ -35,10 +36,20 @@ void signalHandler(int signal) > > static int parseOptions(int argc, char *argv[]) > { > + KeyValueParser formatKeyValue; > + formatKeyValue.addOption("width", "Set width in pixels", I'd drop the "Set " part: "Width in pixels". Same for the other options. > + ArgumentRequired, "w"); I wonder if we need the argumentName. Is "width=w" better than "width=value" ? > + formatKeyValue.addOption("height", "Set height in pixels", > + ArgumentRequired, "h"); > + formatKeyValue.addOption("pixelformat", "Set pixel format", > + ArgumentRequired, "pf"); > + > OptionsParser parser; > > parser.addOption(OptCamera, "Specify which camera to operate on", > "camera", ArgumentRequired, "camera"); > + parser.addOption(OptFormat, "Set format of the cameras first stream", s/cameras/camera's/ > + formatKeyValue, "format"); > parser.addOption(OptHelp, "Display this help message", "help"); > parser.addOption(OptList, "List all cameras", "list"); > > @@ -54,6 +65,42 @@ static int parseOptions(int argc, char *argv[]) > return 0; > } > > +int str2uint(std::string str, unsigned int *dest) > +{ > + if (!dest || str == "" || !sscanf(str.c_str(), "%d", dest)) Wouldn't strtoul be simpler than sscanf ? > + return -EINVAL; > + return 0; > +} > + > +bool configureStreams(Camera *camera, std::vector<Stream> &streams) > +{ > + if (!options.isKeyValue(OptFormat)) Can this happen, given that OptFormat is a key-value argument ? I would drop this check, and possibly the isKeyValue() method of the parser if not needed elsewhere. > + return false; > + > + KeyValueParser::Options format = options.keyValues(OptFormat); > + > + std::map<unsigned int, StreamConfiguration> config = camera->streamConfiguration(streams); > + unsigned int id = streams.front().id(); > + > + if (format.isSet("width")) > + if (str2uint(format["width"], &config[id].width)) > + return false; Down the road I would like the parser to take a value type argument, and expose an API that will automatically convert to the right type. > + > + if (format.isSet("height")) > + if (str2uint(format["height"], &config[id].height)) > + return false; > + > + /* TODO: Translate 4CC string to ID. */ > + if (format.isSet("pixelformat")) > + if (str2uint(format["pixelformat"], &config[id].pixelFormat)) > + return false; > + > + if (camera->configureStreams(config)) > + return false; > + > + return true; > +} > + > int main(int argc, char **argv) > { > int ret; > @@ -63,6 +110,8 @@ int main(int argc, char **argv) > return EXIT_FAILURE; > > CameraManager *cm = CameraManager::instance(); > + std::shared_ptr<Camera> camera; > + std::vector<Stream> streams; > > ret = cm->start(); > if (ret) { > @@ -71,31 +120,62 @@ int main(int argc, char **argv) > return EXIT_FAILURE; > } > > + loop = new EventLoop(cm->eventDispatcher()); > + > + struct sigaction sa = {}; > + sa.sa_handler = &signalHandler; > + sigaction(SIGINT, &sa, nullptr); > + > if (options.isSet(OptList)) { > std::cout << "Available cameras:" << std::endl; > - for (const std::shared_ptr<Camera> &camera : cm->cameras()) > - std::cout << "- " << camera->name() << std::endl; > + for (const std::shared_ptr<Camera> &cam : cm->cameras()) > + std::cout << "- " << cam->name() << std::endl; > } > > if (options.isSet(OptCamera)) { > - std::shared_ptr<Camera> cam = cm->get(options[OptCamera]); > - > - if (cam) { > - std::cout << "Using camera " << cam->name() << std::endl; > - } else { > + camera = cm->get(options[OptCamera]); > + if (!camera) { > std::cout << "Camera " << options[OptCamera] > << " not found" << std::endl; Should errors be printed to cerr ? > + goto out; > + } > + > + streams = camera->streams(); > + if (streams.size() != 1) { > + std::cout << "Camera have " << streams.size() s/have/has/ > + << " streams, I only know how to work with 1" I'm not used to software talking to me, but why not ? :-) > + << std::endl; > + goto out; > + } > + > + if (camera->acquire()) { > + std::cout << "Failed to acquire camera" << std::endl; > + goto out; > } > + > + std::cout << "Using camera " << camera->name() << std::endl; > } > > - loop = new EventLoop(cm->eventDispatcher()); > + if (options.isSet(OptFormat)) { > + if (!camera) { > + std::cout << "Can't configure stream, no camera selected" << std::endl; > + goto out_camera; > + } I think we'll end up with the cam tool working in one of a small subset of modes. I foresee at least a list mode and a capture mode, and probably a few others, but not many. I'd make them mutually exclusive, with list returning immediately after printing the list of cameras, and capture requiring a camera, so this check will likely be moved out of the OptFormat check. We can refactor this later. > > - struct sigaction sa = {}; > - sa.sa_handler = &signalHandler; > - sigaction(SIGINT, &sa, nullptr); > + if (!configureStreams(camera.get(), streams)) { > + std::cout << "Failed to configure camera" << std::endl; > + goto out_camera; > + } > + } > > ret = loop->exec(); > > +out_camera: > + if (camera) { > + camera->release(); > + camera.reset(); > + } > +out: > delete loop; > > cm->stop();
diff --git a/src/cam/main.cpp b/src/cam/main.cpp index bde47a8f17983912..4b4ce9aa29c80bd1 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -20,7 +20,8 @@ using namespace libcamera; OptionsParser::Options options; enum { - OptCamera = 'c', + OptCamera = 'C', + OptFormat = 'f', OptHelp = 'h', OptList = 'l', }; @@ -35,10 +36,20 @@ void signalHandler(int signal) static int parseOptions(int argc, char *argv[]) { + KeyValueParser formatKeyValue; + formatKeyValue.addOption("width", "Set width in pixels", + ArgumentRequired, "w"); + formatKeyValue.addOption("height", "Set height in pixels", + ArgumentRequired, "h"); + formatKeyValue.addOption("pixelformat", "Set pixel format", + ArgumentRequired, "pf"); + OptionsParser parser; parser.addOption(OptCamera, "Specify which camera to operate on", "camera", ArgumentRequired, "camera"); + parser.addOption(OptFormat, "Set format of the cameras first stream", + formatKeyValue, "format"); parser.addOption(OptHelp, "Display this help message", "help"); parser.addOption(OptList, "List all cameras", "list"); @@ -54,6 +65,42 @@ static int parseOptions(int argc, char *argv[]) return 0; } +int str2uint(std::string str, unsigned int *dest) +{ + if (!dest || str == "" || !sscanf(str.c_str(), "%d", dest)) + return -EINVAL; + return 0; +} + +bool configureStreams(Camera *camera, std::vector<Stream> &streams) +{ + if (!options.isKeyValue(OptFormat)) + return false; + + KeyValueParser::Options format = options.keyValues(OptFormat); + + std::map<unsigned int, StreamConfiguration> config = camera->streamConfiguration(streams); + unsigned int id = streams.front().id(); + + if (format.isSet("width")) + if (str2uint(format["width"], &config[id].width)) + return false; + + if (format.isSet("height")) + if (str2uint(format["height"], &config[id].height)) + return false; + + /* TODO: Translate 4CC string to ID. */ + if (format.isSet("pixelformat")) + if (str2uint(format["pixelformat"], &config[id].pixelFormat)) + return false; + + if (camera->configureStreams(config)) + return false; + + return true; +} + int main(int argc, char **argv) { int ret; @@ -63,6 +110,8 @@ int main(int argc, char **argv) return EXIT_FAILURE; CameraManager *cm = CameraManager::instance(); + std::shared_ptr<Camera> camera; + std::vector<Stream> streams; ret = cm->start(); if (ret) { @@ -71,31 +120,62 @@ int main(int argc, char **argv) return EXIT_FAILURE; } + loop = new EventLoop(cm->eventDispatcher()); + + struct sigaction sa = {}; + sa.sa_handler = &signalHandler; + sigaction(SIGINT, &sa, nullptr); + if (options.isSet(OptList)) { std::cout << "Available cameras:" << std::endl; - for (const std::shared_ptr<Camera> &camera : cm->cameras()) - std::cout << "- " << camera->name() << std::endl; + for (const std::shared_ptr<Camera> &cam : cm->cameras()) + std::cout << "- " << cam->name() << std::endl; } if (options.isSet(OptCamera)) { - std::shared_ptr<Camera> cam = cm->get(options[OptCamera]); - - if (cam) { - std::cout << "Using camera " << cam->name() << std::endl; - } else { + camera = cm->get(options[OptCamera]); + if (!camera) { std::cout << "Camera " << options[OptCamera] << " not found" << std::endl; + goto out; + } + + streams = camera->streams(); + if (streams.size() != 1) { + std::cout << "Camera have " << streams.size() + << " streams, I only know how to work with 1" + << std::endl; + goto out; + } + + if (camera->acquire()) { + std::cout << "Failed to acquire camera" << std::endl; + goto out; } + + std::cout << "Using camera " << camera->name() << std::endl; } - loop = new EventLoop(cm->eventDispatcher()); + if (options.isSet(OptFormat)) { + if (!camera) { + std::cout << "Can't configure stream, no camera selected" << std::endl; + goto out_camera; + } - struct sigaction sa = {}; - sa.sa_handler = &signalHandler; - sigaction(SIGINT, &sa, nullptr); + if (!configureStreams(camera.get(), streams)) { + std::cout << "Failed to configure camera" << std::endl; + goto out_camera; + } + } ret = loop->exec(); +out_camera: + if (camera) { + camera->release(); + camera.reset(); + } +out: delete loop; cm->stop();
Add a option to configure the first stream of a camera from an argument with options and parse the width, height and pixel format from that list. The pixel format is still specified as a integer which should correspond to the kernels FOURCC identifiers. Going forward this should be turned into a string representation and the cam parser should translate between the two. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/cam/main.cpp | 104 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 12 deletions(-)