[MPlayer-dev-eng] [PATCH] Playtree-Highlevel-API & fix gui and playlist fallback & import playtree into gui

Fabian Franz FabianFranz at gmx.de
Sun Feb 2 20:07:17 CET 2003


Hi,

this patch adds a highlevel-api to playtree.c & playtree.h ... (also adds some 
out of memory messages and fixes one 10l)

You may ask yourself: Why ?

Because using playtree.c-low-level code in mplayer is really ugly and does 
overcomplicate things a lot...

I had problems with the code, so I code-audited the whole playtree.c and all 
subroutines are Ok, but not rock solid :-(

But the real problem with playtree is how it is used in mplayer.c...

I spent one whole day, to make the futile attempt to add playtree-code to gui 
... :-((

As playtree is not really nicely used by mplayer.c I failed ...

When Albeu is back, I'll talk to him about how to improve the code ... For 
some independent thing like playtree should everything be capsulated into 
functions, which then are rock-solid ...

Ok, you say, but why now a new API, in "feature-freeze time" ?

First: 

It does not break anything. mplayer.c uses still the low-level-api, but can 
hopefully later be changed to use the new.

Second:

I need it for the import_playtree_into_gui-function as thats written using the 
new API and shows how nicely (imho) playtrees can be handled ...

Of course I could write it with old low-level-api, but why write ugly code, if 
other possibilities are available ?

Also, I would like to maintain this new high-level-api ...

I hope you agree with me on that!

Next thing about this patch is that it FIXES playlist fallback, when using the 
gui, before it did break it to unusability ...

Next and you could argue, that this is a feature, (and as such should not be 
added in feature-freeze-time), it removes the playtree_iter from memory and 
add any files available to gui. 

But I think this also fixes a bug, as :

gmplayer somefile

Did play somefile first and only after that switched to the gui-pl ...

Correct behaviour should be to either append it at end or just overwrite 
gui.pl...

At the moment, its just appending, but I'm thinking about an 
command-line-switch: -enqueue

Easy to implement, btw. I also need a switch (for plugin): -guinopl (which 
will not load and save the playlist; but handle it)

Btw2. I think there should be a switch for gui to not open any 
gtkMessageBoxes!!! (I think it is nerving many users; I also need it for 
mozilla mplayer plugin as I cannot quit mplayer without giving a min of 3 
gtkMessageBoxes, that it was interrupted and so on, which is annoying)

I also had to change the gui-code a bit:

Added gtkInsertPlItem and gtkDelCurrPlItem (which removes current item) ...

After I delete the current gtkPlaylist-item, I have to update filename but 
there was only mplNext and mplPrev and I found it a hack to use:

mplNext(); mplPrev(); 

so I implemented

mplCurr(); ...

Of course I could have sent separate patches, because there are independent 
changes, but on thee other hand the all depend on each other ...

mplayer.c
|_ playtree.c/.h: highlevel-api
|_ Gui/interface.c : gtkInsertPlItem/gtkDelCurrPlItem
   |_ Gui/mplayer/play.c : mplCurr();

So what to do ?

Btw. There is another thing changed in Gui/interface.c that I believe was a 
BUG!

case gtkGetNextPlItem: // get current item from playlist
	if ( plCurrent)
	 {
	  plCurrent=plCurrent->next;
	  if ( !plCurrent && plList ) 
	   {
	    plItem * next = plList;
	    while ( next->next ) { if ( !next->next ) break; next=next->next; }
	    plCurrent=next;
	   }
	  return plCurrent;
	 }
        return NULL;

Do you see what this code does ? 

If there is no next entry the last one is used, as such doing gui-next did 
lead to play the same file over and over again ... (Such playing next, and it 
does not stop, but palys the current file from beginning)

I believe this code was created because:

	if ( plCurrent)
	 {
	  plCurrent=plCurrent->next;
	  return plCurrent;
	 }

Of course did not work ...

But doing things like that is IMHO not the right way (sorry Pontscho ;) )

I changed it to:

	if ( plCurrent && plCurrent->next)
	 {
	  plCurrent=plCurrent->next;
	  return plCurrent;
	 }
        return NULL;

I hope that this is the right fix!

I also did MUCH testing!

Note also that all changes to code already used are in #ifdef HAVE_GUI and 
use_gui or directly in gui-code ...

So it can't break anything in normal player!!!

cu

Fabian

PS: I hope you like the patch and that it'll be accepted, because it were 
two-three whole days of hard work ... :-) [And all started with me trying to 
make the mplayer-plug-in use the gui ... *ARRRRRRRG* ;-) ]
PPS: Pontscho, I can now understand why you don't like the implementation 
playtree-stuff... But I do understand it, and see its problems :-/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-gui-and-pl-on-fly-4.diff
Type: text/x-diff
Size: 13082 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20030202/71c6af8d/attachment.diff>


More information about the MPlayer-dev-eng mailing list