[{"id":5077,"web_url":"https://patchwork.libcamera.org/comment/5077/","msgid":"<20200605232013.GQ26752@pendragon.ideasonboard.com>","date":"2020-06-05T23:20:13","subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: Rename ImageFormats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Fri, May 29, 2020 at 01:03:33PM +0200, Jacopo Mondi wrote:\n> The ImageFormats class is actually a map. Convey that in the class name.\n\nIt's a hybrid of a map with extra random features. That's partly why I\ndon't like it. I don't see much value in a rename.\n\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  include/libcamera/internal/camera_sensor.h    |  2 +-\n>  include/libcamera/internal/formats.h          |  2 +-\n>  include/libcamera/internal/v4l2_subdevice.h   |  2 +-\n>  include/libcamera/internal/v4l2_videodevice.h |  2 +-\n>  src/libcamera/formats.cpp                     | 32 +++++++++----------\n>  .../pipeline/raspberrypi/raspberrypi.cpp      | 10 +++---\n>  src/libcamera/pipeline/simple/simple.cpp      |  2 +-\n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  2 +-\n>  src/libcamera/v4l2_subdevice.cpp              |  4 +--\n>  src/libcamera/v4l2_videodevice.cpp            |  4 +--\n>  test/v4l2_subdevice/list_formats.cpp          |  2 +-\n>  11 files changed, 32 insertions(+), 32 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 5c1d5789fe79..52eedb9ef880 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -75,7 +75,7 @@ private:\n>  \n>  \tstd::string model_;\n>  \n> -\tImageFormats<uint32_t> formats_;\n> +\tImageFormatsMap<uint32_t> formats_;\n>  \tSize resolution_;\n>  \tstd::vector<unsigned int> mbusCodes_;\n>  \tstd::vector<Size> sizes_;\n> diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h\n> index e20c031e857f..cc56e256f54c 100644\n> --- a/include/libcamera/internal/formats.h\n> +++ b/include/libcamera/internal/formats.h\n> @@ -19,7 +19,7 @@\n>  namespace libcamera {\n>  \n>  template<typename T>\n> -class ImageFormats\n> +class ImageFormatsMap\n>  {\n>  public:\n>  \tusing iterator = typename std::map<T, std::vector<SizeRange>>::iterator;\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 0ce6da48f58a..c9aa3428f93c 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -51,7 +51,7 @@ public:\n>  \tint setSelection(unsigned int pad, unsigned int target,\n>  \t\t\t Rectangle *rect);\n>  \n> -\tImageFormats<uint32_t> formats(unsigned int pad);\n> +\tImageFormatsMap<uint32_t> formats(unsigned int pad);\n>  \n>  \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n>  \t\t      Whence whence = ActiveFormat);\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h\n> index de4745982e94..02d101db0b2b 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -187,7 +187,7 @@ public:\n>  \n>  \tint getFormat(V4L2DeviceFormat *format);\n>  \tint setFormat(V4L2DeviceFormat *format);\n> -\tImageFormats<V4L2PixelFormat> formats(uint32_t code = 0);\n> +\tImageFormatsMap<V4L2PixelFormat> formats(uint32_t code = 0);\n>  \n>  \tint setSelection(unsigned int target, Rectangle *rect);\n>  \n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index a7922077d9c5..3acb6fe6284e 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -21,7 +21,7 @@ namespace libcamera {\n>  LOG_DEFINE_CATEGORY(Formats)\n>  \n>  /**\n> - * \\class ImageFormats\n> + * \\class ImageFormatsMap\n>   * \\brief Describe V4L2Device and V4L2SubDevice image formats and associated\n>   * image resolutions\n>   *\n> @@ -36,62 +36,62 @@ LOG_DEFINE_CATEGORY(Formats)\n>   */\n>  \n>  /**\n> - * \\typedef ImageFormats::iterator\n> + * \\typedef ImageFormatsMap::iterator\n>   * \\brief Iterator for the formats map\n>   */\n>  \n>  /**\n> - * \\typedef ImageFormats::const_iterator\n> + * \\typedef ImageFormatsMap::const_iterator\n>   * \\brief Const iterator for the formats map\n>   */\n>  \n>  /**\n> - * \\typedef ImageFormats::value_type\n> + * \\typedef ImageFormatsMap::value_type\n>   * \\brief Value type of the entries in the formats map\n>   */\n>  \n>  /**\n> - * \\fn iterator ImageFormats<T>::begin()\n> + * \\fn iterator ImageFormatsMap<T>::begin()\n>   * \\brief Retrieve an iterator to the first element in the formats map\n>   * \\return An iterator to the first format map\n>   */\n>  \n>  /**\n> - * \\fn const_iterator ImageFormats<T>::begin() const\n> + * \\fn const_iterator ImageFormatsMap<T>::begin() const\n>   * \\brief Retrieve an const iterator to the first element in the formats map\n>   * \\return A const iterator to the first format map\n>   */\n>  \n>  /**\n> - * \\fn iterator ImageFormats<T>::end()\n> + * \\fn iterator ImageFormatsMap<T>::end()\n>   * \\brief Retrieve an iterator pointing to the past-the-end element in the\n>   * formats map\n>   * \\return An iterator to the element following the last format\n>   */\n>  \n>  /**\n> - * \\fn const_iterator ImageFormats<T>::end() const\n> + * \\fn const_iterator ImageFormatsMap<T>::end() const\n>   * \\brief Retrieve a const iterator pointing to the past-the-end element in the\n>   * formats map\n>   * \\return A const iterator to the element following the last format\n>   */\n>  \n>  /**\n> - * \\fn iterator ImageFormats::find(const T format)\n> + * \\fn iterator ImageFormatsMap::find(const T format)\n>   * \\brief Find an element with key equal to \\a format\n>   * \\param[in] format The format to search for\n>   * \\return An iterator to the vector of sizes associated with \\a format\n>   */\n>  \n>  /**\n> - * \\fn const_iterator ImageFormats::find(const T format) const\n> + * \\fn const_iterator ImageFormatsMap::find(const T format) const\n>   * \\brief Find a const element with key equal to \\a format\n>   * \\param[in] format The format to search for\n>   * \\return An const iterator to the vector of sizes associated with \\a format\n>   */\n>  \n>  /**\n> - * \\fn std::pair<iterator, bool> ImageFormats::emplace(Args&&... args)\n> + * \\fn std::pair<iterator, bool> ImageFormatsMap::emplace(Args&&... args)\n>   * \\brief Insert a new element in the formats map constructed in place with the\n>   * given \\a args\n>   * \\param[in] args The argument pack used to construct the new entry in place\n> @@ -100,7 +100,7 @@ LOG_DEFINE_CATEGORY(Formats)\n>   */\n>  \n>  /**\n> - * \\fn ImageFormats<T>::addFormat(T format, const std::vector<SizeRange> &sizes)\n> + * \\fn ImageFormatsMap<T>::addFormat(T format, const std::vector<SizeRange> &sizes)\n>   * \\brief Add a format and corresponding sizes to the description\n>   * \\param[in] format Pixel format or media bus code to describe\n>   * \\param[in] sizes List of supported size ranges for the format\n> @@ -110,19 +110,19 @@ LOG_DEFINE_CATEGORY(Formats)\n>   */\n>  \n>  /**\n> - * \\fn ImageFormats<T>::isEmpty() const\n> + * \\fn ImageFormatsMap<T>::isEmpty() const\n>   * \\brief Check if the list of devices supported formats is empty\n>   * \\return True if the list of supported formats is empty\n>   */\n>  \n>  /**\n> - * \\fn ImageFormats<T>::formats() const\n> + * \\fn ImageFormatsMap<T>::formats() const\n>   * \\brief Retrieve a list of all supported image formats\n>   * \\return List of pixel formats or media bus codes\n>   */\n>  \n>  /**\n> - * \\fn ImageFormats<T>::sizes(T format) const\n> + * \\fn ImageFormatsMap<T>::sizes(T format) const\n>   * \\brief Retrieve all sizes for a specific format\n>   * \\param[in] format The pixel format or mbus code\n>   *\n> @@ -134,7 +134,7 @@ LOG_DEFINE_CATEGORY(Formats)\n>   */\n>  \n>  /**\n> - * \\fn ImageFormats<T>::data() const\n> + * \\fn ImageFormatsMap<T>::data() const\n>   * \\brief Retrieve the map that associates formats to image sizes\n>   * \\return The map that associates formats to image sizes\n>   */\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 64364afb3f7e..e0132113b072 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -65,7 +65,7 @@ double scoreFormat(double desired, double actual)\n>  \treturn score;\n>  }\n>  \n> -V4L2DeviceFormat findBestMode(ImageFormats<V4L2PixelFormat> &formatsMap, const Size &req)\n> +V4L2DeviceFormat findBestMode(ImageFormatsMap<V4L2PixelFormat> &formatsMap, const Size &req)\n>  {\n>  \tdouble bestScore = 9e9, score;\n>  \tV4L2DeviceFormat bestMode = {};\n> @@ -424,7 +424,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>  \t\t\t * Calculate the best sensor mode we can use based on\n>  \t\t\t * the user request.\n>  \t\t\t */\n> -\t\t\tImageFormats<V4L2PixelFormat> fmts = data_->unicam_[Unicam::Image].dev()->formats();\n> +\t\t\tImageFormatsMap<V4L2PixelFormat> fmts = data_->unicam_[Unicam::Image].dev()->formats();\n>  \t\t\tV4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);\n>  \t\t\tPixelFormat sensorPixFormat = sensorFormat.fourcc.toPixelFormat();\n>  \t\t\tif (cfg.size != sensorFormat.size ||\n> @@ -478,7 +478,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>  \t\t *\n>  \t\t */\n>  \t\tPixelFormat &cfgPixFmt = config_.at(outSize[i].first).pixelFormat;\n> -\t\tImageFormats<V4L2PixelFormat> fmts;\n> +\t\tImageFormatsMap<V4L2PixelFormat> fmts;\n>  \n>  \t\tif (i == maxIndex)\n>  \t\t\tfmts = data_->isp_[Isp::Output0].dev()->formats();\n> @@ -515,7 +515,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n>  \tRPiCameraData *data = cameraData(camera);\n>  \tCameraConfiguration *config = new RPiCameraConfiguration(data);\n>  \tV4L2DeviceFormat sensorFormat;\n> -\tImageFormats<V4L2PixelFormat> fmts;\n> +\tImageFormatsMap<V4L2PixelFormat> fmts;\n>  \n>  \tif (roles.empty())\n>  \t\treturn config;\n> @@ -602,7 +602,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n>  \t}\n>  \n>  \t/* First calculate the best sensor mode we can use based on the user request. */\n> -\tImageFormats<V4L2PixelFormat> fmts = data->unicam_[Unicam::Image].dev()->formats();\n> +\tImageFormatsMap<V4L2PixelFormat> fmts = data->unicam_[Unicam::Image].dev()->formats();\n>  \tV4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);\n>  \n>  \t/*\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index f3e03ba60196..62906b560d6a 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -275,7 +275,7 @@ int SimpleCameraData::init()\n>  \t\t\treturn ret;\n>  \t\t}\n>  \n> -\t\tImageFormats<V4L2PixelFormat> videoFormats = video_->formats(format.mbus_code);\n> +\t\tImageFormatsMap<V4L2PixelFormat> videoFormats = video_->formats(format.mbus_code);\n>  \n>  \t\tLOG(SimplePipeline, Debug)\n>  \t\t\t<< \"Adding configuration for \" << format.size.toString()\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index dbd835f5c5ef..e1b1ae32b821 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -159,7 +159,7 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,\n>  \tif (roles.empty())\n>  \t\treturn config;\n>  \n> -\tImageFormats<V4L2PixelFormat> v4l2Formats = data->video_->formats();\n> +\tImageFormatsMap<V4L2PixelFormat> v4l2Formats = data->video_->formats();\n>  \tstd::map<PixelFormat, std::vector<SizeRange>> deviceFormats;\n>  \tstd::transform(v4l2Formats.begin(), v4l2Formats.end(),\n>  \t\t       std::inserter(deviceFormats, deviceFormats.begin()),\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 9fa20e84a904..80db4cda82bb 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -320,9 +320,9 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n>   *\n>   * \\return A list of the supported device formats\n>   */\n> -ImageFormats<uint32_t> V4L2Subdevice::formats(unsigned int pad)\n> +ImageFormatsMap<uint32_t> V4L2Subdevice::formats(unsigned int pad)\n>  {\n> -\tImageFormats<uint32_t> formats;\n> +\tImageFormatsMap<uint32_t> formats;\n>  \n>  \tif (pad >= entity_->pads().size()) {\n>  \t\tLOG(V4L2, Error) << \"Invalid pad: \" << pad;\n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index ea952444e0ad..b264915cb73a 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -925,9 +925,9 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)\n>   *\n>   * \\return A list of the supported video device formats\n>   */\n> -ImageFormats<V4L2PixelFormat> V4L2VideoDevice::formats(uint32_t code)\n> +ImageFormatsMap<V4L2PixelFormat> V4L2VideoDevice::formats(uint32_t code)\n>  {\n> -\tImageFormats<V4L2PixelFormat> formats;\n> +\tImageFormatsMap<V4L2PixelFormat> formats;\n>  \n>  \tfor (V4L2PixelFormat pixelFormat : enumPixelformats(code)) {\n>  \t\tstd::vector<SizeRange> sizes = enumSizes(pixelFormat);\n> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp\n> index f66bb633fb00..adcf5606c961 100644\n> --- a/test/v4l2_subdevice/list_formats.cpp\n> +++ b/test/v4l2_subdevice/list_formats.cpp\n> @@ -48,7 +48,7 @@ void ListFormatsTest::printFormats(unsigned int pad,\n>  int ListFormatsTest::run()\n>  {\n>  \t/* List all formats available on existing \"Scaler\" pads. */\n> -\tImageFormats<uint32_t> formats;\n> +\tImageFormatsMap<uint32_t> formats;\n>  \n>  \tformats = scaler_->formats(0);\n>  \tif (formats.isEmpty()) {","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 74376600F7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  6 Jun 2020 01:20:56 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EB3E527C;\n\tSat,  6 Jun 2020 01:20:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MiNANG1S\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591399256;\n\tbh=DaILmPXBzATbyiS0ip+NeYPqtPsBqW36aziMgAoFDxQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MiNANG1SgZuZJNbkkr56i0s24F7dWnyIZwlsfda8RsLZ8PFhYEDglTSsUMgnQhdog\n\tuYfvgZKdVCd/Ylg/Y1/QDN2NSgKyDmZHKPge+fdmSfpAlkqK1eD5yIOj+NG3l0xTR1\n\tXtUwVLA2o35ueoPaVpH0or4IzWunPF3N4zfbsiKs=","Date":"Sat, 6 Jun 2020 02:20:13 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200605232013.GQ26752@pendragon.ideasonboard.com>","References":"<20200529110335.620503-1-jacopo@jmondi.org>\n\t<20200529110335.620503-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200529110335.620503-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 3/5] libcamera: Rename ImageFormats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Fri, 05 Jun 2020 23:20:56 -0000"}}]