| Message ID | 20201005104649.10812-12-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Mon, Oct 05, 2020 at 01:46:45PM +0300, Laurent Pinchart wrote: > From: Jacopo Mondi <jacopo@jmondi.org> > > Loop over the CameraStream instances and use their interface to perform > CameraStream configuration after the Camera::configure(). > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/android/camera_device.cpp | 10 ++++------ > src/android/camera_stream.cpp | 4 ++-- > src/android/camera_stream.h | 3 ++- > 3 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp > index 537883b63f15..62307726aea9 100644 > --- a/src/android/camera_device.cpp > +++ b/src/android/camera_device.cpp > @@ -1298,17 +1298,15 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) > * StreamConfiguration and set the number of required buffers in > * the Android camera3_stream_t. > */ > - for (unsigned int i = 0; i < stream_list->num_streams; ++i) { > - camera3_stream_t *stream = stream_list->streams[i]; > - CameraStream *cameraStream = static_cast<CameraStream *>(stream->priv); > - const StreamConfiguration &cfg = cameraStream->streamConfiguration(); > + for (CameraStream &cameraStream : streams_) { > + camera3_stream_t *stream = cameraStream.androidStream(); > > - int ret = cameraStream->configure(cfg); > + int ret = cameraStream.configure(); > if (ret) > return ret; > > /* Use the bufferCount confirmed by the validation process. */ > - stream->max_buffers = cfg.bufferCount; > + stream->max_buffers = cameraStream.streamConfiguration().bufferCount; Should this move to the CameraStream class ? > } > > return 0; > diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp > index 76e7d240f4e7..dbde1e786300 100644 > --- a/src/android/camera_stream.cpp > +++ b/src/android/camera_stream.cpp > @@ -56,10 +56,10 @@ const Size &CameraStream::size() const > return streamConfiguration().size; > } > > -int CameraStream::configure(const libcamera::StreamConfiguration &cfg) > +int CameraStream::configure() > { > if (encoder_) { > - int ret = encoder_->configure(cfg); > + int ret = encoder_->configure(streamConfiguration()); > if (ret) > return ret; > } > diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h > index e399e17b2a2f..c6ff79230b7e 100644 > --- a/src/android/camera_stream.h > +++ b/src/android/camera_stream.h > @@ -116,13 +116,14 @@ public: > > int outputFormat() const { return androidStream_->format; } > Type type() const { return type_; } > + camera3_stream_t *androidStream() const { return androidStream_; } > > const libcamera::StreamConfiguration &streamConfiguration() const; > libcamera::Stream *stream() const; > const libcamera::PixelFormat &format() const; > const libcamera::Size &size() const; > > - int configure(const libcamera::StreamConfiguration &cfg); > + int configure(); > int process(libcamera::FrameBuffer *source, > MappedCamera3Buffer *dest, > CameraMetadata *metadata);
Hi Jacopo, On 05/10/2020 13:30, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Mon, Oct 05, 2020 at 01:46:45PM +0300, Laurent Pinchart wrote: >> From: Jacopo Mondi <jacopo@jmondi.org> >> >> Loop over the CameraStream instances and use their interface to perform >> CameraStream configuration after the Camera::configure(). >> >> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> >> --- >> src/android/camera_device.cpp | 10 ++++------ >> src/android/camera_stream.cpp | 4 ++-- >> src/android/camera_stream.h | 3 ++- >> 3 files changed, 8 insertions(+), 9 deletions(-) >> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp >> index 537883b63f15..62307726aea9 100644 >> --- a/src/android/camera_device.cpp >> +++ b/src/android/camera_device.cpp >> @@ -1298,17 +1298,15 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) >> * StreamConfiguration and set the number of required buffers in >> * the Android camera3_stream_t. >> */ >> - for (unsigned int i = 0; i < stream_list->num_streams; ++i) { >> - camera3_stream_t *stream = stream_list->streams[i]; >> - CameraStream *cameraStream = static_cast<CameraStream *>(stream->priv); >> - const StreamConfiguration &cfg = cameraStream->streamConfiguration(); >> + for (CameraStream &cameraStream : streams_) { >> + camera3_stream_t *stream = cameraStream.androidStream(); Oh that's nicer indeed. >> >> - int ret = cameraStream->configure(cfg); >> + int ret = cameraStream.configure(); >> if (ret) >> return ret; >> >> /* Use the bufferCount confirmed by the validation process. */ >> - stream->max_buffers = cfg.bufferCount; >> + stream->max_buffers = cameraStream.streamConfiguration().bufferCount; > > Should this move to the CameraStream class ?> >> } >> >> return 0; >> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp >> index 76e7d240f4e7..dbde1e786300 100644 >> --- a/src/android/camera_stream.cpp >> +++ b/src/android/camera_stream.cpp >> @@ -56,10 +56,10 @@ const Size &CameraStream::size() const >> return streamConfiguration().size; >> } >> >> -int CameraStream::configure(const libcamera::StreamConfiguration &cfg) >> +int CameraStream::configure() >> { >> if (encoder_) { >> - int ret = encoder_->configure(cfg); >> + int ret = encoder_->configure(streamConfiguration()); >> if (ret) >> return ret; >> } >> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h >> index e399e17b2a2f..c6ff79230b7e 100644 >> --- a/src/android/camera_stream.h >> +++ b/src/android/camera_stream.h >> @@ -116,13 +116,14 @@ public: >> >> int outputFormat() const { return androidStream_->format; } >> Type type() const { return type_; } >> + camera3_stream_t *androidStream() const { return androidStream_; } If the max_buffers/bufferCount does move to CameraStream, this might not be needed in this patch - but I think it's certainly better to be able to iterate out HAL objects and get the camera3 types from our representations - so I really like this androidStream() in it's potential usage. If it ends up dropped, it can always come in when required later. I'll still add this : Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> >> const libcamera::StreamConfiguration &streamConfiguration() const; >> libcamera::Stream *stream() const; >> const libcamera::PixelFormat &format() const; >> const libcamera::Size &size() const; >> >> - int configure(const libcamera::StreamConfiguration &cfg); >> + int configure(); >> int process(libcamera::FrameBuffer *source, >> MappedCamera3Buffer *dest, >> CameraMetadata *metadata); >
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 537883b63f15..62307726aea9 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1298,17 +1298,15 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) * StreamConfiguration and set the number of required buffers in * the Android camera3_stream_t. */ - for (unsigned int i = 0; i < stream_list->num_streams; ++i) { - camera3_stream_t *stream = stream_list->streams[i]; - CameraStream *cameraStream = static_cast<CameraStream *>(stream->priv); - const StreamConfiguration &cfg = cameraStream->streamConfiguration(); + for (CameraStream &cameraStream : streams_) { + camera3_stream_t *stream = cameraStream.androidStream(); - int ret = cameraStream->configure(cfg); + int ret = cameraStream.configure(); if (ret) return ret; /* Use the bufferCount confirmed by the validation process. */ - stream->max_buffers = cfg.bufferCount; + stream->max_buffers = cameraStream.streamConfiguration().bufferCount; } return 0; diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 76e7d240f4e7..dbde1e786300 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -56,10 +56,10 @@ const Size &CameraStream::size() const return streamConfiguration().size; } -int CameraStream::configure(const libcamera::StreamConfiguration &cfg) +int CameraStream::configure() { if (encoder_) { - int ret = encoder_->configure(cfg); + int ret = encoder_->configure(streamConfiguration()); if (ret) return ret; } diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index e399e17b2a2f..c6ff79230b7e 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -116,13 +116,14 @@ public: int outputFormat() const { return androidStream_->format; } Type type() const { return type_; } + camera3_stream_t *androidStream() const { return androidStream_; } const libcamera::StreamConfiguration &streamConfiguration() const; libcamera::Stream *stream() const; const libcamera::PixelFormat &format() const; const libcamera::Size &size() const; - int configure(const libcamera::StreamConfiguration &cfg); + int configure(); int process(libcamera::FrameBuffer *source, MappedCamera3Buffer *dest, CameraMetadata *metadata);