[libcamera-devel] [PATCH v2 3/4] android: post_processor_jpeg: Make |cameraDevice_| constant

[Hirokazu Honda]

On Wed, Oct 21, 2020 at 12:08 AM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> On 20/10/2020 11:45, Umang Jain wrote:
> > Hi Hiro,
> >
> > Thanks for your work.
> >
> > On 10/20/20 1:12 PM, Hirokazu Honda wrote:
> >> PostProcessorJpeg doesn't have the ownership of CameraDevice given
> >> in the constructor. To clarify it, this makes the member variable
> >> constant.
> >>
> >> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> >> ---
> >>   src/android/jpeg/post_processor_jpeg.cpp | 2 +-
> >>   src/android/jpeg/post_processor_jpeg.h   | 4 ++--
> >>   2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/src/android/jpeg/post_processor_jpeg.cpp
> >> b/src/android/jpeg/post_processor_jpeg.cpp
> >> index 6f33631..f895d1f 100644
> >> --- a/src/android/jpeg/post_processor_jpeg.cpp
> >> +++ b/src/android/jpeg/post_processor_jpeg.cpp
> >> @@ -20,7 +20,7 @@ using namespace libcamera;
> >>     LOG_DEFINE_CATEGORY(JPEG);
> >>   -PostProcessorJpeg::PostProcessorJpeg(CameraDevice *device)
> >> +PostProcessorJpeg::PostProcessorJpeg(CameraDevice const *device)
> >>       : cameraDevice_(device)
> >>   {
> >>   }
> >> diff --git a/src/android/jpeg/post_processor_jpeg.h
> >> b/src/android/jpeg/post_processor_jpeg.h
> >> index ae636ff..8e25b29 100644
> >> --- a/src/android/jpeg/post_processor_jpeg.h
> >> +++ b/src/android/jpeg/post_processor_jpeg.h
> >> @@ -19,7 +19,7 @@ class CameraDevice;
> >>   class PostProcessorJpeg : public PostProcessor
> >>   {
> >>   public:
> >> -    PostProcessorJpeg(CameraDevice *device);
> >> +    PostProcessorJpeg(CameraDevice const *device);
> >>         int configure(const libcamera::StreamConfiguration &incfg,
> >>                 const libcamera::StreamConfiguration &outcfg) override;
> >> @@ -28,7 +28,7 @@ public:
> >>               CameraMetadata *metadata) override;
> >>     private:
> >> -    CameraDevice *cameraDevice_;
> >> +    CameraDevice const *cameraDevice_;
> >>       std::unique_ptr<Encoder> encoder_;
> >>       libcamera::Size streamSize_;
> >>   };
> > Not a hard-blocker but I think libcamera uses the other style for
> > `const` definition - `const class *ptr_;` I would refer to overlords to
> > use their discretion here before pushing.
>
> Indeed, i think this should be a prefixed const.
>

Oops, I made a mistake here.
I intended to change it to `class *const ptr_`, which means the ptr
value (not value pointed by the ptr) cannot be changed.
(The mistake is because I have been writing for it as `class* const
ptr_` in my life.)

I changed so in the next patch.

> I can update locally, assuming there's nothing else critical.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> >
> > Reviewed-by: Umang Jain <email at uajain.com>
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards
> --
> Kieran