| Message ID | 20200703154730.3908-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | b5f6a2ce2fae423f40c4bdaf1be43ad5070b3868 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for the patch. On 7/3/20 9:17 PM, Laurent Pinchart wrote: > Use the new Size::isNull() function through the code base to replace > manual checks. While the new code isn't equivalent, as isNull() checks > that both width and height are zero, it catches the same conditions in > practice. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Umang Jain <email@uajain.com> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- > test/v4l2_subdevice/test_formats.cpp | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 00559ce318e1..af51fb2d1718 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -185,7 +185,7 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) > * Provide a suitable default that matches the sensor aspect > * ratio. > */ > - if (!cfg.size.width || !cfg.size.height) { > + if (cfg.size.isNull()) { > cfg.size.width = 1280; > cfg.size.height = 1280 * cio2Configuration_.size.height > / cio2Configuration_.size.width; > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 3c3f3f3a8049..051d77a68b55 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -506,7 +506,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > MEDIA_BUS_FMT_SGRBG8_1X8, > MEDIA_BUS_FMT_SRGGB8_1X8 }, > cfg.size); > - if (!sensorFormat_.size.width || !sensorFormat_.size.height) > + if (sensorFormat_.size.isNull()) > sensorFormat_.size = sensor->resolution(); > > /* > @@ -517,7 +517,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > */ > const Size size = cfg.size; > > - if (!cfg.size.width || !cfg.size.height) { > + if (cfg.size.isNull()) { > cfg.size.width = 1280; > cfg.size.height = 1280 * sensorFormat_.size.height > / sensorFormat_.size.width; > diff --git a/test/v4l2_subdevice/test_formats.cpp b/test/v4l2_subdevice/test_formats.cpp > index 9635c9948946..679f50b62c4d 100644 > --- a/test/v4l2_subdevice/test_formats.cpp > +++ b/test/v4l2_subdevice/test_formats.cpp > @@ -67,7 +67,7 @@ int FormatHandlingTest::run() > return TestFail; > } > > - if (format.size.width == 0 || format.size.height == 0) { > + if (format.size.isNull()) { > cerr << "Failed to update image format" << endl; > return TestFail; > }
Hi Laurent, Thanks for your work. On 2020-07-03 18:47:30 +0300, Laurent Pinchart wrote: > Use the new Size::isNull() function through the code base to replace > manual checks. While the new code isn't equivalent, as isNull() checks > that both width and height are zero, it catches the same conditions in > practice. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- > test/v4l2_subdevice/test_formats.cpp | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 00559ce318e1..af51fb2d1718 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -185,7 +185,7 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) > * Provide a suitable default that matches the sensor aspect > * ratio. > */ > - if (!cfg.size.width || !cfg.size.height) { > + if (cfg.size.isNull()) { > cfg.size.width = 1280; > cfg.size.height = 1280 * cio2Configuration_.size.height > / cio2Configuration_.size.width; > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 3c3f3f3a8049..051d77a68b55 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -506,7 +506,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > MEDIA_BUS_FMT_SGRBG8_1X8, > MEDIA_BUS_FMT_SRGGB8_1X8 }, > cfg.size); > - if (!sensorFormat_.size.width || !sensorFormat_.size.height) > + if (sensorFormat_.size.isNull()) > sensorFormat_.size = sensor->resolution(); > > /* > @@ -517,7 +517,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > */ > const Size size = cfg.size; > > - if (!cfg.size.width || !cfg.size.height) { > + if (cfg.size.isNull()) { > cfg.size.width = 1280; > cfg.size.height = 1280 * sensorFormat_.size.height > / sensorFormat_.size.width; > diff --git a/test/v4l2_subdevice/test_formats.cpp b/test/v4l2_subdevice/test_formats.cpp > index 9635c9948946..679f50b62c4d 100644 > --- a/test/v4l2_subdevice/test_formats.cpp > +++ b/test/v4l2_subdevice/test_formats.cpp > @@ -67,7 +67,7 @@ int FormatHandlingTest::run() > return TestFail; > } > > - if (format.size.width == 0 || format.size.height == 0) { > + if (format.size.isNull()) { > cerr << "Failed to update image format" << endl; > return TestFail; > } > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 00559ce318e1..af51fb2d1718 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -185,7 +185,7 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) * Provide a suitable default that matches the sensor aspect * ratio. */ - if (!cfg.size.width || !cfg.size.height) { + if (cfg.size.isNull()) { cfg.size.width = 1280; cfg.size.height = 1280 * cio2Configuration_.size.height / cio2Configuration_.size.width; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 3c3f3f3a8049..051d77a68b55 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -506,7 +506,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() MEDIA_BUS_FMT_SGRBG8_1X8, MEDIA_BUS_FMT_SRGGB8_1X8 }, cfg.size); - if (!sensorFormat_.size.width || !sensorFormat_.size.height) + if (sensorFormat_.size.isNull()) sensorFormat_.size = sensor->resolution(); /* @@ -517,7 +517,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() */ const Size size = cfg.size; - if (!cfg.size.width || !cfg.size.height) { + if (cfg.size.isNull()) { cfg.size.width = 1280; cfg.size.height = 1280 * sensorFormat_.size.height / sensorFormat_.size.width; diff --git a/test/v4l2_subdevice/test_formats.cpp b/test/v4l2_subdevice/test_formats.cpp index 9635c9948946..679f50b62c4d 100644 --- a/test/v4l2_subdevice/test_formats.cpp +++ b/test/v4l2_subdevice/test_formats.cpp @@ -67,7 +67,7 @@ int FormatHandlingTest::run() return TestFail; } - if (format.size.width == 0 || format.size.height == 0) { + if (format.size.isNull()) { cerr << "Failed to update image format" << endl; return TestFail; }
Use the new Size::isNull() function through the code base to replace manual checks. While the new code isn't equivalent, as isNull() checks that both width and height are zero, it catches the same conditions in practice. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- test/v4l2_subdevice/test_formats.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)