[libcamera-devel] [PATCH 18/20] libcamera: pipeline: simple: Support configuration of multiple streams
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Mar 2 11:46:28 CET 2021
On 02/03/2021 06:30, paul.elder at ideasonboard.com wrote:
> Hi Laurent,
>
> On Mon, Feb 01, 2021 at 12:47:00AM +0200, Laurent Pinchart wrote:
>> Extend the SimpleCameraConfiguration to support multiple streams, using
>> the multi-stream capability of the SimpleConverter class. Wiring up
>> multi-stream support in the other pipeline handler operations will come
>> in further commits.
>>
>> To keep the code simple, require all streams to use the converter if any
>> stream needs it. It would be possible to generate one stream without
>> conversion (provided the format and size match what the capture device
>> can generate), and this is left as a future optimization.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Seems you've answered Pauls questions:
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>> src/libcamera/pipeline/simple/simple.cpp | 174 ++++++++++++++---------
>> 1 file changed, 104 insertions(+), 70 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
>> index c987e1a0d9cb..58e5f0d23139 100644
>> --- a/src/libcamera/pipeline/simple/simple.cpp
>> +++ b/src/libcamera/pipeline/simple/simple.cpp
>> @@ -538,62 +538,94 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>> }
>>
>> /* Cap the number of entries to the available streams. */
>> - if (config_.size() > 1) {
>> - config_.resize(1);
>> + if (config_.size() > data_->streams_.size()) {
>> + config_.resize(data_->streams_.size());
>> status = Adjusted;
>> }
>>
>> - StreamConfiguration &cfg = config_[0];
>> -
>> - /* Adjust the pixel format. */
>> - auto it = data_->formats_.find(cfg.pixelFormat);
>> - if (it == data_->formats_.end())
>> - it = data_->formats_.begin();
>> -
>> - PixelFormat pixelFormat = it->first;
>> - if (cfg.pixelFormat != pixelFormat) {
>> - LOG(SimplePipeline, Debug) << "Adjusting pixel format";
>> - cfg.pixelFormat = pixelFormat;
>> - status = Adjusted;
>> - }
>> -
>> - pipeConfig_ = it->second;
>> - if (!pipeConfig_->outputSizes.contains(cfg.size)) {
>> - LOG(SimplePipeline, Debug)
>> - << "Adjusting size from " << cfg.size.toString()
>> - << " to " << pipeConfig_->captureSize.toString();
>> - cfg.size = pipeConfig_->captureSize;
>> - status = Adjusted;
>> - }
>> -
>> - needConversion_ = cfg.pixelFormat != pipeConfig_->captureFormat
>> - || cfg.size != pipeConfig_->captureSize;
>> -
>> - cfg.bufferCount = 3;
>> -
>> - /* Set the stride and frameSize. */
>> - if (!needConversion_) {
>> - V4L2DeviceFormat format;
>> - format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
>> - format.size = cfg.size;
>> -
>> - int ret = data_->video_->tryFormat(&format);
>> - if (ret < 0)
>> - return Invalid;
>> -
>> - cfg.stride = format.planes[0].bpl;
>> - cfg.frameSize = format.planes[0].size;
>> -
>> - return status;
>> + /*
>> + * Pick a configuration for the pipeline based on the pixel format for
>> + * the streams (ordered from highest to lowest priority). Default to
>> + * the first pipeline configuration if no streams requests a supported
>> + * pixel format.
>> + */
>> + pipeConfig_ = data_->formats_.begin()->second;
>> +
>> + for (const StreamConfiguration &cfg : config_) {
>> + auto it = data_->formats_.find(cfg.pixelFormat);
>> + if (it != data_->formats_.end()) {
>> + pipeConfig_ = it->second;
>> + break;
>> + }
>> }
>>
>> + /* Adjust the requested streams. */
>> SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe_);
>> SimpleConverter *converter = pipe->converter();
>>
>> - std::tie(cfg.stride, cfg.frameSize) =
>> - converter->strideAndFrameSize(cfg.pixelFormat, cfg.size);
>> - if (cfg.stride == 0)
>> - return Invalid;
>> + /*
>> + * Enable usage of the converter when producing multiple streams, as
>> + * the video capture device can't capture to multiple buffers.
>> + *
>> + * It is possible to produce up to one stream without conversion
>> + * (provided the format and size match), at the expense of more complex
>> + * buffer handling (including allocation of internal buffers to be used
>> + * when a request doesn't contain a buffer for the stream that doesn't
>> + * require any conversion, similar to raw capture use cases). This is
>> + * left as a future improvement.
>> + */
>> + needConversion_ = config_.size() > 1;
>
> Don't we also needConversion_ if we have only one stream but the
> format/size doesn't match?>> +
>> + for (unsigned int i = 0; i < config_.size(); ++i) {
>> + StreamConfiguration &cfg = config_[i];
>> +
>> + /* Adjust the pixel format and size. */
>> + auto it = std::find(pipeConfig_->outputFormats.begin(),
>> + pipeConfig_->outputFormats.end(),
>> + cfg.pixelFormat);
>> + if (it == pipeConfig_->outputFormats.end())
>> + it = pipeConfig_->outputFormats.begin();
>> +
>> + PixelFormat pixelFormat = *it;
>> + if (cfg.pixelFormat != pixelFormat) {
>> + LOG(SimplePipeline, Debug) << "Adjusting pixel format";
>> + cfg.pixelFormat = pixelFormat;
>> + status = Adjusted;
>> + }
>> +
>> + if (!pipeConfig_->outputSizes.contains(cfg.size)) {
>> + LOG(SimplePipeline, Debug)
>> + << "Adjusting size from " << cfg.size.toString()
>> + << " to " << pipeConfig_->captureSize.toString();
>> + cfg.size = pipeConfig_->captureSize;
>> + status = Adjusted;
>> + }
>> +
>> + if (cfg.pixelFormat != pipeConfig_->captureFormat ||
>> + cfg.size != pipeConfig_->captureSize)
>> + needConversion_ = true;
>> +
>> + /* Set the stride, frameSize and bufferCount. */
>> + if (needConversion_) {
>> + std::tie(cfg.stride, cfg.frameSize) =
>> + converter->strideAndFrameSize(cfg.pixelFormat, cfg.size);
>
> Is this the right parameter order (did I miss a change earlier in this
> series)?
>
>
> Paul
>
>> + if (cfg.stride == 0)
>> + return Invalid;
>> + } else {
>> + V4L2DeviceFormat format;
>> + format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
>> + format.size = cfg.size;
>> +
>> + int ret = data_->video_->tryFormat(&format);
>> + if (ret < 0)
>> + return Invalid;
>> +
>> + cfg.stride = format.planes[0].bpl;
>> + cfg.frameSize = format.planes[0].size;
>> + }
>> +
>> + cfg.bufferCount = 3;
>> + }
>>
>> return status;
>> }
>> @@ -628,16 +660,18 @@ CameraConfiguration *SimplePipelineHandler::generateConfiguration(Camera *camera
>> });
>>
>> /*
>> - * Create the stream configuration. Take the first entry in the formats
>> + * Create the stream configurations. Take the first entry in the formats
>> * map as the default, for lack of a better option.
>> *
>> * \todo Implement a better way to pick the default format
>> */
>> - StreamConfiguration cfg{ StreamFormats{ formats } };
>> - cfg.pixelFormat = formats.begin()->first;
>> - cfg.size = formats.begin()->second[0].max;
>> + for ([[maybe_unused]] StreamRole role : roles) {
>> + StreamConfiguration cfg{ StreamFormats{ formats } };
>> + cfg.pixelFormat = formats.begin()->first;
>> + cfg.size = formats.begin()->second[0].max;
>>
>> - config->addConfiguration(cfg);
>> + config->addConfiguration(cfg);
>> + }
>>
>> config->validate();
>>
>> @@ -650,7 +684,6 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>> static_cast<SimpleCameraConfiguration *>(c);
>> SimpleCameraData *data = cameraData(camera);
>> V4L2VideoDevice *video = data->video_;
>> - StreamConfiguration &cfg = config->at(0);
>> int ret;
>>
>> /*
>> @@ -694,28 +727,29 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>> return -EINVAL;
>> }
>>
>> - /* Configure the converter if required. */
>> + /* Configure the converter if needed. */
>> + std::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;
>> data->useConverter_ = config->needConversion();
>> - if (data->useConverter_) {
>> - StreamConfiguration inputCfg;
>> - inputCfg.pixelFormat = pipeConfig->captureFormat;
>> - inputCfg.size = pipeConfig->captureSize;
>> - inputCfg.stride = captureFormat.planes[0].bpl;
>> - inputCfg.bufferCount = kNumInternalBuffers;
>>
>> - ret = converter_->configure(inputCfg, { cfg });
>> - if (ret < 0) {
>> - LOG(SimplePipeline, Error)
>> - << "Unable to configure converter";
>> - return ret;
>> - }
>> + for (unsigned int i = 0; i < config->size(); ++i) {
>> + StreamConfiguration &cfg = config->at(i);
>>
>> - LOG(SimplePipeline, Debug) << "Using format converter";
>> + cfg.setStream(&data->streams_[i]);
>> +
>> + if (data->useConverter_)
>> + outputCfgs.push_back(cfg);
>> }
>>
>> - cfg.setStream(&data->streams_[0]);
>> + if (outputCfgs.empty())
>> + return 0;
>>
>> - return 0;
>> + StreamConfiguration inputCfg;
>> + inputCfg.pixelFormat = pipeConfig->captureFormat;
>> + inputCfg.size = pipeConfig->captureSize;
>> + inputCfg.stride = captureFormat.planes[0].bpl;
>> + inputCfg.bufferCount = kNumInternalBuffers;
>> +
>> + return converter_->configure(inputCfg, outputCfgs);
>> }
>>
>> int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list