[PATCH v2 2/9] android: Move StreamBuffer out of Camera3RequestDescriptor
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Thu Nov 28 14:06:58 CET 2024
Hi Harvey
On Wed, Nov 27, 2024 at 09:25:52AM +0000, Harvey Yang wrote:
> Move StreamBuffer out of Camera3RequestDescriptor as a refactory before
s/as a refactory//
> applying partial result to Android adaptor. For easier adding
> Camera3ResultDescriptor which references both StreamBuffer and
> Camera3RequestDescriptor.
I presume you mean
"This change makes it easier to add Camera3ResultDescriptor, which
references both StreamBuffer and Camera3RequestDescriptor"
>
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
The patch itself is ok, let's see how it is used
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
> src/android/camera_device.cpp | 22 +++++-----
> src/android/camera_device.h | 8 ++--
> src/android/camera_request.cpp | 29 +++++++------
> src/android/camera_request.h | 53 ++++++++++++++----------
> src/android/camera_stream.cpp | 16 +++----
> src/android/camera_stream.h | 6 +--
> src/android/jpeg/encoder.h | 2 +-
> src/android/jpeg/encoder_jea.cpp | 2 +-
> src/android/jpeg/encoder_jea.h | 2 +-
> src/android/jpeg/encoder_libjpeg.cpp | 2 +-
> src/android/jpeg/encoder_libjpeg.h | 2 +-
> src/android/jpeg/post_processor_jpeg.cpp | 2 +-
> src/android/jpeg/post_processor_jpeg.h | 2 +-
> src/android/post_processor.h | 4 +-
> src/android/yuv/post_processor_yuv.cpp | 2 +-
> src/android/yuv/post_processor_yuv.h | 2 +-
> 16 files changed, 82 insertions(+), 74 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index a038131ae..4e3bdc9cc 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -865,7 +865,7 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const
> notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);
>
> for (auto &buffer : descriptor->buffers_)
> - buffer.status = Camera3RequestDescriptor::Status::Error;
> + buffer.status = StreamBuffer::Status::Error;
>
> descriptor->status_ = Camera3RequestDescriptor::Status::Error;
> }
> @@ -1163,7 +1163,7 @@ void CameraDevice::requestComplete(Request *request)
> if (fence)
> buffer.fence = fence->release();
> }
> - buffer.status = Camera3RequestDescriptor::Status::Success;
> + buffer.status = StreamBuffer::Status::Success;
> }
>
> /*
> @@ -1226,12 +1226,12 @@ void CameraDevice::requestComplete(Request *request)
> auto iter = descriptor->pendingStreamsToProcess_.begin();
> while (iter != descriptor->pendingStreamsToProcess_.end()) {
> CameraStream *stream = iter->first;
> - Camera3RequestDescriptor::StreamBuffer *buffer = iter->second;
> + StreamBuffer *buffer = iter->second;
>
> FrameBuffer *src = request->findBuffer(stream->stream());
> if (!src) {
> LOG(HAL, Error) << "Failed to find a source stream buffer";
> - setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error);
> + setBufferStatus(*buffer, StreamBuffer::Status::Error);
> iter = descriptor->pendingStreamsToProcess_.erase(iter);
> continue;
> }
> @@ -1241,7 +1241,7 @@ void CameraDevice::requestComplete(Request *request)
> ++iter;
> int ret = stream->process(buffer);
> if (ret) {
> - setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error);
> + setBufferStatus(*buffer, StreamBuffer::Status::Error);
> descriptor->pendingStreamsToProcess_.erase(stream);
>
> /*
> @@ -1311,7 +1311,7 @@ void CameraDevice::sendCaptureResults()
> for (auto &buffer : descriptor->buffers_) {
> camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR;
>
> - if (buffer.status == Camera3RequestDescriptor::Status::Success)
> + if (buffer.status == StreamBuffer::Status::Success)
> status = CAMERA3_BUFFER_STATUS_OK;
>
> /*
> @@ -1335,11 +1335,11 @@ void CameraDevice::sendCaptureResults()
> }
> }
>
> -void CameraDevice::setBufferStatus(Camera3RequestDescriptor::StreamBuffer &streamBuffer,
> - Camera3RequestDescriptor::Status status)
> +void CameraDevice::setBufferStatus(StreamBuffer &streamBuffer,
> + StreamBuffer::Status status)
> {
> streamBuffer.status = status;
> - if (status != Camera3RequestDescriptor::Status::Success) {
> + if (status != StreamBuffer::Status::Success) {
> notifyError(streamBuffer.request->frameNumber_,
> streamBuffer.stream->camera3Stream(),
> CAMERA3_MSG_ERROR_BUFFER);
> @@ -1363,8 +1363,8 @@ void CameraDevice::setBufferStatus(Camera3RequestDescriptor::StreamBuffer &strea
> * be generated from post-processing have been completed. Mark the descriptor as
> * complete using completeDescriptor() in that case.
> */
> -void CameraDevice::streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *streamBuffer,
> - Camera3RequestDescriptor::Status status)
> +void CameraDevice::streamProcessingComplete(StreamBuffer *streamBuffer,
> + StreamBuffer::Status status)
> {
> setBufferStatus(*streamBuffer, status);
>
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 194ca3030..c92ee1aa4 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -65,8 +65,8 @@ public:
> int configureStreams(camera3_stream_configuration_t *stream_list);
> int processCaptureRequest(camera3_capture_request_t *request);
> void requestComplete(libcamera::Request *request);
> - void streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *bufferStream,
> - Camera3RequestDescriptor::Status status);
> + void streamProcessingComplete(StreamBuffer *bufferStream,
> + StreamBuffer::Status status);
>
> protected:
> std::string logPrefix() const override;
> @@ -97,8 +97,8 @@ private:
> void completeDescriptor(Camera3RequestDescriptor *descriptor)
> LIBCAMERA_TSA_EXCLUDES(descriptorsMutex_);
> void sendCaptureResults() LIBCAMERA_TSA_REQUIRES(descriptorsMutex_);
> - void setBufferStatus(Camera3RequestDescriptor::StreamBuffer &buffer,
> - Camera3RequestDescriptor::Status status);
> + void setBufferStatus(StreamBuffer &buffer,
> + StreamBuffer::Status status);
> std::unique_ptr<CameraMetadata> getResultMetadata(
> const Camera3RequestDescriptor &descriptor) const;
>
> diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp
> index 0d45960d9..52a3ac1f7 100644
> --- a/src/android/camera_request.cpp
> +++ b/src/android/camera_request.cpp
> @@ -141,7 +141,7 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(
> Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
>
> /**
> - * \struct Camera3RequestDescriptor::StreamBuffer
> + * \class StreamBuffer
> * \brief Group information for per-stream buffer of Camera3RequestDescriptor
> *
> * A capture request placed to the libcamera HAL can contain multiple streams.
> @@ -150,35 +150,35 @@ Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
> * generation. The generation of the stream will depend on its type (refer to
> * the CameraStream::Type documentation).
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::stream
> + * \var StreamBuffer::stream
> * \brief Pointer to the corresponding CameraStream
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::camera3Buffer
> + * \var StreamBuffer::camera3Buffer
> * \brief Native handle to the buffer
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::frameBuffer
> + * \var StreamBuffer::frameBuffer
> * \brief Encapsulate the dmabuf handle inside a libcamera::FrameBuffer for
> * direct streams
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::fence
> + * \var StreamBuffer::fence
> * \brief Acquire fence of the buffer
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::status
> + * \var StreamBuffer::status
> * \brief Track the status of the buffer
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::internalBuffer
> + * \var StreamBuffer::internalBuffer
> * \brief Pointer to a buffer internally handled by CameraStream (if any)
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::srcBuffer
> + * \var StreamBuffer::srcBuffer
> * \brief Pointer to the source frame buffer used for post-processing
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::dstBuffer
> + * \var StreamBuffer::dstBuffer
> * \brief Pointer to the destination frame buffer used for post-processing
> *
> - * \var Camera3RequestDescriptor::StreamBuffer::request
> + * \var StreamBuffer::request
> * \brief Back pointer to the Camera3RequestDescriptor to which the StreamBuffer belongs
> */
> -Camera3RequestDescriptor::StreamBuffer::StreamBuffer(
> +StreamBuffer::StreamBuffer(
> CameraStream *cameraStream, const camera3_stream_buffer_t &buffer,
> Camera3RequestDescriptor *requestDescriptor)
> : stream(cameraStream), camera3Buffer(buffer.buffer),
> @@ -186,9 +186,8 @@ Camera3RequestDescriptor::StreamBuffer::StreamBuffer(
> {
> }
>
> -Camera3RequestDescriptor::StreamBuffer::~StreamBuffer() = default;
> +StreamBuffer::~StreamBuffer() = default;
>
> -Camera3RequestDescriptor::StreamBuffer::StreamBuffer(StreamBuffer &&) = default;
> +StreamBuffer::StreamBuffer(StreamBuffer &&) = default;
>
> -Camera3RequestDescriptor::StreamBuffer &
> -Camera3RequestDescriptor::StreamBuffer::operator=(Camera3RequestDescriptor::StreamBuffer &&) = default;
> +StreamBuffer &StreamBuffer::operator=(StreamBuffer &&) = default;
> diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> index 5b479180f..335f1985d 100644
> --- a/src/android/camera_request.h
> +++ b/src/android/camera_request.h
> @@ -26,7 +26,9 @@
> class CameraBuffer;
> class CameraStream;
>
> -class Camera3RequestDescriptor
> +class Camera3RequestDescriptor;
> +
> +class StreamBuffer
> {
> public:
> enum class Status {
> @@ -34,27 +36,34 @@ public:
> Error,
> };
>
> - struct StreamBuffer {
> - StreamBuffer(CameraStream *stream,
> - const camera3_stream_buffer_t &buffer,
> - Camera3RequestDescriptor *request);
> - ~StreamBuffer();
> -
> - StreamBuffer(StreamBuffer &&);
> - StreamBuffer &operator=(StreamBuffer &&);
> -
> - CameraStream *stream;
> - buffer_handle_t *camera3Buffer;
> - std::unique_ptr<HALFrameBuffer> frameBuffer;
> - libcamera::UniqueFD fence;
> - Status status = Status::Success;
> - libcamera::FrameBuffer *internalBuffer = nullptr;
> - const libcamera::FrameBuffer *srcBuffer = nullptr;
> - std::unique_ptr<CameraBuffer> dstBuffer;
> - Camera3RequestDescriptor *request;
> -
> - private:
> - LIBCAMERA_DISABLE_COPY(StreamBuffer)
> + StreamBuffer(CameraStream *stream,
> + const camera3_stream_buffer_t &buffer,
> + Camera3RequestDescriptor *request);
> + ~StreamBuffer();
> +
> + StreamBuffer(StreamBuffer &&);
> + StreamBuffer &operator=(StreamBuffer &&);
> +
> + CameraStream *stream;
> + buffer_handle_t *camera3Buffer;
> + std::unique_ptr<HALFrameBuffer> frameBuffer;
> + libcamera::UniqueFD fence;
> + Status status = Status::Success;
> + libcamera::FrameBuffer *internalBuffer = nullptr;
> + const libcamera::FrameBuffer *srcBuffer = nullptr;
> + std::unique_ptr<CameraBuffer> dstBuffer;
> + Camera3RequestDescriptor *request;
> +
> +private:
> + LIBCAMERA_DISABLE_COPY(StreamBuffer)
> +};
> +
> +class Camera3RequestDescriptor
> +{
> +public:
> + enum class Status {
> + Success,
> + Error,
> };
>
> /* Keeps track of streams requiring post-processing. */
> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> index 1d68540d7..53f292d4b 100644
> --- a/src/android/camera_stream.cpp
> +++ b/src/android/camera_stream.cpp
> @@ -112,14 +112,14 @@ int CameraStream::configure()
>
> worker_ = std::make_unique<PostProcessorWorker>(postProcessor_.get());
> postProcessor_->processComplete.connect(
> - this, [&](Camera3RequestDescriptor::StreamBuffer *streamBuffer,
> + this, [&](StreamBuffer *streamBuffer,
> PostProcessor::Status status) {
> - Camera3RequestDescriptor::Status bufferStatus;
> + StreamBuffer::Status bufferStatus;
>
> if (status == PostProcessor::Status::Success)
> - bufferStatus = Camera3RequestDescriptor::Status::Success;
> + bufferStatus = StreamBuffer::Status::Success;
> else
> - bufferStatus = Camera3RequestDescriptor::Status::Error;
> + bufferStatus = StreamBuffer::Status::Error;
>
> cameraDevice_->streamProcessingComplete(streamBuffer,
> bufferStatus);
> @@ -165,7 +165,7 @@ int CameraStream::waitFence(int fence)
> return -errno;
> }
>
> -int CameraStream::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer)
> +int CameraStream::process(StreamBuffer *streamBuffer)
> {
> ASSERT(type_ != Type::Direct);
>
> @@ -283,7 +283,7 @@ void CameraStream::PostProcessorWorker::start()
> Thread::start();
> }
>
> -void CameraStream::PostProcessorWorker::queueRequest(Camera3RequestDescriptor::StreamBuffer *dest)
> +void CameraStream::PostProcessorWorker::queueRequest(StreamBuffer *dest)
> {
> {
> MutexLocker lock(mutex_);
> @@ -306,7 +306,7 @@ void CameraStream::PostProcessorWorker::run()
> if (state_ != State::Running)
> break;
>
> - Camera3RequestDescriptor::StreamBuffer *streamBuffer = requests_.front();
> + StreamBuffer *streamBuffer = requests_.front();
> requests_.pop();
> locker.unlock();
>
> @@ -316,7 +316,7 @@ void CameraStream::PostProcessorWorker::run()
> }
>
> if (state_ == State::Flushing) {
> - std::queue<Camera3RequestDescriptor::StreamBuffer *> requests =
> + std::queue<StreamBuffer *> requests =
> std::move(requests_);
> locker.unlock();
>
> diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h
> index 395552dac..30f64f690 100644
> --- a/src/android/camera_stream.h
> +++ b/src/android/camera_stream.h
> @@ -127,7 +127,7 @@ public:
> CameraStream *sourceStream() const { return sourceStream_; }
>
> int configure();
> - int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer);
> + int process(StreamBuffer *streamBuffer);
> libcamera::FrameBuffer *getBuffer();
> void putBuffer(libcamera::FrameBuffer *buffer);
> void flush();
> @@ -146,7 +146,7 @@ private:
> ~PostProcessorWorker();
>
> void start();
> - void queueRequest(Camera3RequestDescriptor::StreamBuffer *request);
> + void queueRequest(StreamBuffer *request);
> void flush();
>
> protected:
> @@ -158,7 +158,7 @@ private:
> libcamera::Mutex mutex_;
> libcamera::ConditionVariable cv_;
>
> - std::queue<Camera3RequestDescriptor::StreamBuffer *> requests_
> + std::queue<StreamBuffer *> requests_
> LIBCAMERA_TSA_GUARDED_BY(mutex_);
>
> State state_ LIBCAMERA_TSA_GUARDED_BY(mutex_) = State::Stopped;
> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
> index ed033c191..25425d92c 100644
> --- a/src/android/jpeg/encoder.h
> +++ b/src/android/jpeg/encoder.h
> @@ -20,7 +20,7 @@ public:
> virtual ~Encoder() = default;
>
> virtual int configure(const libcamera::StreamConfiguration &cfg) = 0;
> - virtual int encode(Camera3RequestDescriptor::StreamBuffer *buffer,
> + virtual int encode(StreamBuffer *buffer,
> libcamera::Span<const uint8_t> exifData,
> unsigned int quality) = 0;
> };
> diff --git a/src/android/jpeg/encoder_jea.cpp b/src/android/jpeg/encoder_jea.cpp
> index 25dc43173..b3ad04638 100644
> --- a/src/android/jpeg/encoder_jea.cpp
> +++ b/src/android/jpeg/encoder_jea.cpp
> @@ -33,7 +33,7 @@ int EncoderJea::configure(const libcamera::StreamConfiguration &cfg)
> return 0;
> }
>
> -int EncoderJea::encode(Camera3RequestDescriptor::StreamBuffer *buffer,
> +int EncoderJea::encode(StreamBuffer *buffer,
> libcamera::Span<const uint8_t> exifData,
> unsigned int quality)
> {
> diff --git a/src/android/jpeg/encoder_jea.h b/src/android/jpeg/encoder_jea.h
> index 91115d2e9..79265ebc6 100644
> --- a/src/android/jpeg/encoder_jea.h
> +++ b/src/android/jpeg/encoder_jea.h
> @@ -20,7 +20,7 @@ public:
> ~EncoderJea();
>
> int configure(const libcamera::StreamConfiguration &cfg) override;
> - int encode(Camera3RequestDescriptor::StreamBuffer *buffer,
> + int encode(StreamBuffer *buffer,
> libcamera::Span<const uint8_t> exifData,
> unsigned int quality) override;
>
> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp
> index cb242b5ec..cb123576a 100644
> --- a/src/android/jpeg/encoder_libjpeg.cpp
> +++ b/src/android/jpeg/encoder_libjpeg.cpp
> @@ -180,7 +180,7 @@ void EncoderLibJpeg::compressNV(const std::vector<Span<uint8_t>> &planes)
> }
> }
>
> -int EncoderLibJpeg::encode(Camera3RequestDescriptor::StreamBuffer *buffer,
> +int EncoderLibJpeg::encode(StreamBuffer *buffer,
> libcamera::Span<const uint8_t> exifData,
> unsigned int quality)
> {
> diff --git a/src/android/jpeg/encoder_libjpeg.h b/src/android/jpeg/encoder_libjpeg.h
> index 4ac85c22e..de04b17d1 100644
> --- a/src/android/jpeg/encoder_libjpeg.h
> +++ b/src/android/jpeg/encoder_libjpeg.h
> @@ -22,7 +22,7 @@ public:
> ~EncoderLibJpeg();
>
> int configure(const libcamera::StreamConfiguration &cfg) override;
> - int encode(Camera3RequestDescriptor::StreamBuffer *buffer,
> + int encode(StreamBuffer *buffer,
> libcamera::Span<const uint8_t> exifData,
> unsigned int quality) override;
> int encode(const std::vector<libcamera::Span<uint8_t>> &planes,
> diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp
> index 89b8a401e..f5a90785d 100644
> --- a/src/android/jpeg/post_processor_jpeg.cpp
> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> @@ -106,7 +106,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source,
> }
> }
>
> -void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer)
> +void PostProcessorJpeg::process(StreamBuffer *streamBuffer)
> {
> ASSERT(encoder_);
>
> diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h
> index 6fe214577..5421f233e 100644
> --- a/src/android/jpeg/post_processor_jpeg.h
> +++ b/src/android/jpeg/post_processor_jpeg.h
> @@ -22,7 +22,7 @@ public:
>
> int configure(const libcamera::StreamConfiguration &incfg,
> const libcamera::StreamConfiguration &outcfg) override;
> - void process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override;
> + void process(StreamBuffer *streamBuffer) override;
>
> private:
> void generateThumbnail(const libcamera::FrameBuffer &source,
> diff --git a/src/android/post_processor.h b/src/android/post_processor.h
> index b504a3796..f2e00d2b1 100644
> --- a/src/android/post_processor.h
> +++ b/src/android/post_processor.h
> @@ -27,7 +27,7 @@ public:
>
> virtual int configure(const libcamera::StreamConfiguration &inCfg,
> const libcamera::StreamConfiguration &outCfg) = 0;
> - virtual void process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) = 0;
> + virtual void process(StreamBuffer *streamBuffer) = 0;
>
> - libcamera::Signal<Camera3RequestDescriptor::StreamBuffer *, Status> processComplete;
> + libcamera::Signal<StreamBuffer *, Status> processComplete;
> };
> diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> index c998807b0..b05623b23 100644
> --- a/src/android/yuv/post_processor_yuv.cpp
> +++ b/src/android/yuv/post_processor_yuv.cpp
> @@ -49,7 +49,7 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg,
> return 0;
> }
>
> -void PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer)
> +void PostProcessorYuv::process(StreamBuffer *streamBuffer)
> {
> const FrameBuffer &source = *streamBuffer->srcBuffer;
> CameraBuffer *destination = streamBuffer->dstBuffer.get();
> diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
> index ed7bb1fbe..459c6685e 100644
> --- a/src/android/yuv/post_processor_yuv.h
> +++ b/src/android/yuv/post_processor_yuv.h
> @@ -18,7 +18,7 @@ public:
>
> int configure(const libcamera::StreamConfiguration &incfg,
> const libcamera::StreamConfiguration &outcfg) override;
> - void process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override;
> + void process(StreamBuffer *streamBuffer) override;
>
> private:
> bool isValidBuffers(const libcamera::FrameBuffer &source,
> --
> 2.47.0.338.g60cca15819-goog
>
More information about the libcamera-devel
mailing list