ilia3101 / MLV-App

All in one MLV processing app.
https://mlv.app/
GNU General Public License v3.0
283 stars 29 forks source link

QtApp: Crash on enabling filter #74

Closed seescho closed 6 years ago

seescho commented 6 years ago

With no mlv loaded: no crash With a mlv loaded: crash With qt-creator in debug-mode: no crash ???

When I enable filter, mlvapp freezes. After 10 seconds mlvapp crash:

Speicherzugriffsfehler (Speicherabzug geschrieben)

I went back to this commit: "fixed compilation error saying type 'uint16_t' is not defined" Result: crash

My compile-log tell me this warning for a implicit declaration:

g++ -o filter.o ../../src/processing/filter/filter.c ../../src/processing/filter/filter.c: In function ‘initFilterObject’: ../../src/processing/filter/filter.c:41:24: warning: implicit declaration of function ‘fmemopen’; did you mean ‘freopen’? [-Wimplicit-function-declaration]

No clue, if that´s important for the crash

I can reproduce the crash with opensuse-leap and opensuse-tumbleweed

See also discussion on ml-forum: https://www.magiclantern.fm/forum/index.php?topic=20025.msg197099#msg197099

masc4ii commented 6 years ago

@bouncyball-git & @ilia3101 : is it necessary to have in filter.c (line 25) this linux ifdefs? If #else is working on OSX and Windows, why not on Linux? Would simplify this part... and maybe fix this bug?! ;-)

#ifdef __linux__
    file = fmemopen(text, strlen(text), "rb");
#else
    file = fopen("__filter_temp__", "wb");
    fputs(text, file);
    fclose(file);
    file = fopen("__filter_temp__", "rb");
#endif

    return file;
}

#ifdef __linux__
#define close_filter(filter) fclose(filter);
#else
#define close_filter(filter) { fclose(filter); remove("__filter_temp__"); }
#endif
bouncyball-git commented 6 years ago

I personally never experienced this problem or crashes on ubuntu but we can get rid of fmemopen and use temp files for all OSes.

fmemopen needs _POSIX_C_SOURCE >= 200809L to not emmit this warning:

include

   FILE *fmemopen(void *buf, size_t size, const char *mode);

Feature Test Macro Requirements for glibc (see feature_test_macros(7)):

   fmemopen():
       Since glibc 2.10:
           _POSIX_C_SOURCE >= 200809L
       Before glibc 2.10:
           _GNU_SOURCE
bouncyball-git commented 6 years ago

Commit 9ca92de7d87c8afd7d940c486f5dde1bf8122083

masc4ii commented 6 years ago

@bouncyball-git : Is that enough? Does that delete the temp file on Linux?

#ifdef __linux__
#define close_filter(filter) fclose(filter);
#else
#define close_filter(filter) { fclose(filter); remove("__filter_temp__"); }
#endif
bouncyball-git commented 6 years ago

Haha Yes! Right!!! commited.

seescho commented 6 years ago

No succcess. First freeze and then crash: QT-Creator tells me: The process was ended forcefully. I don´t understand, why this works in QT-Creator in debug-mode without any crash, but in normal mode the crash appears.

Not sure at the moment, what I can do to nail down his bug?

seescho commented 6 years ago

I will load an older version of my OS into a VM and play with it. Maybe filter and Tumbleweed together have hickup at the moment ???

seescho commented 6 years ago

filter.c line 86 ff

/* We need a copy or it gets messed up on many threads */
    genann * net;

    if (filter->filter_option == 0)
        net = genann_copy(filter->net_fj);
    else if (filter->filter_option == 1)
        net = genann_copy(filter->net_vis3);
    else if (filter->filter_option == 2)
        net = genann_copy(filter->net_p400);
    else if (filter->filter_option == 3)
        net = genann_copy(filter->net_toyc);
    else if (filter->filter_option == 4)
        net = genann_copy(filter->net_sepia);

if I set a return right after genann * net: no crash if move this return to the end of the "if/else if"-construction: crash

Looks like something happens in genann...

seescho commented 6 years ago

genann.c line 153 ff

