[MPlayer-cvslog] r33448 - trunk/gui/mplayer/gui_common.c

[Reimar Döffinger]

On Sun, May 22, 2011 at 10:52:30PM +0200, Ingo Brückl wrote:
> Reimar Döffinger wrote on Sun, 22 May 2011 20:02:45 +0200:
> 
> > I missed that the variables are all unsigned, why not
> > int64_t duration = fileh->play_duration;
> > duration = FFMAX(duration - 10000*fileh->preroll, 0);
> > asf->movielength=duration/10000000.0;
> 
> Nah. Why casting to int64_t only to be able to use FFMAX to figure out which
> of two values is greater?
> 
> If you don't like the if-else, maybe a ?:-one-liner is more appealing to you?

Well, I considered it still a bit of code duplication, due to the subtraction
and comparison having the same arguments.

> @@ -543,7 +544,8 @@
>        asf->packetsize=fileh->max_packet_size;
>        asf->packet=malloc(asf->packetsize); // !!!
>        asf->packetrate=fileh->max_bitrate/8.0/(double)asf->packetsize;
> -      asf->movielength=(fileh->play_duration-10000*fileh->preroll)/10000000.0;
> +      preroll_duration=10000*fileh->preroll;

I just realized this can actually overflow.

> +      asf->movielength=fileh->play_duration>preroll_duration ? (fileh->play_duration-preroll_duration)/10000000.0 : 0.0;

So why not change it to something like
asf->movielength = FFMAX(0.0, (fileh->play_duration / 10000.0 - fileh->preroll) / 1000.0);
? I am not a fan of floating-point, but since the result is floating-point anyway we could simplify
all that overflow handling by switching to it right from the start.