lit-robotics / libcamera-rs

Experimental Rust bindings for libcamera
Apache License 2.0
46 stars 15 forks source link

Remove unused padding in libcamera_stream_configuration #19

Closed twistedfall closed 1 year ago

twistedfall commented 1 year ago

Hi!

I think we should just drop the padding altogether and don't compare the structure size in tests. The primary reason for that is failing static_assert on 32-bit architectures (Raspberry Pi Zero for example). It's due to the std::string or std::map containing pointers to memory that take more 4 bytes vs 8 bytes on 64-bit archs (partially reported in https://github.com/lit-robotics/libcamera-rs/issues/17).

We could introduce a more complicated padding like:

    void *__padding0[6];
    uint8_t __padding1[24];

But I don't feel like this approach has any benefit. The Rust code doesn't make any use of that padding space and only uses that struct through a pointer so only the alignment of the important fields matter and this is already checked with offsetof.

chemicstry commented 1 year ago

Hi, thanks for the PR!

LGTM