 |
@MatN I have marked this Resolved and will close in a couple of days if no followup. |
|
|
@MatN This will be marked resolved/closed after it is included in the October 31 release as there does not appear to be any outstanding issues. |
 |
@PhyllisSmith, sorry false alarm - just quick automatic recompilation apparently as usual failed to pich up *. h file update... now it works without crash (both gui and cli) |
|
|
@Andrew-R @MatN Oh, that is bad. Is it something I can duplicate? |
|
|
now I have crash at shutdown... [LWP 2879 exited] [LWP 2880 exited] awindow.C:68:14: runtime error: member call on null pointer of type 'AWindowGUI' --Type <RET> for more, q to quit, c to continue without paging-- Thread 9 "cin" received signal SIGSEGV, Segmentation fault. [Switching to LWP 2876] 0x019e5958 in AWindowGUI::save_defaults (this=0x0, defaults= 0xf6569b00) at awindowgui.C:1636 1636 defaults->update("PLUGIN_VISIBILTY", plugin_visibility); (gdb) bt full #0 0x019e5958 in AWindowGUI::save_defaults (this=0x0, defaults=0xf6569b00) at awindowgui.C:1636 No locals. 0000001 0x0247c10c in MWindow::save_defaults (this=0xfffeef18) at mwindow.C:4982 No locals. 0000002 0x02eb7b24 in BC_DialogThread::run (this=0xec0ec6c0) at bcdialog.C:103 result = 1 0000003 0x0317c5d0 in Thread::entrypoint (parameters=0xec0ec6c0) at thread.C:68 thread = 0xec0ec6c0 #4 0xf70a4544 in __pthread_start(void*) () from /apex/com.android.runtime/lib/bionic/libc.so No symbol table info available. 0000005 0xf705ec44 in __start_thread () from /apex/com.android.runtime/lib/bionic/libc.so No symbol table info available. 0000006 0x00000000 in ?? () No symbol table info available. Backtrace stopped: previous frame identical to this frame (corrupt stack?) (gdb) |
|
|
@MatN @Andrew-R Checked into GIT - although previously I was able to get a crash using Batch Render, I could not do so again BEFORE applying the mod, so did not test patch. Reviewed the changes to all of render.C, batchrender,C, renderfarmclient.C, mwindow.C and mwindow.h and all look reasonable. But I really am C++ clueless. P.S. I have not forgotten about Termux but it is a MAJOR enhancement that needs to be verified and documented and hopefully this winter, I will be smarter !! |
|
|
@MatN, sadly I can't compile ladspa suppirt in termux, and audio-encoding still segfailt cin :( not completely self-contained appimage a bit of disappointment, but guess thisbis error for another time and another bug entry. I'll wait a bit for your patch to appear in main git and then rebase my pile on top of that, then write here. Hopefully at least crashes on startup and cli video-only render will be gone! |
 |
