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

KO Myung-Hun komh at chollian.net
Tue Jan 13 17:10:47 CET 2009


Hi/2.

Reimar Döffinger wrote:
> 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.
>
>   

On OS/2, DosDevIOCtl() expect a packed struct or a byte buffer. So I 
decide to use '#pragma pack' globally because all structs are for 
DosDevIOCtl() except 'mp_vcd_priv_t'. And the system headers for them do 
not exist.

>> +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).
>
>   

I'm confused. Are you commenting on my patches or saying your opinion 
about 'structure' ?

Of course, I absolutely agree with you if these codes are for general 
platforms and OSes. But these are only for OS/2. And those structs are 
defined in according to OS/2 API DOCs. Why should I consider what you said ?

And I don't understand a byte buffer has a better readability than a 
structure. If not using a structure, how can I know the meaning of each 
index ?

And just a question. Is there a compiler not supporting 'packed structure' ?

>> +    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.
>
>   

Yes, many compiler set the default stack size for OS/2 to 32K or 64K. 
But I think that size is enough to use 2K stack variable. In addition, 
GCC 4.3.2 are being ported to OS/2. It set the default stack size to 1M. 
Currently I'm using that GCC 4.3.2. Nevertheless I'm setting the stack 
size to 16M for MPlayer. So I think your apprehension is unnecessary.

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 1.1.14
Under OS/2 Warp 4 for Korean with FixPak #15
On AMD ThunderBird 1 GHz with 512 MB RAM

Korean OS/2 User Community : http://www.ecomstation.co.kr





More information about the MPlayer-dev-eng mailing list