[Patch] Audioplayer Memory Leak

[jojo]

Ok, nachdem momentan grad Lied Nr 82 aus einer gemischten FLAC/MP3 Liste ohne Jitter und ohne "pthread_create: out of memory" läuft, denke ich daß ich das Problem gefunden habe.

(Sorry, hatte das Diff ohne "ignore-whitespace" generiert, deshalb ein paar Zeilen mehr als tatsächlich notwendig).

Code: Alles auswählen

Index: apps/tuxbox/neutrino/src/gui/audioplayer.cpp
===================================================================
RCS file: /cvs/tuxbox/apps/tuxbox/neutrino/src/gui/audioplayer.cpp,v
retrieving revision 1.61
diff -u -u -r1.61 audioplayer.cpp
--- a/apps/tuxbox/neutrino/src/gui/audioplayer.cpp	24 Feb 2009 19:27:59 -0000	1.61
+++ b/apps/tuxbox/neutrino/src/gui/audioplayer.cpp	6 Mar 2009 14:21:06 -0000
@@ -385,10 +385,13 @@
 			// stop if mode was changed in another thread
 			loop = false;
 		}
-		if ((m_state != CAudioPlayerGui::STOP) && 
-				(CAudioPlayer::getInstance()->getState() == CBaseDec::STOP) && 
+		if ((m_state != CAudioPlayerGui::STOP) &&
+				(CAudioPlayer::getInstance()->getState() == CBaseDec::STOP) &&
 				(!m_playlist.empty()))
 		{
+			// we must stop the current running decoder thread
+			fprintf(stderr,"CAudioPlayerGui:show - end of song detected...\n");
+			CAudioPlayer::getInstance()->stop();
 			if(m_curr_audiofile.FileType != CFile::STREAM_AUDIO)
 				playNext();
 		}

[seife]

Ich habe mir das ganz kurz angeschaut. Sollte man das "stop()" nicht im playNext() oder noch besser im play() machen?
Und wenn ich dann schaue, was im src/driver/audioplay.cpp im dortigen play() passiert, dann steht da als erstes

Code: Alles auswählen

bool CAudioPlayer::play(const CAudiofile* file, const bool highPrio)
{
        if (state != CBaseDec::STOP)
                stop();

...und es wundert mich, warum das dann den vorigen Thread nicht stoppt...

Also nichts gegen deinen Fix, aber ich vermute, er ist nicht ausreichend

[jojo]

Ich habe mir das ganz kurz angeschaut. Sollte man das "stop()" nicht im playNext() oder noch besser im play() machen?
Und wenn ich dann schaue, was im src/driver/audioplay.cpp im dortigen play() passiert, dann steht da als erstes

Code: Alles auswählen

bool CAudioPlayer::play(const CAudiofile* file, const bool highPrio)
{
        if (state != CBaseDec::STOP)
                stop();

...und es wundert mich, warum das dann den vorigen Thread nicht stoppt...

Also nichts gegen deinen Fix, aber ich vermute, er ist nicht ausreichend

Ehrlich gesagt, verstehe ich es auch nicht - zumal mir die fprintfs zeigen, das zwei stop() ausgeführt werden. Deshalb würde mich ja das Ergebnis bei den anderen Anwendern interessieren.

Allerdings läuft die Kiste mit diesem Flickwerk. Ohne sie nach ca 35 MP3 ab und liefert "pthread_create: out of memory". Bei flac hingegen tauchten nach 8-10 Stücken die Jitter und Timeouts auf.

[seife]

Hm. Muss ich mir mal anschauen.

Jedenfalls fixt dein patch es nur dann, wenn die Stücke automatisch weitergeschaltet werden. Wenn man per Fernbedienung ein anderes Stück auswählt, dann leaked es vermutlich immer noch.

[jojo]

Der Audioplayer und die Codecs sind eh' ein heilloses Chaos, was die Behandlung der Stati (STOP_REQ, STOP etc) angeht. Da muß vollständig aufgeräumt werden.

[seife]

Willkommen! Du hast den Job!
;-)

[jojo]

Du bist so großzügig

[Striper]

*push*

Damits nicht vergessen wird ;)

[seife]

Haut euch doch einfach mal selbst valgrind auf die box und geht die warnings durch. Aber Vorsicht: nicht nach dem Frühstück

64MB RAM und eine box mit etwas schnellerem Prozessor sollten es allerdings sein, damit neutrino in unter 15min. startet...

Als Vorlage, falls es jemand ins tuxbox-cdk machen will:

Code: Alles auswählen

$(DEPDIR)/valgrind:
        tar -C $(BUILD_TMP) -xpf $(ARCHIVE)/valgrind-3.3.1.tar.bz2
        pushd $(BUILD_TMP)/valgrind-3.3.1 && \
                export ac_cv_path_GDB=/bin/gdb && \
                export AR=$(TARGET)-ar && \
                $(CONFIGURE) --prefix=/ --enable-only32bit --enable-tls && \
                make all && \
                make install DESTDIR=$(TARGETPREFIX)
        rm -rf $(BUILD_TMP)/valgrind-3.3.1
        touch $@

[jojo]

64MB RAM und eine box mit etwas schnellerem Prozessor sollten es allerdings sein, damit neutrino in unter 15min. startet...

Wenn wir Schinken hätten, hätten wir Rührei mit Schinken, wenn wir Eier hätten...

Leider nur 32MB - und 66MHz...

[rhabarber1848]

Haut euch doch einfach mal selbst valgrind auf die box

Warum nutzt Du nicht valgrind-3.4.1?
Gibt es damit Probleme auf der Dbox2?

EDIT: Habe die Antwort gefunden:
http://valgrind.org/docs/manual/dist.news.html

the next major release, 3.4.0, will drop
support for the old LinuxThreads threading library

Wird gdb für valgrind als zwingende Abhängigkeit benötigt?

[seife]

Warum nutzt Du nicht valgrind-3.4.1?
Gibt es damit Probleme auf der Dbox2?

Weil er mir auf der TD keine Funktionsnamen / Zeilennummern angezeigt hat.

EDIT: Habe die Antwort gefunden:
http://valgrind.org/docs/manual/dist.news.html

the next major release, 3.4.0, will drop
support for the old LinuxThreads threading library

...und jetzt weiss ich auch warum. Auf der dbox mit kernel 2.6 wäre das zwar kein Problem, aber....
...mit 32MB und 66MHz will man kein valgrind benutzen (mit den 250MHz der STB04xx CPU dauert das Starten von neutrino geschätzt eine Minute... Und die 64MB sind dann auch voll)

Wird gdb für valgrind als zwingende Abhängigkeit benötigt?

Nein, ich glaube nicht. valgrind muss nur wissen, wo der gdb zu finden ist, wenn er denn benutzt werden soll (man kann beim Auftreten von Fehlern automatisch den gdb attachen). Aber das will man gleich zweimal nicht, weil dann 64MB definitiv zuwenig sein werden

Ich muss doch mal nen dummy-FB schreiben, damit ich neutrino auf dem PC debuggen kann... weil man Findet wirklich bugs, die seit 2003 unverändert im Code sind, wenn man da mal systematisch rangeht

[rhabarber1848]

Als Vorlage, falls es jemand ins tuxbox-cdk machen will:

Code: Alles auswählen

$(DEPDIR)/valgrind:

Erledigt:
http://article.gmane.org/gmane.comp.vid ... x.scm/1098
http://article.gmane.org/gmane.comp.vid ... x.scm/1099