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

Alexei Svitkine alexei.svitkine
Sun Jul 4 04:22:10 CEST 2010


Updated patch attached per Stefano's feedback.

Also describes -autoexit in ffplay-doc.texi which appears to have
previously been omitted.

-Alexei

On Sat, Jul 3, 2010 at 7:02 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 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.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iD8DBQFML8F6YR7HhwQLD6sRAvgSAJoCUF/Rjcmz6OZ2VL9XiR2B/eHs1wCfTcJ4
> EVWaNV3Xe0fBJPXcKR/9S0Y=
> =jilA
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
-------------- next part --------------
Index: ffplay.c
===================================================================
--- ffplay.c	(revision 24034)
+++ ffplay.c	(working copy)
@@ -260,6 +260,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;
 
@@ -2819,6 +2821,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:
@@ -2887,6 +2893,10 @@
             }
             break;
         case SDL_MOUSEBUTTONDOWN:
+            if (exit_on_mousedown) {
+                do_exit();
+                break;
+            }
         case SDL_MOUSEMOTION:
             if(event.type ==SDL_MOUSEBUTTONDOWN){
                 x= event.button.x;
@@ -3068,6 +3078,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" },
Index: doc/ffplay-doc.texi
===================================================================
--- doc/ffplay-doc.texi	(revision 24034)
+++ doc/ffplay-doc.texi	(working copy)
@@ -93,6 +93,12 @@
 used for debugging purposes.
 @item -threads @var{count}
 Set the thread count.
+ at item -autoexit
+Exit when video is done playing.
+ at item -exitonkeydown
+Exit if any key is pressed.
+ at item -exitonmousedown
+Exit if any mouse button is pressed.
 @item -ast @var{audio_stream_number}
 Select the desired audio stream number, counting from 0. The number
 refers to the list of all the input audio streams. If it is greater



More information about the ffmpeg-devel mailing list