[MPlayer-dev-eng] [PATCH] seekbar support for mp4 files

[Moritz Bunkus]

Hey,

Some comments about your patch. It seems to work fine (both Diego and I
have tested it), but I have a couple of requests.

> +void demux_mov_info(demuxer_t *demuxer, float* fps, float* length, float* timepos){ 

New patch policy says that all functions have to be documented with
doxygen-compatible comments. Please add that for the two functions
you've added.

> +    *fps = trk->timescale/((trk->durmap_size>=1)?(float)trk->durmap[0].dur:1);
> +    *length = trk->length / trk->timescale; /* seconds */
> +    *timepos = trk->pos / *fps; /* seconds */

Please make sure that you never do a division by zero. In this case
checking trk->timescale is enough for both divisions.

> +}		    

Your patch has leading whitespace on some lines. Please remove those
whitespaces.

> +        case DEMUXER_CTRL_GET_PERCENT_POS:
> +            *((int *)arg)=(int)(100 * timepos / length);

length may be zero here, e.g. if there was no video track found. Again
make sure that no division by zero can occur.

The rest is ok. I'll apply your patch once you have fixed those simple
things ;)

Thanks.

Mosu


-- 
If Darl McBride was in charge, he'd probably make marriage
unconstitutional too, since clearly it de-emphasizes the commercial
nature of normal human interaction, and probably is a major impediment
to the commercial growth of prostitution. - Linus Torvalds