View Issue Details

IDProjectCategoryView StatusLast Update
0000591Cinelerra-GG[All Projects] Bugpublic2021-10-16 01:26
ReporterMatN Assigned ToPhyllisSmith  
PrioritynormalSeverityminorReproducibilityalways
Status confirmedResolutionopen 
PlatformX86_64OSMint XFCEOS Version20.2
Product Version 
Target VersionFixed in Version 
Summary0000591: AppImage reloads LV2 plugins on every restart. Version 2021-09, but has probably been there since AppImgae format releases.
DescriptionA plugin reload (really creating a plugin index) should only happen if file Cinelerra_plugins is missing.
This works fine on normal builds, but the AppImage version does the reload on every start.
TagsNo tags attached.

Activities

Andrew-R

Andrew-R

2021-10-16 01:26

reporter   ~0005087

@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)
PhyllisSmith

PhyllisSmith

2021-10-16 00:46

manager   ~0005086

@Andrew-R
@MatN
Oh, that is bad. Is it something I can duplicate?
Andrew-R

Andrew-R

2021-10-16 00:44

reporter   ~0005085

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)
PhyllisSmith

PhyllisSmith

2021-10-15 23:12

manager   ~0005084

@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 !!
Andrew-R

Andrew-R

2021-10-15 19:09

reporter   ~0005083

@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!
MatN

MatN

2021-10-15 14:56

reporter   ~0005082

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?
PhyllisSmith

PhyllisSmith

2021-10-14 21:47

manager   ~0005079

@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.
MatN

MatN

2021-10-14 20:09

reporter   ~0005078

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)
diff -bur cinelerra_master/cinelerra-5.1/cinelerra/batchrender.C cinelerra_work/cinelerra-5.1/cinelerra/batchrender.C
--- cinelerra_master/cinelerra-5.1/cinelerra/batchrender.C	2021-09-25 20:52:12.607422095 +0200
+++ cinelerra_work/cinelerra-5.1/cinelerra/batchrender.C	2021-10-14 21:23:40.066002349 +0200
@@ -597,8 +597,10 @@
 	else {
 		BC_Trace::disable_locks();
 	}
-
+// In batch mode there is no mwindow, so init_plugins is called with
+// mwindow* = NULL.
 	MWindow::init_plugins(0, preferences);
+	MWindow::init_ladspa_plugins(0, preferences);  
 	char font_path[BCTEXTLEN];
 	strcpy(font_path, preferences->plugin_dir);
 	strcat(font_path, "/" FONT_SEARCHPATH);
diff -bur cinelerra_master/cinelerra-5.1/cinelerra/mwindow.C cinelerra_work/cinelerra-5.1/cinelerra/mwindow.C
--- cinelerra_master/cinelerra-5.1/cinelerra/mwindow.C	2021-10-12 16:25:14.011443994 +0200
+++ cinelerra_work/cinelerra-5.1/cinelerra/mwindow.C	2021-10-14 21:31:09.950000928 +0200
@@ -240,7 +240,6 @@
 	sighandler = 0;
 	restart_status = 0;
 	screens = 1;
-	appimageDir = getenv("APPDIR"); //NULL if not running as appimage
 	in_destructor = 0;
 	speed_edl = 0;
 	beeper = 0;
@@ -561,9 +560,8 @@
 * @brief Load plugins according to an index file.
 * 
 * @details Builds an ArrayList of plugin servers only if there is no
-* mismatch for file layout version, index identifier, or executable
-* timestamp mismatch for the built-in plugins. If OK, add the plugin
-* servers to the global list.
+* mismatch for file layout version, index identifier, or timestamp of
+* the built-in plugins. If OK, add the plugin servers to the global list.
 * 
 * @note If an error is returned the index file needs to be rebuilt, and
 * then this function must be called again.
@@ -673,9 +671,13 @@
 	}
 	return 0;
 }
