[Prototyp] RPI Ausgabeplugin

Spricht eigentlich etwas dagegen den Fix von djagobert ins Git zu übernehmen?

--- vdr-plugin-rpihddevice/audio.c      2014-04-10 13:26:34.171303249 +0200
+++ b/audio.c   2014-04-10 13:26:52.241163627 +0200
@@ -21,9 +21,9 @@ class cAudioParser


 public:


-       cAudioParser()
+       cAudioParser() :
+               m_mutex(new cMutex())
        {
-               m_mutex = new cMutex();
        }


        ~cAudioParser()

Damit läuft es unter Arch Linux ARM wieder

Zitat von seahawk1986

Spricht eigentlich etwas dagegen den Fix von djagobert ins Git zu übernehmen?

Diff

--- vdr-plugin-rpihddevice/audio.c      2014-04-10 13:26:34.171303249 +0200
+++ b/audio.c   2014-04-10 13:26:52.241163627 +0200
@@ -21,9 +21,9 @@ class cAudioParser


 public:


-       cAudioParser()
+       cAudioParser() :
+               m_mutex(new cMutex())
        {
-               m_mutex = new cMutex();
        }


        ~cAudioParser()

Alles anzeigen

Damit läuft es unter Arch Linux ARM wieder

Ich kann mir eigentlich nur vorstellen, daß es sich dabei um einen Compiler-Fehler handeln muß.
Die beiden Varianten sollten sich eigentlich vollkommen gleich verhalten, mit dem winzigen Unterschied, daß bei 'm_mutex = new cMutex()' die Variable u.U. zuerst auf NULL initialiisiert wird, was aber an der Funktionalität nichts ändern sollte. Ich würde es daher auch so handhaben wie Thomas und erstmal nach einer schlüssigen Erklärung suchen...

Klaus

Zitat von reufer

als dass in gcc, libc, etc. noch ein Fehler steckt

Die sind nicht auszuschließen, gerade bei Optimierungen.

Ich hatte das mal, dass eine "inline"-int-Berechnung einen anderen Wert ergeben hat, als wenn ich den Wert vorher berechnet habe und in einer Debug-Meldung ausgegeben habe. Im ersten Fall war das Ergebnis schlicht falsch, im zweiten Fall richtig - und der Code war theoretisch der gleiche, nur wurde das Zwischenergebnis benutzt statt wegoptimiert.

Zur Mutex: In der Theorie sind die beiden Varianten vollkommen gleich, aber es kann sein, dass der Compiler etwas anderes daraus macht. Da hilft nur Dissamblieren der beiden Varianten, anders kommt man dem nicht auf die Schliche.

Lars.

Hi,

ich hab mal nen Freund, der weiß was er tut und sich mit C++ auskennt, auf dieses Problem angesprochen.
Der Unterschied zwischen den Beiden Varianten ist nicht, ob m_mutex mit NULL initialisiert wird oder nicht, sondern die Reihenfolge in der der Speicher für die Objekt Variablen alloziert und die Objekte initialisiert werden. Mit dem original Code werden zuerst die Variablen und Objekte m_packet, m_codec, m_channels, m_samplingRate, m_size und m_ptsQueue Initialisiert und danach der m_mutex, und bei der gepatchten Variante wird zuerst m_mutex alloziert und initialisiert.
Das macht in dem Fall einen Unterschied, wenn irgendwo auf eine falsche Adresse außerhalb des reservierten Speicherbereiches zugegriffen wird. Im vorliegenden Fall mit dem Mutex sollte die Objekt Initialisierungs Reihenfolge meiner Meinung nach nicht relevant sein.
Dabei ist meinem Freund ein Schönheitsfehler aufgefallen, der das lesen und erkennen von solchen Fehlern ein wenig schwieriger macht. Warum ist der m_mutex als einzige Objektvariable ein Zeiger und wird mit new initialisiert?

Auf Grund der obigen Erkenntnis haben wir uns dann mal gemeinsam die Speicherzugriffe in der audio.c angeschaut und konnten auch einen Fehler finden. Ob der nun für das Problem verantwortlich ist, bzw. sein kann, kann ich nicht beurteilen. In der Funktion MpegCheck wird auf p[3] zugegriffen. Es wird aber zuvor nur sicher gestellt das maximal p[2] verfügbar ist. Mit anderen Worten, die Abfrage if (size < 3) müsste in ein if (size < 4) geändert werden, sofern der später folgende Zugriff auf p[3] korrekt ist.

Ich hoffe ich konnte hiermit ein wenig helfen.

Claus

PS.: In meiner langjährigen ehemals aktiven C++ Programmierzeit kam es immer wieder vor, dass ein Compiler Fehler vermutet wurde. Das hatte sich ich jedoch in 99.9% der Fälle als falsch erwiesen hat. Lediglich ein einziges mal konnte mein Freund diese Vermutung bestätigen, nachdem er sich den erzeugten Assembler Code ausgiebig zu Gemüte geführt hatte.

