[MPlayer-dev-eng] [PATCH] VCD support for OS/2
KO Myung-Hun
komh at chollian.net
Tue Jan 13 15:01:24 CET 2009
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>
More information about the MPlayer-dev-eng
mailing list