[MPlayer-dev-eng] [PATCH] (2nd) Fix demux_avi >=2GB overflow & enhance avi info output

[eviv bulgroz]

Same as before but with proper [PATCH] subject line !

Oops, oh well ... ;-)
I saw a few minor differences like no a/v dwLength fix (I guess it's intended), no audio bps
printed, no tidle ~ printed when using fallback and also a few more relevant ones :
- uses video time length instead of audio time length to calculate audio size in fallback ?
- differences in fallback for priv->numberofframes calculation
- run time not printed (nice to have though, at least in verbose)

I prepared a small patch for the last three items


--- Arpi <arpi at thot.banki.hu> wrote:
> Hi,
> 
> > Here is second try to that demux_avi patch :
> 
> eh a bit too late
> i've already applied your first version with my heavy changes.
> 
> > 
> > - fix dwLength now counted in samples, not in chunks,
> >   use priv->audio_block_size for audio dwlength calc.
> > - audio_sh != NULL now always tested 
> > - fallback for a/v size calc. included again 
> >   and fixed in audio size calc.to use sh_audio->wf->nAvgBytesPerSec.
> > (prev. email)
> > - fix fallback in priv->numberofframes calc. (new)
> > - tilda "~" printed when a/v size calc. fallback used.
> > 
> > --- eviv bulgroz <ebulgroz at yahoo.com> wrote:
> > > Hi,
> > > Thanks for your help and advices, I'll try that and maybe shift this whole dwLength tangling
> to
> > > read_avi_header too. I've one question though. As asked, I won't remvove the original i_bps
> > > calulation code and use it as a fallback when no index is available. But I can't figure out
> what
> > >  
> > > 
> > >   if(sh_audio) sh_video->i_bps-=sh_audio->audio.dwLength;
> > > 
> > > is doing? shouldn't it be rather something like:
> > > 
> > >   if(sh_audio && sh_audio->wf->nAvgBytesPerSec) 
> > >     sh_video->i_bps-= (float)sh_audio->wf->nAvgBytesPerSec*
> > >     sh_audio->audio.dwLength*sh_audio->audio.dwScale/sh_audio->audio.dwRate;
> > > 
> > > 
> > > --- Arpi <arpi at thot.banki.hu> wrote:
> > > > Hi,
> > > > 
> > > > ok, the problem is right (and known, by me) but your fix is buggy too:
> > > > 
> > > > > -  // calculating video bitrate:
> > > > > -  sh_video->i_bps=demuxer->movi_end-demuxer->movi_start-priv->idx_size*8;
> > > > > -  if(sh_audio) sh_video->i_bps-=sh_audio->audio.dwLength;
> > > > > -  mp_msg(MSGT_DEMUX,MSGL_V,"AVI video length=%lu\n",(unsigned long)sh_video->i_bps);
> > > > > - 
> sh_video->i_bps=((float)sh_video->i_bps/(float)sh_video->video.dwLength)*sh_video->fps;
> > > > > -  
> > > > > -  if((priv->numberofframes=sh_video->video.dwLength)<=1)
> > > > > -    // bad video header, try to get number of frames from audio
> > > > > -    if(sh_audio && sh_audio->wf->nAvgBytesPerSec)
> > > > priv->numberofframes=sh_video->fps*sh_audio->audio.dwLength/sh_audio->wf->nAvgBytesPerSec;
> > > > 
> > > > this code shouldn't be removed, but used as fallback if no index (maybe with
> > > > a warning saying that values are inaccurate, and/or printing '~' sign before
> > > > teh numbers)
> > > > 
> > > > > +  // calc video bitrate, count idx, calc a/v size & fix dwLength if needed:
> > > > 
> > > > > +    sh_video->video.dwLength=0;
> > > > > +    sh_audio->audio.dwLength=0;
> > > > > +    for(i=0;i<priv->idx_size;i++){ 
> > > > > +      if(sh_video->ds->id == avi_stream_id(((AVIINDEXENTRY *)priv->idx)[i].ckid)) {
> > > > > +        vsize+=((AVIINDEXENTRY *)priv->idx)[i].dwChunkLength;
> > > > > +        sh_video->video.dwLength++;
> > > > > +      }
> > > > > +      else if(sh_audio->ds->id == avi_stream_id(((AVIINDEXENTRY *)priv->idx)[i].ckid))
> {
> > > > > +        asize+=((AVIINDEXENTRY *)priv->idx)[i].dwChunkLength;
> > > > > +        sh_audio->audio.dwLength++;
> > > > > +      }
> > > > 
> > > > this is BAD.
> > > > dwLength is counted in samples, not in chunks.
> > > > also, calculating the right sample number is very tricky for audio, esp. for
> > > > VBR. you need to round up chunksize to N * nBlockAlign, so:
> > > > 
> > > > dwLength += (dwChunkLength+nBlockAlign-1)/nBlockAlign;
> > > > 
> > > > but also check that nBlockAlign is valid, and fallback to dwSampleSize if
> > > > that valid and fallback to nBitsPerSample/8*mChannels and so on...
> > > > see demux_avi.c, for the "right" way of calculating number of samples...
> > > > 
> > > > 
> > > > A'rpi / Astral & ESP-team
> > > > 
> > > > --
> > > > Developer of MPlayer, the Movie Player for Linux - http://www.MPlayerHQ.hu
> > > > _______________________________________________
> > > > MPlayer-dev-eng mailing list
> > > > MPlayer-dev-eng at mplayerhq.hu
> > > > http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
> > > 
> > > 
> > > __________________________________________________
> > > Do you Yahoo!?
> > > Yahoo! Mail Plus - Powerful. Affordable. Sign up now.
> > > http://mailplus.yahoo.com
> > > _______________________________________________
> > > MPlayer-dev-eng mailing list
> > > MPlayer-dev-eng at mplayerhq.hu
> > > http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
> > 
> > __________________________________________________
> > Do you Yahoo!?
> > Yahoo! Mail Plus - Powerful. Affordable. Sign up now.
> > http://mailplus.yahoo.com
> > 
> 
> 
> A'rpi / Astral & ESP-team
> 
> --
> Developer of MPlayer, the Movie Player for Linux - http://www.MPlayerHQ.hu
> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
> http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng


__________________________________________________
Do you Yahoo!?
Yahoo! Mail Plus - Powerful. Affordable. Sign up now.
http://mailplus.yahoo.com