[MPlayer-dev-eng] [PATCH] -slave-idle option

[D Richard Felker III]

On Sat, Feb 26, 2005 at 04:18:48PM +0100, Alexander Strasser wrote:
> Didn't dive into play tree mistery, but anyway a few comments
> from a quick look.
> 
> Oded Shimon wrote ( On Thu, Feb 24, 2005 at 07:04:50PM +0200 ):
> > Index: cfg-mplayer.h
> > ===================================================================
> > RCS file: /cvsroot/mplayer/main/cfg-mplayer.h,v
> > retrieving revision 1.240
> > diff -u -r1.240 cfg-mplayer.h
> > --- cfg-mplayer.h	30 Jan 2005 10:27:26 -0000	1.240
> > +++ cfg-mplayer.h	24 Feb 2005 16:58:15 -0000
> > @@ -392,6 +388,8 @@
> >  #endif
> >  
> >  	{"slave", &slave_mode, CONF_TYPE_FLAG,CONF_GLOBAL , 0, 1, NULL},
> > +	{"slave-idle", &slave_idle, CONF_TYPE_FLAG,CONF_GLOBAL , 0, 1, NULL},
> > +	{"noslave-idle", &slave_idle, CONF_TYPE_FLAG,CONF_GLOBAL , 0, 0, NULL}, 
> 
> Should all be changed to idle, as mentioned in the other mail.

yes.

> [skip]
> > +while (slave_idle && !filename) {
> > +    play_tree_t * entry = NULL;
> > +    mp_cmd_t * cmd;
> > +    fprintf(stderr,"got in");
> 
> No spam, madam ;)

:)

> > +        
> > +    while (entry) { // I use a while instead of an if so i can break at any point
> > +        if (playtree) play_tree_free_list(playtree->child, 1);
> > +        else playtree=play_tree_new();
> > +        if (!playtree) break;
> > +            
> > +        play_tree_set_child(playtree, entry);
> > +        playtree_iter = play_tree_iter_new(playtree, mconfig);
> > +        if (!playtree_iter) break;
> > +            
> > +        if (play_tree_iter_step(playtree_iter,0,0) != PLAY_TREE_ITER_ENTRY) {
> > +            play_tree_iter_free(playtree_iter);
> > +            playtree_iter = NULL;
> > +            break;
> > +        }
> > +        filename = play_tree_iter_get_file(playtree_iter, 1);
> > +        break;
> > +    }
> > +}
> 
> This looks bit messy. In any case it would be better to add a few comments
> about what happens in there. Maybe substituting the whole thing with a switch
> or so if possible would also be cleaner.

also the while should be removed, since this is not a loop, only a
conditional with breaks to jump out. the c language has "goto" for
this purpose; there's no reason to obfuscate the code by making it
look like a loop.

rich