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@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.