[MPlayer-dev-eng] [PATCH] Add size based support for -endpos in MPlayer

[Reimar Döffinger]

On Sat, Dec 25, 2010 at 04:56:50PM +0100, Clément Bœsch wrote:
> A small patch to add size based support for -endpos in MPlayer. I tested
> it with random videos, audio only files and -loop option and it seems to
> work. Anything else I should test?

-loop 0 both before and after the filename might be a good idea (it
behaves differently, putting it after will use seeking to loop).

> However, I don't understand why mpctx->sh_audio is tested when
> mpctx->sh_video is NULL; is there a way to call the code around
> mplayer.c:L3698 without having neither mpctx->sh_video and
> mpctx->sh_audio?

Well, I guess the reason is that it's hard to know and adding that kind
of stuff later is quite a pain.
Usually I'd expect this kind of case to happen if you have an audio-only
file and then the audio parameters change to something neither the ao
nor automatic conversion can handle.
It should drop out of that code very quickly, but I am not confident it
might not run across this first.

> And then, does this mean mpctx->stream could be NULL?

Unlikely, audio e.g. can also become null any time by audio switching
(audio off is one of the states), we do not really have any such complex
stuff for streams.

> @@ -3711,7 +3706,8 @@ if(!mpctx->sh_video) {
>    if(!quiet)
>      print_status(a_pos, 0, 0);
>  
> -  if(end_at.type == END_AT_TIME && end_at.pos < a_pos)
> +  if(end_at.type == END_AT_TIME && end_at.pos < a_pos ||
> +     end_at.type == END_AT_SIZE && end_at.pos < stream_tell(mpctx->stream))
>      mpctx->eof = PT_NEXT_ENTRY;
>    update_subtitles(NULL, a_pos, mpctx->d_sub, 0);
>    update_osd_msg();
> @@ -3819,10 +3815,9 @@ if(auto_quality>0){
>       if (play_n_frames <= 0) mpctx->eof = PT_NEXT_ENTRY;
>   }
>  
> -
> -// FIXME: add size based support for -endpos
> - if (end_at.type == END_AT_TIME &&
> -         !frame_time_remaining && end_at.pos <= mpctx->sh_video->pts)
> + if (!frame_time_remaining &&
> +     ((end_at.type == END_AT_TIME &&       mpctx->sh_video->pts >= end_at.pos) ||
> +      (end_at.type == END_AT_SIZE && stream_tell(mpctx->stream) >= end_at.pos)))

In principle fine, but maybe it's time to factor out this duplicated
code?
E.g. (not valid C, too lazy to look up the types)
static int is_at_end(&end_at, mpctx) {
    switch (end_at->type)
    {
    case END_AT_TIME:
        return mpctx->sh_video->pts >= end_at->pos;
    ...
    }
    return 0;
}

Particularly when extending this the switch is far more readable than
the if, too.