Memory Alignment and stride issues

[sylpe]

Hello,

Not really a bug, but I don't know how to correctly handle the situation.

Some sensors have exotic resolutions. ST vd56g3 for example : https://patchwork.linuxtv.org/project/linux-media/cover/20240904145238.21099-1-sylvain.petinot@foss.st.com/ has a 1124x1364 native pixel array.

With such exotic resolution, we can face stride issues with pyv4l2 (assertion will fail in videodev.py : https://github.com/tomba/pyv4l2/blob/7f1f80045ff949ba040f658bad1d948ecda618fd/v4l2/videodev.py#L328)

I generally observe that most receivers align in memory on multiples of 16 bytes. Thus the following commit sylpe@c9f0357 prevents most of the stride issues.
However this is nothing standardized: 16-byte alignment works well on RPI5 and Qcom RB5 but at the same time RPI4 align memory on 32 bytes...

Because this is platform-dependent I don't know what is the best option to handle this.
I would suggest to first change the default value of align parameter to "common" values like 16 or 32. That being done, there is still a need to adjust this alignment on certain platforms and maybe it would be good to have a dedicated parameter in the init().

What are your thoughts ?

Thanks in advance

[kbingham]

@sylpe Can you test if #6 solves your issue ? (I think it's possibly closely related, but I'm not entirely sure yet - I hope so - if it helps you !!)

Ultimately - the platforms indeed should report the stride - and this wasn't being handled correctly in all cases. I hit this too - and I hope solving my case might also be your solution too!

[sylpe]

Hi @kbingham, In fact I currently solved my problem with the following : sylpe@99a3cca

It still requires some work (the reason why I don't PR yet) ....
I need to expose the 'memory_alignment' parameter in PixelFormats Class to allow specific instantiation on platforms where the default alignment is not suitable.

I saw your PR few weeks back, it was on my todo to give it a try (-> will do that next week).

[tomba]

I don't think there's an issue here, just unimplemented features in pyv4l2. If the platform requires a specific stride alignment, that information must come from the drivers (DRM or V4L2). Pixutils can't guess it, instead the user of pixutils (pykms/pyv4l2) must provide the correct stride. And those users can't guess it either, but need to ask from the driver.

Currently, for simplicity, pyv4l2 just asserts and fails if the stride that v4l2 gives us back is not the same as we requested. Instead, it should either find out the correct stride before calling set_format(), or somehow use the stride we get back from set_format(). Solving this properly probably requires bigger changes, as the buffer management is not very well designed. And buffer management is not easy...

[sylpe]

Hi @tomba, you're totally right ... I don't think it's an "issue" at all.
In my commit, I am just trying to make it easier for the user (pyv4l2) to configure this stride value.
I also observe that many platforms make use of 16 as an alignment value and make it the default.

In my use case, I have some pytests on top of pyv4l2. I have some platform helpers that contains the stride size per platform.

[kbingham]

Are we talking about stride at capture time or display here? I know the pr I just got merged was about capture device application requirements, so it should have fixed RPI issues which indeed have hardware specific alignments.

I'm curious if there is another side to this where you need to enforce assignments to be able to reuse the buffer across two devices with different alignment requirements?

[tomba]

@kbingham This is (mostly) about pyv4l2 stride management on buffers it allocates. That feature is kind of missing, and pyv4l2 expects the buffers from the v4l2 device are "perfectly" aligned without stride (i.e. the default stride which pixutils uses).

[sylpe]

@kbingham on my side, I'm only talking about stride at capture time.

I tested your merged PR and it doesn't solve my "issue" (which - as @tomba mentioned - is not really an issue).

My sensor outputs 1124x1364.
On a RPI5 (which is 16-byte aligned), the 'bytesperline' = 1136 (while by default pixutils compute a "stride" of 1124)

[kbingham]

But I thought my PR ensures that the stride reported by the kernel (RPi5 in this case) is now considered - so I would absolutely expect my PR to solve the issue for you ... (and if it hasn't - I'd be keen to see what's going on).

Aha - unless I've only handled the raw paths - and you're also seeing the same topic on other paths...

Can you share more logs about what pixel formats and configuration you are handling ?

[kbingham]

Aha - ok - following the links - I suspect it's because of different code paths. I was fixing the conversion utilities which are used by camshark - so camshark is handling the buffer allocations - while your issues are more directly in local allcoations in local tools here perhaps ... ok - so likely more to fix indeed to make sure the stride reported by the v4l2 device is correctly managed in those helpers.

[tomba]

@kbingham You fixed the pixutils RAW -> RGB conversion when there's padding. @sylpe is talking about buffer allocation/handling with stride (not related to pixutil's conversion at all).

[sylpe]

Exactly, I'm failing in the set_format() here : https://github.com/tomba/pyv4l2/blob/master/v4l2/videodev.py#L333