[MPlayer-dev-eng] [PATCH] Remove leading/trailing margin for VCD

[Nico Sabbi]

Zuxy Meng wrote:
> Hi,
> 
> Playing around with VCD, I found that the MPEG packs don't start
> precisely at the beginning of a track, and don't end precisely at the
> end of a track either: they are wrapped by all zero sectors. I don't
> have a copy of the White Book so I can't tell if it's mandated by the
> standard or just an implementation issue. I guess the rationale behind
> these margins is that old VCD players have problems in accurate
> seeking. Nevertheless, the leading margin makes lavf demuxer reject it
> as a legitimate MPEG PS stream.
> 
> Two notes about the patch:
> 1. Experience shows that reading sector by sector in the backward
> direction is extremely slow so I commented such code out. As a result
> trailing margin isn't stripped and fortunately it doesn't cause much
> trouble in demuxing too.
> 2. Each MPEG pack (as well as a non-null VCD sector) begins with
> 0x000001ba and I picked the last byte (0xba) as the probe.
> 
> 
> ------------------------------------------------------------------------
> 
> Index: stream/stream_vcd.c
> ===================================================================
> --- stream/stream_vcd.c	£¨°æ±¾ 22543£©
> +++ stream/stream_vcd.c	£¨¹¤×÷¸±±¾£©
> @@ -79,7 +79,7 @@
>  
>  static int open_s(stream_t *stream,int mode, void* opts, int* file_format) {
>    struct stream_priv_s* p = (struct stream_priv_s*)opts;
> -  int ret,ret2,f;
> +  int ret,ret2,f,sect,tmp;
>    mp_vcd_priv_t* vcd;
>  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
>    int bsize = VCD_SECTOR_SIZE;
> @@ -135,6 +135,19 @@
>      m_struct_free(&stream_opts,opts);
>      return STREAM_ERROR;
>    }
> +#if 0 // search backward incurrs significant performance degradation!
> +  /* search backward up to 3 seconds to skip trailing margin */
> +  sect = ret2 / VCD_SECTOR_DATA;
> +  for (tmp = sect - 1; tmp > sect - 3 * 75; tmp--) {
> +    char mem[VCD_SECTOR_DATA];
> +    vcd_set_msf(vcd, tmp);
> +    /* If the sector contains an MPEG pack, mem[3] == 0xBA != 0 */
> +    if (vcd_read(vcd, mem) != VCD_SECTOR_DATA || mem[3] != 0)
> +      break;
> +  }
> +  mp_msg(MSGT_OPEN, MSGL_DBG2, "%d trailing sectors skipped\n", sect - tmp);
> +  ret2 = (tmp + 1) * VCD_SECTOR_DATA - 1;
> +#endif
>    ret=vcd_seek_to_track(vcd,p->track);
>    if(ret<0){
>      mp_msg(MSGT_OPEN,MSGL_ERR,MSGTR_ErrTrackSelect " (seek)\n");
> @@ -143,6 +156,17 @@
>      m_struct_free(&stream_opts,opts);
>      return STREAM_ERROR;
>    }
> +  /* search forward up to 3 seconds to skip leading margin */
> +  sect = ret / VCD_SECTOR_DATA;
> +  for (tmp = sect; tmp < sect + 3 * 75; tmp++) {
> +    char mem[VCD_SECTOR_DATA];
> +    if (vcd_read(vcd, mem) != VCD_SECTOR_DATA || mem[3] != 0)
> +      break;
> +  }
> +  mp_msg(MSGT_OPEN, MSGL_DBG2, "%d leading sectors skipped\n", tmp - sect);
> +  vcd_set_msf(vcd, tmp);
> +  ret = tmp * VCD_SECTOR_DATA;
> +
>    mp_msg(MSGT_OPEN,MSGL_V,"VCD start byte position: 0x%X  end: 0x%X\n",ret,ret2);
>  
>  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
> 

from docs/vcdimager.texi:

"
@item
Up to 98 @acronym{MPEG} mode 2 form 2 tracks wrapped in front and rear
margin at footnote{Margins seem to be used, in order to compensate for
inaccurate sector addressing issues on @acronym{CD-ROM}
media. Interestingly, they have been abandoned for the Super Video
CD.} empty sectors and preceded by (at least) 150 empty sector
pre-gaps."

it doesn't say that
so I'd say that
  for (tmp = sect; tmp < sect + 3 * 75; tmp++) {
    char mem[VCD_SECTOR_DATA];
    if (vcd_read(vcd, mem) != VCD_SECTOR_DATA || mem[2] != 0 || mem[3] 
!= 0)
     break;
  }

is safer, but why that last
vcd_set_msf(vcd, tmp); ?

The rest is fine for me, but without that if-0 code

-- 
"Without a frontend, mplayer is useless" - someone in mplayer-users