[libcamera-devel] [PATCH 1/5] android: yuv: Use CameraBuffer::stride() in PostProcessorYuv
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Aug 31 23:30:24 CEST 2021
Hi Hiro,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
On Wed, Sep 01, 2021 at 03:11:00AM +0900, Hirokazu Honda wrote:
> On Tue, Aug 31, 2021 at 7:28 PM Jacopo Mondi wrote:
> > On Tue, Aug 31, 2021 at 03:34:34PM +0900, Hirokazu Honda wrote:
> > > PostProcessorYuv computes strides for a CameraBuffer. CameraBuffer has
> > > stride() function. This replaces the computation with the function.
> > >
> > > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > > ---
> > > src/android/yuv/post_processor_yuv.cpp | 13 +++++++------
> > > src/android/yuv/post_processor_yuv.h | 1 -
> > > 2 files changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp
> > > index 6952fc38..12d6297d 100644
> > > --- a/src/android/yuv/post_processor_yuv.cpp
> > > +++ b/src/android/yuv/post_processor_yuv.cpp
> > > @@ -69,9 +69,9 @@ int PostProcessorYuv::process(const FrameBuffer &source,
> > > sourceStride_[1],
> > > sourceSize_.width, sourceSize_.height,
> > > destination->plane(0).data(),
> > > - destinationStride_[0],
> > > + destination->stride(0),
> > > destination->plane(1).data(),
> > > - destinationStride_[1],
> > > + destination->stride(1),
> > > destinationSize_.width,
> > > destinationSize_.height,
> > > libyuv::FilterMode::kFilterBilinear);
> > > @@ -115,8 +115,8 @@ bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source,
> > > << destination.plane(0).size() << ", "
> > > << destination.plane(1).size()
> > > << "}, expected size: {"
> > > - << sourceLength_[0] << ", "
> > > - << sourceLength_[1] << "}";
> > > + << destinationLength_[0] << ", "
> > > + << destinationLength_[1] << "}";
> >
> > Is this intended ?
>
> Yes, I found this was wrong.
Could you please mention it in the commit message ?
> > > return false;
> > > }
> > >
> > > @@ -132,13 +132,14 @@ void PostProcessorYuv::calculateLengths(const StreamConfiguration &inCfg,
> > > const PixelFormatInfo &nv12Info = PixelFormatInfo::info(formats::NV12);
> > > for (unsigned int i = 0; i < 2; i++) {
> > > sourceStride_[i] = inCfg.stride;
> > > - destinationStride_[i] = nv12Info.stride(destinationSize_.width, i, 1);
> > > + const unsigned int destinationStride =
> > > + nv12Info.stride(destinationSize_.width, i, 1);
> > >
> > > const unsigned int vertSubSample =
> > > nv12Info.planes[i].verticalSubSampling;
> > > sourceLength_[i] = sourceStride_[i] *
> > > ((sourceSize_.height + vertSubSample - 1) / vertSubSample);
> > > - destinationLength_[i] = destinationStride_[i] *
> > > + destinationLength_[i] = destinationStride *
> > > ((destinationSize_.height + vertSubSample - 1) / vertSubSample);
> >
> > A comment on the existing implementation:
> > destinationLength_[i] is used in isValidBuffer() only for this
> > comparison:
> >
> > if (destination.plane(0).size() < destinationLength_[0] ||
> > destination.plane(1).size() < destinationLength_[1]) {
> >
> > shouldn't we compare the source and dest ?
>
> Do you mean to add the comment? Where shall I add?
>
> > Also as destination is a CameraBuffer, shouldn't we use
> > CameraBuffer::size(i) there ?
I thought about that but then realized that the check is meant to ensure
that the buffer has a size compatible with the size that was set at
configuration time.
I think this should be reworked to use outCfg.stride and move the
destination stride calculation to the caller, but maybe a todo comment
would be enough for now.
/* \todo Move stride calculation to the caller and use outCfg.stride */
> > > }
> > > }
> > > diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h
> > > index f8b1ba23..1a0b5e89 100644
> > > --- a/src/android/yuv/post_processor_yuv.h
> > > +++ b/src/android/yuv/post_processor_yuv.h
> > > @@ -36,7 +36,6 @@ private:
> > > unsigned int sourceLength_[2] = {};
> > > unsigned int destinationLength_[2] = {};
> > > unsigned int sourceStride_[2] = {};
> > > - unsigned int destinationStride_[2] = {};
> > > };
> > >
> > > #endif /* __ANDROID_POST_PROCESSOR_YUV_H__ */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list