Parsing any Rectangle array causes assertion

arekm commented 8 months ago

/control web interface has an option:

AfWindows
?
[(0, 0)/0x0..(65535, 65535)/65535x65535]

which is supposed to take a single entry or array of windows (I would assume an array) of (A,B)/ZxQ "windows" (where A,B are raw pixel coordinates of beginning of the window, Z is width, Q is height in raw pixels).

When trying to use

(1382,1296)/1382x1296

there camera streamer fails with assert

 camera-streamer: ../include/libcamera/controls.h:167: T libcamera::ControlValue::get() const [with T = libcamera::Span<const libcamera::Rectangle>; typename std::enable_if<(libcamera::details::is_span<U>::value || std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1> >::type>::value), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `isArray_' failed.

Not I'm not sure if I'm specifying it in wrong way or if that's a bug in parsing?

Trying to repeat similar scenario with libcamera-still it seems to work there:

libcamera-still -o test.jpg --autofocus-range macro --autofocus-window 0.3,0.5,0.3,0.5 --immediate

(my sensor is 4608x2592 from rpi v3 camera; --autofocus-window internally recalculates that using specified 0.N coefficients and sensor size)

Testing done on:

camera-streamer --version
v0.2.7 (e43e93c)

arekm commented 8 months ago

Easier way of testing:

$ camera-streamer  --camera-path=/base/soc/i2c0mux/i2c@1/imx708@1a --camera-type=libcamera --camera-options="ScalerCrop=(0,0)/4608x2592" 2>&1 |grep -iE "(ScalerCrop|assert)"
device/libcamera/options.cc: CAMERA: Configuring option 'ScalerCrop' (00000019, type=7) = [ (0, 0)/0x0, (0, 0)/0x0 ]
camera-streamer: ../include/libcamera/controls.h:153: T libcamera::ControlValue::get() const [with T = libcamera::Rectangle; typename std::enable_if<((! libcamera::details::is_span<U>::value) && (! std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1> >::type>::value)), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `!isArray_' failed.

It thinks that two elements with all zeroes were passed in array: [ (0, 0)/0x0, (0, 0)/0x0 ]

->>> The problem is in libcamera_parse_control_value() which splits by "," (even for values inside brackets) causing libcamera_parse_rectangle() to be called twice, one for (0 and then again for 0)/4608x2592

maximweb commented 5 months ago

I arrived here from Octoprint (new camera stack) and was impressed as my camera was detected out of the box by camera-streamer.

Recently, I started to tweak camera settings and stumbled across similar issues.

Setup

Raspberry Pi 4B - 2 GB
Raspberry Pi Camera Module 3
libcamera build: v0.0.5+83-bde9b04f (returned by libcamera-hello --version)
libcamera-apps build: 7e4d3d71867f 17-07-2023 (07:34:37) (returned by libcamera-hello --version)
camera-streamer version: 0.2.8 (bc23191)

What works

libcamera-still with crop and autofocus-window

libcamera-still -v 2 --camera=0 --roi 0.1,0.1,0.8,0.8 -o testCrop.jpg returns (excerpt)

Using crop (460, 259)/3686x2073

And the image is indeed cropped. Interestingly, the image has full sensor resolution but the visible image is clearly cropped as desired.

Regarding --autofocus-window 0.1,0.1,0.8,0.8, the log entry is similar (Using AfWindow (460, 259)/3686x2073) and resulting stills are noticeably focussed on the desired area (I tested several areas).

camera-streamer with crop A

Running the camera-stream.service and setting ScalerCrop to (0,0)/128x128 in the webUI (/webcam/control) also has an immediate visual effect on MJPEG stream.

This is also seen in the logs: Jan 30 19:20:25 octopi sh[9384]: device/libcamera/buffer.cc: CAMERA:capture:buf0: Metadata: ScalerCrop (00000016, type=7): (0, 0)/128x128

Setting anything else after this, without restarting the service, does not change anything. Only when I restart the service, I get back to the full video.

What does not work

camera-streamer with crop B

Setting any other pixel values in the webUI, e.g. (0,0)/256x256 still changes the crop, but not as desired to 256x256 but again to 128x128

log: Jan 30 19:26:00 octopi sh[9578]: device/libcamera/buffer.cc: CAMERA:capture:buf0: Metadata: ScalerCrop (00000016, type=7): (0, 0)/128x128

camera-streamer with AfWindows

Setting this in the GUI or as --camera-options does not work.

If I can help sorting this out, let me know. I am willing to test drafts or provide more logs if desired.

arekm commented 5 months ago

Why it is happening is known, see:

https://github.com/ayufan/camera-streamer/issues/114#issuecomment-1792851339

That code simply needs to be fixed to parse correctly.

maximweb commented 5 months ago

Why it is happening is known, see:

#114 (comment)

That code simply needs to be fixed to parse correctly.

Hmm, somehow I read this but didn't fully understand. Now I see, thank you. I am already trying to pull the repo and see if I can compile it. If yes, maybe I can come up with a fix.

maximweb commented 5 months ago

I thin I found the issue:

https://github.com/ayufan/camera-streamer/blob/54bdd9d4d6a2c67291ecc419cc482423605dedea/device/libcamera/options.cc#L427-L430

calls libcamera_parse_control_value, which itself separates the values by ,:

https://github.com/ayufan/camera-streamer/blob/54bdd9d4d6a2c67291ecc419cc482423605dedea/device/libcamera/options.cc#L357-L372

and calls libcamera_parse_rectangle for each ,-divided section individually.

for 0,0,1000,1000 it is called four times with each number individually
for (0,0)/1000x1000 it is called four times with (0 0)/ 1000 and 1000

So either libcamera_parse_control_value has to be rewritten to not subdivide by comma (why does it?) or libcamera_parse_rectangle needs to be called directly from the switch case entry.

Unfortunately, I cannot test it properly. While I can compile camera-streamer, execution fails with:

[72:46:11.188575463] [19419] ERROR V4L2 v4l2_videodevice.cpp:1697 /dev/video13[25:out]: Failed to queue buffer 0: Invalid argument
[72:46:11.188636963] [19419] ERROR RPISTREAM rpi_stream.cpp:276 Failed to queue buffer for ISP Input

arekm commented 5 months ago

It does it for other parameters that need such split.

You need to kind of walk character by character and if you encounter "(" stop splitting by "," until ")" is closed (also not sure if there are multiple () inside other () allowed or not). Or alternative for that.

maximweb commented 5 months ago

Hmm, as case libcamera::ControlTypeRectangle: already looks like it knows that it is expecting a Rectangle.

So an easy fix would be:

case libcamera::ControlTypeRectangle: 
  control_value.set<libcamera::Rectangle>(libcamera_parse_rectangle(value));

But I don't know if this might have any other implications.

grunkyb commented 3 months ago

I've tried out the suggestion from @maximweb. It'll parse a rectangle fine, including using the (%d,%d)/%ux%u format. Libcamera expects an array/Span<>, so the windowing fails when libcamera tries to unpack the tuple rather than tuples in a Span. I've tried out change the separator from a comma to a semicolon, but no joy there, either.

I'm not familiar with C++ grammar, but it looks like there ought to be a simple fix based on @maximweb's suggestion with a tweak or two.

maximweb commented 2 months ago

Last time I tried to test some local changes, I encountered error messages I did not understand. At least the error message looks identical to the recently merged #140 . So I might take another look in the next couple of days.

@grunkyb How do you know it passed fine and how do you know windowing failed? What exactly did you test?

grunkyb commented 2 months ago

@maximweb This is what I should have included in my previous post.

I'm running this as part of octopi/octoprint, so I have two instances of camera-streamer running, the pre-installed version (v0.25) in /usr/bin/ and the one I compiled with your suggestions in /usr/local/bin/ (v0.28). I'm running Bullseye. I've tried changing the options both in /etc/camera-streamer.conf.d/libcamera.conf and /etc/systemd/system/camera-streamer-libcamera.service and they behave identically.

Case 1: no array set in options for AfWindows OPTIONS='--camera-options="AfMetering=1" --camera-options="AfWindows=(1152,1296)/2304x1296"

Output from journalctl -e -u camera-streamer-libcamera.service shows that it recognised libcamera::Rectangle because it's not null/[(0,0)/0x0)].

Apr 05 09:53:24 octopi sh[1097]: device/libcamera/options.cc: CAMERA: Configuring option 'AfMode' (0000001c, type=3) = 2
Apr 05 09:53:24 octopi sh[1097]: device/libcamera/options.cc: CAMERA: Configuring option 'AfRange' (0000001d, type=3) = 2
Apr 05 09:53:24 octopi sh[1097]: device/libcamera/options.cc: CAMERA: Configuring option 'AfMetering' (0000001f, type=3) = 1
Apr 05 09:53:24 octopi sh[1097]: device/libcamera/options.cc: CAMERA: Configuring option 'AfWindows' (00000020, type=7) = (1152, 1296)/2304x1296
Apr 05 09:53:24 octopi sh[1097]: device/libcamera/options.cc: CAMERA: Configuring option 'AeEnable' (00000001, type=1) = true

But using the web control interface at http://octopi.local/webcam/control gives this error in the unit output in the log:

Apr 05 10:01:10 octopi sh[1176]: util/http/http.c: HTTP8080/0: Request 'GET' '/snapshot' '_cb=1712307670613'
Apr 05 10:01:10 octopi sh[1176]: [0:14:27.163993235] [1190]  WARN IPARPI ipa_base.cpp:1070 Could not set AF_TRIGGER - no AF algorithm or not Auto
Apr 05 10:01:10 octopi sh[1176]: camera-streamer: ../include/libcamera/controls.h:167: T libcamera::ControlValue::get() const [with T = libcamera::Span<const libcamera::Rectangle>; typename std::enable_if<(libcamera::details::is_span<U>::value || std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1> >::type>::value), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `isArray_' failed.
Apr 05 10:01:10 octopi systemd[1]: camera-streamer-libcamera.service: Main process exited, code=killed, status=6/ABRT
Apr 05 10:01:10 octopi systemd[1]: camera-streamer-libcamera.service: Failed with result 'signal'.

I read the line before the service restarts as libcamera expecting an array, not getting one, then killing the process.

Case 2: array set in options for AfWindows OPTIONS='--camera-options="AfMetering=1" --camera-options="AfWindows=[(1152,1296,2304,1296)]"

This time, it doesn't recognise the rectangle and passes a single-value null array.

Apr 05 10:06:42 octopi sh[1330]: device/libcamera/options.cc: CAMERA: Configuring option 'AfMode' (0000001c, type=3) = 2
Apr 05 10:06:42 octopi sh[1330]: device/libcamera/options.cc: CAMERA: Configuring option 'AfRange' (0000001d, type=3) = 2
Apr 05 10:06:42 octopi sh[1330]: device/libcamera/options.cc: CAMERA: Configuring option 'AfMetering' (0000001f, type=3) = 1
Apr 05 10:06:42 octopi sh[1330]: device/libcamera/options.cc: CAMERA: Configuring option 'AfWindows' (00000020, type=7) = (0, 0)/0x0

The result is the same, not an array, process dies.

Apr 05 10:10:10 octopi sh[1411]: camera-streamer: ../include/libcamera/controls.h:167: T libcamera::ControlValue::get() const [with T = libcamera::Span<const libcamera::Rectangle>; typename std::enable_if<(libcamera::details::is_span<U>::value || std::is_same<std::__cxx11::basic_string<char>, typename std::remove_cv< <template-parameter-1-1> >::type>::value), std::nullptr_t>::type <anonymous> = nullptr]: Assertion `isArray_' failed.

Commenting out those options makes the camera work fine again, but without a specified set of AfWindows.

The pieces seem to be in place in the code here: there's facility to parse arrays and a Rectangle template, but I haven't worked out how to get them to play nicely together.

maximweb commented 2 months ago

I retested with the latest fix in main branch and the libcamera option ScalerCrop, which does not need a span, and is supposed to crop the output video.

So I did the same as suggested earlier, replacing the following

https://github.com/ayufan/camera-streamer/blob/6deb9c513c67f9c8ba85d05198bedccc7b716f16/device/libcamera/options.cc#L427-L430

with

    case libcamera::ControlTypeRectangle:
      control_value.set<libcamera::Rectangle>(libcamera_parse_rectangle(value));
      break;

And it seems to work when running (sorry, no time to reduce the command to the bare minimum required to test this):

./camera-streamer --camera-path=/base/soc/i2c0mux/i2c@1/imx708@1a --camera-type=libcamera --camera-format=YUYV --camera-width=2304 --camera-height=1296 --http-listen=0.0.0.0 --camera-nbufs=2 --camera-video.height=1056 --camera-snapshot.height=1056 --camera-options="ScalerCrop=0,0,1000,1000"

The cli output of the command still notes

device/libcamera/options.cc: CAMERA: Configuring option 'ScalerCrop' (00000019, type=7) = (0, 0)/1000x1000

but my video is clearly cropped.

Unfortunately, I have to attend to other things for now, but here are some thoughts: Does the case libcamera::ControlTypeRectangle work for AfWindows in the first place? Maybe we need to check differently as it might be case libcamera::Span or a nested version with span and Rectangle, if such a check is even possible.

maximweb commented 2 months ago

Case 2: array set in options for AfWindows OPTIONS='--camera-options="AfMetering=1" --camera-options="AfWindows=[(1152,1296,2304,1296)]"

Ok, looking at how libcamera_parse_rectangle works, it either accepts AfWindows=x,y,width,height or AfWindows=(x,y)/width,height. Passing an array of multiple rectangles would require to alter libcamera_parse_rectangle, or wrap it accordingly.

So instead, we could construct a Span of a single Rectangle specifically for AfWindows, as it seems to be the only control type accepting multiple rectangles.

So the following quick-and-dirty replacement does not throw any errors:

    case libcamera::ControlTypeRectangle:
      if (control_key == "AfWindows") {
        libcamera::Rectangle rectangle = libcamera_parse_rectangle(value);
        libcamera::Span<libcamera::Rectangle> rectangles(&rectangle, 1);
        control_value.set<libcamera::Span<libcamera::Rectangle> >(rectangles);
      } else {
        control_value.set<libcamera::Rectangle>(libcamera_parse_rectangle(value));
      }
      break;

using --camera-options="AfWindows=0,0,1000,1000" in command line.

I have not tested, though, if it actually works as desired.

ayufan / camera-streamer