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

[Oded Shimon]

On Saturday 26 February 2005 17:29, D Richard Felker III wrote:
> On Sat, Feb 26, 2005 at 04:18:48PM +0100, Alexander Strasser wrote:
> > Should all be changed to idle, as mentioned in the other mail.
>
> yes.

Done.
Still used 'slave_idle' for the internal variable, because a variable of 
"idle" just seems too short...

> > > +    fprintf(stderr,"got in");
> >
> > No spam, madam ;)
> >
> :)

oops... I completely forgot to clean up the patch. I also used a 'sleep();' 
which I meant to clean up later to a usec_sleep or something better.
Currently the get cmd loop has a 30ms delay. Anyone feels it should be a 
different delay?...

> > 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.

I don't see how a switch helps here.. It is all safe, as entry starts out with 
NULL, and is only changed if a proper command is given. Added quite a few 
comments, as the whole playtree thing is admittedly very confusing.

> 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.

Changed to an 'if', only instead of 'goto', I realized "continue;" is 
appropriate here, as the bigger loop simply waits for the next command each 
time. a goto would simply goto right before the end of the loop.

On Thursday 24 February 2005 19:44, Attila Kinali wrote:
> On Thu, 24 Feb 2005 19:04:50 +0200 Oded Shimon wrote:
> > -if(use_gui || playtree_iter != NULL){
> > -
> > +if(use_gui || playtree_iter != NULL || slave_idle){
> > +  if (!playtree_iter) filename = NULL;
>
> I'm quite sure that this causes a memleak, but
> i cannot say whether it's safe to free filename.
> I dont understand playtree enough.
> But actualy i'd say that play_tree_iter_get_file()
> should strdup the filename it gives back so
> the caller could free it.

I've looked into it thoroughly now, there is no memleak.
playtree does NOT strdup the filename in that function, it strdups it when 
ADDING a file.
Even though playtree_iter is NULL, filename is still pointing at legitimate 
unfreed data - The 2 are not in sync. filename is pointing at the previously 
played item, which is STILL in the "main" playtree! This is later freed by 
the slave_idle code  in the begginning.

I think I'm starting to understand playtree more and more. Don't ask me to 
document it. :)

Compiled and tested some more, works beautifully as far as I can see.

- ods15
-------------- next part --------------
A non-text attachment was scrubbed...
Name: slave-idle.patch
Type: text/x-diff
Size: 4569 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20050227/3b2f000a/attachment.patch>