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

[Michael Niedermayer]

On Sat, Jul 03, 2010 at 10:40:44PM +0200, Stefano Sabatini wrote:
> 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?

if you consider this usefull iam not against but iam also not sure what this
is good for

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct awnser.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100704/63df2913/attachment.pgp>