Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing any Rectangle array causes assertion #114

Open
arekm opened this issue Nov 3, 2023 · 12 comments
Open

Parsing any Rectangle array causes assertion #114

arekm opened this issue Nov 3, 2023 · 12 comments

Comments

@arekm
Copy link

arekm commented Nov 3, 2023

/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
Copy link
Author

arekm commented Nov 3, 2023

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

@arekm arekm changed the title Passing array in control interface AfWindows causes assertion Passing any Rectangle array causes assertion Nov 3, 2023
@arekm arekm changed the title Passing any Rectangle array causes assertion Parsing any Rectangle array causes assertion Nov 3, 2023
@maximweb
Copy link

maximweb commented Jan 30, 2024

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
Copy link
Author

arekm commented Jan 30, 2024

Why it is happening is known, see:

#114 (comment)

That code simply needs to be fixed to parse correctly.

@maximweb
Copy link

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
Copy link

maximweb commented Jan 30, 2024

I thin I found the issue:

case libcamera::ControlTypeRectangle:
libcamera_parse_control_value<libcamera::Rectangle>(
control_value, value, libcamera_parse_rectangle);
break;

calls libcamera_parse_control_value, which itself separates the values by ,:

while (value && *value) {
std::string current_value;
if (const char *next = strchr(value, ',')) {
current_value.assign(value, next);
value = &next[1];
} else {
current_value.assign(value);
value = NULL;
}
if (current_value.empty())
continue;
parsed.push_back(fn(current_value.c_str()));
}

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
Copy link
Author

arekm commented Jan 30, 2024

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
Copy link

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
Copy link

grunkyb commented Apr 2, 2024

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
Copy link

maximweb commented Apr 4, 2024

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
Copy link

grunkyb commented Apr 5, 2024

@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
Copy link

maximweb commented Apr 7, 2024

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

case libcamera::ControlTypeRectangle:
libcamera_parse_control_value<libcamera::Rectangle>(
control_value, value, libcamera_parse_rectangle);
break;

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
Copy link

maximweb commented Apr 7, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants