Skip to content
New issue

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

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

Already on GitHub? # to your account

With 64bit python, result of G722.decode() is broken. #6

Closed
ynkdir opened this issue Jul 25, 2024 · 4 comments
Closed

With 64bit python, result of G722.decode() is broken. #6

ynkdir opened this issue Jul 25, 2024 · 4 comments

Comments

@ynkdir
Copy link

ynkdir commented Jul 25, 2024

With 64bit python (3.12.4 amd64), result of G722.decode() is broken.

Reproducible code:

from G722 import G722

while True:
    arr = G722(16000, 64000).decode(b"00")
    print(len(arr))
    assert len(arr) == 4

Result:

PS C:\Users\yukih\Downloads\libg722> .\venv\Scripts\python.exe .\test.py
4   <- expected
140728898420740   <- invalid length
Traceback (most recent call last):
  File "C:\Users\yukih\Downloads\libg722\test.py", line 6, in <module>
    assert len(arr) == 4
           ^^^^^^^^^^^^^
AssertionError

This is caused by sizeof(long) (4) != sizeof(np_intp) (8).

diff --git a/python/G722_mod.c b/python/G722_mod.c
index 10f969e..0d8d3cb 100644
--- a/python/G722_mod.c
+++ b/python/G722_mod.c
@@ -217,7 +217,7 @@ PyG722_decode(PyG722* self, PyObject* args) {
     owner->data = array;

     // Create a new numpy array to hold the integers
-    long dims[1] = {olength};
+    npy_intp dims[1] = {olength};
     PyObject* numpy_array = PyArray_SimpleNewFromData(1, dims, NPY_INT16, (void *)array);
     if (numpy_array == NULL) goto e1;
     PyArray_SetBaseObject((PyArrayObject*)numpy_array, (PyObject*)owner);
@ynkdir
Copy link
Author

ynkdir commented Jul 29, 2024

I forgot to mention that I'm using Windows.

@sobomax
Copy link
Member

sobomax commented Jul 29, 2024

@ynkdir would it be possible for you to create a unit test? I have added some minimal build/test on Windows recently. See test.py & .github/workflows/build_and_test.yml. Thanks!

sobomax added a commit that referenced this issue Jul 29, 2024
@sobomax
Copy link
Member

sobomax commented Jul 30, 2024

I've fixed the issue, added unit test and rolled a new release 1.2.0. Thanks for reporting!

@sobomax sobomax closed this as completed Jul 30, 2024
@ynkdir
Copy link
Author

ynkdir commented Jul 30, 2024

Thank you.

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

No branches or pull requests

2 participants