| Message ID | 20220804120620.1696350-1-hanlinchen@chromium.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Han-Lin, Thank you for the patch. On Thu, Aug 04, 2022 at 08:06:20PM +0800, Han-Lin Chen via libcamera-devel wrote: > Using Size::expandTo() to find the max resolution might generate a non-existent > resolution. For example, when application request streams for 1920x1080 and > 1600x1200, the max resolution will be wrongly 1920x1200 and fails the > configuration. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=139 > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 75231156..335b6c94 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -254,7 +254,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() > rawSize.expandTo(cfg.size); Doesn't this need to be addressed too ? > } else { > yuvCount++; > - maxYuvSize.expandTo(cfg.size); > + maxYuvSize = std::max(maxYuvSize, cfg.size); Is this enough though ? In the example above, if the user requests two streams, in 1920x1080 and 1600x1200 resolutions, shouldn't we pick a size from the sensor that is larger than both, and then crop at the output ? Looking at the rest of the validate() function, the size selection seems quite fragile. > } > } >
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 75231156..335b6c94 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -254,7 +254,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() rawSize.expandTo(cfg.size); } else { yuvCount++; - maxYuvSize.expandTo(cfg.size); + maxYuvSize = std::max(maxYuvSize, cfg.size); } }
Using Size::expandTo() to find the max resolution might generate a non-existent resolution. For example, when application request streams for 1920x1080 and 1600x1200, the max resolution will be wrongly 1920x1200 and fails the configuration. Bug: https://bugs.libcamera.org/show_bug.cgi?id=139 Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)