epgsearch: Kleine Änderung Löschen aus Timeredit Menü

  • Wenn man das epgsearch eigene TimerEdit Menü verwendet und dort einen Timer löscht, dann crasht der VDR bei mir (verwende nOpacity als Skin).


    Ich habe mir mal die Funktion cMenuMyEditTimer::DeleteTimer in menu_myedittimer.c angeschaut.


    Diese liefert, falls man den Timer löscht, osBack als Rückgabewert. Daraufhin versucht nOpacity das Timer Menü neu zu zeichnen. Weil ein Menüpunkt nun zu einem gelöschten Timer gehört, geht´s schief.


    Ich habe den Rückgabewert an dieser Stelle nun in osTimers geändert und alles geht gut.


    Spricht etwas dagegen, dies im Plugin so zu übernehmen?

  • Hi,


    erstmal Danke für Deine Mühe und den Patch. Wo im Code passiert der Crash genau? Ich konnte das bisher in keinem Skin nachvollziehen, nOpacity hab ich leider nicht zum Test.
    Laut Deinem Bugreport ändert sich durch Deinen Patch ja auch das Verhalten etwas (Rücksprung ins Timer-Menü, auch wenn man aus der Übersicht kommt), weshalb ich das so nicht gern übernehmen würde. Wenn es nur bei nOpacity passiert, sollte man vielleicht auch mal louis dazuholen.


    Gruß,
    winni

  • Moin,


    ich hatte da schon mit Ramirez drüber diskutiert...meiner Ansicht nach liegt das Problem eher darin, dass der Timer wohl nicht sofort, sondern asynchron gelöscht wird (ich habe mir den Code nicht angesehen, aber in dem Thread von damals, als Sören den Patch vorgeschlagen hat, hies es, dass der Timer per svdrp gelöscht wird, da ein Löschen im VDR Hauptthred ein Problem war) und das wohl doch nicht 100% Threadsafe ist. Vielleicht ist das gar nicht mehr so...wollte ich nur mal gesagt haben ;) Bei nOpacity scheint das dann wohl Probleme zu geben, da die Anzeige länger dauert als bei schlanken Skins...


    Ciao Louis

  • Ich hatte fast den kompletten Sonntag das Timer Menü geöffnet, dann das Timer Edit Menü und einen Timer gelöscht. Mit nOpacity komme ich auf eine Absturzquote von 99%. Daher hatte ich es ursprünglich louis als Bug gemeldet.


    Ich konnte den Absturz allerdings auch mit LCARS erzeugen. Habe einfach mal 4-5 Timer angelegt und diese in schneller Weise auch so gelöscht.


    Das Problem ist eben, dass der VDR auf das osBack hin zum aufrufenden Menü zurück springt. Das geht eben überall gut, außer im Timer Menü. Das Löschen passiert, wie louis geschrieben hat, nicht synchron. Diese kleine Verzögerung sorgt eben manchmal dafür, dass der Skin einen Menüpunkt zeichnen will, der einem Timer zugeordnet ist, der inzwischen gelöscht ist. Daher hatte ich den Gedanken, dass man dem VDR doch einfach nur sagen müsste, dass er das Timer Menü neu aufbauen soll.


    Meine Lösung war, wie in dem Bugreport geschrieben, aber etwas kurz gedacht. Als ich dann den VDR wieder produktiv genutzt habe, habe ich mal einen Timer aus dem Programm Menü gelöscht. Dann springt er natürlich auch beim osBack ins Timer Menü, was dann natürlich nicht gewollt ist.


    Was ich eigentlich als Patch dann erstellen wollte, war den Konstruktor von cMenuMyEditTimer um einen Parameter bool isCalledFromTimerMenu zu erweitern, um dann an dieser Stelle eben selektiv osBack bzw. osTimers zurückgeben zu können. Aber wie man dann diesen Parameter füllen kann, da bin ich nun ein wenig überfordert. So tief bin ich leider weder im VDR Code noch im epgsearch Code.

  • Moin!


    Wenn die Timerliste von verschiedenen Threads benutzt wird, dann muss sie gelockt werden. Anders wird's nicht gehen.
    Aber der vdr muss das dann auch beachten, das wird also ohne Patch nicht funktionieren.


    Das Original-Menü löscht das zugehörige OSD-Objekt gleichzeitig mit dem Timer-Objekt, deshalb gibt es da keinen Konflikt, schätze ich.
    Das Problem könnte sein, dass man bei epgsearch außerhalb des Timer-Listenmenüs den Timer löschen kann.


    Ich weiß nicht, wie Klaus sich das mit dem Zugriff auf diese Listen von außen gedacht hat (Timers, Channels usw.). Bei den Events gibt es ja einen Lock, den man erst holen muss, wenn man die auslesen oder bearbeiten will.
    Eventuell muss man sich eine eigene Timerliste anlegen und dann auf das cStatus-Event lauschen, wenn sie verändert wird? Klingt aber auch nicht gut...


    Lars.

  • Vielleicht könnte man Klaus jetzt mal daran erinnern, den Plugins ein Timer-Interface zu gönnen... :)


    Heißt das, dass das Skin die Timer eigentlich auch per SVDRP abholen sollte?
    Alternativ evtl. asynchron im MainThreadHook?


    Lars.


  • Heißt das, dass das Skin die Timer eigentlich auch per SVDRP abholen sollte?
    Alternativ evtl. asynchron im MainThreadHook?


    Hmmm...das wäre sicherlich nicht sehr zuträglich für die Performance...klingt für mich außerdem nach dem falschen Weg, der Verursacher sollte es "richtig" machen.


    Ciao Louis

  • Vielleicht könnte man Klaus jetzt mal daran erinnern, den Plugins ein Timer-Interface zu gönnen... :)


    Heißt das, dass das Skin die Timer eigentlich auch per SVDRP abholen sollte?
    Alternativ evtl. asynchron im MainThreadHook?
    .


    Eine Skin soll anzeigen und sonst nichts!
    Solange sie das im Vordergrund tut ist auch alles OK. Siehe z.B. die Timeranzeige im LCARS Hauptmenü.
    Im Hintergrund an den Timern rumzufummeln ist nicht gut.


    Klaus


  • Eine Skin soll anzeigen und sonst nichts!
    Solange sie das im Vordergrund tut ist auch alles OK. Siehe z.B. die Timeranzeige im LCARS Hauptmenü.
    Im Hintergrund an den Timern rumzufummeln ist nicht gut.


    Klaus


    Nur um Missverständnisse zu vermeiden...ich zeige nur an...rumfummeln tut epgsearch :) Und es ist eben nicht alles ok...mit LCARS knallt es ja laut Ramirez auch.


    Ciao Louis

  • Nur um Missverständnisse zu vermeiden...ich zeige nur an...rumfummeln tut epgsearch :) Und es ist eben nicht alles ok...mit LCARS knallt es ja laut Ramirez auch.


    Dann hatte ich wohl "Heißt das, dass das Skin die Timer eigentlich auch per SVDRP abholen sollte?" mißverstanden.
    Auf jeden Fall ist es falsch, Timer im Hintergrund zu manipulieren oder gar zu löschen. Dann ist halt epgsearch der Schuldige ;-).


    Klaus

  • mini73 : Danke für die Erklärung. Nun verstehe ich langsam die Zusammenhänge und vor allem weiß ich jetzt, warum es beim Löschen (Gelb) aus dem Timer Menü nicht kracht. Hier wird einfach das MenuItem entfernt.


    Zunächst einmal muss ich Klaus und louis zustimmen. Ein Skin sollte sich wirklich nicht um sowas kümmern. Und es crasht auch mit den VDR eigenen Skins. Nur halt nicht so oft, weil die weniger Zeit zum Zeichnen brauchen. Dadurch ergeben sich aber andere lustige Abstürze, wenn man dann etwas mit den gezeichneten Timern machen will.


    Das Problem ist ja, dass im epgsearch Plugin ein Timers.Del gefolgt von einem Timers.SetModified aufgerufen wird und dies keinerlei Auswirkung auf das Timer Menü hat.


    Meiner Meinung nach wäre eine saubere Implementierung, wenn sich weder epgsearch noch der Skin um diese Sache kümmern würden. Eigentlich sollte sich das VDR eigene Timer Menü um die Timer Events kümmern und im Löschen Fall das zugeordnete MenuItem entfernen.


    Und nun schlagt mich ;)

  • Dann frage ich mich, wie es knallen kann, wenn das Edit-Menü von epgsearch im ProcessKey Timers.Del aufruft.
    Wie wird denn zum aufrufenden Menü zurückgegangen? Wird das nicht noch mal neu gezeichnet bzw. mit neuen Daten gefüttert?
    Vermutlich ist es immer noch das selbe Objekt, dass dann selbst seine Anzeigedaten aktualisieren muss?


    Lars.

  • Dann frage ich mich, wie es knallen kann, wenn das Edit-Menü von epgsearch im ProcessKey Timers.Del aufruft.
    Wie wird denn zum aufrufenden Menü zurückgegangen? Wird das nicht noch mal neu gezeichnet bzw. mit neuen Daten gefüttert?
    Vermutlich ist es immer noch das selbe Objekt, dass dann selbst seine Anzeigedaten aktualisieren muss?


    Lars.

    Nein, es knallt ja nicht, wenn Timers.Del aufgerufen wird.


    Mal zur Vollständigkeit, der Code um den es geht:


    Code
    1. LogFile.iSysLog("deleting timer %s", *timer->ToDescr());
    2. Timers.Del(timer);
    3. gl_timerStatusMonitor->SetConflictCheckAdvised();
    4. Timers.SetModified();
    5. return osBack;


    Dieser läuft bis zum Ende durch. Soweit habe ich mich durchgehangelt.


    Der Absturz kommt erst nach dem osBack ausgewertet wird. Denn dann fängt der Skin an das Timer Menü neu zu zeichnen. Hier gibt es jetzt aber ein Item, dass keinem Timer mehr zugeordnet ist => Crash.


    Das Timer Menü des VDR implementiert, wie Du mich richtig hingewiesen hast, eine eigene Löschfunktion, die im Prinzip dasselbe macht und zusätzlich ein

    Code
    1. cOsdMenu::Del(Current());


    aufruft.


    Daraus schließe ich, dass der Code davon ausgeht, dass dies die einzige Möglichkeit ist einen Timer zu löschen.


    Und mit plain VDR stimmt das ja auch. epgsearch führt halt eine zusätzliche Möglichkeit Timer zu löschen, während das Menü im Hintergrund liegt.


    Daher meine Schlußfolgerung, dass der eben erwähnte Code eben bei jeglichem Löschen stattfinden sollte.

  • Jup, ich hatte dich richtig verstanden und du mich auch. :)
    Deshalb ja meine Frage, ob das epgsearch-Menü mit den Timern nicht zerstört wird, wenn das Edit-Menü aufgerufen wird. Denn wenn es neu erstellt werden würde, würde es sich ja eine neue Liste zusammenstellen.


    Ich weiß jetzt nicht genau, wo bei epgsearch wieder in den Code gesprungen wird, wenn man aus dem Edit-Menü zurück kommt. Wenn das Edit-Menü ein entsprechendes Flag setzen würde, wenn ein Timer gelöscht wird, dann könnte die Timer-Liste dort wieder neu eingelesen werden und die OSD-Elemente entsprechend überarbeitet werden.


    Aber ich kenn mich in epgsearch nicht gut genug aus, um da was Qualifiziertes zu sagen zu können.
    Es scheint ja auch an anderen Stellen über die cTimer-Objekte zu iterieren, das sind dann genauso Kandidaten für Crashes, wenn das nicht im Main-Thread passiert.


    Read-/Write-Locks für die Timerliste wären evtl. ein nettes Feature für einen der nächsten Developer-vdr-Versionen.
    Oder man muss im Plugin eben alle Zugriffe aus Threads auf die Timer-Liste asynchron über den MainThreadHook machen. Das ist auch nicht so schwierig...


    Lars.


  • Read-/Write-Locks für die Timerliste wären evtl. ein nettes Feature für einen der nächsten Developer-vdr-Versionen.


    Dazu werde ich mir was einfallen lassen, wenn ich SVDRP für mehrere gleichzeitige Verbindungen mache und Peering zwischen VDRs (zunächst zum Timer-Handling) einbaue.
    Das wird aber noch etwas dauern...


    Klaus

  • Als Ergebnis des Threads sehe ich hier, dass es eine wohl wirklich saubere Lösung in einer VDR vnext irgendwann mal geben wird.


    Nach wie vor sehe ich als die pragmatischte Lösung an, wenn epgsearch in diesem konkreten Fall einfach osTimers zurück gibt. In allen anderen Fällen eben weiterhin osBack. Es gibt nun aber in epgsearch sehr viele Stellen, wo das EditTimerMenü instanziert wird und ich weiß leider nicht, wie man dort jeweils rauskriegen kann in welchem Menü der VDR gerade ist. Denn dann könnte man diese Information einfach an den Konstruktor weiterreichen.


    @winnie: Wäre das überhaupt möglich?

  • Lohnt sich der Mob eigentlich, nur damit man aus der TimerEdit Ansicht einen Timer löschen kann? Ich finde das braucht kein Mensch, ich lösche meine Timer aus der Timerliste, wie es der VDR vorsieht *scnr*


    Ciao Louis

  • Kann man aus der Timerliste von epgsearch denn Timer löschen? War die gelbe Taste da nicht irgendwie anders belegt?
    Ich bin zu selten im OSD, um das zu wissen...


    winni wird sich da bestimmt was sinnvolles einfallen lassen, das hat er bisher immer. :)


    Lars.