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

[Stefano Sabatini]

On date Thursday 2010-06-24 23:01:43 -0400, Alexei Svitkine encoded:
> > 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?
> 
> Not really. I want any input to skip the cutscene - i.e. so players
> can hit space (the biggest key on the keyboard, so easiest to hit) to
> skip it - by default anyway. We'll probably allow the users to tweak
> the ffplay invocation command via an .ini file, if they wish, though.
>
> > 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.
> 
> Hmm, I would have thought the refactoring would be welcome - since it
> makes the code cleaner (the current input loop code is too giant of a
> function, imho). Seems the current policy would lead towards bad code
> lingering, since there will be less incentive to clean up code if it
> has to be done separately.
> 
> For instance, I don't really care so much about this specific code
> being cleaner - but I thought I'd do a favor to fix it up while I'm
> here. Clearly this is not welcome (in the way I did it), so I'd
> probably settle for just the simple patch that keeps the function huge
> as it is now, and not bother with the cleanup patch after.
> 
> So then any cleanup would be left to the actual maintainers - which I
> suppose gives you control on what clean up takes place, but seems like
> a result of more work for the maintainers.
> 
> Sorry for the rant. Please don't consider it when deciding the
> usefulness of this patch. Thanks.
> 
> Updated patch is attached.
> 
> -Alexei

> Index: ffplay.c
> ===================================================================
> --- ffplay.c	(revision 23764)
> +++ 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;
>  
> @@ -2815,6 +2817,10 @@
>          SDL_WaitEvent(&event);
>          switch(event.type) {
>          case SDL_KEYDOWN:
> +            if (exit_on_keydown) {
> +                do_exit();
> +                break;
> +            }
>              switch(event.key.keysym.sym) {
>              case SDLK_ESCAPE:
>              case SDLK_q:
> @@ -2883,6 +2889,10 @@
>              }
>              break;
>          case SDL_MOUSEBUTTONDOWN:
> +            if (exit_on_mousedown) {
> +                do_exit();
> +                break;
> +            }
>          case SDL_MOUSEMOTION:
>              if(event.type ==SDL_MOUSEBUTTONDOWN){
>                  x= event.button.x;
> @@ -3064,6 +3074,8 @@
>      { "sync", HAS_ARG | OPT_FUNC2 | OPT_EXPERT, {(void*)opt_sync}, "set audio-video sync. type (type=audio/video/ext)", "type" },
>      { "threads", HAS_ARG | OPT_FUNC2 | OPT_EXPERT, {(void*)opt_thread_count}, "thread count", "count" },
>      { "autoexit", OPT_BOOL | OPT_EXPERT, {(void*)&autoexit}, "exit at the end", "" },
> +    { "exitonkeydown", OPT_BOOL | OPT_EXPERT, {(void*)&exit_on_keydown}, "exit on key down", "" },
> +    { "exitonmousedown", OPT_BOOL | OPT_EXPERT, {(void*)&exit_on_mousedown}, "exit on mouse down", "" },
>      { "loop", OPT_INT | HAS_ARG | OPT_EXPERT, {(void*)&loop}, "set number of times the playback shall be looped", "loop count" },
>      { "framedrop", OPT_BOOL | OPT_EXPERT, {(void*)&framedrop}, "drop frames when cpu is too slow", "" },
>      { "window_title", OPT_STRING | HAS_ARG, {(void*)&window_title}, "set window title", "window title" },

Missing ffplay-doc.texi docs.

Apart that the change is possibly useful in such scenarios and it has
small impact on the code, so I'm in favor of this patch.

Michael?

Regards.
-- 
FFmpeg = Fiendish and Frightening Mere Philosofic Enhancing Gnome