[MPlayer-dev-eng] [PATCH] rff - demux_control patch v1.2

[Arpi]

Hi,

> Here is the updated demux_control patch
> Btw is there a reason for not using function pointers for the demux_control 
> stuff ? That way these can be plugins, which can be added and removed easily. 
> Or am I missing something ?
yes.
(hint: "demuxer API"... do you remember of the beginning of this thread?)

> +      asf_movielength=fileh.send_duration/10000000;
maybe should be /10000000LL; or so

> +  if((priv->numberofframes=sh_video->video.dwLength)<=1){
> +    // bad video header, try to get it from audio
> +    if(sh_audio) priv->numberofframes=sh_video->fps*sh_audio
> ->audio.dwLength/sh_audio->wf->nAvgBytesPerSec;
> +        if(priv->numberofframes<=1){
> +		mp_msg(MSGT_SEEK,MSGL_WARN,MSGTR_CouldntDetFNo);
> +        	priv->numberofframes=1;
> +        }
> +  }  

this looks bad
- check for nAvgBytesPerSec!=0
- priv->numberofframes=1; will confuse control()
- length calc will be INACCURATE when numberofframes calced from audio rate

> -  sh_video->i_bps=((float)sh_video->i_bps/(float)sh_video->v
> ideo.dwLength)*sh_video->fps;
> +  sh_video->i_bps=((float)sh_video->i_bps/(float)priv->numbe
> rofframes)*sh_video->fps;
why?

> -              mp_msg(MSGT_SEEK,MSGL_WARN,MSGTR_CouldntDetFNo);
> -	      total=0;

> +    switch(cmd) {
> +	case DEMUXER_CTRL_GET_TIME_LENGTH:
> +    	    *((unsigned long *)arg)=priv->numberofframes/sh_video->fps;
> +	    return DEMUXER_CTRL_OK;

numberofframes should be set to 0 if unknown, so you could handle it here

> +	case DEMUXER_CTRL_GET_PERCENT_POS:
> +	    *((int *)arg)=(int)(priv->video_pack_no*100/priv->numberofframes);

calc pos from (demux->filepos-movi_start)/(movi_end-movi_start) if
no_of_frames not available

> +	case DEMUXER_CTRL_GET_PERCENT_POS:
> +	    if (demuxer->movi_end==demuxer->movi_start) 
> +    		return DEMUXER_CTRL_DONTKNOW;
> +    	    *((int *)arg)=(int)(demuxer->filepos/((demuxer->mov
> i_end-demuxer->movi_start)/100));	    

should be (demuxer->filepos-demuxer->movi_start)/...

teh rest looks ok


A'rpi / Astral & ESP-team

--
Developer of MPlayer, the Movie Player for Linux - http://www.MPlayerHQ.hu