[SkinNopacity] Aktuelle Probleme

  • Es scheint mir, in diesem Commit werden mehrere verschiedene Sachen geaendert, die auch so nicht in der Beschreibung erklaert werden.

    Ja, mit den Beschreibungen tue ich mich recht schwer. Das ist für mich Neuland. Prinzipiell finde ich das gut, und bemühe mich auch, damit besser zu werden.


    OK, ich werde mal versuchen, ein wenig Licht ins Dunkel zu bringen, warum ich diese beiden Commits in der ein oder anderen Form für notwendig halte.


    Die Ausgangssituation war, das das erste DisplayChannel beim Start des VDR als Workaround für CreateCacheDelayed() unterdrückt wurde. Das sollte mit dem Commit 990d88480c9392da73438d44d1a82b1290e307ef behoben werden. Im Nachgang hat sich dann herausgestellt, das das nicht immer, und zwar in Abhängigkeit vom Theme, funktioniert hat.


    Aus meiner Sicht gehört das CreateCacheDelayed() in nopacity.c. Nach weiteren Analysen hat sich dann gezeigt, das man das aber nicht einfach im Constructor von cNopacity unterbringen kann, und das war sicher auch der Grund dafür das CreateCacheDelayed() vom CreateCache() zu trennen, weil der Constructor zu zeitig beim Start vom VDR aufgerufen wird. Zu diesem Zeitpunkt sind die Daten, die dafür gebraucht werden, das Theme, noch nicht bekannt. Auch kann man das nicht z.B. in cSkinDisplayChannel *cNopacity::DisplayChannel unterbringen, das funktioniert auch nur abhängig vom Theme.


    Wo also das CreateCacheDelayed() unterbringen? Da erschien nun die Funktion cNopacity::ReloadCaches() prädestiniert dafür, da die ja sowieso nach Ändern der Geometrie oder des Theme den Cache neu anlegt. Der Ausrutscher mit dem Show() war hier leider ein Denkfehler.


    Das Problem ist nun aber, das cNopacity::ReloadCaches(void) beim ersten Start des Skin gar nicht aufgerufen wird, weil im geometrymanager die Koordinaten bereits im Constructor gesetzt werden und damit die Funktion GeometryChanged(void) false zurückliefert. Die Änderung im geometrymanager passt das so an, das im Constructor jetzt die Werte mit 0 initialisiert werden, und damit jetzt auch beim ersten Start cNopacity::ReloadCaches(void) durchlaufen wird und den ImageCache korrekt initialisiert. Der Commit "Eliminate cGeometryManager::SetOSDSize" für sich alleine, macht natürlich wenig Sinn und gehört in den letzten Commit mit rein.


    CreateCacheDelayed() vom CreateCache() sind somit in cNopacity überflüssig, weil die dann über imgCache->Reload() aufgerufen werden. Dann habe ich noch den Rest aus dem Constructor von cNopacity in cNopacity::ReloadCaches(void) untergebracht, weil das sowieso dort schon vorhanden war oder da mit hin passt.


    Die Änderung im fontmanager war dann nötig, weil beim ersten Aufruf von cNopacity::ReloadCaches(void) die fonts noch nicht definiert waren und es beim fontManager->DeleteFonts() zu einem segfault kam. Dazu habe ich in fontmanager.h die ganzen Fonts mit NULL initilisiert, weil ich ursprünglich die einzelnen Fonts in cFontManager::DeleteFonts* in if () einbauen wollte, habe mich dann aber entschieden hier das DELETENULL() zu verwenden.


    Diese ganzen Änderungen führen im Endeffekt dazu, dass über cNopacity::ReloadCaches(void) nur beim ersten Aufrufen des Skins oder bei Änderungen der Geometrie bzw. des Theme die Caches korrekt initialisiert werden.


    Im Nachgang schwebten mir dann noch ein paar weitere Anpassungen vor:

    - Zusammenfassen von CreateCacheDelayed() und CreateCache(), hier ist ein separates Handling nun nicht mehr nötig.

    - Zusammenfassen von cGeometryManager::SetGeometry() und den ganzen SetDisplay*, weil die nur dort verwendet werden.

    - Zusammenfassen von DeleteFonts() und SetFonts(), das habe ich mir aber noch nicht angesehen.


    Ich schaue mal, wie ich diese beiden Commits noch etwas Kleinteiliger anlegen kann.


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Wenn Du den Commit aufteilst (die NULL-Geschichten, Fix der Debug-Logs, was da jetzt wieso anders initialisiert wird) und das jeweils erklaerst, dann kann man vielleicht auch nachvollziehen, was da schief geht.

    Ich habe mal die beiden commits neu aufgeteilt und hoffentlich besser beschrieben.

    Aber erst mit dem letzten Commit funktioniert das mit den Grafiken im Menü richtig, zumindest bei mir.


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Ich habe mal die beiden commits neu aufgeteilt und hoffentlich besser beschrieben.

    Sehr schoen! Macht die ganze Sache viel uebersichtlicher.



    Aber erst mit dem letzten Commit funktioniert das mit den Grafiken im Menü richtig

    Bei mir immer noch der selbe Fehler. Aber jetzt ist der commit uebersichtlich genug, dass man etwas sieht.

    Ich denke, dass Problem ist, dass jetzt SetGeometry() vor config.LoadThemeSpecificConfigs(); config.SetThemeSpecificDefaults(); kommt, und dadurch nicht die richtigen Settings aktiv sind.


    Zusammenfassen von DeleteFonts() und SetFonts(), das habe ich mir aber noch nicht angesehen.

    Der ganze fontManager mit dem neuen Commit ist etwas unuebersichtlich. Statt SetFonts() und DeleteFonts() koennte man das doch in Konstruktor und Destruktor packen, und den ganzen fontMager erst anlegen, wenn die Settings klar sind, bzw. bei Aenderungen den ganzen fontManager loeschen und neu anlegen. Macht den Code bestimmt uebersichtlicher. Geht aber erstmal auch so.


    Gruss,

    S:oren

  • Ich denke, dass Problem ist, dass jetzt SetGeometry() vor config.LoadThemeSpecificConfigs(); config.SetThemeSpecificDefaults(); kommt, und dadurch nicht die richtigen Settings aktiv sind.

    Das schaue ich mir an, ob da irgendwas fehlt.

    Verwunderlich ist nur, das es bei mir scheinbar geht. Mhh...

    Statt SetFonts() und DeleteFonts() koennte man das doch in Konstruktor und Destruktor packen, und den ganzen fontMager erst anlegen, wenn die Settings klar sind, bzw. bei Aenderungen den ganzen fontManager loeschen und neu anlegen. Macht den Code bestimmt uebersichtlicher.

    Gute Idee, muss ich mir mal Gedanken darüber machen.


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Neuer Versuch:

    Ich denke, dass Problem ist, dass jetzt SetGeometry() vor config.LoadThemeSpecificConfigs(); config.SetThemeSpecificDefaults(); kommt, und dadurch nicht die richtigen Settings aktiv sind.

    Ich würde diese ganzen config.* gerne in cNopacity::ReloadCaches(void) lassen, damit man die später noch zusammenfassen kann, z.B. durch ein config.Init(). Deshalb habe ich jetzt 2 von den letzten 3 commits nochmal etwas umgestellt und damit das geoManager->SetGeometry() in Nopacity::ReloadCaches(void) erhalten. Damit sollte die Reihenfolge wieder korrekt sein.


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Die letzten Commits sind jetzt in den Branch light_version gemerged.

    Der ganze fontManager mit dem neuen Commit ist etwas unuebersichtlich. Statt SetFonts() und DeleteFonts() koennte man das doch in Konstruktor und Destruktor packen,

    Dieses, und noch ein paar weitere sich anbietende Sachen, habe ich im Branch Devel-Light neu angelegt.


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Die letzten Commits sind jetzt in den Branch light_version gemerged.

    OK, danke.


    Dieses, und noch ein paar weitere sich anbietende Sachen, habe ich im Branch Devel-Light neu angelegt.

    Von C++ habe ich eigentlich nicht wirklich viel Ahnung, aber einige Sachen gefallen mir nicht so recht.


    Zunaechst meinte ich, den alten Commit zum Fontmanager anzupassen, und nicht zwei neue dazuzuerfinden. Das haette ich in einem Patch uebersichtlicher gefunden. Oder zumindest in aufeinanderfolgnden Patches. Aber OK, wenn es am Ende einfacher wird, bringt es etwas.


    Code
    1. + if (fontManager)
    2. + delete fontManager;

    Eher unguenstig. Das delete prueft selbst auf nullptr. Sowas ist auch an anderen Stellen noch drin.


    DELETENULL() finde ich generell sehr sinnvoll. Aber in einem Destruktor ist es nun wirklich vergebene Liebesmueh. Warum am Ende die Attribute noch schnell auf NULL setzen, wenn das ganze Objekt sowieso gerade zerstoert wird?

    Auch die andere Aenderung in dem ersten Fontmanager-Patch (in der alten Serie), die Null-Initialisierung im Header, ist unnoetig, wenn man sowieso im Konstruktor die ganzen Werte initialisiert. Das Nullen ist nun ueberfluessigerweise immer noch drin.

    Die ganze Idee, den fontManager zu loeschen und neu anzulegen, bringt ja nur etwas, weil man diese Zusatzinitialisierungen weglassen kann. Ansonsten ist es ja nur Overhead, wenn man den Speicher fuer das Objekt selber freigibt und gleich wieder alloziert.


    Weil wir gerade beim fontManager sind, hier sind nur die meisten Fonts drin, aber in displaymenuview.c, menudetailview.c und displaychannelview.c werden doch noch ein paar zusaetzlich lokal benutzt. Vermutlich noch ein Ziel fuer Aufraeumarbeiten. Solche Inkonsistenzen gibt es auch an anderen Stellen noch, so werden channellogos beim channelview aus dem imagecache genommen, aber beim Programm-Menue-Header nicht...


    Der letzte Patch zu cNopacityConfig ist auch wieder ein Vorbereitungspatch, um die ganzen Sachen dann direkt in Init zusammenzufassen?


    Ansonsten habe ich die neue Serie bei mir auch getestet, laeuft noch alles einwandfrei.


    Gruss,

    S:oren

  • Das delete prueft selbst auf nullptr.

    Das dachte ich auch. Ich hatte allerdings auch schon Fälle, wo es beim delete einen segfault gab und dann nur die Prüfung half.

    Da müsste ich wohl doch noch mal solche Stellen tiefgründiger untersuchen.

    Habe ich in den aktuellen commits geändert.

    Auch die andere Aenderung in dem ersten Fontmanager-Patch (in der alten Serie), die Null-Initialisierung im Header, ist unnoetig, wenn man sowieso im Konstruktor die ganzen Werte initialisiert. Das Nullen ist nun ueberfluessigerweise immer noch drin.

    Die ganze Idee, den fontManager zu loeschen und neu anzulegen, bringt ja nur etwas, weil man diese Zusatzinitialisierungen weglassen kann. Ansonsten ist es ja nur Overhead, wenn man den Speicher fuer das Objekt selber freigibt und gleich wieder alloziert.

    Den ersten Patch habe ich so gelassen, die neuen aber noch zusammengeführt und angepasst.

    Weil wir gerade beim fontManager sind, hier sind nur die meisten Fonts drin, aber in displaymenuview.c, menudetailview.c und displaychannelview.c werden doch noch ein paar zusaetzlich lokal benutzt. Vermutlich noch ein Ziel fuer Aufraeumarbeiten.

    Das hatte ich auch gesehen. Hier ist allerdings zumindest bei 2 von den 3 Dateien das Problem, das die neuen Fonts von lokalen Variablen abhängen.

    Da müsste man mal schauen, ob das zu lösen ist, deshalb erst einmal vertagt.

    Der letzte Patch zu cNopacityConfig ist auch wieder ein Vorbereitungspatch, um die ganzen Sachen dann direkt in Init zusammenzufassen?

    Das ist momentan nur dazu gedacht, cNopacity::ReloadCaches() etwas übersichtlicher zu gestalten. In einem weiteren Schritt müssen sicher auch cNopacityConfig und NopacitySetup nochmal überarbeitet werden, die liegen auch in einer VDR-unüblichen Form vor. Da das aber eine größere Baustelle ist, habe ich es vorerst auf später verschoben.


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Das dachte ich auch. Ich hatte allerdings auch schon Fälle, wo es beim delete einen segfault gab und dann nur die Prüfung half.

    Das hängt wohl davon ab, ob es da einen Operator Overload für das delete gibt (und wie der genau implementiert ist): https://stackoverflow.com/a/615408

    yaVDR-Dokumentation (Ceterum censeo enchiridia esse lectitanda.)

  • Das hängt wohl davon ab, ob es da einen Operator Overload für das delete gibt (und wie der genau implementiert ist): https://stackoverflow.com/a/615408

    Ah, OK. Das heißt dann doch tiefgründigere Recherche...


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Das hängt wohl davon ab, ob es da einen Operator Overload für das delete gibt (und wie der genau implementiert ist)

    Ein einfacher Blick in den Code ergibt: so ein Operator ist nicht definiert (hier ging es um die Klasse cFontManager), es gilt also die default-Implementierung, die perfekt mit nullptr beim delete() klar kommt...


    Gruss,

    S:oren

  • Mir ist erst jetzt aufgefallen: es gab noch Patches im Devel-Light-Branch. Hab es ausprobiert, funktioniert alles .

    Ueberhaupt hatte ich (mit diesem Branch) schon lange keinerlei Abstuerze oder andere Probleme mehr. Funktioniert fuer mich perfekt.


    kamel5 Waere es mal wieder Zeit fuer eine neue Version? Oder bist Du gerade noch an den oben angekuendigten Anpassungen dran?


    Gruss,

    S:oren

  • Mir ist erst jetzt aufgefallen: es gab noch Patches im Devel-Light-Branch. Hab es ausprobiert, funktioniert alles .

    Ueberhaupt hatte ich (mit diesem Branch) schon lange keinerlei Abstuerze oder andere Probleme mehr. Funktioniert fuer mich perfekt.

    Sehr schön, das zu hören.

    Waere es mal wieder Zeit fuer eine neue Version? Oder bist Du gerade noch an den oben angekuendigten Anpassungen dran?

    Eine neue Version kann ich vorerst gerne machen.

    An den anderen Sachen bin ich noch dran, die dauern aber doch noch etwas länger.

    In letzter Zeit musste ich mich um andere Baustellen kümmern...


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Ich habe mal aus gegebenem Anlass, [ansible] vdr 2.4.7 Segmentation Fault beim Start mit softhddevice und aktivem skinnopacity oder skinflat, ein paar neue commits im Brach Devel für die "normale" Version angelegt.


    Im Moment bin ich noch an der Überarbeitung des Flush() im Menü.

    Wenn das fertig ist, werde ich das, soweit möglich und sinnvoll, auch in die light_version übernehmen.


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5

  • Es gibt einen aktualisierten Branch devel.


    Zusätzlich zu internen Anpassungen sind folgende Änderungen gegenüber der Version 1.1.8 erwähnenswert:


    - Überarbeitung von fade-in in allen OSD's

    -- Es werden jetzt alle OSD-Elemente berücksichtigt

    -- Insgesamt etwas smoother

    - Fade-out hinzugefügt

    - Recording errors in detail view hinzugefügt

    - skinnopacity Setup kann nur noch aufgerufen werden, wenn der Skin aktiv ist.

    - Neue Setup-Optionen "Use scraper infos and pictures" und "Use animation" hinzugefügt (ist dadurch zentral abschaltbar)

    - Icon für UHD hinzugefügt

    - Light Version integriert (im Setup über die Option "Use tabs in detail view" konfigurierbar)


    Die Integration der Light Version ist noch nicht optimiert. Das werde ich später noch machen.


    Zum Setup:

    Da bei diesem Skin die Parameter Theme-spezifisch abgespeichert werden, das benutze Theme aber nur bekannt ist, wenn der Skin aktiv ist, lassen sich auch nur dann Parameter fehlerfrei in die setup.conf abspeichern.

    Das führte in der Vergangenheit dazu, das z.T. unsinnige Dinge in die setup.conf gespeichert wurden, wenn bei nicht aktiven Skin Änderungen im Skin-Setup gemacht wurden.

    Ich habe deshalb, wenn der Skin nicht aktiv ist, jetzt das Setup-Menü vom Skin deaktiviert.


    S:oren ,

    beim Übernehmen von commits in die "light_version" hat sich gezeigt, das nur noch wenige Unterschiede zur "normalen" Version bestehen.

    Ich habe deshalb die verbliebenen Sachen:

    - weniger Threads (sind jetzt so weit wie möglich eliminiert)

    - lineare Detailansicht

    in die "normale" Version übernommen.


    Wenn Du die 3 Optionen "Use tabs in detail view" , "Use scraper infos and pictures" und "Use animation" auf Nein stellst, sollte sich kein Unterschied zur "light_version" ergeben.


    Es wäre schön, wenn Du den Branch devel mal testen könntest.


    Noch was TT6400 spezifisches bzgl. Animation:

    Das erste cPixmap::Lock() dauert gegenüber anderen Ausgabewegen unverhältnismäßig lange. Das erschließt sich mir nicht ganz, da an dieser Stelle die Hardware der TT6400 ja noch nicht beteiligt sein sollte. Alle nachfolgenden cPixmap::Lock() sind dann unauffällig. Möglicherweise liegt das am dvbhddevice Plugin.


    Bei mir treten da Verzögerungszeiten von z.T. über einer 1/2 sec auf. Debug-Ausgabe bei Kanalanzeige:

    [270948] skinnopacity: First Lock(): 565ms


    Das Ganze ist abhängig von der OSD Größe und der Anzahl der OSD Elemente.


    Grüße

    kamel5

    VDR 2.5.6: ASUS Prime X470-PRO, Ryzen 7 2700, 64GB, 6TB HD, GT1030, Fedora 35 Kernel 5.14 X86_64, Devicebonding 2 x 1 auf 2, TT6400, DVBSky S952 V3

    Git-Repo: gitlab.com/kamel5