Update to projectM v4.x (another try)#137
Conversation
Kodiai Review SummaryWhat ChangedUpdates visualization.projectm to use projectM v4.x API, replacing the old C++ API with new C-style API functions and playlist management. Requested minimal review; timeout risk auto-reduced the effective review to minimal and covered 50/4286 changed files via large-PR triage (30 full, 20 abbreviated; 4236 not reviewed). Reviewed: core logic Strengths
ObservationsImpact[MAJOR] src/Main.cpp (282, 284-286): Redundant function call and unused variable in GetActivePreset() [MAJOR] src/Main.cpp (98-103, 350, 354, 374): Type mismatch between constructor and SetSetting() Verdict🔴 Address before merging -- 2 blocking issue(s) found
Review Details
Files not fully reviewed (sorted by risk score)
|
![kodiai[bot]](https://avatars.githubusercontent.com/in/2832814?s=60&v=4){kind=small}
|
@kodiai review this please |
Kodiai Review SummaryWhat ChangedMigration from projectM v3 C++ API to v4 C API, replacing class-based calls with C-style function calls and introducing playlist management. Reviewed: core logic, docs Strengths
ObservationsImpact[CRITICAL] src/Main.cpp (100-102): Dangling pointer in texture path setup [MAJOR] src/Main.cpp (469-473): Failed reinitialization leaves object in broken state [MEDIUM] src/Main.cpp (608-612): PresetSwitchedEvent callback lacks defensive validation Suggestions
Verdict🔴 Address before merging -- 2 blocking issue(s) found (CRITICAL/MAJOR) |
|
@kodiai review this please |
Kodiai Review SummaryWhat ChangedMigration from projectM v3 to v4.x API, refactoring from C++ object-oriented API to C-style handle-based API. Reviewed: core logic, docs Strengths
ObservationsImpact[MAJOR] src/Main.cpp (178, 201): Incomplete audio channel handling Suggestions
Verdict🔴 Address before merging -- 1 blocking issue(s) found |
|
@kodiai review this please |
Kodiai Review SummaryWhat ChangedMajor update from projectM v3.x to v4.x API, migrating from C++ class-based API to C handle-based API with separate playlist management. Reviewed: core logic, docs Strengths
ObservationsImpact[MAJOR] src/Main.cpp (138-171): Destructor lacks mutex protection for shared resources [MAJOR] src/Main.cpp (294): Missing null check for C API return value [MAJOR] src/Main.cpp (624-633): Race condition in callback during shutdown [MEDIUM] src/Main.cpp (211): Missing validation for channel count [MEDIUM] src/Main.cpp (446-459): Logic error in user preset folder handling Preference[MINOR] src/Main.cpp (639): Unused variable Suggestions
Verdict🔴 Address before merging -- 3 blocking issue(s) found (CRITICAL/MAJOR) |
|
@kodiai review this please |
Kodiai Review SummaryWhat ChangedThis PR updates the visualization.projectm addon from v3.1.12 to v4.x, migrating from the C++ API to the new C API with significant refactoring of initialization, playlist management, and settings handling. Reviewed: core logic, C++ source changes Strengths
ObservationsImpact[MAJOR] src/Main.cpp (201): InitProjectM() return value not checked on Windows [MAJOR] src/Main.cpp (644-647): Race condition in PresetSwitchedEvent callback Verdict🔴 Address before merging -- 2 blocking issue(s) found |
|
@kodiai review this please |
Kodiai Review SummaryWhat ChangedThis PR updates the visualization addon from projectM v3 to v4, migrating to the new C API, reorganizing settings into a structured format, and including preset files. Reviewed: core logic, docs Strengths
ObservationsImpact[CRITICAL] src/Main.cpp (96-142): Constructor leaves object in invalid state if initialization fails [MAJOR] src/Main.cpp (439): Type mismatch for smooth_duration setting [MAJOR] src/Main.cpp (451): Type mismatch for preset_duration setting [MAJOR] src/Main.cpp (493): Type mismatch for beat_sens setting [MAJOR] src/Main.cpp (478): Wrong variable compared in user_preset_folder handler Suggestions
Verdict🔴 Address before merging -- 5 blocking issue(s) found (CRITICAL/MAJOR) |
Relates to comming changes. As the AI becomes not happy about comming "CVisualizationProjectM::PresetSwitchedEvent" without mutex, becomes it required to have a recursive_mutex as this callback function becomes called from main thread with CVisualizationProjectM::Render as base and where his called ProjectM function calls the callback on add-on. Without freeze the add-on and Kodi no more usable. During tests before was "mutex" two times called from main thread and one time from audio thread by "AudioData" call.
…lue)" Before was about needed preset URL every ID checked with a "switch (pvalue)". This now adds a map where makes more easy and smaller for changes in case used presets becomes updated in future.
This done to have easier an overview what comes from add-on settings.
Further are the names and the flow equal to settings.xml.
NOTE: The only slithly bad part about is that few used values have a name starting with "last_*".
Maybe we can look to rename they to have more matching to active and past time.
This done to have prepared for future API changes where add-on can return a init status to Kodi and inform add-on is usable or not. To make this then working for the new API must only the call from construction removed and the commented override uncommented.
AlwinEsch
force-pushed
the
Piers-Dev
branch
2 times, most recently
from
60b4620 to
261dfae
Compare
AlwinEsch
changed the title
|
NOTE: has fixed a crash on NVIDIA Shield TV, for some reason has it on init crashed there, but all other OS and all other Android systems was OK. The crash was produced by this line: The strange was that the crash was not directly come by this code line and call. Crash was coming later on call from Kodi of But I'm having a lot of guesswork because it was only there with NVIDIA (probably because of Android 11 🤔?) and with all the others it was OK. |
|
I've attempted to test this everywhere I can:
|
|
OK, I was able to verify success on Android using the zip from the build artifacts: https://jenkins.kodi.tv/blue/organizations/jenkins/xbmc%2Fvisualization.projectm/detail/PR-137/34/artifacts/ Shield TV working great. |
Implemented in ProjectM on Dec 7, 2025 with c6e02c9 Not included in the 4.1.6 release
Before was it hardcoded to Stereo and where the AI was not happy. This includes now the support for diverent channels amount
On versions lower as 10.15 for OSX and 13.0 for embedded is std::filesystem not complete supported and compile fails there.
Unfortunately, due to numerous problems, it currently no longer makes sense to use this add-on under Windows. On the one hand, Angle rendering using DirectX causes too many problems and incorrect representations. On the other hand, projectM version 4 for GLES requires minimum version 3.2, which is not available under Windows using DirectX. In addition, Kodi crashes when changing full screen when this add-on is in use. Support for Windows could possibly be brought back in the future.
|
Pull request code has been updated again, as a new change the Windows support for the add-on will unfortunately be removed, see description in the associated commit. |
|
It comes in now. Is not nice but Windows support then removed 😢. |
|
Nice work! |
3 participants
No description provided.