Tested batch rendering with different audio type plugins as follows. Set up project with video jellyfish-10-mbps-hd-h264.mkv, audio stereo (some music) of the same length as the video, 4 different types of audio plugins, one after the other with no-effect spaces in between. At 0:00:02, 5 seconds of L_Barry's Satan, knee point set at -20 dB, ladspa plugin from CinGG (gives distorted audio) At 0:00:10, 5 seconds of L2_ACE_delay, default settings, LV2 plugin from Ardour on Manjaro (gives echo). At 0:00:16, 6 seconds of F_aecho, default settings, ffmpeg plugin from CinGG (gives different kind of echo) At 0:00:24, 5 seconds of Chorus, 2 voices per channel, CinGG plugin (chorus effect). These plugins give a clear different effect each, so are easy to hear if they work. Project saved on shared storage, with appimage, video audio, and project xml file in the same directory. Manually editied the xml file to change all paths to ./ . For testing. copy this whole directory to a local directory, start AppImage, set up batch render with output file in same directory, test batch render, close, stop CinGG, delete output file, start it again with -r parameter. It should give an output with each of the audio effects audible, proving that all 4 types of audio plugins work with batch rendering using the AppImage. I tested on 5 platforms, two were OK, the other are a local problem: - Mint 20.2 (build platform): works fine. - VirtualBox VM, Debian11: works fine. - VirtualBox VM, MXlinux: AppImage fails because of old glibc version. - Native hardware Manjaro: AppImage fails because of libjbig.so.0 - Native hardware Fedora 34: AppImage fails because of libbz2.so.1.0 I think it is quite solid now. Because previously for batch and renderfarmclient the ladspa plugins were not initialized, I wonder if they ever worked that way. @Andrew-R, can you test on your arm setup with my patches from yesterday? |
|
|
@MatN @Andrew-R Will test both Andrews's "fix batchrender patch" and Mat's Diff, but will wait until final patches are for sure done. |
|
|
Cleaned up modules for batch render (and renderfarm too). Diff against current git (20211014 2000 UTC, last update was for the titler). Note that both batch render and renderfarmclient did not initialize the ladspa plugins, and therefore did not rebuild the ladspa index(es) when needed. I have added that, but testing of rendering with plugins actually active remains to be done. Had not had time yet to look at actual use of mwindow by the plugin servers.
Diff_20211014_MatN.diff (5,986 bytes) |
|
|
Yep. that line should indeed be TransportCommand *command = new TransportCommand(render->preferences); The if mwindow then copy part above is not needed, because that is already done in RenderThread::run() . In addition, both MWindow::init_plugins and init_ladspa_plugins should not use mwindow-> . I have replaced them for now with direct multiple getenv calls, will clean up before I send it in. But with these changes, cin -r runs fine, even rebuilding the indexes. The mwindow value of 0 does get passed on to the plugin servers and stored there, that also needs a look. I'll continue tomorrow. |
|
|
@MatN, I think I broke it with my variable rewind speed patches.. try attached patch?
0001-FIX-cli-batchrender.patch (1,036 bytes) |
|
|
Confirmed. I created two projects, set up the rendering but did not render and saved the project (.xml file). Then in the GUI, I set up batch rendering for those two projects (had to read the nice manual...) . Tested the batch from the GUI, worked fine. Deleted the output files, stopped CinGG, started it as ./cin -r , and crash! Probably would crash also with just one job. Time to wake up the debugger. |
|
|
@MatN, it indeed aborts from cli: -- rm 1.mp4 $ bin/cin -r ~/.bcast5/batchrender.rc Cinelerra Infinity - built: Oct 11 2021 21:42:12 git://git.cinelerra-gg.org/goodguy/cinelerra.git (c) 2006-2019 Heroine Virtual Ltd. by Adam Williams 2007-2020 mods for Cinelerra-GG by W.P.Morrow aka goodguy Cinelerra is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. There is absolutely no warranty for Cinelerra. filesystem.C:354:29: runtime error: subtraction of unsigned offset from 0x00000000 overflowed to 0x00000001 Render::run: /data/data/com.termux/files/home/cingg/cinelerra/cinelerra-5.1/p.xml render.C:688:60: runtime error: member access within null pointer of type 'MWindow' ** segv at 0x0 in pid 23209, tid 23209 created on Wed Oct 13 22:07:08 2021 by 10116:10116 u0_a116(*) OS: CPUS: 8 CPUINFO: processor : 0 model name : ARMv8 Processor rev 4 (v8l) BogoMIPS : 26.00 Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt lpae evtstrm aes pmull sha1 sha2 crc32 CPU implementer : 0x41 CPU architecture: 8 CPU variant : 0x0 CPU part : 0xd03 CPU revision : 4 processor : 1 model name : ARMv8 Processor rev 4 (v8l) BogoMIPS : 26.00 Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt lpae evtstrm aes pmull sha1 sha2 crc32 CPU implementer : 0x41 CPU architecture: 8 CPU variant : 0x0 CPU part : 0xd03 CPU revision : 4 processor : 2 model name : ARMv8 Processor rev 4 (v8l) BogoMIPS : 26.00 Features : half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt lpae evtstrm aes pmull sha1 sha2 crc32 CPU implementer : 0x41 CPU architecture: 8 CPU variant : 0x0 CPU part : 0xd03 CPU revision : 4 processor : 3 model name : ARMv8 Processor rev 4 (v8l) THREADS: TRACES: LOCKS: signal_entry: lock table size=1 lock_items: 0 lock_frees: 1 BUFFERS: SHMMEM: MAIN HOOK: EDL: batchrender.C:727:11: runtime error: member call on null pointer of type 'MWindow' mwindow.C:4884:15: runtime error: member call on null pointer of type 'MWindow *' Segmentation fault -- |
|
|
@Andrew-R, - porting an application to another platform or set of build tools is a very good way to find hidden problems. - If you do batchrender from the GUI, you already have a mwindow. But you can start a batchrender also as "./cin -r batchjob.rc" . Have you tried that? I will do so later and look at the code too. |
|
|
@MatN, batchrender works without warnings (from gui) |
|
|
@MatN, may be this hybrid of android's libc and clang somewhat more sensitive to various subtle 'bugs' (or undefined behaviors?) than my usual x86/glibc/gcc installation. For example, after I added '-fsanitize=undefined' to both CFLAGS and LDFLAGS and recompiled CinGg (main prigram, guicast and plugins) I see some runtime errors, especially one I silenced by adding check for existence of mwindow. Right now I try to make sense out of some pointer overflow bug (?) in vframe.C, it printed right before crash on arm32/termux with X11 DIRECT if composer window at default size. If I set it to 50% zoom on 720x576 video - it does not crash on playback. It also work if I uncheck use direct X11 checkbox in preferences. So yeah, it looks like somethkng strange goes on, but both parameters I printf'ed out of create_row_pointers() func look sane! not overly large or negative... So I can debug but w/o success) ( will try batchrender with updated codebase... |
|
|
The patch is OK. @Andrew-R, - a strcpy will crash the program if source and/or destination are NULL. You test for a getenv return value, but getenv is used in many more places, and CinGG creates this value itself. It will only return NULL if something changed that env var in CinGG's process space. I guess it is slightly more safe. I don't quite understand how it could crash for you without the patch. The appimageDir may be NULL or a string. I do see that batchrender calls this function with a zero for mwindow. I wonder how this can work, have not looked at it yet. Is this maybe the cause of your crash? Are you able to debug? Then a breakpoint on function entry shows you the call stack. @PhyllisSmith, have you tested batchrender by any chance? |
|
|
@PhyllisSmith @Andrew-R will look at the patch tomorrow. |
|
|
may be just adding ' mwindow && ' under if() condition will be enough? It seems sometimes this function can be called with uninitialized mwindow pointer/parameter... But then I am not sure how strcpy will react with NULL'ed string... |
|
|
@Andrew-R @MatN Yes, Andrew it works for AppImages as well as the builds. Mat, if you have time to look at this final patch and let me know if I should check it into GIT? |
|
|
revised patch
0001-Attempt-at-fixing-mwindow-appimageDir-NULL.patch (2,747 bytes) |
|
|
well, then I guess '__attribute__((no_sanitize("undefined")));' should also be deleted - I using it because I feared clang's instrumentation interfered badly with Cingg's code in this function.. I'll try to make cleaner patch.. |
|
|
@Andrew-R When applying the patch, "ci" is rejected as well as "plugin_config". So I leave them out but it will not compile -- tried twice because sometimes my laptop flakes out so I will reboot and start fresh and maybe only apply "mwindow.C" and "mwindow.h". |
|
|
this patch (relative to latest git) *does not* crash for me, but does it still work in appimage case?
fsanitize_undefined_and_appimage_crashfix.diff (3,388 bytes) |
|
|
@Andrew-R Yes, I have been running after compiling and it has not been a problem so far. I also ran on older ubuntu 16 with no problems. But it is new code so I will test on other operating systems -- especially Debian 32-bit. |
|
|
does it work as non-appimage? for me it seems to segfault on accessing NULL'ed member of mwindow... (on arm32/termux, compiled with clang). |
|
|
@MatN All of the changes have been checked into GIT. Thank you very much. There is 1 side affect -- when you switch from running a build to running an AppImage, or vice versus, it always reloads the regular plugins. But this only affects mostly me since I do quite a bit of multiple type testing. Ordinary users will only have to reload the plugins when there is a new release AND THAT IS A GOOD THING in case there are new plugins |
 |
It works perfectly, thank you. |
|
|
@MatN Additional testing and looking at the code changes is all good. Waiting to hear back from Andrea for his test results also and hopefully he tests for reload of LV2 plugins, because I do not have any of those to easily test. |
|
|
@Andrea_Paz There are now a newer and an older AppImage for testing at: https://cinelerra-gg.org/download/testing/cin_pluginsLoad_fade-newer_distros.AppImage https://cinelerra-gg.org/download/testing/cin_pluginsLoad_fade-older_distros.AppImage I tested both and have so far seen no issues but am multitasking so not being very thorough. P.S. it includes your new DPX.dpx and CineformHD.mov which I checked into GIT this morning. |
|
|
@PhyllisSmith If you prepare an appimage I can try to test it on Arch, since it gave MatN problems on Manjaro. |
|
|
I use "meld" to look at code changes, I never got comfortable with git. I do a make clean first on the modified Cin, then compare it to the "master", which is kept in sync with the git and never built or modified. Then I look for "Modified" and "New" only. Very comfortable. Had an issue yesterday when trying to run on Manjaro, an error about libjbig.so.0 , but that is included in the AppImage. Have to look further, but next few day are busy. Also want to do MXlinux. |
|
|
@MatN Works REALLY WELL! and will save a lot of time for the users -- I never noticed it on my laptop mostly because I usually only run a built CinGG instead of the AppImage as I test things and this laptop is pretty fast. I would like to do a few more tests yet and study the code changes (even though I do not know C++) before checking the changes into GIT. |
|
|
Aaargh! I included the wrong mwindow.C yesterday, with that one you cannot build due to a compile error. Attached is the correct one. My apologies. MatN mwindow.C.zip (35,142 bytes) |
|
|
Great! I will be testing this tomorrow and will see if anyone needs me to provide an AppImage so they can test also. |
|
|
Here are the fixes for BT588 and BT591; unzip the zip in the Cinelerra-5.1 directory. I have tested with two ladspa directories (/usr/lib/ladspa and CinGG's one) . The index files (one for built-in + ffmpeg + LV2, plus one per ladspa directory) are now not rebuilt anymore each time you load the AppImage. The index file naming has been changed to fix that. Explanation: Naming convention for the index files is always "Cinelerra_plugins" for built-ins, ffmpeg, and LV2; and ladspa_plugins.xxxx for each ladspa directory, where xxxx is basically the path to the ladspa directory. But the path for the ladspa plugins included in Cin-GG changes for every AppImage start, and an index file will be rebuilt if the name of the file does not match (which causes BT591). It also gets rebuilt if the second line of the file is not what is should be (also a cause of BT591), or if the timestamp of any of the built-in plugins has changed. And because the path changes, there will be a new ladspa_plugin.xxxx for every AppImage start (BT588). In case of an AppImage, the filename pointing to the CinGG plugins, and the second line of the index file will now contain a CinGG identifier if the form CINGG_20211008_152354, the date/time of the cin or AppImage executable. And because the name for built-in ladspa plugins does not change anymore every AppImage start, there will only be one ladspa index file for those with a fixed name, solving BT588. Have fun testing! Addition: A utility called doxygen will produce some html documentation about the code. Install at least packages doxygen and graphviz. doxygen-doc and dozygen-gui are not needed. There is good doxygen documentation online. To create the doxygen documentation, place the supplied "Doxyfile" (case sensitive!) in cinelerra-5.1, and run doxygen from a terminal in there. It runs some time, generates 45k files taking 2.4GB. In the html directory is an index.html, open it with your browser. The Doxyfile supplies the directories to include, only CinGG code ones. If you include thirdparty, it grows to 9.6 G and takes ages to build. I have added some Doxygen-recognisable comments here and there. To see how it looks, after generating the doxygen docs, open the index file in your browser, in the upper right is a search field, type in "load_plu" , from the list select load_plugin_index . Class hierarchy is also interesting. It will get better if more functions get doxygen compatible comment. During Doxygen generation I saw some postscript errors fly past, have not looked at those yet. I have not looked at callgraphs yet, those helped a lot when I was still programming for a living. I have choosen @ instead of \ to indicated doxygen specific keywords, because @ is used a lot less in the code than the escape character \. So @brief instead if \brief. Makes it easier to locate doxygen keywords. BT588_591.zip (101,940 bytes) |
|
|
Just a note that I have (locally) fixed this BT591 (LV2 plugin release every AppImage start), and BT588 (new ladspa file in ./bcast5 directory for every AppImage start). Now I need to do some code cleanup. Hopefully I can send the files to Phyllis tomorrow. |
|
|
@Andrea_Paz, I had forgotten about that mailing list entry, thanks for reminding me. I found the reason why it reloads if an AppImage. The file "Cinelerra_plugins" has as first two lines: - version number (clever) - path the to built-in plugins. Example: 4 /tmp/.mount_cin-01yxjenR/usr/bin/plugins For an AppImage this path changes every start, so it reloads all plugins. BT588 is caused by the same problem. Now to fix it. Fortunately it is dead easy to detect if you are running as an AppImage, the environment variable AppImage is set. |
|
|
I confirm that the "lv2 plugins reload" has been there from the beginning: https://www.mail-archive.com/[email protected]/msg02513.html |
- Anonymous
