Message ID | 20200804214711.177645-9-kieran.bingham@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Kieran, Thank you for the patch. On Tue, Aug 04, 2020 at 10:47:06PM +0100, Kieran Bingham wrote: > The camera3buffer describes the number of filedescriptors given. > Don't try to construct more planes than that. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/android/camera_device.cpp | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index e4dce680d46f..087e226f5a82 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -1082,9 +1082,20 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) > FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer) > { > std::vector<FrameBuffer::Plane> planes; > - for (unsigned int i = 0; i < 3; i++) { > + for (int i = 0; i < camera3buffer->numFds; i++) { > + /* Skip unused planes. */ > + if (camera3buffer->data[i] == -1) > + break; > + > FrameBuffer::Plane plane; > plane.fd = FileDescriptor(camera3buffer->data[i]); > + if (!plane.fd.isValid()) { > + LOG(HAL, Error) << "Failed to obtain FileDescriptor (" > + << camera3buffer->data[i] << ") " > + << " on plane " << i; > + break; Shouldn't you return nullptr here ? > + } > + > /* > * Setting length to zero here is OK as the length is only used > * to map the memory of the plane. Libcamera do not need to poke
On 05/08/2020 00:49, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Tue, Aug 04, 2020 at 10:47:06PM +0100, Kieran Bingham wrote: >> The camera3buffer describes the number of filedescriptors given. >> Don't try to construct more planes than that. >> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/android/camera_device.cpp | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp >> index e4dce680d46f..087e226f5a82 100644 >> --- a/src/android/camera_device.cpp >> +++ b/src/android/camera_device.cpp >> @@ -1082,9 +1082,20 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) >> FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer) >> { >> std::vector<FrameBuffer::Plane> planes; >> - for (unsigned int i = 0; i < 3; i++) { >> + for (int i = 0; i < camera3buffer->numFds; i++) { >> + /* Skip unused planes. */ >> + if (camera3buffer->data[i] == -1) >> + break; >> + >> FrameBuffer::Plane plane; >> plane.fd = FileDescriptor(camera3buffer->data[i]); >> + if (!plane.fd.isValid()) { >> + LOG(HAL, Error) << "Failed to obtain FileDescriptor (" >> + << camera3buffer->data[i] << ") " >> + << " on plane " << i; >> + break; > > Shouldn't you return nullptr here ? Ugh, I'm sure I'd fixed that already... Sorry not sure where that hunk went. Must have got squashed to the wrong patch perhaps. Now corrected on this patch. Ah I must have done this for the SEEK_END implementation, but yes I think it should return nullptr here too. > >> + } >> + >> /* >> * Setting length to zero here is OK as the length is only used >> * to map the memory of the plane. Libcamera do not need to poke >
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index e4dce680d46f..087e226f5a82 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1082,9 +1082,20 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer) { std::vector<FrameBuffer::Plane> planes; - for (unsigned int i = 0; i < 3; i++) { + for (int i = 0; i < camera3buffer->numFds; i++) { + /* Skip unused planes. */ + if (camera3buffer->data[i] == -1) + break; + FrameBuffer::Plane plane; plane.fd = FileDescriptor(camera3buffer->data[i]); + if (!plane.fd.isValid()) { + LOG(HAL, Error) << "Failed to obtain FileDescriptor (" + << camera3buffer->data[i] << ") " + << " on plane " << i; + break; + } + /* * Setting length to zero here is OK as the length is only used * to map the memory of the plane. Libcamera do not need to poke