Moin!

Danke für den Hinweis.

Und wie du sagst, sollte die Reihenfolge der Initialisierung keinen Einfluss haben, es passiert ja alles während des Konstruktors und auch im gleichen Thread. Ich vermute auch, dass da irgendwo anders der Hase im Pfeffer liegt.

Lars.

Nicht ganz. Oder schon, aber das ändern der Initialisierungsreihenfolge macht in diesem Fall halt möglicherweise nen Speicherzugriffsfehler sichtbar...

Claus

Hi,
soooo ich habe jetzt herausgefunden, dass wieso auch immer der Constructor nicht ausgeführt wird, wenn innerhalb des Rumpfes etwas ausgeführt wird.
Wenn der Constructor leer ist (aber was in der init-liste steht), dann wird er ausgeführt.
Ich habe langsam auch die Vermutung, dass es sich um einen Compiler Fehler handelt...

Abgesehen davon finde ich es "moderner", wenn man die member Variablen in der initializer list initialisiert und wenn man die Klassendefinition etc in die Header-Datei schreibt
Wenn es gewünscht ist, kann ich da gerne einen Patch machen.

Ach ja, macht es nicht Probleme, wenn man zum Beispiel in der Shrink(..) den Mutex setzt und dort dann die Reset() Funktion aufruft, die selber auch nochmal den Mutex verwenden möchte? oder ist das innerhalb eines threads kein Problem?

Greets
Patrick

Zitat von djdagobert

Hi,
soooo ich habe jetzt herausgefunden, dass wieso auch immer der Constructor nicht ausgeführt wird, wenn innerhalb des Rumpfes etwas ausgeführt wird.
Wenn der Constructor leer ist (aber was in der init-liste steht), dann wird er ausgeführt.
Ich habe langsam auch die Vermutung, dass es sich um einen Compiler Fehler handelt...

Abgesehen davon finde ich es "moderner", wenn man die member Variablen in der initializer list initialisiert und wenn man die Klassendefinition etc in die Header-Datei schreibt
Wenn es gewünscht ist, kann ich da gerne einen Patch machen.

Ach ja, macht es nicht Probleme, wenn man zum Beispiel in der Shrink(..) den Mutex setzt und dort dann die Reset() Funktion aufruft, die selber auch nochmal den Mutex verwenden möchte? oder ist das innerhalb eines threads kein Problem?

Greets
Patrick

Alles anzeigen

Hallo Patrick,

ich kann es fast nicht glauben, dass der Konstruktor nicht durchlaufen wird, rein aus Interesse, wie hast du dies getestet?
Falls dieser nicht durchlaufen würde, dann müsste es ja sofort knallen, wenn auf diese Member zugegriffen würde.

Persönlich initialisiere ich meine Members immer im Stiel
cAudioParser() :
m_mutex(new cMutex())

In dem Fall hätte ich auf den Pointer ganz verzichtet und den Mutex im Header einfach so angelegt:
cMutex m_mutex;

wobei ich sagen muss, meine Programmierkenntnisse liegen ausschließlich auf Windows und nicht unter Linux.

Dann noch ein Frage zur Aussage von Klaus

Zitat

Die beiden Varianten sollten sich eigentlich vollkommen gleich
verhalten, mit dem winzigen Unterschied, daß bei 'm_mutex = new
cMutex()' die Variable u.U. zuerst auf NULL initialiisiert wird

ist dies unter Linux wirklich so, dass ein Pointer einer Klassenmember zunächst auf NULL durch den Compiler gesetzt wird.
Ich kenne dies eher umkehrt, dass man diesen Pointer explizit auf NULL setzen muss. Dann gibt es unter Windows noch
den Unterschied zwischen einer Debug und Release Version, in der Debug Version werden Variablen auf 0 gesetzt und
in der Release Version nicht mehr, dann laufen plötzlich Release Versionen anders als erwartet.
Mutex Aufrufe im selben Threadkontext können mehrfach ausgeführt werden, die verklemmen sich nicht,
wobei auf ein Lock auch ein Unlock ausgeführt werden muss, also paarweise Aufgerufen werden muss.

Gruß
Guido

Ich hab zwar keine Ahnung vom Programmieren, aber konnte zumindest die Ursache heute zufällig reproduzieren.
Der seg fault scheint an ffmpeg zu liegen - habe beide meine RPI's auf gcc-4.8 upgedated und den git Stand neu kompiliert.
Der eine ist auf wheezy default - libavformat.so.53 - keinerlei Probleme.
Auf dem anderen kam es zum seg fault - der ist auf ffmpeg-1.0.8 - Abhilfe schaffte Djagobert's fix.

Gruss

