[libcamera-devel] [PATCH v2 2/4] android: Modify Encoder interface
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Oct 20 17:05:27 CEST 2020
On 20/10/2020 15:57, Kieran Bingham wrote:
> Hi Hiro,
>
> On 20/10/2020 11:38, Umang Jain wrote:
>> Hi Hiro,
>>
>> Thanks for your work.
>>
>> On 10/20/20 1:12 PM, Hirokazu Honda wrote:
>>> In Encoder::encode(), the |source| argument doesn't have to be a
>>> pointer. This replaces its type, const pointer, with const
>>> reference as the latter is preferred to the former.
>>> libcamera::Span is chea to construct/copy/move. We should deal
>> s/chea/cheap
>
> Hrm, this is fine in the original. Must have caught it while editing.
Ignore that - I was comparing against a different patch ;-)
Fixed up here locally.
>
>>> with the type as pass-by-value parameter. Therefore this also
>>> drops the const reference in the |destination| argument.
>>>
>>> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
>> LGTM again.
>> Reviewed-by: Umang Jain <email at uajain.com>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>>> ---
>>> src/android/jpeg/encoder.h | 4 ++--
>>> src/android/jpeg/encoder_libjpeg.cpp | 6 +++---
>>> src/android/jpeg/encoder_libjpeg.h | 4 ++--
>>> src/android/jpeg/post_processor_jpeg.cpp | 2 +-
>>> 4 files changed, 8 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/android/jpeg/encoder.h b/src/android/jpeg/encoder.h
>>> index 4483153..4fe71cf 100644
>>> --- a/src/android/jpeg/encoder.h
>>> +++ b/src/android/jpeg/encoder.h
>>> @@ -17,8 +17,8 @@ public:
>>> virtual ~Encoder() {};
>>> virtual int configure(const libcamera::StreamConfiguration
>>> &cfg) = 0;
>>> - virtual int encode(const libcamera::FrameBuffer *source,
>>> - const libcamera::Span<uint8_t> &destination,
>>> + virtual int encode(const libcamera::FrameBuffer &source,
>>> + libcamera::Span<uint8_t> destination,
>>> const libcamera::Span<const uint8_t> &exifData) = 0;
>>> };
>>> diff --git a/src/android/jpeg/encoder_libjpeg.cpp
>>> b/src/android/jpeg/encoder_libjpeg.cpp
>>> index 8995ba5..4236c9d 100644
>>> --- a/src/android/jpeg/encoder_libjpeg.cpp
>>> +++ b/src/android/jpeg/encoder_libjpeg.cpp
>>> @@ -179,11 +179,11 @@ void EncoderLibJpeg::compressNV(const
>>> libcamera::MappedBuffer *frame)
>>> }
>>> }
>>> -int EncoderLibJpeg::encode(const FrameBuffer *source,
>>> - const libcamera::Span<uint8_t> &dest,
>>> +int EncoderLibJpeg::encode(const FrameBuffer &source,
>>> + libcamera::Span<uint8_t> dest,
>>> const libcamera::Span<const uint8_t> &exifData)
>>> {
>>> - MappedFrameBuffer frame(source, PROT_READ);
>>> + MappedFrameBuffer frame(&source, PROT_READ);
>
> Perhaps MappedFrameBuffer should also take a reference, but lets not
> worry about that here.
>
>>> if (!frame.isValid()) {
>>> LOG(JPEG, Error) << "Failed to map FrameBuffer : "
>>> << strerror(frame.error());
>>> diff --git a/src/android/jpeg/encoder_libjpeg.h
>>> b/src/android/jpeg/encoder_libjpeg.h
>>> index 934caef..391a53c 100644
>>> --- a/src/android/jpeg/encoder_libjpeg.h
>>> +++ b/src/android/jpeg/encoder_libjpeg.h
>>> @@ -21,8 +21,8 @@ public:
>>> ~EncoderLibJpeg();
>>> int configure(const libcamera::StreamConfiguration &cfg)
>>> override;
>>> - int encode(const libcamera::FrameBuffer *source,
>>> - const libcamera::Span<uint8_t> &destination,
>>> + int encode(const libcamera::FrameBuffer &source,
>>> + libcamera::Span<uint8_t> destination,
>>> const libcamera::Span<const uint8_t> &exifData) override;
>>> private:
>>> diff --git a/src/android/jpeg/post_processor_jpeg.cpp
>>> b/src/android/jpeg/post_processor_jpeg.cpp
>>> index 55b9088..6f33631 100644
>>> --- a/src/android/jpeg/post_processor_jpeg.cpp
>>> +++ b/src/android/jpeg/post_processor_jpeg.cpp
>>> @@ -67,7 +67,7 @@ int PostProcessorJpeg::process(const
>>> libcamera::FrameBuffer &source,
>>> if (exif.generate() != 0)
>>> LOG(JPEG, Error) << "Failed to generate valid EXIF data";
>>> - int jpeg_size = encoder_->encode(&source, destination,
>>> exif.data());
>>> + int jpeg_size = encoder_->encode(source, destination, exif.data());
>> Perfect. I was expecting this from v1.
>>> if (jpeg_size < 0) {
>>> LOG(JPEG, Error) << "Failed to encode stream image";
>>> return jpeg_size;
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list