[MPlayer-dev-eng] [PATCH] Dohm found some Bug: Playlist wrong

[Arpi]

Hi,

> Btw. Please comment on this patch overall ...
> Do you think its OK ? 

good q.
you know it's freezee now...
you also know that Albeu has been kidnapped by UFOs or sth, at least no one
can reach him.

so i would split your patch to 2 parts:
common changes, to playtree.c
and the gui-only parts.

the later is simple case, if it fixes playback with gui and .so says OK then
it may be commited now.

the playtree.c changes are various things:

+  //DEBUG_FF: Where are the childs freed ?
+  // Attention in using this function!
adding comments shouldn't hurt :)

-  if(pt->params == NULL)
-    printf("Can't realloc params\n");
+  if(pt->params == NULL) {
+      mp_msg(MSGT_PLAYTREE,MSGL_ERR,"Can't allocate %d bytes of memory\n",(n+2)
+      return;
+  }

i don't like these changes, the error msg should be something meaningful...
users usually aren't interested in how many bytes can't bea allocated, but
are interested in the reason, ie which part caused memory problems.

and the last one:
   iter->stack_size--;
-  iter->loop = iter->status_stack[iter->stack_size];
   if(iter->stack_size > 0)
+  {
+    iter->loop = iter->status_stack[iter->stack_size];
     iter->status_stack = (int*)realloc(iter->status_stack,iter->stack_size*sizeof(int));
+  }

i have no idea, but i would be happy if you could prove this is required and
is a bug or something. or if Albeu would say: "ok, commit!".
(ok don't run firing your fake-mail sender app yet :))

what's the value/purpose of iter->loop ?

> Are there parts that need to be rewritten, made different ?

it would be better if the new functions you added to mplayer.c would be in a
new file inside the Gui/ dir. i'm getting ideg by seeing the word 'gtk' in
mplayer.c :)


A'rpi / Astral & ESP-team

--
Developer of MPlayer, the Movie Player for Linux - http://www.MPlayerHQ.hu
    "However, many people beg for its inclusion in Debian. Why?" - Gabucino
  "Because having new software in Debian is good." - Josselin Mouette
"Because having good software in Debian is new." - Gabucino