| Message ID | 20190403011235.12782-3-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On 03/04/2019 02:12, Niklas Söderlund wrote: > Extend the cam tool to allow configuring more than one stream. Add an > optional parameter to the --stream option to specify a usage role for > the stream. The stream role is passed to libcamera to give it control > over which streams to use. > > To support multiple streams, creation of requests needs to be reworked > to limit the number of requests to match the stream with the least > number of buffers. This should be improved in the future as the tool and > the library evolve. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/main.cpp | 78 ++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 66 insertions(+), 12 deletions(-) > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index 6bf5e5926704d6e9..b47bda21cbb7f220 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -5,8 +5,10 @@ > * main.cpp - cam - The libcamera swiss army knife > */ > > +#include <algorithm> > #include <iomanip> > #include <iostream> > +#include <limits.h> > #include <map> > #include <signal.h> > #include <string.h> > @@ -42,6 +44,9 @@ void signalHandler(int signal) > static int parseOptions(int argc, char *argv[]) > { > KeyValueParser streamKeyValue; > + streamKeyValue.addOption("role", OptionString, > + "Role for the stream (viewfinder, video, still)", > + ArgumentRequired); > streamKeyValue.addOption("width", OptionInteger, "Width in pixels", > ArgumentRequired); > streamKeyValue.addOption("height", OptionInteger, "Height in pixels", > @@ -61,7 +66,7 @@ static int parseOptions(int argc, char *argv[]) > "The default file name is 'frame-#.bin'.", > "file", ArgumentOptional, "filename"); > parser.addOption(OptStream, &streamKeyValue, > - "Set configuration of a camera stream", "stream"); > + "Set configuration of a camera stream", "stream", true); > parser.addOption(OptHelp, OptionNone, "Display this help message", > "help"); > parser.addOption(OptList, OptionNone, "List all cameras", "list"); > @@ -80,12 +85,51 @@ static int parseOptions(int argc, char *argv[]) > > static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > { > - std::set<Stream *> streams = camera->streams(); > - *config = camera->streamConfiguration({ Stream::VideoRecording() }); > - Stream *stream = config->begin()->first; > + std::vector<StreamRole> roles; > > - if (options.isSet(OptStream)) { > - KeyValueParser::Options conf = options[OptStream]; > + /* If no configuration is provided assume a single video stream. */ > + if (!options.isSet(OptStream)) { > + *config = camera->streamConfiguration({ Stream::VideoRecording() }); > + return 0; > + } > + > + const std::vector<OptionValue> &streamopts = > + options[OptStream].toArray(); > + > + /* Use roles and get a default configuration. */ > + for (auto const &value : streamopts) { > + KeyValueParser::Options conf = value.toKeyValues(); > + > + if (!conf.isSet("role")) { > + roles.push_back(Stream::VideoRecording()); > + } else if (conf["role"].toString() == "viewfinder") { > + roles.push_back(Stream::Viewfinder(conf["width"], > + conf["height"])); > + } else if (conf["role"].toString() == "video") { > + roles.push_back(Stream::VideoRecording()); > + } else if (conf["role"].toString() == "still") { > + roles.push_back(Stream::StillCapture()); > + } else { > + std::cerr << "Unknown stream role " > + << conf["role"].toString() << std::endl; > + return -EINVAL; > + } > + } > + > + *config = camera->streamConfiguration(roles); > + > + if (config->size() != streamopts.size()) { > + std::cerr << "Failed to get default stream configuration" > + << std::endl; > + return -EINVAL; > + } > + > + /* Apply configuration explicitly requested. */ > + std::map<Stream *, StreamConfiguration>::iterator it = config->begin(); > + for (auto const &value : streamopts) { > + KeyValueParser::Options conf = value.toKeyValues(); > + Stream *stream = it->first; > + it++; > > if (conf.isSet("width")) > (*config)[stream].width = conf["width"]; > @@ -137,7 +181,6 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *> > static int capture() > { > std::map<Stream *, StreamConfiguration> config; > - std::vector<Request *> requests; > int ret; > > ret = prepareCameraConfig(&config); > @@ -152,8 +195,6 @@ static int capture() > return ret; > } > > - Stream *stream = config.begin()->first; > - > ret = camera->allocateBuffers(); > if (ret) { > std::cerr << "Failed to allocate buffers" > @@ -163,9 +204,18 @@ static int capture() > > camera->requestCompleted.connect(requestComplete); > > - BufferPool &pool = stream->bufferPool(); > + /* Figure out which stream s the lower number of buffers. */ /s/has/ ? Perhaps: /* Identify the stream with the least number of buffers. */ > + unsigned int nbuffers = UINT_MAX; > + for (auto const &it : config) > + nbuffers = std::min(nbuffers, it.first->bufferPool().count()); > > - for (Buffer &buffer : pool.buffers()) { > + /* > + * TODO: make cam tool smarter to support still capture by for > + * example pushing a button. For now run all streams all the time. > + */ > + > + std::vector<Request *> requests; > + for (unsigned int i = 0; i < nbuffers; i++) { > Request *request = camera->createRequest(); > if (!request) { > std::cerr << "Can't create request" << std::endl; > @@ -174,7 +224,11 @@ static int capture() > } > > std::map<Stream *, Buffer *> map; > - map[stream] = &buffer; > + for (auto const &it : config) { > + Stream *stream = it.first; > + map[stream] = &stream->bufferPool().buffers()[i]; > + } > + > ret = request->setBuffers(map); > if (ret < 0) { > std::cerr << "Can't set buffers for request" << std::endl; >
Hi Niklas, Thank you for the patch. On Wed, Apr 03, 2019 at 03:12:33AM +0200, Niklas Söderlund wrote: > Extend the cam tool to allow configuring more than one stream. Add an > optional parameter to the --stream option to specify a usage role for > the stream. The stream role is passed to libcamera to give it control > over which streams to use. > > To support multiple streams, creation of requests needs to be reworked > to limit the number of requests to match the stream with the least > number of buffers. This should be improved in the future as the tool and > the library evolve. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/main.cpp | 78 ++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 66 insertions(+), 12 deletions(-) > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index 6bf5e5926704d6e9..b47bda21cbb7f220 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -5,8 +5,10 @@ > * main.cpp - cam - The libcamera swiss army knife > */ > > +#include <algorithm> > #include <iomanip> > #include <iostream> > +#include <limits.h> > #include <map> > #include <signal.h> > #include <string.h> > @@ -42,6 +44,9 @@ void signalHandler(int signal) > static int parseOptions(int argc, char *argv[]) > { > KeyValueParser streamKeyValue; > + streamKeyValue.addOption("role", OptionString, > + "Role for the stream (viewfinder, video, still)", > + ArgumentRequired); > streamKeyValue.addOption("width", OptionInteger, "Width in pixels", > ArgumentRequired); > streamKeyValue.addOption("height", OptionInteger, "Height in pixels", > @@ -61,7 +66,7 @@ static int parseOptions(int argc, char *argv[]) > "The default file name is 'frame-#.bin'.", > "file", ArgumentOptional, "filename"); > parser.addOption(OptStream, &streamKeyValue, > - "Set configuration of a camera stream", "stream"); > + "Set configuration of a camera stream", "stream", true); > parser.addOption(OptHelp, OptionNone, "Display this help message", > "help"); > parser.addOption(OptList, OptionNone, "List all cameras", "list"); > @@ -80,12 +85,51 @@ static int parseOptions(int argc, char *argv[]) > > static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > { > - std::set<Stream *> streams = camera->streams(); > - *config = camera->streamConfiguration({ Stream::VideoRecording() }); > - Stream *stream = config->begin()->first; > + std::vector<StreamRole> roles; > > - if (options.isSet(OptStream)) { > - KeyValueParser::Options conf = options[OptStream]; > + /* If no configuration is provided assume a single video stream. */ > + if (!options.isSet(OptStream)) { > + *config = camera->streamConfiguration({ Stream::VideoRecording() }); > + return 0; > + } > + > + const std::vector<OptionValue> &streamopts = s/streamopts/streamOpts/ (or even streamOptions) ? > + options[OptStream].toArray(); > + > + /* Use roles and get a default configuration. */ > + for (auto const &value : streamopts) { > + KeyValueParser::Options conf = value.toKeyValues(); > + > + if (!conf.isSet("role")) { > + roles.push_back(Stream::VideoRecording()); > + } else if (conf["role"].toString() == "viewfinder") { > + roles.push_back(Stream::Viewfinder(conf["width"], > + conf["height"])); > + } else if (conf["role"].toString() == "video") { > + roles.push_back(Stream::VideoRecording()); > + } else if (conf["role"].toString() == "still") { > + roles.push_back(Stream::StillCapture()); > + } else { > + std::cerr << "Unknown stream role " > + << conf["role"].toString() << std::endl; > + return -EINVAL; > + } > + } > + > + *config = camera->streamConfiguration(roles); > + > + if (config->size() != streamopts.size()) { > + std::cerr << "Failed to get default stream configuration" > + << std::endl; > + return -EINVAL; > + } > + > + /* Apply configuration explicitly requested. */ > + std::map<Stream *, StreamConfiguration>::iterator it = config->begin(); > + for (auto const &value : streamopts) { > + KeyValueParser::Options conf = value.toKeyValues(); > + Stream *stream = it->first; > + it++; > > if (conf.isSet("width")) > (*config)[stream].width = conf["width"]; > @@ -137,7 +181,6 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *> > static int capture() > { > std::map<Stream *, StreamConfiguration> config; > - std::vector<Request *> requests; > int ret; > > ret = prepareCameraConfig(&config); > @@ -152,8 +195,6 @@ static int capture() > return ret; > } > > - Stream *stream = config.begin()->first; > - > ret = camera->allocateBuffers(); > if (ret) { > std::cerr << "Failed to allocate buffers" > @@ -163,9 +204,18 @@ static int capture() > > camera->requestCompleted.connect(requestComplete); > > - BufferPool &pool = stream->bufferPool(); > + /* Figure out which stream s the lower number of buffers. */ s/stream s/stream has/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + unsigned int nbuffers = UINT_MAX; > + for (auto const &it : config) > + nbuffers = std::min(nbuffers, it.first->bufferPool().count()); > > - for (Buffer &buffer : pool.buffers()) { > + /* > + * TODO: make cam tool smarter to support still capture by for > + * example pushing a button. For now run all streams all the time. > + */ > + > + std::vector<Request *> requests; > + for (unsigned int i = 0; i < nbuffers; i++) { > Request *request = camera->createRequest(); > if (!request) { > std::cerr << "Can't create request" << std::endl; > @@ -174,7 +224,11 @@ static int capture() > } > > std::map<Stream *, Buffer *> map; > - map[stream] = &buffer; > + for (auto const &it : config) { > + Stream *stream = it.first; > + map[stream] = &stream->bufferPool().buffers()[i]; > + } > + > ret = request->setBuffers(map); > if (ret < 0) { > std::cerr << "Can't set buffers for request" << std::endl;
Hi Niklas, one general question below On Wed, Apr 03, 2019 at 03:12:33AM +0200, Niklas Söderlund wrote: > Extend the cam tool to allow configuring more than one stream. Add an > optional parameter to the --stream option to specify a usage role for > the stream. The stream role is passed to libcamera to give it control > over which streams to use. > > To support multiple streams, creation of requests needs to be reworked > to limit the number of requests to match the stream with the least > number of buffers. This should be improved in the future as the tool and > the library evolve. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/cam/main.cpp | 78 ++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 66 insertions(+), 12 deletions(-) > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index 6bf5e5926704d6e9..b47bda21cbb7f220 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -5,8 +5,10 @@ > * main.cpp - cam - The libcamera swiss army knife > */ > > +#include <algorithm> > #include <iomanip> > #include <iostream> > +#include <limits.h> > #include <map> > #include <signal.h> > #include <string.h> > @@ -42,6 +44,9 @@ void signalHandler(int signal) > static int parseOptions(int argc, char *argv[]) > { > KeyValueParser streamKeyValue; > + streamKeyValue.addOption("role", OptionString, > + "Role for the stream (viewfinder, video, still)", > + ArgumentRequired); > streamKeyValue.addOption("width", OptionInteger, "Width in pixels", > ArgumentRequired); > streamKeyValue.addOption("height", OptionInteger, "Height in pixels", > @@ -61,7 +66,7 @@ static int parseOptions(int argc, char *argv[]) > "The default file name is 'frame-#.bin'.", > "file", ArgumentOptional, "filename"); > parser.addOption(OptStream, &streamKeyValue, > - "Set configuration of a camera stream", "stream"); > + "Set configuration of a camera stream", "stream", true); > parser.addOption(OptHelp, OptionNone, "Display this help message", > "help"); > parser.addOption(OptList, OptionNone, "List all cameras", "list"); > @@ -80,12 +85,51 @@ static int parseOptions(int argc, char *argv[]) > > static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > { > - std::set<Stream *> streams = camera->streams(); > - *config = camera->streamConfiguration({ Stream::VideoRecording() }); > - Stream *stream = config->begin()->first; > + std::vector<StreamRole> roles; > > - if (options.isSet(OptStream)) { > - KeyValueParser::Options conf = options[OptStream]; > + /* If no configuration is provided assume a single video stream. */ > + if (!options.isSet(OptStream)) { > + *config = camera->streamConfiguration({ Stream::VideoRecording() }); > + return 0; > + } > + > + const std::vector<OptionValue> &streamopts = > + options[OptStream].toArray(); > + > + /* Use roles and get a default configuration. */ > + for (auto const &value : streamopts) { > + KeyValueParser::Options conf = value.toKeyValues(); > + > + if (!conf.isSet("role")) { > + roles.push_back(Stream::VideoRecording()); > + } else if (conf["role"].toString() == "viewfinder") { > + roles.push_back(Stream::Viewfinder(conf["width"], > + conf["height"])); > + } else if (conf["role"].toString() == "video") { > + roles.push_back(Stream::VideoRecording()); > + } else if (conf["role"].toString() == "still") { > + roles.push_back(Stream::StillCapture()); > + } else { > + std::cerr << "Unknown stream role " > + << conf["role"].toString() << std::endl; > + return -EINVAL; > + } > + } Open question and not specifically on this patch, but on this and the other series you have in flight: this implementation does not allow to specify multiple roles for a stream, as you could have done using a bitmaks. For applications to stay as generic as possible, is it very unlikely to have them as for something like (VIEWFINDER | VIDEOCAPTURE) ? If I'm not wrong this implementation makes roles mutually exclusive, doesn't it? This would call for pipeline handler to match at least one of the intended usages, to assign a stream to a stream usage. > + > + *config = camera->streamConfiguration(roles); > + > + if (config->size() != streamopts.size()) { > + std::cerr << "Failed to get default stream configuration" > + << std::endl; > + return -EINVAL; > + } > + > + /* Apply configuration explicitly requested. */ > + std::map<Stream *, StreamConfiguration>::iterator it = config->begin(); > + for (auto const &value : streamopts) { > + KeyValueParser::Options conf = value.toKeyValues(); > + Stream *stream = it->first; > + it++; > > if (conf.isSet("width")) > (*config)[stream].width = conf["width"]; > @@ -137,7 +181,6 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *> > static int capture() > { > std::map<Stream *, StreamConfiguration> config; > - std::vector<Request *> requests; > int ret; > > ret = prepareCameraConfig(&config); > @@ -152,8 +195,6 @@ static int capture() > return ret; > } > > - Stream *stream = config.begin()->first; > - > ret = camera->allocateBuffers(); > if (ret) { > std::cerr << "Failed to allocate buffers" > @@ -163,9 +204,18 @@ static int capture() > > camera->requestCompleted.connect(requestComplete); > > - BufferPool &pool = stream->bufferPool(); > + /* Figure out which stream s the lower number of buffers. */ s/ s/has/ ? > + unsigned int nbuffers = UINT_MAX; > + for (auto const &it : config) > + nbuffers = std::min(nbuffers, it.first->bufferPool().count()); > > - for (Buffer &buffer : pool.buffers()) { > + /* > + * TODO: make cam tool smarter to support still capture by for > + * example pushing a button. For now run all streams all the time. > + */ > + > + std::vector<Request *> requests; > + for (unsigned int i = 0; i < nbuffers; i++) { > Request *request = camera->createRequest(); > if (!request) { > std::cerr << "Can't create request" << std::endl; > @@ -174,7 +224,11 @@ static int capture() > } > > std::map<Stream *, Buffer *> map; > - map[stream] = &buffer; > + for (auto const &it : config) { > + Stream *stream = it.first; > + map[stream] = &stream->bufferPool().buffers()[i]; > + } > + > ret = request->setBuffers(map); > if (ret < 0) { > std::cerr << "Can't set buffers for request" << std::endl; > -- > 2.21.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thanks for your feedback. On 2019-04-03 15:39:47 +0200, Jacopo Mondi wrote: > Hi Niklas, > one general question below > > On Wed, Apr 03, 2019 at 03:12:33AM +0200, Niklas Söderlund wrote: > > Extend the cam tool to allow configuring more than one stream. Add an > > optional parameter to the --stream option to specify a usage role for > > the stream. The stream role is passed to libcamera to give it control > > over which streams to use. > > > > To support multiple streams, creation of requests needs to be reworked > > to limit the number of requests to match the stream with the least > > number of buffers. This should be improved in the future as the tool and > > the library evolve. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/cam/main.cpp | 78 ++++++++++++++++++++++++++++++++++++++++-------- > > 1 file changed, 66 insertions(+), 12 deletions(-) > > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > > index 6bf5e5926704d6e9..b47bda21cbb7f220 100644 > > --- a/src/cam/main.cpp > > +++ b/src/cam/main.cpp > > @@ -5,8 +5,10 @@ > > * main.cpp - cam - The libcamera swiss army knife > > */ > > > > +#include <algorithm> > > #include <iomanip> > > #include <iostream> > > +#include <limits.h> > > #include <map> > > #include <signal.h> > > #include <string.h> > > @@ -42,6 +44,9 @@ void signalHandler(int signal) > > static int parseOptions(int argc, char *argv[]) > > { > > KeyValueParser streamKeyValue; > > + streamKeyValue.addOption("role", OptionString, > > + "Role for the stream (viewfinder, video, still)", > > + ArgumentRequired); > > streamKeyValue.addOption("width", OptionInteger, "Width in pixels", > > ArgumentRequired); > > streamKeyValue.addOption("height", OptionInteger, "Height in pixels", > > @@ -61,7 +66,7 @@ static int parseOptions(int argc, char *argv[]) > > "The default file name is 'frame-#.bin'.", > > "file", ArgumentOptional, "filename"); > > parser.addOption(OptStream, &streamKeyValue, > > - "Set configuration of a camera stream", "stream"); > > + "Set configuration of a camera stream", "stream", true); > > parser.addOption(OptHelp, OptionNone, "Display this help message", > > "help"); > > parser.addOption(OptList, OptionNone, "List all cameras", "list"); > > @@ -80,12 +85,51 @@ static int parseOptions(int argc, char *argv[]) > > > > static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) > > { > > - std::set<Stream *> streams = camera->streams(); > > - *config = camera->streamConfiguration({ Stream::VideoRecording() }); > > - Stream *stream = config->begin()->first; > > + std::vector<StreamRole> roles; > > > > - if (options.isSet(OptStream)) { > > - KeyValueParser::Options conf = options[OptStream]; > > + /* If no configuration is provided assume a single video stream. */ > > + if (!options.isSet(OptStream)) { > > + *config = camera->streamConfiguration({ Stream::VideoRecording() }); > > + return 0; > > + } > > + > > + const std::vector<OptionValue> &streamopts = > > + options[OptStream].toArray(); > > + > > + /* Use roles and get a default configuration. */ > > + for (auto const &value : streamopts) { > > + KeyValueParser::Options conf = value.toKeyValues(); > > + > > + if (!conf.isSet("role")) { > > + roles.push_back(Stream::VideoRecording()); > > + } else if (conf["role"].toString() == "viewfinder") { > > + roles.push_back(Stream::Viewfinder(conf["width"], > > + conf["height"])); > > + } else if (conf["role"].toString() == "video") { > > + roles.push_back(Stream::VideoRecording()); > > + } else if (conf["role"].toString() == "still") { > > + roles.push_back(Stream::StillCapture()); > > + } else { > > + std::cerr << "Unknown stream role " > > + << conf["role"].toString() << std::endl; > > + return -EINVAL; > > + } > > + } > > Open question and not specifically on this patch, but on this and the > other series you have in flight: this implementation does not allow to > specify multiple roles for a stream, as you could have done using a > bitmaks. For applications to stay as generic as possible, is it very > unlikely to have them as for something like (VIEWFINDER | VIDEOCAPTURE) ? > If I'm not wrong this implementation makes roles mutually exclusive, > doesn't it? Yes it makes roles mutually exclusive on a stream level. It is not possible to use the same stream simultaneously for both still image capture and as a view finder. You can however use the same stream for both purposes in two different capture sessions. Did I understand your question correctly? > > This would call for pipeline handler to match at least one of the intended > usages, to assign a stream to a stream usage. > > > + > > + *config = camera->streamConfiguration(roles); > > + > > + if (config->size() != streamopts.size()) { > > + std::cerr << "Failed to get default stream configuration" > > + << std::endl; > > + return -EINVAL; > > + } > > + > > + /* Apply configuration explicitly requested. */ > > + std::map<Stream *, StreamConfiguration>::iterator it = config->begin(); > > + for (auto const &value : streamopts) { > > + KeyValueParser::Options conf = value.toKeyValues(); > > + Stream *stream = it->first; > > + it++; > > > > if (conf.isSet("width")) > > (*config)[stream].width = conf["width"]; > > @@ -137,7 +181,6 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *> > > static int capture() > > { > > std::map<Stream *, StreamConfiguration> config; > > - std::vector<Request *> requests; > > int ret; > > > > ret = prepareCameraConfig(&config); > > @@ -152,8 +195,6 @@ static int capture() > > return ret; > > } > > > > - Stream *stream = config.begin()->first; > > - > > ret = camera->allocateBuffers(); > > if (ret) { > > std::cerr << "Failed to allocate buffers" > > @@ -163,9 +204,18 @@ static int capture() > > > > camera->requestCompleted.connect(requestComplete); > > > > - BufferPool &pool = stream->bufferPool(); > > + /* Figure out which stream s the lower number of buffers. */ > > s/ s/has/ ? > > > + unsigned int nbuffers = UINT_MAX; > > + for (auto const &it : config) > > + nbuffers = std::min(nbuffers, it.first->bufferPool().count()); > > > > - for (Buffer &buffer : pool.buffers()) { > > + /* > > + * TODO: make cam tool smarter to support still capture by for > > + * example pushing a button. For now run all streams all the time. > > + */ > > + > > + std::vector<Request *> requests; > > + for (unsigned int i = 0; i < nbuffers; i++) { > > Request *request = camera->createRequest(); > > if (!request) { > > std::cerr << "Can't create request" << std::endl; > > @@ -174,7 +224,11 @@ static int capture() > > } > > > > std::map<Stream *, Buffer *> map; > > - map[stream] = &buffer; > > + for (auto const &it : config) { > > + Stream *stream = it.first; > > + map[stream] = &stream->bufferPool().buffers()[i]; > > + } > > + > > ret = request->setBuffers(map); > > if (ret < 0) { > > std::cerr << "Can't set buffers for request" << std::endl; > > -- > > 2.21.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 6bf5e5926704d6e9..b47bda21cbb7f220 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -5,8 +5,10 @@ * main.cpp - cam - The libcamera swiss army knife */ +#include <algorithm> #include <iomanip> #include <iostream> +#include <limits.h> #include <map> #include <signal.h> #include <string.h> @@ -42,6 +44,9 @@ void signalHandler(int signal) static int parseOptions(int argc, char *argv[]) { KeyValueParser streamKeyValue; + streamKeyValue.addOption("role", OptionString, + "Role for the stream (viewfinder, video, still)", + ArgumentRequired); streamKeyValue.addOption("width", OptionInteger, "Width in pixels", ArgumentRequired); streamKeyValue.addOption("height", OptionInteger, "Height in pixels", @@ -61,7 +66,7 @@ static int parseOptions(int argc, char *argv[]) "The default file name is 'frame-#.bin'.", "file", ArgumentOptional, "filename"); parser.addOption(OptStream, &streamKeyValue, - "Set configuration of a camera stream", "stream"); + "Set configuration of a camera stream", "stream", true); parser.addOption(OptHelp, OptionNone, "Display this help message", "help"); parser.addOption(OptList, OptionNone, "List all cameras", "list"); @@ -80,12 +85,51 @@ static int parseOptions(int argc, char *argv[]) static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config) { - std::set<Stream *> streams = camera->streams(); - *config = camera->streamConfiguration({ Stream::VideoRecording() }); - Stream *stream = config->begin()->first; + std::vector<StreamRole> roles; - if (options.isSet(OptStream)) { - KeyValueParser::Options conf = options[OptStream]; + /* If no configuration is provided assume a single video stream. */ + if (!options.isSet(OptStream)) { + *config = camera->streamConfiguration({ Stream::VideoRecording() }); + return 0; + } + + const std::vector<OptionValue> &streamopts = + options[OptStream].toArray(); + + /* Use roles and get a default configuration. */ + for (auto const &value : streamopts) { + KeyValueParser::Options conf = value.toKeyValues(); + + if (!conf.isSet("role")) { + roles.push_back(Stream::VideoRecording()); + } else if (conf["role"].toString() == "viewfinder") { + roles.push_back(Stream::Viewfinder(conf["width"], + conf["height"])); + } else if (conf["role"].toString() == "video") { + roles.push_back(Stream::VideoRecording()); + } else if (conf["role"].toString() == "still") { + roles.push_back(Stream::StillCapture()); + } else { + std::cerr << "Unknown stream role " + << conf["role"].toString() << std::endl; + return -EINVAL; + } + } + + *config = camera->streamConfiguration(roles); + + if (config->size() != streamopts.size()) { + std::cerr << "Failed to get default stream configuration" + << std::endl; + return -EINVAL; + } + + /* Apply configuration explicitly requested. */ + std::map<Stream *, StreamConfiguration>::iterator it = config->begin(); + for (auto const &value : streamopts) { + KeyValueParser::Options conf = value.toKeyValues(); + Stream *stream = it->first; + it++; if (conf.isSet("width")) (*config)[stream].width = conf["width"]; @@ -137,7 +181,6 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *> static int capture() { std::map<Stream *, StreamConfiguration> config; - std::vector<Request *> requests; int ret; ret = prepareCameraConfig(&config); @@ -152,8 +195,6 @@ static int capture() return ret; } - Stream *stream = config.begin()->first; - ret = camera->allocateBuffers(); if (ret) { std::cerr << "Failed to allocate buffers" @@ -163,9 +204,18 @@ static int capture() camera->requestCompleted.connect(requestComplete); - BufferPool &pool = stream->bufferPool(); + /* Figure out which stream s the lower number of buffers. */ + unsigned int nbuffers = UINT_MAX; + for (auto const &it : config) + nbuffers = std::min(nbuffers, it.first->bufferPool().count()); - for (Buffer &buffer : pool.buffers()) { + /* + * TODO: make cam tool smarter to support still capture by for + * example pushing a button. For now run all streams all the time. + */ + + std::vector<Request *> requests; + for (unsigned int i = 0; i < nbuffers; i++) { Request *request = camera->createRequest(); if (!request) { std::cerr << "Can't create request" << std::endl; @@ -174,7 +224,11 @@ static int capture() } std::map<Stream *, Buffer *> map; - map[stream] = &buffer; + for (auto const &it : config) { + Stream *stream = it.first; + map[stream] = &stream->bufferPool().buffers()[i]; + } + ret = request->setBuffers(map); if (ret < 0) { std::cerr << "Can't set buffers for request" << std::endl;
Extend the cam tool to allow configuring more than one stream. Add an optional parameter to the --stream option to specify a usage role for the stream. The stream role is passed to libcamera to give it control over which streams to use. To support multiple streams, creation of requests needs to be reworked to limit the number of requests to match the stream with the least number of buffers. This should be improved in the future as the tool and the library evolve. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/cam/main.cpp | 78 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 12 deletions(-)