[Video] StdVideoH265HrdFlags layout differs on Windows and Linux platforms

[MarijnS95]

Originally noticed in ash-rs/ash#417 (comment), Windows and Linux C compilers treat the total size of bitfields differently. The struct:

typedef struct StdVideoH265HrdFlags {
    uint32_t nal_hrd_parameters_present_flag : 1;
    uint32_t vcl_hrd_parameters_present_flag : 1;
    uint32_t sub_pic_hrd_params_present_flag : 1;
    uint32_t sub_pic_cpb_params_in_pic_timing_sei_flag : 1;
    uint8_t  fixed_pic_rate_general_flag; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
    uint8_t  fixed_pic_rate_within_cvs_flag; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
    uint8_t  low_delay_hrd_flag; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
} StdVideoH265HrdFlags;

Takes up 1 byte for the first four bitflags on Linux, but 4 bytes on Windows, leading to a - perhaps unexpected - mismatch. See https://godbolt.org/z/bev9eEzdv for a compiler comparison. Is this known and desired?

[DadSchoorse]

Please absolutely avoid such differences, they will make our job in Wine a lot harder. Not using bitfields in general might be a good idea because they are very implementation defined. There are some structs related to RT which require them to match the gpu hardware, but as far as I can tell that's not the case here.

[MarijnS95]

It's just a single boolean per field, the suggested alternative is to use bitflags like everywhere else in the Vulkan API which have proven to work nicely already.

[oddhack]

@zlatinski please bring to attention of the Video TSG.

[zlatinski]

Originally noticed in MaikKlein/ash#417 (comment), Windows and Linux C compilers treat the total size of bitfields differently. The struct:

typedef struct StdVideoH265HrdFlags {
    uint32_t nal_hrd_parameters_present_flag : 1;
    uint32_t vcl_hrd_parameters_present_flag : 1;
    uint32_t sub_pic_hrd_params_present_flag : 1;
    uint32_t sub_pic_cpb_params_in_pic_timing_sei_flag : 1;
    uint8_t  fixed_pic_rate_general_flag; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
    uint8_t  fixed_pic_rate_within_cvs_flag; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
    uint8_t  low_delay_hrd_flag; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
} StdVideoH265HrdFlags;

Takes up 1 byte for the first four bitflags on Linux, but 4 bytes on Windows, leading to a - perhaps unexpected - mismatch. See https://godbolt.org/z/bev9eEzdv for a compiler comparison. Is this known and desired?

Thank you, for reporting this issue!
We'll fix the reported issue by adding bitfield and struct padding:

typedef struct StdVideoH265HrdFlags {
    uint32_t nal_hrd_parameters_present_flag : 1;
    uint32_t vcl_hrd_parameters_present_flag : 1;
    uint32_t sub_pic_hrd_params_present_flag : 1;
    uint32_t sub_pic_cpb_params_in_pic_timing_sei_flag : 1;
    uint32_t fixed_pic_rate_general_flag : 8; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
    uint32_t fixed_pic_rate_within_cvs_flag : 8; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
    uint32_t low_delay_hrd_flag : 8; // each bit represents a sublayer, bit 0 - vps_max_sub_layers_minus1
} StdVideoH265HrdFlags;

[MarijnS95]

@zlatinski Thanks! The people I have spoken to seem to prefer flags like the rest of the Vulkan API, but given that this is outside the scope of vk.xml (and the category="bitmask"/VkFlags type needed to describe this properly) this should be good enough.

[misyltoad]

It would be preferable to resolve this by replacing bitmasks with flags.

Nowhere else in Vulkan is using bitmasks and this will break FFI integrations and language bindings significantly.

Not every language supports C-styled bitfields.

[oddhack]

The original issue has been fixed in the 1.2.197 spec update.

[zlatinski]

Not every language supports C-styled bitfields.

Please note that those are C-header files, so C/C++ language is implied. We are working on converting the std* video headers to XML, so other languages can generate the language-specific output files, according to their requirements.