[libcamera-devel] [PATCH] android: Cleanup libcamera namespace usage
Hirokazu Honda
hiroh at chromium.org
Fri Sep 3 11:07:16 CEST 2021
Hi Umang,
On Fri, Sep 3, 2021 at 5:40 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
>
> Hiro,
>
>
> On 9/3/21 2:04 PM, Hirokazu Honda wrote:
> > Hi Umang, thank you for the patch.
> >
> >
> > On Fri, Sep 3, 2021 at 4:52 PM Umang Jain <umang.jain at ideasonboard.com> wrote:
> >> Usually .cpp files are equipped with using namespace libcamera;
> >> Hence, it is unnecessary mentioning the explicit namespace of
> >> libcamera at certain places.
> >>
> >> While at it, a small typo in a comment was noticed and fixed as
> >> part of this patch.
> >>
> >> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> >> ---
> >> src/android/camera_capabilities.cpp | 4 ++--
> >> src/android/camera_device.cpp | 6 +++---
> >> src/android/camera_stream.cpp | 4 ++--
> >> src/android/camera_worker.cpp | 2 +-
> >> src/android/mm/cros_camera_buffer.cpp | 6 +++---
> >> src/android/mm/generic_camera_buffer.cpp | 11 +++++------
> > $ grep -l -r "libcamera::" --include \*.cpp
> > camera_device.cpp
> > camera_capabilities.cpp
> > camera_worker.cpp
> > camera_stream.cpp
> > mm/generic_camera_buffer.cpp
> > mm/cros_camera_buffer.cpp
> > jpeg/encoder_libjpeg.cpp
> > jpeg/thumbnailer.cpp
> >
> > There are also cases in jpeg directories.
> > Could you apply this change to them?
>
> Are you running this with the patch applied? :-)
>
Sorry, I executed grep with my patch series
https://patchwork.libcamera.org/project/libcamera/list/?series=2423.
It contains the unnecessary "libcamera::" in the change. I am
definitely going to remove them in my patch series.
I confirmed all the unnecessary libcamera in top of tree are removed
in android with this patch.
Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
Best Regards,
-Hiro
> With his patch applied here, I get:
>
> [uajain at fedora android]$ grep -l -r "libcamera::" --include \*.cpp
> camera_device.cpp
> camera_worker.cpp
> camera_stream.cpp
> [uajain at fedora android]$ git grep -ni "libcamera::" | grep \.cpp
> camera_device.cpp:54: * Reorder the configurations so that
> libcamera::Camera can accept them as much
> camera_device.cpp:253: * The CameraDevice class wraps a
> libcamera::Camera instance, and implements
> camera_device.cpp:315: * If the libcamera::Camera provides those
> information as retrieved
> camera_device.cpp:1250: * will have to be passed to the
> libcamera::Camera and extracted
> camera_device.cpp:1251: * from libcamera::Request::metadata.
> camera_stream.cpp:28: * camera framework to a
> libcamera::StreamConfiguration.
> camera_stream.cpp:40: * the associated libcamera::Stream, including the
> creation of the encoder
> camera_worker.cpp:23: * \brief Wrap a libcamera::Request associated with
> buffers and fences
> camera_worker.cpp:27: * by the CameraWorker which queues it to the
> libcamera::Camera after handling
>
> I don't think it would be wise to remove the `libcamera::` on comments.
> They probably have some "contextual" value
>
>
> >
> > Thanks,
> > -Hiro
> >> 6 files changed, 16 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> >> index c16e755f..e92bca42 100644
> >> --- a/src/android/camera_capabilities.cpp
> >> +++ b/src/android/camera_capabilities.cpp
> >> @@ -378,7 +378,7 @@ void CameraCapabilities::computeHwLevel(
> >> hwLevel_ = hwLevel;
> >> }
> >>
> >> -int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> >> +int CameraCapabilities::initialize(std::shared_ptr<Camera> camera,
> >> int orientation, int facing)
> >> {
> >> camera_ = camera;
> >> @@ -429,7 +429,7 @@ CameraCapabilities::initializeYUVResolutions(const PixelFormat &pixelFormat,
> >> }
> >>
> >> std::vector<Size>
> >> -CameraCapabilities::initializeRawResolutions(const libcamera::PixelFormat &pixelFormat)
> >> +CameraCapabilities::initializeRawResolutions(const PixelFormat &pixelFormat)
> >> {
> >> std::unique_ptr<CameraConfiguration> cameraConfig =
> >> camera_->generateConfiguration({ StreamRole::Raw });
> >> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> >> index 8ca76719..d782067e 100644
> >> --- a/src/android/camera_device.cpp
> >> +++ b/src/android/camera_device.cpp
> >> @@ -746,8 +746,8 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> >> }
> >>
> >> FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer,
> >> - libcamera::PixelFormat pixelFormat,
> >> - const libcamera::Size &size)
> >> + PixelFormat pixelFormat,
> >> + const Size &size)
> >> {
> >> FileDescriptor fd;
> >> /*
> >> @@ -1140,7 +1140,7 @@ void CameraDevice::requestComplete(Request *request)
> >> if (!resultMetadata) {
> >> notifyError(descriptor.frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);
> >>
> >> - /* The camera framework expects an empy metadata pack on error. */
> >> + /* The camera framework expects an empty metadata pack on error. */
> >> resultMetadata = std::make_unique<CameraMetadata>(0, 0);
> >> }
> >>
> >> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp
> >> index 01909ec7..0f5ae947 100644
> >> --- a/src/android/camera_stream.cpp
> >> +++ b/src/android/camera_stream.cpp
> >> @@ -98,7 +98,7 @@ int CameraStream::configure()
> >> return 0;
> >> }
> >>
> >> -int CameraStream::process(const libcamera::FrameBuffer &source,
> >> +int CameraStream::process(const FrameBuffer &source,
> >> buffer_handle_t camera3Dest,
> >> const CameraMetadata &requestMetadata,
> >> CameraMetadata *resultMetadata)
> >> @@ -139,7 +139,7 @@ FrameBuffer *CameraStream::getBuffer()
> >> return buffer;
> >> }
> >>
> >> -void CameraStream::putBuffer(libcamera::FrameBuffer *buffer)
> >> +void CameraStream::putBuffer(FrameBuffer *buffer)
> >> {
> >> if (!allocator_)
> >> return;
> >> diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp
> >> index 98dddd9e..91313183 100644
> >> --- a/src/android/camera_worker.cpp
> >> +++ b/src/android/camera_worker.cpp
> >> @@ -27,7 +27,7 @@ LOG_DECLARE_CATEGORY(HAL)
> >> * by the CameraWorker which queues it to the libcamera::Camera after handling
> >> * fences.
> >> */
> >> -CaptureRequest::CaptureRequest(libcamera::Camera *camera)
> >> +CaptureRequest::CaptureRequest(Camera *camera)
> >> : camera_(camera)
> >> {
> >> request_ = camera_->createRequest(reinterpret_cast<uint64_t>(this));
> >> diff --git a/src/android/mm/cros_camera_buffer.cpp b/src/android/mm/cros_camera_buffer.cpp
> >> index 44993379..ec45e04c 100644
> >> --- a/src/android/mm/cros_camera_buffer.cpp
> >> +++ b/src/android/mm/cros_camera_buffer.cpp
> >> @@ -21,7 +21,7 @@ class CameraBuffer::Private : public Extensible::Private
> >>
> >> public:
> >> Private(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,
> >> - libcamera::PixelFormat pixelFormat, const libcamera::Size &size,
> >> + PixelFormat pixelFormat, const Size &size,
> >> int flags);
> >> ~Private();
> >>
> >> @@ -53,8 +53,8 @@ private:
> >>
> >> CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
> >> buffer_handle_t camera3Buffer,
> >> - [[maybe_unused]] libcamera::PixelFormat pixelFormat,
> >> - [[maybe_unused]] const libcamera::Size &size,
> >> + [[maybe_unused]] PixelFormat pixelFormat,
> >> + [[maybe_unused]] const Size &size,
> >> [[maybe_unused]] int flags)
> >> : handle_(camera3Buffer), numPlanes_(0), mapped_(false),
> >> registered_(false)
> >> diff --git a/src/android/mm/generic_camera_buffer.cpp b/src/android/mm/generic_camera_buffer.cpp
> >> index 22efc4d4..27a30e2d 100644
> >> --- a/src/android/mm/generic_camera_buffer.cpp
> >> +++ b/src/android/mm/generic_camera_buffer.cpp
> >> @@ -20,14 +20,13 @@ using namespace libcamera;
> >> LOG_DECLARE_CATEGORY(HAL)
> >>
> >> class CameraBuffer::Private : public Extensible::Private,
> >> - public libcamera::MappedBuffer
> >> + public MappedBuffer
> >> {
> >> LIBCAMERA_DECLARE_PUBLIC(CameraBuffer)
> >>
> >> public:
> >> Private(CameraBuffer *cameraBuffer, buffer_handle_t camera3Buffer,
> >> - libcamera::PixelFormat pixelFormat, const libcamera::Size &size,
> >> - int flags);
> >> + PixelFormat pixelFormat, const Size &size, int flags);
> >> ~Private();
> >>
> >> unsigned int numPlanes() const;
> >> @@ -58,13 +57,13 @@ private:
> >>
> >> CameraBuffer::Private::Private([[maybe_unused]] CameraBuffer *cameraBuffer,
> >> buffer_handle_t camera3Buffer,
> >> - libcamera::PixelFormat pixelFormat,
> >> - const libcamera::Size &size, int flags)
> >> + PixelFormat pixelFormat,
> >> + const Size &size, int flags)
> >> : fd_(-1), flags_(flags), bufferLength_(-1), mapped_(false)
> >> {
> >> error_ = 0;
> >>
> >> - const auto &info = libcamera::PixelFormatInfo::info(pixelFormat);
> >> + const auto &info = PixelFormatInfo::info(pixelFormat);
> >> if (!info.isValid()) {
> >> error_ = -EINVAL;
> >> LOG(HAL, Error) << "Invalid pixel format: "
> >> --
> >> 2.31.0
> >>
More information about the libcamera-devel
mailing list