Mal ne kurze Zwischenfrage:
Bringt denn gcc-4.8 irgendwelche Vorteile weshalb man Updaten sollte?

Ich kann mich erinnern, daß es (vor Jahren) manchmal Probleme mit dem generierten Code gab, wenn eine höhere Optimierungsstufe als O2 verwendet wurde. Evtl. einfach mal -O2 statt -O3 versuchen.

CU
Oliver

Hi Guido,

Zitat von Guido17

ich kann es fast nicht glauben, dass der Konstruktor nicht durchlaufen wird, rein aus Interesse, wie hast du dies getestet?

Ich habe eine neue Member-Variable (int) angelegt und im Constructor diese auf den Wert 3 gesetzt. Später in der Init Funktion habe ich dann die Variable ausgegeben und diese hatte dann einen großen negativen Wert anstatt 3. Je nachdem was und wieviel ich im Constructor schreibe, wird er manchmal ausgeführt und manchmal wieder nicht!?!?!? Und jetzt wo ich das ganze nochmal teste, geht auch der ursprüngliche code..... Ich versteh es einfach nicht mehr...

Also für mich bestätigt das alles nur meine Anfangs Vermutung. Irgendwo wird auf den falschen Speicherbereich zugegriffen. Die zusätzliche member Variable wird immer im Konstruktor gesetzt, nur manchmal wird die später durch nen fehlerhaften Speicherzugriff überschrieben. Das Ergebnis ist dann je nach Optimierung und zusätzlichen Aktionen immer unterschiedlich.

Claus

Zitat von clausmuus

Also für mich bestätigt das alles nur meine Anfangs Vermutung. Irgendwo wird auf den falschen Speicherbereich zugegriffen. Die zusätzliche member Variable wird immer im Konstruktor gesetzt, nur manchmal wird die später durch nen fehlerhaften Speicherzugriff überschrieben. Das Ergebnis ist dann je nach Optimierung und zusätzlichen Aktionen immer unterschiedlich.

Claus

Das was Claus schreibt, ist für mich auch die einzige Erklärung, da wird etwas überschrieben, am Konstruktor kann es nicht liegen, sonst könnte man dies einfach nachvollziehen.
Wenn dies so ist, dann ist dies ein "Sche..."-Fehler und fast nur mit Hilfswerkzeugen zu finden, die den Speicher auf überschreiben überwachen. Eigentlich müsste dies zwischen dem
Erzeugen des Objektes und der Init-Funktion liegen bzw. an der Datenstruktur die vor dem Objekt erzeugt wird und Speicher dynamisch erzeugt. Schau mir den Sourcecode darauf noch einmal an.

Gruß
Guido

Zitat von Guido17

sonst könnte man dies einfach nachvollziehen

Wenn da Optimierungen des Compilers dazwischen funken, ist das nicht so einfach nachzuvollziehen, weil unterschiedlicher Code unterschiedliche Optimierungen auslösen können.

Aber wahrscheinlicher kann es schon sein, dass da irgendwo was überschrieben wird.

Lars.

Hallo Thomas,

Reden wir hier eigentlich von 2.1.6 oder 2.0.6? Ich verwende die Developer Version (2.1.6). Gibt es da relevante Unterschiede?

Kann ich irgendwie weiterhelfen um das Problem zu Debuggen?

Gruß, ollo

Hi,
sooo ich glaub ich hab jetzt den Grund gefunden...
Ich habe mich gerade durch den Code mit dbg "gedebugt" und bin dabei auf das das Verhalten gestoßen, dass er den Konstruktor der "falschen" cAudioParser-Klasse aus der remux.c nimmt und ausführt.

#0  cFrameParser::cFrameParser (this=this@entry=0x74cde0) at remux.c:1055
#1  0x000d9c68 in cAudioParser::cAudioParser (this=0x74cde0) at remux.c:1069
#2  0xb68cf794 in cAudioDecoder::cAudioDecoder (this=0x4e2b18, omx=0x4e2990) at audio.c:680
#3  0xb68d0c6c in cOmxDevice::cOmxDevice (this=0x4e0030, onPrimaryDevice=<optimized out>) at omxdevice.c:42
#4  0xb68c7e64 in cPluginRpiHdDevice::Initialize (this=0x1ad488) at rpihddevice.c:62
#5  0x000ca3f4 in cPluginManager::InitializePlugins (this=this@entry=0x1) at plugin.c:363
#6  0x00063900 in main (argc=12, argv=0xbefff764) at vdr.c:730

das liegt daran, dass beide Klassen gleich heißen. Keine Ahnung, ob der Compiler das richtig macht, oder da ein Bug hat?
Vielleicht hilft es ja die cAudioParser Klasse hier in der audio.c umzubenennen.

Soweit meine momentanen Erkenntnisse. Vielleicht helfen diese ja weiter!
Greets
Patrick