[libcamera-devel] [PATCH 11/11] libcamera: pipeline: Add RKISP1 pipeline

[Laurent Pinchart]

Hi Niklas,

On Tue, Apr 16, 2019 at 01:29:29AM +0200, Niklas Söderlund wrote:
> On 2019-04-16 02:09:52 +0300, Laurent Pinchart wrote:
> >>> +int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request 
> >>> *request)
> >>> +{
> >>> +	RkISP1CameraData *data = cameraData(camera);
> >>> +	Stream *stream = &data->stream_;
> >>> +
> >>> +	Buffer *buffer = request->findBuffer(stream);
> >>> +	if (!buffer) {
> >>> +		LOG(RkISP1, Error)
> >>> +			<< "Attempt to queue request with invalid stream";
> >>> +		return -ENOENT;
> >>> +	}
> >> 
> >> Should we move this check to Camera::queueRequest() ?
> > 
> > The Camera class doesn't have access to data->stream_. I expect this to
> > be reworked with multiple streams support anyway, so I'd keep it as-is
> > for now.
> 
> The Camera has access to something better, all streams configured. With 
> the pending patch [1] from Jacopo adding Request::buffers(), something 
> like this in Camera::queueRequest() would address both single and 
> multiple streams.
> 
> +       for (auto const &it : request->buffers()) {
> +               Stream *stream = it.first;
> +               if (activeStreams_.find(stream) == activeStreams_.end()) {
> +                       LOG(Camera, Error)
> +                               << "Attempt to queue request with invalid stream";
> +                       return -EINVAL;
> +               }
> +       }
> 
> 1. [PATCH v5 6/7] libcamera: request: Expose the Stream to Buffers map

To avoid adding a dependency between the two series, let's do this on
top when they will be merged. Would you like to send a patch ? :-)

-- 
Regards,

Laurent Pinchart