[FFmpeg-devel] [PATCH] Re: new command-line option for ffplay

[Stefano Sabatini]

On date Wednesday 2010-06-23 19:07:12 -0400, Alexei Svitkine encoded:
> Patch is attached. Makes the input function a little cleaner, too.
> 
> -Alexei
> 
> On Wed, Jun 23, 2010 at 9:13 AM, Alexei Svitkine
> <alexei.svitkine at gmail.com> wrote:
> > Hello,
> >
> > I wish to add an extra command-line option to ffplay that will disable
> > the regular input events - but treat any key press or mouse click as
> > the "quit" event.
> >
> > This would be useful for my application that would launch ffplay
> > processes programmatically (cutscenes for a game).
> >
> > I am wondering, would a patch with this change be accepted upstream?

I have no strong opinion on the feature and I'll leave Michael to
decide. One question comes to mind, won't be the possiblity to execute
all the ffplay commands be a bonus rather than an issue in your
particular scenario?

> >
> > -Alexei
> >

> Index: ffplay.c
> ===================================================================
> --- ffplay.c	(revision 23733)
> +++ ffplay.c	(working copy)
> @@ -259,6 +259,8 @@
>  static int error_concealment = 3;
>  static int decoder_reorder_pts= -1;
>  static int autoexit;
> +static int exit_on_keydown;
> +static int exit_on_mousedown;
>  static int loop=1;
>  static int framedrop=1;
>  
> @@ -2804,119 +2806,142 @@
>      }
>  }
>  
> +static void handle_keydown_event(SDL_Event *event)
> +{
> +    double incr, pos;
> +
> +    switch(event->key.keysym.sym) {
> +    case SDLK_ESCAPE:
> +    case SDLK_q:
> +        do_exit();
> +        break;
> +    case SDLK_f:
> +        toggle_full_screen();
> +        break;
> +    case SDLK_p:
> +    case SDLK_SPACE:
> +        toggle_pause();
> +        break;
> +    case SDLK_s: //S: Step to next frame
> +        step_to_next_frame();
> +        break;
> +    case SDLK_a:
> +        if (cur_stream)
> +            stream_cycle_channel(cur_stream, AVMEDIA_TYPE_AUDIO);
> +        break;
> +    case SDLK_v:
> +        if (cur_stream)
> +            stream_cycle_channel(cur_stream, AVMEDIA_TYPE_VIDEO);
> +        break;
> +    case SDLK_t:
> +        if (cur_stream)
> +            stream_cycle_channel(cur_stream, AVMEDIA_TYPE_SUBTITLE);
> +        break;
> +    case SDLK_w:
> +        toggle_audio_display();
> +        break;
> +    case SDLK_LEFT:
> +        incr = -10.0;
> +        goto do_seek;
> +    case SDLK_RIGHT:
> +        incr = 10.0;
> +        goto do_seek;
> +    case SDLK_UP:
> +        incr = 60.0;
> +        goto do_seek;
> +    case SDLK_DOWN:
> +        incr = -60.0;
> +    do_seek:
> +        if (cur_stream) {
> +            if (seek_by_bytes) {
> +                if (cur_stream->video_stream >= 0 && cur_stream->video_current_pos >= 0) {
> +                    pos= cur_stream->video_current_pos;
> +                } else if (cur_stream->audio_stream >= 0 && cur_stream->audio_pkt.pos >= 0) {
> +                    pos= cur_stream->audio_pkt.pos;
> +                } else
> +                    pos = url_ftell(cur_stream->ic->pb);
> +                if (cur_stream->ic->bit_rate)
> +                    incr *= cur_stream->ic->bit_rate / 8.0;
> +                else
> +                    incr *= 180000.0;
> +                pos += incr;
> +                stream_seek(cur_stream, pos, incr, 1);
> +            } else {
> +                pos = get_master_clock(cur_stream);
> +                pos += incr;
> +                stream_seek(cur_stream, (int64_t)(pos * AV_TIME_BASE), (int64_t)(incr * AV_TIME_BASE), 0);
> +            }
> +        }
> +        break;
> +    default:
> +        break;
> +    }
> +}
> +
> +static void handle_mouse_event(SDL_Event *event)
> +{
> +    double x, frac;
> +
> +    if (event->type == SDL_MOUSEBUTTONDOWN) {
> +        x = event->button.x;
> +    } else if (event->type == SDL_MOUSEMOTION && event->motion.state == SDL_PRESSED) {
> +        x = event->motion.x;
> +    } else {
> +        return;
> +    }
> +
> +    if (cur_stream) {
> +        if (seek_by_bytes || cur_stream->ic->duration <= 0) {
> +            uint64_t size=  url_fsize(cur_stream->ic->pb);
> +            stream_seek(cur_stream, size*x/cur_stream->width, 0, 1);
> +        } else {
> +            int64_t ts;
> +            int ns, hh, mm, ss;
> +            int tns, thh, tmm, tss;
> +            tns = cur_stream->ic->duration/1000000LL;
> +            thh = tns/3600;
> +            tmm = (tns%3600)/60;
> +            tss = (tns%60);
> +            frac = x/cur_stream->width;
> +            ns = frac*tns;
> +            hh = ns/3600;
> +            mm = (ns%3600)/60;
> +            ss = (ns%60);
> +            fprintf(stderr, "Seek to %2.0f%% (%2d:%02d:%02d) of total duration (%2d:%02d:%02d)       \n", frac*100,
> +                    hh, mm, ss, thh, tmm, tss);
> +            ts = frac*cur_stream->ic->duration;
> +            if (cur_stream->ic->start_time != AV_NOPTS_VALUE)
> +                ts += cur_stream->ic->start_time;
> +            stream_seek(cur_stream, ts, 0, 0);
> +        }
> +    }
> +}
[...]

All this refactoring (move most part of the event_loop code to a
function) should be a separate patch. Possibly also avoid to re-indent
the code (see the patch submission guidelines), huge patches are less
attractive than small nice patches.

Thanks, regards.
-- 
FFmpeg = Frenzy and Fascinating Magic Practical Efficient Gorilla