-/*
+
+/**
 * @brief Load built-in and LV2 plugins as specified in index file,
 *        rebuild the index file if needed.
+* @param[in]	mwindow: GUI class pointer, will be NULL for batch
+*               rendering or renderfarm client.
+* @param[in]	preferences: Information from cinelerra_rc file.
 */
 int MWindow::init_plugins(MWindow *mwindow, Preferences *preferences)
 {
@@ -689,7 +691,7 @@
 	// index_id is 2nd line of the index file, normally full plugin path,
 	// but fixed value if AppImage because the path changes on each run.
 	// And if the second line does not match on the next run the index is rebuilt.
-	if( mwindow->appimageDir ) strcpy(index_id, getenv("CINGG_BUILD"));
+	if( getenv("APPDIR") && getenv("CINGG_BUILD")) strcpy(index_id, getenv("CINGG_BUILD"));
 	else strcpy(index_id, plugin_path);
 	FILE *fp = fopen(index_path,"a+");
 	if( !fp ) {
@@ -723,14 +725,18 @@
 	return ret;
 }
 
-/*
+/**
 * @brief Load ladspa plugins as specified in index files, for each ladspa
 *        directory keep a separate index file. Rebuild index file(s) if needed.
-**/
+* @param[in]	mwindow: GUI class pointer, will be NULL for batch
+*               rendering or renderfarm client.
+* @param[in]	preferences: Information from cinelerra_rc file.
+*/
 int MWindow::init_ladspa_plugins(MWindow *mwindow, Preferences *preferences)
 {
 #ifdef HAVE_LADSPA
 	char *path = getenv("LADSPA_PATH");
+	char *appdir = getenv("APPDIR");
 	char ladspa_path[BCTEXTLEN];
 	if( !path ) {  			// if no env var, use CinGG's own ladspa dir
 		strncpy(ladspa_path, File::get_ladspa_path(), sizeof(ladspa_path));
@@ -749,7 +755,7 @@
 		// If the first part of the plugin_path matches the APPDIR, we are
 		// referring to CinGG's ladspa, replace the path by a fixed ID. APPDIR
 		// only exists if we are running as AppImage (with variable mount points).
-		if( mwindow->appimageDir && strncmp(plugin_path, mwindow->appimageDir, strlen(mwindow->appimageDir)) == 0 )
+		if( appdir && strncmp(plugin_path, appdir, strlen(appdir)) == 0 )
 			strcpy(index_id, getenv("CINGG_BUILD"));
 		else strcpy(index_id, plugin_path);
 
@@ -818,7 +824,7 @@
 		char fs_path[BCTEXTLEN], path[BCTEXTLEN];
 		get_plugin_path(fs_path, 0, fs.dir_list[i]->path);
 		get_plugin_path(path, plug_dir, fs_path);
-		if( fs.is_dir(fs_path) ) {
+		if( fs.is_dir(fs_path) ) {	// recursively scan child directory
 			scan_plugin_index(mwindow, preferences, fp, plug_dir, path, idx);
 			continue;
 		}
diff -bur cinelerra_master/cinelerra-5.1/cinelerra/mwindow.h cinelerra_work/cinelerra-5.1/cinelerra/mwindow.h
--- cinelerra_master/cinelerra-5.1/cinelerra/mwindow.h	2021-10-12 16:25:14.011443994 +0200
+++ cinelerra_work/cinelerra-5.1/cinelerra/mwindow.h	2021-10-14 21:12:59.142004373 +0200
@@ -858,7 +858,6 @@
 	SigHandler *sighandler;
 	int restart_status;
 	int screens;
-	const char *appimageDir;
 	int in_destructor;
 	Shuttle *shuttle;
 	WinTV *wintv;
diff -bur cinelerra_master/cinelerra-5.1/cinelerra/render.C cinelerra_work/cinelerra-5.1/cinelerra/render.C
--- cinelerra_master/cinelerra-5.1/cinelerra/render.C	2021-09-25 20:52:12.679421084 +0200
+++ cinelerra_work/cinelerra-5.1/cinelerra/render.C	2021-10-14 20:55:40.122007653 +0200
@@ -684,7 +684,7 @@
 	render->result = 0;
 
 // Create rendering command
-	TransportCommand *command = new TransportCommand(mwindow->preferences);
+	TransportCommand *command = new TransportCommand(render->preferences);
 	command->command = NORMAL_FWD;
 	command->get_edl()->copy_all(edl);
 	command->change_type = CHANGE_ALL;
diff -bur cinelerra_master/cinelerra-5.1/cinelerra/renderfarmclient.C cinelerra_work/cinelerra-5.1/cinelerra/renderfarmclient.C
--- cinelerra_master/cinelerra-5.1/cinelerra/renderfarmclient.C	2021-09-25 20:52:12.679421084 +0200
+++ cinelerra_work/cinelerra-5.1/cinelerra/renderfarmclient.C	2021-10-14 21:29:17.998001282 +0200
@@ -77,6 +77,7 @@
 	boot_preferences = new Preferences;
 	boot_preferences->load_defaults(boot_defaults);
 	MWindow::init_plugins(0, boot_preferences);
+	MWindow::init_ladspa_plugins(0, boot_preferences);  
 	BC_Signals::set_catch_segv(boot_preferences->trap_sigsegv);
 	BC_Signals::set_catch_intr(0);
         if( boot_preferences->trap_sigsegv ) {

Diff_20211014_MatN.diff (5,986 bytes)
MatN

MatN

2021-10-13 20:46

reporter   ~0005075

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.
Andrew-R

Andrew-R

2021-10-13 19:59

reporter   ~0005074

@MatN, I think I broke it with my variable rewind speed patches..

try attached patch?

0001-FIX-cli-batchrender.patch (1,036 bytes)
From 7614d3472db8b28135a4949676950414aa0e0f18 Mon Sep 17 00:00:00 2001
From: Andrew Randrianasulu <[email protected]>
Date: Wed, 13 Oct 2021 22:36:00 +0300
Subject: [PATCH] FIX cli batchrender

---
 cinelerra-5.1/cinelerra/render.C | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/cinelerra-5.1/cinelerra/render.C b/cinelerra-5.1/cinelerra/render.C
index d9760706..be5a9448 100644
--- a/cinelerra-5.1/cinelerra/render.C
+++ b/cinelerra-5.1/cinelerra/render.C
@@ -683,9 +683,12 @@ void RenderThread::render_single(int test_overwrite, Asset *asset, EDL *edl,
 	render->default_asset = asset;
 	render->progress = 0;
 	render->result = 0;
+	
+	if(mwindow)
+		render->preferences->copy_from(mwindow->preferences);
 
 // Create rendering command
-	TransportCommand *command = new TransportCommand(mwindow->preferences);
+	TransportCommand *command = new TransportCommand(render->preferences);
 	command->command = NORMAL_FWD;
 	command->get_edl()->copy_all(edl);
 	command->change_type = CHANGE_ALL;
-- 
2.33.0

MatN

MatN

2021-10-13 19:17

reporter   ~0005073

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.
Andrew-R

Andrew-R

2021-10-13 19:11

reporter   ~0005072

@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

--
MatN

MatN

2021-10-13 17:33

reporter   ~0005071

@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.
Andrew-R

Andrew-R

2021-10-13 16:22

reporter   ~0005070

@MatN, batchrender works without warnings (from gui)
Andrew-R

Andrew-R

2021-10-13 15:51

reporter   ~0005069

@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...
MatN

MatN

2021-10-13 14:29

reporter   ~0005068

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?
MatN

MatN

2021-10-12 19:46

reporter   ~0005066

@PhyllisSmith @Andrew-R
will look at the patch tomorrow.
Andrew-R

Andrew-R

2021-10-12 16:38

reporter   ~0005065

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...
PhyllisSmith

PhyllisSmith

2021-10-12 15:46

manager   ~0005064

@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?
Andrew-R

Andrew-R

2021-10-12 08:25

reporter   ~0005062

revised patch

0001-Attempt-at-fixing-mwindow-appimageDir-NULL.patch (2,747 bytes)
From 9943c9f8b7067d3cd1354b24cc4ee85082646d90 Mon Sep 17 00:00:00 2001
From: Andrew Randrianasulu <[email protected]>
Date: Tue, 12 Oct 2021 11:17:51 +0300
Subject: [PATCH] Attempt at fixing mwindow->appimageDir = NULL

---
 cinelerra-5.1/cinelerra/mwindow.C | 15 ++++++++++++---
 cinelerra-5.1/cinelerra/mwindow.h |  1 +
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/cinelerra-5.1/cinelerra/mwindow.C b/cinelerra-5.1/cinelerra/mwindow.C
index 2a86aaae..b3a12239 100644
--- a/cinelerra-5.1/cinelerra/mwindow.C
+++ b/cinelerra-5.1/cinelerra/mwindow.C
@@ -240,7 +240,7 @@ MWindow::MWindow()
 	sighandler = 0;
 	restart_status = 0;
 	screens = 1;
-	appimageDir = getenv("APPDIR"); //NULL if not running as appimage
+	appimageDir = getenv("APPDIR"); //NULL if not running as appimage 
 	in_destructor = 0;
 	speed_edl = 0;
 	beeper = 0;
@@ -675,6 +675,15 @@ int MWindow::check_plugin_index(ArrayList<PluginServer*> &plugins,
 	}
 	return 0;
 }
+
+int MWindow::is_appimage()
+{
+	if (getenv("APPDIR"))
+	return 1;
+    return 0;
+}
+
+
 /*
 * @brief Load built-in and LV2 plugins as specified in index file,
 *        rebuild the index file if needed.
@@ -691,7 +700,7 @@ int MWindow::init_plugins(MWindow *mwindow, Preferences *preferences)
 	// index_id is 2nd line of the index file, normally full plugin path,
 	// but fixed value if AppImage because the path changes on each run.
 	// And if the second line does not match on the next run the index is rebuilt.
-	if( mwindow->appimageDir ) strcpy(index_id, getenv("CINGG_BUILD"));
+	if( mwindow->is_appimage() && getenv("CINGG_BUILD")) strcpy(index_id, getenv("CINGG_BUILD"));
 	else strcpy(index_id, plugin_path);
 	FILE *fp = fopen(index_path,"a+");
 	if( !fp ) {
@@ -751,7 +760,7 @@ int MWindow::init_ladspa_plugins(MWindow *mwindow, Preferences *preferences)
 		// If the first part of the plugin_path matches the APPDIR, we are
 		// referring to CinGG's ladspa, replace the path by a fixed ID. APPDIR
 		// only exists if we are running as AppImage (with variable mount points).
-		if( mwindow->appimageDir && strncmp(plugin_path, mwindow->appimageDir, strlen(mwindow->appimageDir)) == 0 )
+		if( mwindow->is_appimage() && strncmp(plugin_path, mwindow->appimageDir, strlen(mwindow->appimageDir)) == 0 )
 			strcpy(index_id, getenv("CINGG_BUILD"));
 		else strcpy(index_id, plugin_path);
 
diff --git a/cinelerra-5.1/cinelerra/mwindow.h b/cinelerra-5.1/cinelerra/mwindow.h
index 46e96643..ed20bfbd 100644
--- a/cinelerra-5.1/cinelerra/mwindow.h
+++ b/cinelerra-5.1/cinelerra/mwindow.h
@@ -859,6 +859,7 @@ public:
 	int restart_status;
 	int screens;
 	const char *appimageDir;
+	int is_appimage();
 	int in_destructor;
 	Shuttle *shuttle;
 	WinTV *wintv;
-- 
2.33.0

Andrew-R

Andrew-R

2021-10-12 06:46

reporter   ~0005060

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..
PhyllisSmith

PhyllisSmith

2021-10-12 01:13

manager   ~0005059

@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".
Andrew-R

Andrew-R

2021-10-11 22:19

reporter   ~0005058

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)
diff --git a/cinelerra-5.1/cinelerra/Makefile b/cinelerra-5.1/cinelerra/Makefile
index c660ee37..5c5bbd09 100644
--- a/cinelerra-5.1/cinelerra/Makefile
+++ b/cinelerra-5.1/cinelerra/Makefile
@@ -501,7 +501,7 @@ CFLAGS += -DUSE_ALPHA
 
 else
 
-LDFLAGS1 = -Wl,-export-dynamic -g
+LDFLAGS1 = -Wl,-export-dynamic -g -fsanitize=undefined
 LDFLAGS2 = $(LDFLAGS)
 LINKER = $(CXX) -o $(OUTPUT)
 
diff --git a/cinelerra-5.1/cinelerra/ci b/cinelerra-5.1/cinelerra/ci
index bc987833..22858982 120000
--- a/cinelerra-5.1/cinelerra/ci
+++ b/cinelerra-5.1/cinelerra/ci
@@ -1 +1 @@
-../bin/cinelerra
\ No newline at end of file
+../bin/cin
\ No newline at end of file
diff --git a/cinelerra-5.1/cinelerra/mwindow.C b/cinelerra-5.1/cinelerra/mwindow.C
index 2a86aaae..c6a87626 100644
--- a/cinelerra-5.1/cinelerra/mwindow.C
+++ b/cinelerra-5.1/cinelerra/mwindow.C
@@ -240,7 +240,7 @@ MWindow::MWindow()
 	sighandler = 0;
 	restart_status = 0;
 	screens = 1;
-	appimageDir = getenv("APPDIR"); //NULL if not running as appimage
+	appimageDir = getenv("APPDIR"); //NULL if not running as appimage 
 	in_destructor = 0;
 	speed_edl = 0;
 	beeper = 0;
@@ -675,6 +675,15 @@ int MWindow::check_plugin_index(ArrayList<PluginServer*> &plugins,
 	}
 	return 0;
 }
+
+int MWindow::is_appimage()
+{
+	if (getenv("APPDIR"))
+	return 1;
+    return 0;
+}
+
+
 /*
 * @brief Load built-in and LV2 plugins as specified in index file,
 *        rebuild the index file if needed.
@@ -691,7 +700,7 @@ int MWindow::init_plugins(MWindow *mwindow, Preferences *preferences)
 	// index_id is 2nd line of the index file, normally full plugin path,
 	// but fixed value if AppImage because the path changes on each run.
 	// And if the second line does not match on the next run the index is rebuilt.
-	if( mwindow->appimageDir ) strcpy(index_id, getenv("CINGG_BUILD"));
+	if( mwindow->is_appimage() && getenv("CINGG_BUILD")) strcpy(index_id, getenv("CINGG_BUILD"));
 	else strcpy(index_id, plugin_path);
 	FILE *fp = fopen(index_path,"a+");
 	if( !fp ) {
diff --git a/cinelerra-5.1/cinelerra/mwindow.h b/cinelerra-5.1/cinelerra/mwindow.h
index 46e96643..780001e6 100644
--- a/cinelerra-5.1/cinelerra/mwindow.h
+++ b/cinelerra-5.1/cinelerra/mwindow.h
@@ -810,7 +810,7 @@ public:
 	void init_tipwindow();
 // Used by MWindow and RenderFarmClient
 	static void get_plugin_path(char *path, const char *plug_dir, const char *fs_path);
-	static int init_plugins(MWindow *mwindow, Preferences *preferences);
+	static int init_plugins(MWindow *mwindow, Preferences *preferences) __attribute__((no_sanitize("undefined")));
 	static int init_ladspa_plugins(MWindow *mwindow, Preferences *preferences);
 	static void init_plugin_tips(ArrayList<PluginServer*> &plugins, const char *lang);
 	static int check_plugin_index(ArrayList<PluginServer*> &plugins,
@@ -859,6 +859,7 @@ public:
 	int restart_status;
 	int screens;
 	const char *appimageDir;
+	int is_appimage();
 	int in_destructor;
 	Shuttle *shuttle;
 	WinTV *wintv;
diff --git a/cinelerra-5.1/plugin_config b/cinelerra-5.1/plugin_config
index 56f43511..08e0238c 100644
--- a/cinelerra-5.1/plugin_config
+++ b/cinelerra-5.1/plugin_config
@@ -4,6 +4,7 @@ CFLAGS += -I../ -I$(CINELERRA) -I$(GUICAST) -I../colors -fPIC
 CFLAGS += $(static_incs)
 LFLAGS += $(static_libs)
 LFLAGS += $(LDFLAGS)
+LFLAGS += -fsanitize=undefined
 LDLINKER ?= $(CXX) -shared
 $(shell mkdir -p $(OBJDIR))
 
PhyllisSmith

PhyllisSmith

2021-10-11 21:15

manager   ~0005057

@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.
Andrew-R

Andrew-R

2021-10-11 20:08

reporter   ~0005056

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).
PhyllisSmith

PhyllisSmith

2021-10-11 17:11

manager   ~0005053

@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
Andrea_Paz

Andrea_Paz

2021-10-11 08:13

manager   ~0005048

It works perfectly, thank you.
PhyllisSmith

PhyllisSmith

2021-10-10 20:27

manager   ~0005041

@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.
PhyllisSmith

PhyllisSmith

2021-10-10 19:08

manager   ~0005037

Last edited: 2021-10-10 19:09

View 2 revisions

@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.

Andrea_Paz

Andrea_Paz

2021-10-10 12:12

manager   ~0005030

@PhyllisSmith
If you prepare an appimage I can try to test it on Arch, since it gave MatN problems on Manjaro.
MatN

MatN

2021-10-10 06:07

reporter   ~0005028

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.
PhyllisSmith

PhyllisSmith

2021-10-10 00:25

manager   ~0005024

@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.
MatN

MatN

2021-10-09 19:16

reporter   ~0005023

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)
PhyllisSmith

PhyllisSmith

2021-10-09 00:59

manager   ~0005019

Great! I will be testing this tomorrow and will see if anyone needs me to provide an AppImage so they can test also.
MatN

MatN

2021-10-08 21:31

reporter   ~0005017

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)
MatN

MatN

2021-10-07 15:58

reporter   ~0005016

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.
MatN

MatN

2021-10-05 12:05

reporter   ~0005014

@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.
Andrea_Paz

Andrea_Paz

2021-10-05 08:16

manager   ~0005013

I confirm that the "lv2 plugins reload" has been there from the beginning:

https://www.mail-archive.com/[email protected]/msg02513.html

Issue History

Date Modified Username Field Change
2021-10-05 05:45 MatN New Issue
2021-10-05 08:16 Andrea_Paz Note Added: 0005013
2021-10-05 12:05 MatN Note Added: 0005014
2021-10-07 15:58 MatN Note Added: 0005016
2021-10-08 21:31 MatN File Added: BT588_591.zip
2021-10-08 21:31 MatN Note Added: 0005017
2021-10-09 00:59 PhyllisSmith Assigned To => PhyllisSmith
2021-10-09 00:59 PhyllisSmith Status new => confirmed
2021-10-09 00:59 PhyllisSmith Note Added: 0005019
2021-10-09 19:16 MatN File Added: mwindow.C.zip
2021-10-09 19:16 MatN Note Added: 0005023
2021-10-10 00:25 PhyllisSmith Note Added: 0005024
2021-10-10 06:07 MatN Note Added: 0005028
2021-10-10 12:12 Andrea_Paz Note Added: 0005030
2021-10-10 19:08 PhyllisSmith Note Added: 0005037
2021-10-10 19:09 PhyllisSmith Note Edited: 0005037 View Revisions
2021-10-10 20:27 PhyllisSmith Note Added: 0005041
2021-10-11 08:13 Andrea_Paz Note Added: 0005048
2021-10-11 17:11 PhyllisSmith Note Added: 0005053
2021-10-11 20:08 Andrew-R Note Added: 0005056
2021-10-11 21:15 PhyllisSmith Note Added: 0005057
2021-10-11 22:19 Andrew-R File Added: fsanitize_undefined_and_appimage_crashfix.diff
2021-10-11 22:19 Andrew-R Note Added: 0005058
2021-10-12 01:13 PhyllisSmith Note Added: 0005059
2021-10-12 06:46 Andrew-R Note Added: 0005060
2021-10-12 08:25 Andrew-R File Added: 0001-Attempt-at-fixing-mwindow-appimageDir-NULL.patch
2021-10-12 08:25 Andrew-R Note Added: 0005062
2021-10-12 15:46 PhyllisSmith Note Added: 0005064
2021-10-12 16:38 Andrew-R Note Added: 0005065
2021-10-12 19:46 MatN Note Added: 0005066
2021-10-13 14:29 MatN Note Added: 0005068
2021-10-13 15:51 Andrew-R Note Added: 0005069
2021-10-13 16:22 Andrew-R Note Added: 0005070
2021-10-13 17:33 MatN Note Added: 0005071
2021-10-13 19:11 Andrew-R Note Added: 0005072
2021-10-13 19:17 MatN Note Added: 0005073
2021-10-13 19:59 Andrew-R File Added: 0001-FIX-cli-batchrender.patch
2021-10-13 19:59 Andrew-R Note Added: 0005074
2021-10-13 20:46 MatN Note Added: 0005075
2021-10-14 20:09 MatN File Added: Diff_20211014_MatN.diff
2021-10-14 20:09 MatN Note Added: 0005078
2021-10-14 21:47 PhyllisSmith Note Added: 0005079
2021-10-15 14:56 MatN Note Added: 0005082
2021-10-15 19:09 Andrew-R Note Added: 0005083
2021-10-15 23:12 PhyllisSmith Note Added: 0005084
2021-10-16 00:44 Andrew-R Note Added: 0005085
2021-10-16 00:46 PhyllisSmith Note Added: 0005086
2021-10-16 01:26 Andrew-R Note Added: 0005087