Skip to content

Regression in opencv 4.11 when reading double precision matrix data from yaml files #1078

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

Closed
lostoliv opened this issue Jan 23, 2025 · 2 comments
Assignees
Labels

Comments

@lostoliv
Copy link

Reading this yaml file in opencv 4.10 works while failing with opencv 4.11 (all int values like 0 get corrupted in 4.11 - while 0.0 is fine):

%YAML:1.0
matrix1:
  rows: 1
  cols: 4
  dt: d
  data: [ 0, 0.0, 1, 1.0 ]

Note that changing dt: d to dt: f works fine.

You can run the following code to repro (assert failing only with 4.11):

# Good: python -m pip install --upgrade opencv-python==4.10.0.84
# Bad:  python -m pip install --upgrade opencv-python==4.11.0.86
import cv2, numpy as np
mat_str = '[ 0, 0.0, 1, 1.0 ]'
yaml_file =\
'%YAML:1.0\n'\
'matrix1:\n'\
'  rows: 1\n'\
'  cols: 4\n'\
'  dt: d\n'\
f'  data: {mat_str}\n'
node_name = 'matrix1'
fs = cv2.FileStorage(yaml_file, cv2.FILE_STORAGE_MEMORY)
mat = fs.getNode(node_name).mat().flatten()
expected = np.array(eval(mat_str))
print(f'mat from yaml: {mat}')
print(f'mat expected:  {expected}')
assert np.allclose(mat, expected)

I see some changes related to FileStorage in 4.11 (like opencv/opencv#26434) that might have introduced this regression.

@asmorkalov
Copy link
Collaborator

@dkurt Could you take a look? May it be related to int64 support?

@asmorkalov asmorkalov added the bug label Jan 28, 2025
@dkurt
Copy link
Member

dkurt commented Jan 28, 2025

opencv/opencv#26846 resolves this issue too

asmorkalov pushed a commit to opencv/opencv that referenced this issue Jan 28, 2025
…rt_for_FileStorage

Fix bug with int64 support for FileStorage #26846

### Pull Request Readiness Checklist

Fix #26829, opencv/opencv-python#1078

In current implementation of `int64` support raw size of recorded integer is variable (`4` or `8` bytes depending on value). But then we iterate over nodes we need to know it exact value
https://github.com/opencv/opencv/blob/dfad11aae7ef3b3a0643379266bc363b1a9c3d40/modules/core/src/persistence.cpp#L2596-L2609

Bug is that `rawSize` method still return `4` for any integer. I haven't figured out a way how to get variable raw size for integer in this method. I made raw size for integer is constant and equal to `8`.

Yes, after this patch memory consumption for integers will increase, but I don't know a better way to do it yet. At least this fixes bug and implementation becomes more correct

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
NanQin555 pushed a commit to NanQin555/opencv that referenced this issue Feb 24, 2025
…_support_for_FileStorage

Fix bug with int64 support for FileStorage opencv#26846

### Pull Request Readiness Checklist

Fix opencv#26829, opencv/opencv-python#1078

In current implementation of `int64` support raw size of recorded integer is variable (`4` or `8` bytes depending on value). But then we iterate over nodes we need to know it exact value
https://github.com/opencv/opencv/blob/dfad11aae7ef3b3a0643379266bc363b1a9c3d40/modules/core/src/persistence.cpp#L2596-L2609

Bug is that `rawSize` method still return `4` for any integer. I haven't figured out a way how to get variable raw size for integer in this method. I made raw size for integer is constant and equal to `8`.

Yes, after this patch memory consumption for integers will increase, but I don't know a better way to do it yet. At least this fixes bug and implementation becomes more correct

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants