[MPlayer-dev-eng] [PATCH] VCD support for OS/2

[Reimar Döffinger]

Hello,
I do very much dislike just globally using "#pragma pack", use
__attribute__((packed)) on a per-struct basis or even better
avoid it completely, e.g. by using uint8_t buffers and bit
functions and AV_RL*. Or find a system header where they are defined.

> +int vcd_seek_to_track(mp_vcd_priv_t *vcd, int track)
> +{
> +    struct {
> +        UCHAR   auchSign[4];
> +        BYTE    bTrack;
> +    } sParam = {{'C', 'D', '0', '1'},};
> +
> +    struct {
> +        msf     msfStart;
> +        BYTE    bAdr     : 4;
> +        BYTE    bControl : 4;
> +    } sData;

E.g. using those structs, unless they are defined in some system header
is not better than using
static const char sParam[5] = "CD01";
uint8_t sData[5];
Actually this is even more portable, since you do not rely on things
like bitfield order, support for packed structs etc.
This applies to all structs you defined, though a struct with only
uint8_t elements and no bitfields instead of an array is ok (and
possibly better for readability).

> +    struct {
> +        BYTE    abSync[12];
> +        BYTE    abHeader[4];
> +        BYTE    abSubHeader[8];
> +        BYTE    abData[2324];
> +        BYTE    abSpare[4];
> +    } sData;

I think the default stack size on OS/2 is rather small, so IMO you
should not allocate more than 2kB on the stack if it can be avoided.

Greetings,
Reimar Döffinger