-
Notifications
You must be signed in to change notification settings - Fork 68
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
qcam crashes after selecting camera [CM4, ov9281] when decimal separator <> '.'
#29
Comments
I think this is fixed by ipa: raspberrypi: Rename ov9281.json to ov9281_mono.json You could probably check by renaming it yourself: |
No, this is not it. I did this already. The crash must be something different.
|
looks like a null-pointer in libcamera/src/ipa/raspberrypi/raspberrypi.cpp Lines 1186 to 1188 in cb4c5f3
[...] |
You should not be seeing any of these error messages. I suggest you pull the latest libcamera and libcamera-apps builds from apt to reset your environment and try again. Would you be able to also run the |
What do you mean? To my knowledge, these are the latest:
|
Those log messages suggest the ov9281 tuning file installed on your system is an old format file. The new format file can be found in the latest releases such as https://github.com/raspberrypi/libcamera/blob/main/src/ipa/raspberrypi/data/ov9281.json |
no, that's not it
Edited to add:
|
Those are indeed the up-to-date versions of the libraries. I can't see what exactly is going wrong unfortunately. I'll get hold of an ov9281 and give it a try myself. |
Starting with a new clean 32-bit image, and doing a
Once I do a
You can ignore the last three warning messages, they really ought to be removed for any mono sensor. With manually building and installing the up-to-date libcamera tree at commit aa7b374054c7, I don't need to do the @kralo are you able to manually build/install the latest libcamera tree on your setup. Alternatively, you might try |
on (fresh)
and same with updated
btw, I am running Aren't you able to reproduce this? |
Unfortunately I have not been able to reproduce. I will try again with a fresh image in case I did something wrong. EDIT: Running
|
Hello all, I've got the same problem as @kralo but I'm using an imx477 sensor. I think the bug is caused by an interaction between QT and recent versions of libcamera. Both of these programs are a few months old and ran fine with old versions of libcamera (git20220705+f30ad033-1) but the qt version (as well as qcam) doesn't work with the latest releases (git20221109+d528119f-1). |
@NatGr Are you compiling these applications yourself? Are you compiling libcamera yourself? I'm worried we're hitting ABI instability issues here or multiple installed versions. You may need to recompile your custom application to match the updated libcamera install. It sounds like your issue may be independent, please create a new/separate issue and paste a full backtrace / log of the crash - and if possible - use gdb to identify exactly where the crash is occurring that you are seeing. |
I'm compiling my two applications myself but libcamera and qcam come from the package manager. Here is a backtrace of qcam. I just mentionned my application to give a bit of context, I don't think it's worth creating another issue but I'll do it if you disagree.
At This point the selection dialog appears and when I click on Ok:
|
So, to recap: I get the crash in @NatGr gets it with imx447 at the same function. I just tested and get the crash with an inno-maker ov9281 also. This is with
qcam works when compiled from source, The packaged @naushir you really have a working qcam on aarch64 ? do earlier packaged versions work for you as well? Might there be something off with the packaging? |
It definitely works for me. When I get some spare time, I'll try reverting to earlier packages and test out. |
Given that |
This does not tell me anything.
|
@naushir could this because libcamera-apps are setting some environment variable that gets used which isn't set in qcam or anything like that ? |
The only env variable it sets (as far as I'm aware) is for the tuning file, if a custom one is specified on the command line. |
Ok - shouldn't be that then. This is very perplexing that it is crashing in the core code/ipa with qcam, but not libcamera-apps. @kralo - could you check with 'cam' ... something like |
@kralo Do you know how to use git-bisect ? If you can identify a specific commit in these ranges it may be enlightening.
|
As i wrote earlier, I can compile the latest tips without problems. Thats why I suggested something was off with the packaging. I have been trying to recompile the deb from source, but it hangs when compiling. I suppose the packages are cross-built ? |
You have a hint where it is packaged? |
That's why I've asked to bisect ... bisect means compile in stages between the failing version and the working version to see which commit introduces the issue. if cam isn't packaged- that's up to RPi. When you build from source, it's build/src/apps/cam/cam -c1 -C20 --metadata - but if that's already known to work it's not going to tell us much. |
If you have known working versions, and known bad versions: |
I know how to bisect. Please explain to me the search trajectory.
When I compile the current tip of rpi/libcamera, which is d528119, it works. Where should I go with bisecting? I don't have a "known bad" commit, because when I compile myself, it works. |
Ok - I hadn't realised your reference to the 'bad' and 'good' commits were the same. |
Given that it works as expected for me with the exact same packages, I have run out of ideas as well. We will be making a new package release imminently, so let's see what happens there. |
I got the sources via apt and rebuilt. I can produce an equally "broken" version. What really bogs me is the error about the "format of the tuning file", when it is in reality "2.0" but the parser does not seem to understand it. I added additional output but cannot believe it can not parse "2.0".
|
Erm... that seems very strange indeed! libcamera uses the libyaml parser that gets included as a meson subproject:
I don't know too much about meson subprojects, but if you already have libyaml installed in your system, it may not re-build as part of the libcamera build. In such cases, it is possible that you may have a different version from my setup causing a failure. |
hmm, but it does not seem to be relevant? It is not even downloaded ?
|
When I remove
|
I don't have
|
@kbingham this looks like a veritable libcamera issue to me. Fasten your seatbelts, I think this is a locale issue ( "2.0" style vs. "2,0" style) Crashes
works
I have stepped into YamlObject::get libcamera/src/libcamera/yaml_parser.cpp Lines 285 to 289 in d528119
And the situation is
This suggests that strtod has parsed until the dot. And the comparison in l. 288 fails For example with the german system, see 1 the packaged qcam also works with I don't know a quick remedy. looks like strtod_l is a candidate, but I don't see through the targeted c++ std versions. |
Yikes - How does that exhibit itself as a difference from when run when installed as a package, vs run from a self built tree? |
I see no difference when accounting for that, i.e. I have a libcamera tip that I can crash or use, depending on which LC_ALL I prepend |
@kralo great debugging. Is this something that can be solved if the "version" tag is read as a string instead of a float like it currently does? If this does impact any float type numbers... there are plenty more in the tuning files to be read :( |
Indeed - and there may be other locales with the same (but different) issue :-(
|
I wonder if we need to use something like this in libcamera? |
Given we define the yaml files in a specific locale , we could also set the locale when parsing (and then restore it after?) |
haha you beat me to it. The only thing there is we could only set it while parsing, so we'd have to 'get' the existing locale and cache it to then restore after parsers have completed. Otherwise we could end up affecting other application behavious. |
I got there first :-) |
decimal separator <> '.'
I have sent out a patch to libcamera that prints more verbose information which file it has read with which version number. That at least improves the speed of discovery. Then I will let you experts ponder what the right fix is regarding the locale. I'll let this ticket stay open until a solution is found. I consider this reported to libcamera, because kb has seen it. ok? |
Sounds good, thanks for the debugging here! |
Discussing this more today, I think the solution will be that we'll implement our own (non-locale affected) version of strtod in our utils::strtod() namespace which shouldn't be affected by this locale issue. |
The libc implementations we support (glibc, musl, uclibc and bionic) all provide stdtod_l() which takes the desired locale as an extra parameter. We could use those (possibly with an alias in utils::, and a rule in checkstyle.py to catch usage to plain strtod()). |
Aha, and that's even what @kralo suggested in #29 (comment) |
Also now reported separately on our bugzilla. |
@kralo I'm not able to replicate this issue locally - even if I generate a de_DE.utf8 locale ... so I'm struggling to validate this. But regardless I've thrown this patch together to test. https://github.com/kbingham/libcamera/tree/kbingham/clocale - or specifically this commit kbingham/libcamera@3025a88 |
When parsing configuration files on systems with differing locales, the use of strtod can produce different results, or in the worst case - fail to parse expected values. Fix this by using strtod_l() instead. To avoid constructing and destructing a locale_t instance for every use of strtod_l(), create an RAII class that wraps the locale_t and use it to provide a global "C" locale. Bug: https://bugs.libcamera.org/show_bug.cgi?id=174 Bug: raspberrypi/libcamera#29 Reported-by: https://github.com/kralo Reported-by: Hannes Winkler <hanneswinkler2000@web.de> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The fix for this has now been merged upstream at https://git.libcamera.org/libcamera/libcamera.git/commit/?id=e8ae254970cfdeb1b5aba307a95a3189b09c9784 Please let me know if this resolves the issue fully for you. |
I was just able to positively assert that qcam crashes on kbingham/libcamera@0081e4e6 (parent to e8ae254), but it works with kbingham/libcamera@e8ae2549.
Same with rpi/libcamera 0684c373 - it crashes by default, but not when your patch applied by hand:
Thank you very much for your work on this, @kbingham ! Rpi folks might consider a new release ? |
this is with pretty recent updates.
The qcam camera selection dialogue appears. Upon clicking "OK", the SIGSEV happens.
Might this be related to the latest updates?
The text was updated successfully, but these errors were encountered: