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

[Diego Biurrun]

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

> +typedef struct {
> +    BYTE bFrame;
> +    BYTE bSecond;
> +    BYTE bMinute;
> +    BYTE bReserved;
> +} msf;

Do you really need a typedef here?

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

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

ditto

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

again

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

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

ditto

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

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

see above

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

Diego