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

[KO Myung-Hun]

Hi/2.

Diego Biurrun wrote:
> On Tue, Jan 13, 2009 at 12:24:25PM +0900, KO Myung-Hun wrote:
>   
>> I've attached the updated patches as your suggestion.
>>
>> --- stream/vcd_read_os2.h   (revision 0)
>> +++ stream/vcd_read_os2.h   (revision 0)
>> @@ -0,0 +1,275 @@
>> +/*
>> + * Implementation of VCD IO for OS/2
>>     
>
> implementation
>
>   

Fixed.

>> +typedef struct {
>> +    BYTE bFrame;
>> +    BYTE bSecond;
>> +    BYTE bMinute;
>> +    BYTE bReserved;
>> +} msf;
>>     
>
> Do you really need a typedef here?
>
>   

Any problem ?

>> +    rc = DosDevIOCtl(
>> +            vcd->hcd, IOCTL_CDROMAUDIO, CDROMAUDIO_GETAUDIOTRACK,
>> +            &sParam, sizeof(sParam), &ulParamLen,
>> +            &sData, sizeof(sData), &ulDataLen);
>>     
>
> This is an ugly way to indent, instead please use
>
>     rc = DosDevIOCtl(vcd->hcd, IOCTL_CDROMAUDIO, CDROMAUDIO_GETAUDIOTRACK,
>                      &sParam, sizeof(sParam), &ulParamLen,
>                      &sData, sizeof(sData), &ulDataLen);
>
>   

I don't know why it is ugly, but it's not important.

Fixed.

>> +    rc = DosDevIOCtl(
>> +            fd, IOCTL_CDROMAUDIO, CDROMAUDIO_GETAUDIODISK,
>> +            &sParamDisk, sizeof(sParamDisk), &ulParamLen,
>> +            &sDataDisk, sizeof(sDataDisk), &ulDataLen);
>>     
>
> ditto
>
>   

Fixed.

>> +            rc = DosDevIOCtl(
>> +                    fd, IOCTL_CDROMAUDIO, CDROMAUDIO_GETAUDIOTRACK,
>> +                    &sParamTrack, sizeof(sParamTrack), &ulParamLen,
>> +                    &sDataTrack, sizeof(sDataTrack), &ulDataLen);
>>     
>
> again
>
>   

Fixed.

>> +            mp_msg(MSGT_OPEN, MSGL_INFO, "track %02d:  adr=%d  ctrl=%d  %02d:%02d:%02d\n",
>> +                i,
>> +                sDataTrack.bControlInfo & 0x0F,
>> +                (sDataTrack.bControlInfo >> 4) & 0x0F,
>> +                sDataTrack.msfStart.bMinute,
>> +                sDataTrack.msfStart.bSecond,
>> +                sDataTrack.msfStart.bFrame);
>>     
>
> weird indentation
>
>   

Fixed, but why is it weird ?

>> +                mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VCD_TRACK_%d_MSF=%02d:%02d:%02d\n",
>> +                        i - 1, iMinute, iSecond, iFrame);
>>     
>
> ditto
>
>   

Fixed.

>> +    vcd = calloc(1, sizeof(mp_vcd_priv_t));
>> +    vcd->hcd = fd;
>> +    vcd->iFirstTrack = sDataDisk.bFirstTrack;
>> +    vcd->iLastTrack = sDataDisk.bLastTrack;
>> +    vcd->msfLeadOut = sDataDisk.msfLeadOut;
>>     
>
> This could be aligned.
>
>   

Ok.

>> +    rc = DosDevIOCtl(
>> +            vcd->hcd, IOCTL_CDROMDISK, CDROMDISK_READLONG,
>> +            &sParam, sizeof(sParam), &ulParamLen,
>> +            &sData, sizeof(sData), &ulDataLen);
>>     
>
> see above
>
>   

Fixed.

> The rest looks fine.  If there are no further comments I would soon
> apply an updated patch.
>   

Good.

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


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: vcd.diff
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20090113/67e1988a/attachment.asc>