`ColorCamera::getSensorCrop()` fails; and various get/set crop+scaling APIs inconsistent

[diablodale]

ColorCamera::getSensorCrop() always returns 0.0f,0.0f due to code defect.

There is also a suspicious interaction between the underlying properties.sensorCropX and Y, ISP scaling, and setVideoSize() cropping. I think there is a bug in this space but I can't confirm without better understanding the image frame pipeline. depthai docs https://docs.luxonis.com/projects/api/en/latest/components/nodes/color_camera/ don't have needed detail and I can't find an ISP doc for the myriad x chip.

The known code defect and the iteractions are one subject area on frame "cropping" so I've put them both into this issue for now.

Setup

• OAK-D via BW1098OBC
• Microsoft Windows [Version 10.0.19044.1415]
• depthai-core v2.13.3 + xlink move/thread fixes
• VS2019 v16.11.8

Repro Case 1

1. cmake config and build depthai-core for x64, debug, static library
2. cmake install
3. create main app that outputs color video with a scale+crop via ISP
4. point main app config to install location
5. config and build main app with below pseudocode
6. Run main app

pseudocode

// goal to output 832x480 by downsampling the 1920x1080 sensor image to 853.3x480 then crop 832x480
rgbCamera->setResolution(THE_1080_P);
rgbCamera->setIspScale(4, 9);
rgbCamera->setVideoSize(832, 480);
rgbCamera->sensorCenterCrop();
const auto crop = rgbCamera->getSensorCrop();
// crop tuple will be <0.0f,0.0f> when it should be <some_fraction, 0.0f>

Result

the return of getSensorCrop() is always <0.0f,0.0f>

Expected

the return of getSensorCrop() to be top left of the needed center crop

Code defect in ColorCamera.cpp

https://github.com/luxonis/depthai-core/blob/29e2a5479e9edece375e4596e6561afcc8bfb806/src/pipeline/node/ColorCamera.cpp#L365-L366

First, one must remember that according to depthai documentation and the intention of this code...the crop is in NORMALIZED coordinates. Can not use floor() like that. Since crop is a normalized [0:1) value, the floor() will always return 0.0f.

One could use it around the first half of the formula if-and-only-if the device firmware/HW that actually does the crop follows the exact same rounding process -- otherwise the crop behavior can cause cascading pixel +/- shift issues which accumulate as they are used further in depthai-core or in main app.

Depending on the firmware/HW implementation, a fix to this code could be...

float x = ((getResolutionWidth() - getVideoWidth()) / 2.0f) / getResolutionWidth();
float y = ((getResolutionHeight() - getVideoHeight()) / 2.0f) / getResolutionHeight();

Repro Case 2

As I look at code and its behavior, there is a lack of clarity on the sequence of steps for...

1. sensor cropping
2. ISP scaling
3. video size cropping

What is the exact and unchanging sequence of these? I think there are some bugs in the APIs that support the interrelation of these but I can't clearly report a bug without first knowing the exact hw/firmware intention and sequence of steps.

sensorCenterCrop() why does this exist? There is only center crop. No API support for off-center cropping. setSensorCrop() takes two normalized floats that represent the center crop to apply; no off-center support getSensorCrop() write in its code "AUTO - center crop by default". However, there is only center crop. There is no API support for off-center cropping.

The code in ColorCamera::getIspWidth() and ColorCamera::getIspHeight() suggests that the ISP takes the sensor frame (e.g. the full 1920x1080 frame) and then applies scaling with (num/denom) to output a new frame size defined by getScaledSize()...

...but...code in getSensorCrop() and getVideoSize() doesn't consistently consider ISP scaling.

• When sensorCropX != ColorCameraProperties::AUTO it returns the normalized crop values set by ColorCamera::setSensorCrop()
• When sensorCropX == ColorCameraProperties::AUTO, it calls getVideoWidth() which changes behavior depending on defaults or if the main app called setVideoSize() like my pseudocode above.

It is unclear to me what is correct. I don't know the hardware/firmware implementation. Should the crop set by setSensorCrop() be a normalized value of the original sensor frame -or- a normalized value for the ISP scaled frame. And how does this relate to a call to setVideoSize()?

Related

https://github.com/luxonis/depthai-core/issues/320

[Luxonis-Brandon]

Thanks @diablodale ! Super appreciate the detail here! We're at CES this week (just landed) so will be a bit slow to respond most likely.

[diablodale]

Got it. Vegas nights! Fond blurry memories.....😁

[Luxonis-Brandon]

There will be much coding. :-).

[diablodale]

Answering one of my own questions above...from what I can discern colorcam::setVideoSize() is actually crop. And preview downscales this crop image. These are unexpected to me.

The behavior as I look at on-screen images is that setVideoSize(640,480) center crops any image that comes out of the ISP to make it a center cropped 640x480 images. It does not downscale or "resize" the image from ISP. This is unexpected to me. I would expect it to downscale/resize to the resolution given by setVideoSize(). And separately to have functionality to crop with crop-specific apis. I see setSensorCrop(). That is ambiguous to me. Crop original sensor, crop ISP, crop video? What does it do?

Further unexpected is that this setVideoSize() cropped image is sent to the input of its sibling preview pipeline. This cropped image is downsampled/resized based on setPreviewSize() and setPreviewKeepAspectRatio(). I like that it actually downsamples, but it is unexpected that it does this to the cropped image. I would expect it to downsample the ISP image.

Other questions above yet remain. The device-side image mutating is yet unclear to me.

[diablodale]

good to see the case 1 getSensorCrop() bug was fixed in https://github.com/luxonis/depthai-core/commit/a30aa777b947ee5fed958321cc456cb8953f6afe

I'm going to close this issue as resolved. All the other isp,scaling,cropping problems I saw are invalidated given the rewrite of the ISP and imagemanip rewrite this year. If I see problems, I'll open a new bug.