tiagocoutinho / linuxpy

Human friendly interface to linux subsystems using python
https://tiagocoutinho.github.io/linuxpy/
GNU General Public License v3.0
28 stars 3 forks source link

Fixes to codegen and video devices handling #4

Closed Chimstaz closed 6 months ago

Chimstaz commented 6 months ago

This pull request aim to fix issues with v4l2 that I encountered. 1) codegen/base.py isn't able to parse structure without members. Such structure was introduced in kernel 6.5. struct usb_ssp_cap_descriptor uses __DECLARE_FLEX_ARRAY macro which generate empty structure. 2) Changed codegen to replace [unsigned] long long int with c[u]longlong. Without this change some structures used members with different size of types (e.g. u64 for unsigned long int which usually has 32 bits). 3) Some ioctls may be not implemented by V4L2 drivers. To support that drivers, iter_read ignores ENOTTY error which indicates that ioctl is not implemented by the driver. 4) Some V4L2 drivers may report correct frame size using ENUM_FRAMESIZES, but doesn't implement ENUM_FRAMEINTERVALS. In that case frame size is still returned, to give an user information about frame size.

amstan commented 6 months ago

This totally fixed my problem. Thanks Tomasz!

FWIW, I'm on arm32. Though I could not reproduce the same problem on my x86_64 desktop.

I had something similar to this:

import linuxpy.video.raw as v4l2

def video_map_buf(fd, idx):
  buf = v4l2.v4l2_buffer()
  buf.type = v4l2.BufType.VIDEO_CAPTURE
  fcntl.ioctl(fd, v4l2.IOC.QUERYBUF, buf)
  return mmap.mmap(fd, buf.length, offset=buf.m.offset)

And I kept getting Invalid Argument for the mmap since length was set to 0. I compared the list(bytes(buf)) to other libraries and it seemed the length field was in a different spot.

Luckly I just discovered @Chimstaz's patches and they do seem to do the trick to fix the format of buf and now my device is happy. I look forward to a new release to incorporate this fix.

tiagocoutinho commented 6 months ago

🤔 looks like it's failing in the linting phase. Would you mind having a look @Chimstaz ?

Chimstaz commented 6 months ago

I pushed new version of this patch. I think lint will be happy now.

tiagocoutinho commented 6 months ago

It still fails. I suspect it is not your fault. I've merged into master changes that fix a misalignment between pre-commit hook and CI lint checks. Maybe if you rebase from master it will help (assuming you have pre-commit hook installed).

Also codegen is using black but pre-commit hook and CI are using ruff which is far from ideal.

tiagocoutinho commented 6 months ago

FYI, I have another PR #7 that replaces black with ruff on code generation

Chimstaz commented 6 months ago

I rebased this branch on top of PR #7, so you could merge it after merging that another PR.

Also while I was on codegen changes, I added one extra patch which adds V4L2 DT timings enums. I would like to use them, to get some information from my device. DT timings ioctls in general are useful if you want to query new timing information after resolution change without closing a device (more about that in kernel documentation https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/vidioc-query-dv-timings.html#vidioc-query-dv-timings).

Please review this extra commit.

tiagocoutinho commented 6 months ago

Still fails. I know the problem and made a quick fix in #8. Already merged.

If you rebase from latest master it should work now.

I'm sorry for the trouble

Chimstaz commented 6 months ago

Rebased to the master. I hope it will be OK now.

tiagocoutinho commented 6 months ago

Oh, forgot to tell you to re-generate the code and commit after you rebased master

Chimstaz commented 6 months ago

Ok, now it should be it. I added extra commit which removes extra newline at the end of linuxpy/usb/ids files. Basically it is the same fix you did for the codegen/base.py copied to codegen/usbids.py

Chimstaz commented 6 months ago

I have no idea why "Add Coverage PR Comment" step have failed.