| Message ID | 20211126104045.4756-1-david.plowman@raspberrypi.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi David,
forgot to notify it yesterday, but the series breaks the IPU3 and
the Simple pipeline handlers.
The following will fix it
diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index 59dda56bdd3d..f4e8c6632c2f 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -322,10 +322,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int>
return {};
}
- V4L2SubdeviceFormat format{
- .mbus_code = bestCode,
- .size = bestSize,
- };
+ V4L2SubdeviceFormat format{};
+ format.mbus_code = bestCode;
+ format.size = bestSize;
return format;
}
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 701fb4be0b71..a3108fc0b0f6 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -457,7 +457,9 @@ int SimpleCameraData::init()
* formats on the video node.
*/
for (unsigned int code : sensor_->mbusCodes()) {
- V4L2SubdeviceFormat format{ code, sensor_->resolution() };
+ V4L2SubdeviceFormat format{};
+ format.mbus_code = code;
+ format.size = sensor_->resolution();
ret = setupFormats(&format, V4L2Subdevice::TryFormat);
if (ret < 0) {
@@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
return ret;
const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();
- V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };
+ V4L2SubdeviceFormat format{};
+ format.mbus_code = pipeConfig->code;
+ format.size = data->sensor_->resolution();
ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);
if (ret < 0)
On Fri, Nov 26, 2021 at 10:40:38AM +0000, David Plowman wrote:
> Hi everyone
>
> Here's version 7 of the colour space patches.
>
> Mostly it's just tidying up, improving documentation and making things
> a bit clearer, so thank you very much for all those suggestions.
>
> The other change is that I've decided to leave it up to the caller to
> check what has happened when you try to set a colour space. As some
> have pointed out, some pads already don't have colour spaces, and
> there might be more in future. I think it's only really the pipeline
> handler that understands this context, so it's best so leave it to the
> pipeline handler to check.
>
> I also decided to leave the to/fromColorSpace methods public. I could
> imagine a day in the future when a pipeline handler might want to
> check (for example) that V4L2 understands the ColorSpace it wants to
> use, and these methods give a handy way to do that.
>
> Thanks again for all the reviews so far!
>
> Best regards
> David
>
> David Plowman (7):
> libcamera: Add ColorSpace class
> libcamera: Add ColorSpace fields to StreamConfiguration
> libcamera: Convert between ColorSpace class and V4L2 formats
> libcamera: Support passing ColorSpaces to V4L2 video devices
> libcamera: Support passing ColorSpaces to V4L2 subdevices
> libcamera: Add validateColorSpaces to CameraConfiguration class
> libcamera: pipeline: raspberrypi: Support color spaces
>
> include/libcamera/camera.h | 2 +
> include/libcamera/color_space.h | 72 +++++
> include/libcamera/internal/v4l2_device.h | 7 +
> include/libcamera/internal/v4l2_subdevice.h | 2 +
> include/libcamera/internal/v4l2_videodevice.h | 2 +
> include/libcamera/meson.build | 1 +
> include/libcamera/stream.h | 3 +
> src/libcamera/camera.cpp | 38 +++
> src/libcamera/camera_sensor.cpp | 1 +
> src/libcamera/color_space.cpp | 305 ++++++++++++++++++
> src/libcamera/meson.build | 1 +
> .../pipeline/raspberrypi/raspberrypi.cpp | 42 +++
> src/libcamera/stream.cpp | 19 ++
> src/libcamera/v4l2_device.cpp | 192 +++++++++++
> src/libcamera/v4l2_subdevice.cpp | 25 +-
> src/libcamera/v4l2_videodevice.cpp | 32 ++
> 16 files changed, 743 insertions(+), 1 deletion(-)
> create mode 100644 include/libcamera/color_space.h
> create mode 100644 src/libcamera/color_space.cpp
>
> --
> 2.30.2
>
Hi Jacopo Ah, thanks for those fixes. I must have turned off building all that other stuff at some point. Let me fold those into version 8 with your other suggestions and then I'll post that. Best regards David On Wed, 1 Dec 2021 at 13:20, Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hi David, > forgot to notify it yesterday, but the series breaks the IPU3 and > the Simple pipeline handlers. > > The following will fix it > > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp > index 59dda56bdd3d..f4e8c6632c2f 100644 > --- a/src/libcamera/pipeline/ipu3/cio2.cpp > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp > @@ -322,10 +322,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector<unsigned int> > return {}; > } > > - V4L2SubdeviceFormat format{ > - .mbus_code = bestCode, > - .size = bestSize, > - }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = bestCode; > + format.size = bestSize; > > return format; > } > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 701fb4be0b71..a3108fc0b0f6 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -457,7 +457,9 @@ int SimpleCameraData::init() > * formats on the video node. > */ > for (unsigned int code : sensor_->mbusCodes()) { > - V4L2SubdeviceFormat format{ code, sensor_->resolution() }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = code; > + format.size = sensor_->resolution(); > > ret = setupFormats(&format, V4L2Subdevice::TryFormat); > if (ret < 0) { > @@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c) > return ret; > > const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig(); > - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() }; > + V4L2SubdeviceFormat format{}; > + format.mbus_code = pipeConfig->code; > + format.size = data->sensor_->resolution(); > > ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat); > if (ret < 0) > > > On Fri, Nov 26, 2021 at 10:40:38AM +0000, David Plowman wrote: > > Hi everyone > > > > Here's version 7 of the colour space patches. > > > > Mostly it's just tidying up, improving documentation and making things > > a bit clearer, so thank you very much for all those suggestions. > > > > The other change is that I've decided to leave it up to the caller to > > check what has happened when you try to set a colour space. As some > > have pointed out, some pads already don't have colour spaces, and > > there might be more in future. I think it's only really the pipeline > > handler that understands this context, so it's best so leave it to the > > pipeline handler to check. > > > > I also decided to leave the to/fromColorSpace methods public. I could > > imagine a day in the future when a pipeline handler might want to > > check (for example) that V4L2 understands the ColorSpace it wants to > > use, and these methods give a handy way to do that. > > > > Thanks again for all the reviews so far! > > > > Best regards > > David > > > > David Plowman (7): > > libcamera: Add ColorSpace class > > libcamera: Add ColorSpace fields to StreamConfiguration > > libcamera: Convert between ColorSpace class and V4L2 formats > > libcamera: Support passing ColorSpaces to V4L2 video devices > > libcamera: Support passing ColorSpaces to V4L2 subdevices > > libcamera: Add validateColorSpaces to CameraConfiguration class > > libcamera: pipeline: raspberrypi: Support color spaces > > > > include/libcamera/camera.h | 2 + > > include/libcamera/color_space.h | 72 +++++ > > include/libcamera/internal/v4l2_device.h | 7 + > > include/libcamera/internal/v4l2_subdevice.h | 2 + > > include/libcamera/internal/v4l2_videodevice.h | 2 + > > include/libcamera/meson.build | 1 + > > include/libcamera/stream.h | 3 + > > src/libcamera/camera.cpp | 38 +++ > > src/libcamera/camera_sensor.cpp | 1 + > > src/libcamera/color_space.cpp | 305 ++++++++++++++++++ > > src/libcamera/meson.build | 1 + > > .../pipeline/raspberrypi/raspberrypi.cpp | 42 +++ > > src/libcamera/stream.cpp | 19 ++ > > src/libcamera/v4l2_device.cpp | 192 +++++++++++ > > src/libcamera/v4l2_subdevice.cpp | 25 +- > > src/libcamera/v4l2_videodevice.cpp | 32 ++ > > 16 files changed, 743 insertions(+), 1 deletion(-) > > create mode 100644 include/libcamera/color_space.h > > create mode 100644 src/libcamera/color_space.cpp > > > > -- > > 2.30.2 > >