genann *genann_copy(genann const *ann) {

    const int size = sizeof(genann) + sizeof(double) * (ann->total_weights + ann->total_neurons + (ann->total_neurons - ann->inputs));
    genann *ret = malloc(size);
    if (!ret) return 0;

    memcpy(ret, ann, size);

    /* Set pointers. */
    ret->weight = (double*)((char*)ret + sizeof(genann));
    ret->output = ret->weight + ret->total_weights;
    ret->delta = ret->output + ret->total_neurons;

    return ret;
}

if I replace the "return ret" by "return 0" : no crash

Now I don´t know, how to proceed ...

bouncyball-git commented 6 years ago

Is it happening for all MLV clips?

seescho commented 6 years ago

Yes, happens with the old ones, with new ones, happens with mlvs from 600D and from 6D, happens with 10 bit too.

masc4ii commented 6 years ago

filter.c line 86 ff

/ We need a copy or it gets messed up on many threads / genann * net;

if (filter->filter_option == 0)
    net = genann_copy(filter->net_fj);
else if (filter->filter_option == 1)
    net = genann_copy(filter->net_vis3);
else if (filter->filter_option == 2)
    net = genann_copy(filter->net_p400);
else if (filter->filter_option == 3)
    net = genann_copy(filter->net_toyc);
else if (filter->filter_option == 4)
    net = genann_copy(filter->net_sepia);

if I set a return right after genann * net: no crash if move this return to the end of the "if/else if"-construction: crash

Looks like something happens in genann...

Sounds like the filter parameters were not read (at all or wrong). I had that when inserting new filters and had forgotten some lines...

seescho commented 6 years ago

It took some time to find out, how to share a folder between host and guest in KVM/QEMU...

I installed openSUSE Leap 42.3 in a VM. That´s the standard openSUSE-version. In Leap I have the same crash... So it´s not Tumbleweed, which forces this crash.

I would like to see the content of some variable (like net). Can someone give me a code-snippet, please, which I can insert?

bouncyball-git commented 6 years ago

Ehh... I don't think fmemopen was the reason of the crash. The bad thing is I can not reproduce this on ubuntu. @seescho: can you share the MLV (or small part of it) which crashes the app? Maybe then I could reproduce this.

masc4ii commented 6 years ago

Get that crash on Ubuntu 12.04 and Qt5.6.3 as well. Unfortunately I don't get a debugger to work on this VM. Crash is in line 155 in genann.c, called from applyFilterObject(..) malloc(size) ann is 0 before. So with a additional line if(!ann) return 0; there is no crash, but also no filter... It looks like the filter read in fails: filter->net_fj and all others are 0.

bouncyball-git commented 6 years ago

Why the f... the filter can not be read on a particular Linux distro? I'm lost...

No crashes on ubuntu 16.04. Never experienced the crash using filters.

BTW I'm using gcc 5.4.0. May be it's that?

masc4ii commented 6 years ago

No idea. I'm using gcc 4.6.3.

ilia3101 commented 6 years ago

So does this appear to be caused by things going on inside genann library?

ilia3101 commented 6 years ago

Finally a crash I didn't directly cause

ilia3101 commented 6 years ago

Filter files cannot be read for some reason?

masc4ii commented 6 years ago

Yes I think so - I don't know if they cannot be read. What I know: when applyFilter...(...) is called filter->net_... are 0.

ilia3101 commented 6 years ago

Uhh maybe I'll have to find some linux VM I have and see if I can do anything.

masc4ii commented 6 years ago

Okay, for me genann_read(..) returns with NULL. Just one thought: what happens, if mlvapp has no write permission? I also can't export any file on the desktop via mlvapp on this Linux. I guess mlvapp can't write the temp files - so it can't read - then the filter is not loaded the right way, and if filter applied - boooom.

ls -al brings -rwxrwxr-x for mlvapp.

ilia3101 commented 6 years ago

Well maybe fmemopen was only likely to be helping rather than causing problems? As it requires no write permission...

See if putting it back in will help in your case

masc4ii commented 6 years ago

But it was crashing before with fmemopen too. (that's why this bug exists)

bouncyball-git commented 6 years ago

BTW congratulations with passing the 1000 commits :)

