[libcamera-devel] [PATCH] libcamera: stream: Rename StillCaptureRaw to Raw

David Plowman david.plowman at raspberrypi.com
Fri Sep 25 08:36:43 CEST 2020


Hi Niklas

I like this change, am happy to apply it to the RPi pipeline handler,
and would like to give it my thumbs-up! Just one very small
question...

On Mon, 21 Sep 2020 at 19:23, Niklas Söderlund
<niklas.soderlund at ragnatech.se> wrote:
>
> With the buffer copy removed from all pipelines for the raw capture
> rename the StillCaptureRaw to Raw to better represent the role.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/stream.h                         | 2 +-
>  src/android/camera_device.cpp                      | 4 ++--
>  src/cam/stream_options.cpp                         | 4 ++--
>  src/libcamera/pipeline/ipu3/ipu3.cpp               | 2 +-
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 2 +-
>  src/libcamera/stream.cpp                           | 5 ++---
>  src/qcam/main_window.cpp                           | 2 +-
>  7 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index f502b35957fc250c..bb47c390f8a12387 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -59,8 +59,8 @@ private:
>  };
>
>  enum StreamRole {
> +       Raw,
>         StillCapture,
> -       StillCaptureRaw,
>         VideoRecording,
>         Viewfinder,
>  };
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 70d77a17ef43cf48..751699cd21138245 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -313,7 +313,7 @@ std::vector<Size> CameraDevice::getYUVResolutions(CameraConfiguration *cameraCon
>  std::vector<Size> CameraDevice::getRawResolutions(const libcamera::PixelFormat &pixelFormat)
>  {
>         std::unique_ptr<CameraConfiguration> cameraConfig =
> -               camera_->generateConfiguration({ StillCaptureRaw });
> +               camera_->generateConfiguration({ StreamRole::Raw });
>         StreamConfiguration &cfg = cameraConfig->at(0);
>         const StreamFormats &formats = cfg.formats();
>         std::vector<Size> supportedResolutions = formats.sizes(pixelFormat);
> @@ -895,7 +895,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>
>         /* Report if camera supports RAW. */
>         std::unique_ptr<CameraConfiguration> cameraConfig =
> -               camera_->generateConfiguration({ StillCaptureRaw });
> +               camera_->generateConfiguration({ StreamRole::Raw });
>         if (cameraConfig && !cameraConfig->empty()) {
>                 const PixelFormatInfo &info =
>                         PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> diff --git a/src/cam/stream_options.cpp b/src/cam/stream_options.cpp
> index 4168e5d2d1deff9b..27cc39122989f256 100644
> --- a/src/cam/stream_options.cpp
> +++ b/src/cam/stream_options.cpp
> @@ -119,8 +119,8 @@ bool StreamKeyValueParser::parseRole(StreamRole *role,
>         } else if (name == "still") {
>                 *role = StreamRole::StillCapture;
>                 return true;
> -       } else if (name == "stillraw") {
> -               *role = StreamRole::StillCaptureRaw;
> +       } else if (name == "raw") {
> +               *role = StreamRole::Raw;
>                 return true;
>         }
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 221259c7fe61df03..17ffed2b0e2ff0a6 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -343,7 +343,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
>
>                         break;
>
> -               case StreamRole::StillCaptureRaw: {
> +               case StreamRole::Raw: {
>                         StreamConfiguration cio2Config =
>                                 data->cio2_.generateConfiguration(sensorResolution);
>                         pixelFormat = cio2Config.pixelFormat;
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 50f07182457046ff..fe46dbd386d2a5ea 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -376,7 +376,7 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,
>         unsigned int outCount = 0;
>         for (const StreamRole role : roles) {
>                 switch (role) {
> -               case StreamRole::StillCaptureRaw:
> +               case StreamRole::Raw:
>                         size = data->sensor_->resolution();
>                         fmts = data->unicam_[Unicam::Image].dev()->formats();
>                         sensorFormat = findBestMode(fmts, size);
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 4d0ab90d73f96e0f..6d51e5da794f9f87 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -380,12 +380,11 @@ std::string StreamConfiguration::toString() const
>   * are specified by applications and passed to cameras, that then select the
>   * most appropriate streams and their default configurations.
>   *
> + * \var Raw
> + * The stream is intended to capture high-resolution raw images.

I wonder just a little about saying "high-resolution raw images". I
can use the raw streams to capture lower-resolution binned raw images
too, so perhaps we should reflect that? But other than that, can I add

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David

>   * \var StillCapture
>   * The stream is intended to capture high-resolution, high-quality still images
>   * with low frame rate. The captured frames may be exposed with flash.
> - * \var StillCaptureRaw
> - * The stream is intended to capture high-resolution, raw still images with low
> - * frame rate.
>   * \var VideoRecording
>   * The stream is intended to capture video for the purpose of recording or
>   * streaming. The video stream may produce a high frame rate and may be
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 985743f3233405d0..ecb9dd6642400697 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -380,7 +380,7 @@ int MainWindow::startCapture()
>                 break;
>         case 2:
>                 if (roles[0] != StreamRole::Viewfinder ||
> -                   roles[1] != StreamRole::StillCaptureRaw) {
> +                   roles[1] != StreamRole::Raw) {
>                         qWarning() << "Only viewfinder + raw supported for dual streams";
>                         return -EINVAL;
>                 }
> --
> 2.28.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel


More information about the libcamera-devel mailing list