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

[Clément Bœsch]

On Sat, Jan 08, 2011 at 05:37:39PM +0100, Reimar Döffinger wrote:
> On 8 Jan 2011, at 10:20, Clément Bœsch <ubitux at gmail.com> wrote:
> > On Wed, Jan 05, 2011 at 04:22:12PM +0100, Reimar Döffinger wrote:
> >>> @@ -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;
> >> }
> >> 
> > 
> > Sure. Since I don't want to do that in the first patch, here is a second
> > one. But there is a few things I think may be improved: doesn't end_at and
> > frame_time_remaining belong to the MPContext? Putting end_at in it will
> > likely clarify the code, while for the other one it could avoid an
> > unnecessary check before calling is_at_end (it could get it from the
> > mpctx, just like it would take end_at).
> 
> Doesn't seem like much of an improvement but I don't mind.
> However for this second patch I'd prefer if you applied merging the two
> if(!quiet) separately, also because the patch you have attached ends up
> with the comment at a wrong and confusing place.
> 

Ok, I'll send a new patch. Btw, what is the "handle this better" refering
to?

> > By the way, I will apply the first patch (not the one attached) adding the
> > mplayer -endpos bytes support in the next days.
> 
> No need to wait from my side.

Applied.

-- 
Clément B.