| Message ID | 20190418164638.400-5-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Thu, Apr 18, 2019 at 06:46:36PM +0200, Jacopo Mondi wrote: > Add support for queueing requests for multiple streams in the IPU3 > pipeline handler class. The output video node should be queued with > buffers even if not part of the requested streams. Is this still the case ? > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 23 +++++++++-------------- > 1 file changed, 9 insertions(+), 14 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index d14ba47e1c2c..8353272642bd 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -545,25 +545,20 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > { > - IPU3CameraData *data = cameraData(camera); > - V4L2Device *output = data->imgu_->output_.dev; > - IPU3Stream *stream = &data->outStream_; > + int ret = 0; > > - /* Queue a buffer to the ImgU output for capture. */ > - Buffer *buffer = request->findBuffer(stream); > - if (!buffer) { > - LOG(IPU3, Error) > - << "Attempt to queue request with invalid stream"; > - return -ENOENT; > - } > + for (auto it : request->buffers()) { > + IPU3Stream *stream = static_cast<IPU3Stream *>(it.first); > + Buffer *buffer = it.second; > > - int ret = output->queueBuffer(buffer); > - if (ret < 0) > - return ret; > + int status = stream->device_->dev->queueBuffer(buffer); > + if (status < 0) > + ret = status; I'd rather s/ret/error/ and s/status/ret/ as ret is a very common name for temporary return values. Or possibly just swap ret and status. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + } > > PipelineHandler::queueRequest(camera, request); > > - return 0; > + return ret; > } > > bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
Hi Laurent, On Thu, Apr 18, 2019 at 11:41:39PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Thu, Apr 18, 2019 at 06:46:36PM +0200, Jacopo Mondi wrote: > > Add support for queueing requests for multiple streams in the IPU3 > > pipeline handler class. The output video node should be queued with > > buffers even if not part of the requested streams. > > Is this still the case ? > Not anymore. I have not updated the commit message. Sorry about this :( > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 23 +++++++++-------------- > > 1 file changed, 9 insertions(+), 14 deletions(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index d14ba47e1c2c..8353272642bd 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -545,25 +545,20 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > > > int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) > > { > > - IPU3CameraData *data = cameraData(camera); > > - V4L2Device *output = data->imgu_->output_.dev; > > - IPU3Stream *stream = &data->outStream_; > > + int ret = 0; > > > > - /* Queue a buffer to the ImgU output for capture. */ > > - Buffer *buffer = request->findBuffer(stream); > > - if (!buffer) { > > - LOG(IPU3, Error) > > - << "Attempt to queue request with invalid stream"; > > - return -ENOENT; > > - } > > + for (auto it : request->buffers()) { > > + IPU3Stream *stream = static_cast<IPU3Stream *>(it.first); > > + Buffer *buffer = it.second; > > > > - int ret = output->queueBuffer(buffer); > > - if (ret < 0) > > - return ret; > > + int status = stream->device_->dev->queueBuffer(buffer); > > + if (status < 0) > > + ret = status; > > I'd rather s/ret/error/ and s/status/ret/ as ret is a very common name > for temporary return values. Or possibly just swap ret and status. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Thanks j > > + } > > > > PipelineHandler::queueRequest(camera, request); > > > > - return 0; > > + return ret; > > } > > > > bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index d14ba47e1c2c..8353272642bd 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -545,25 +545,20 @@ void PipelineHandlerIPU3::stop(Camera *camera) int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) { - IPU3CameraData *data = cameraData(camera); - V4L2Device *output = data->imgu_->output_.dev; - IPU3Stream *stream = &data->outStream_; + int ret = 0; - /* Queue a buffer to the ImgU output for capture. */ - Buffer *buffer = request->findBuffer(stream); - if (!buffer) { - LOG(IPU3, Error) - << "Attempt to queue request with invalid stream"; - return -ENOENT; - } + for (auto it : request->buffers()) { + IPU3Stream *stream = static_cast<IPU3Stream *>(it.first); + Buffer *buffer = it.second; - int ret = output->queueBuffer(buffer); - if (ret < 0) - return ret; + int status = stream->device_->dev->queueBuffer(buffer); + if (status < 0) + ret = status; + } PipelineHandler::queueRequest(camera, request); - return 0; + return ret; } bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
Add support for queueing requests for multiple streams in the IPU3 pipeline handler class. The output video node should be queued with buffers even if not part of the requested streams. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-)