Please try the latest commit 77a0740052011f8c73f3f64dc2d5f1054286c1ff

Yes I know that now all this huge filters are on stack but just try if this helps :)

Here on my Linux it works without hiccup...

masc4ii commented 6 years ago

Have no crash... but it does not look as it should look... white, blue, pink, pictures... :stuck_out_tongue_winking_eye: ...on linux - OSX & Win32 is fine. "Huge filters"?... some kB?!

ilia3101 commented 6 years ago

I get nice filtered pictures on ubuntu mate vm no problems 😕

ilia3101 commented 6 years ago
Yes I know that now all this huge filters are on stack but just try if this helps :)

Why will they be huge?

masc4ii commented 6 years ago

linux

bouncyball-git commented 6 years ago

Haha :) When I was a kid I started programming in the end of 80s 88-89 and 640kb mem and 20mb HDD was huge... so it's a habit - everything that is more then 32bytes long is huge for stack ;) (well, I'm kidding)

But seriously on my ubuntu everything works fine, those false colors mean that values are not read correctly.... hmmm...

ilia3101 commented 6 years ago

Argh why is it broken again :(( well, still working perfect for me

And I'm extremely confused by that OS

seescho commented 6 years ago

Back from work... Cloned from git and compiled 2 minutes ago. Result: No more crash on enabling filter but funny colors. I guess, a mlv from me isn´t needed anymore?

bouncyball-git commented 6 years ago

One more question: are you both using 64 bit or 32 bit Linux?

bouncyball-git commented 6 years ago

Some more polishing, please try again. commit c62e3345f12cb8a27a6dc86f7850305561c4c655.

seescho commented 6 years ago

64 bit Linux

All looks fine now. The funny colors came in from the strengh slider in the filter section, which was set to 100.

bouncyball-git commented 6 years ago

@seescho: Now slider works? You can set it to whatever value you want and no funny colors?

bouncyball-git commented 6 years ago

@masc4ii Please confirm that it works with your linux VM as well :)

masc4ii commented 6 years ago

Hm... also with a 100 on the slider the colors shouldn't be "funny". I use 32bit ubuntu. Will try tomorrow.

masc4ii commented 6 years ago

Funny colors are still there.

bouncyball-git commented 6 years ago

Do they appear on 100 of the slider or in any case?

bouncyball-git commented 6 years ago

I have one last fix that I can think of for maybe 32bit linux issue.

masc4ii commented 6 years ago

The slider does what it should do - sure: at 100 it is the strongest, at 0 there pic is okay. BTW: Win 32bit is fine here.

bouncyball-git commented 6 years ago

I mean bad colors appear when any other value than zero is set?

bouncyball-git commented 6 years ago

@masc4ii

In your 32bit linux vm do 1 experiment please. Pull last commit, compile and run.

The log output in console will print double values of the filter. please compare them to the originals in film.h. (they will be printed as double not as long double like in film.h, genann uses double anyway) If values matched there is something wrong but not with reading of the filter.

If values differ then:

In genann.c in function genann_read() comment this line: ann->weight[i] = strtod(strtok(NULL, " \t\n"), NULL); and uncomment this line: //sscanf(strtok(NULL, " \t\n"), "%le", &ann->weight[i]);

Do the same comparison

masc4ii commented 6 years ago

All values look like integers: +/-x.000000e+00, without changing anything in the code. With that change - the same - also all x.00000.... Just the filter itself seems to be wrong. The linear amount of strength is just fine.

masc4ii commented 6 years ago

Got it: it is a localization problem: if I change the dots to commas in film.h, it works :stuck_out_tongue_closed_eyes:

What can we do here!? Is it possible to change that in the std library?

bouncyball-git commented 6 years ago

HUH! s..t :) Now everything gets their places. Escho has german loc as you do, but me always use us loc.

I don't know really how to deal with this. Inet is our friend :)

ilia3101 commented 6 years ago

So because of the dots/commas it will now be dependant on where in the world it is compiled on Linux?

ilia3101 commented 6 years ago

Something like:

#ifdef GERMANY
#define DOT ","
#else
#define DOT "."
#end

If there is a way to test for country...