Lmms: ZynAddSubFX resets its sound to default

[Seedix1]

An issue in LMMS 1.2.0-rc4. When you open ZynAddSubFX and you create a sound and then close ZynAddSubFX, it resets the sound to default.
The sound resets when you click on "Show GUI" to close the GUI.

Running Windows 10/64 bit

I'm adding a video of me showing that issue: https://youtu.be/NJ4zkQKZa74

[tresf]

Possibly related to https://github.com/LMMS/lmms/pull/3688#issuecomment-333741046

[PhysSong]

@Seedix1 Could you try this and tell me what happens?

1. Drop a ZynAddSubFX preset into song editor.
2. Play a note.
3. Open ZynAddSubFX GUI.
4. Play the same note.

If it sounds different after opening GUI, it can be a permission error or something similar.

[Seedix1]

@PhysSong I did it and it sounds different after opening GUI. It resets to the default sound again.

[PhysSong]

It seems to be a QTemporaryFile issue. Does it occur if you run LMMS as administrator?

[zonkmachine]

It seems to be a QTemporaryFile issue

Related? https://github.com/LMMS/lmms/issues/2854

[tresf]

Does it occur if you run LMMS as administrator?

We don't set permissions on files and this plugin hasn't changed. I would advise against this advise on all platforms. It can cause more problems with other files created.

[musikBear]

RC4 win32 xp3
@PhysSong I cant reproduce this bug.
The method-steps you advised here:
https://github.com/LMMS/lmms/issues/3855#issuecomment-334069243
i have exact the same output from Arpeggio| Arp0007

• before i open the zasfx gui
• during opening of gui
• after closing gui

Can others reproduce on win32? / win64??

[PhysSong]

@Seedix1 Does your user name contain non-ASCII character? It may cause several problems.

[Seedix1]

@PhysSong Yes it does. In 1.1.3 ZynAddSubFX worked perfectly though.

[musikBear]

@Seedix1 is your win 10 Enterprise ed. ?
There is an issue with unsigned software afaik

[Seedix1]

@musikBear It's Windows 10 Home

[musikBear]

@Seedix1 the issue here is that it is not a general fail. It must be local on your pc. What i would do was to backup all own stuff, remove lmms completely (including config file), and then reinstall in a folder structure with only ISO characters in all names and path. It would be great if you would report the result back

[PhysSong]

I can confirm this bug. If I use non-ASCII username, Zyn resets sound when toggling GUI as @Seedix1 experienced.
@musikBear I think it is a general problem.

[tresf]

Possible duplicate of #2662, #1191.

Somewhat related because of UTF8-ness: #1998 #1325 #1933 #1995

[PhysSong]

Yes. Seems related. For Zyn(this issue) and VST(#2662), we may re-convert UTF-8 string into QString and somehow use that.
However, fixing the issue for samples(#1191) looks tricky since we use 3rd-party libraries (ex. libsndfile) which expects C-style string(or std::string).

[tresf]

, fixing the issue for samples(#1191) looks tricky since we use 3rd-party libraries (ex. libsndfile) which expects C-style string(or std::string).

I saw that too. QSTR_TO_STDSTR, QString::fromStdString(...) and this Windows logic should do the trick, no?

[tresf]

... actually, read the 5th comment here... https://stackoverflow.com/a/4214397/3196753. Seems Qt5 has improved toStdString() a bit. Platform compat may be a moot point?

[PhysSong]

Clicked wrong button... :/

[PhysSong]

The problems are:

• QString::toLocal8Bit() is lossy, so it can't represent some characters(ex. Ѭ for english code page).
• Our QtXmlWrapper is used for both communication with LMMS and internal file loading/saving, so we should make sure fixing one doesn't break the other.

[PhysSong]

QTemporaryFile path issue also affects VST chunk loading/saving. According to QDir documentation, it seem to use TEMP or TMP enviornment variable. I think we may force QTemporaryFile to use some appropriate path (ex. C:\Temp) on windows.

[tresf]

@PhysSong we should use the system value of temp. C:\Temp isn't valid and we shouldn't make assumptions. I think we just need to support UTF8 properly.

[musikBear]

@PhysSong it general, IF user has non-ISO characters in their local installation -Thats why i asked him to remediate that.
Non-ISO chars will also hamper usage of VSTs on windows. It is however news to me, that even a username with non-ISO chars, can cause this 👻 ..spooky

[PhysSong]

@Seedix1 Could you set your Tmp and/or TEMP environment variable to a path without non-ASCII characters and test if it works?

[PhysSong]

Okay. I found a solution for most filename encoding issues: open a file with QFile first, and then re-open it using fdopen() or something similar.
The only exception is VST dlls. LMMS uses LoadLibrary() to load VST dlls. I think converting utf-8 to wide char(with Win32 API or something else) and using LoadLibraryW().

[tresf]

@PhysSong good find. Note, this article suggests to define the character type as TCHAR and LoadLibrary will automatically switch between LoadLibraryA and LoadLibraryW for you.
https://stackoverflow.com/a/5208515/3196753 (although the answer at the bottom suggests wide support is built-in since Windows 2000, so UTF8 to wide should be fine for 100% of use-cases).

[PhysSong]

Note, this article suggests to define the character type as TCHAR and LoadLibrary will automatically switch between LoadLibraryA and LoadLibraryW for you.

Right. In this case, however, I think LoadLibraryW() should be used explicitly to support every unicode characters. I think calling LoadLibraryA() is bad there.

[tresf]

Right. In this case, however, I think LoadLibraryW() should be used explicitly to support every unicode characters. I think calling LoadLibraryA() is bad there.

Agreed. The data should never be ANSI, LoadLibraryW() will work 100% of the time.

[PhysSong]

This issue seems to affect loading/saving VST settings, as well as ZynAddSubFX.

[PhysSong]

Both QFile and wide-char Windows API functions work fine with such file/directory names, even with emoji characters(ex. 뼱؁ѪѨ😃.dll). I'll create a pull request as soon as I can.

[PhysSong]

Here's the list of file name issues that I found so far:

• ZynAddSubFX setting loading/saving (this)
• VST DLL loading (#2662)
• VST setting loading/saving
• Sample file decoding except OGG Vorbis (#1191)
• .wav exporting
• .gig file loading
• .pat file loading
• MIDI import
• STK rawwave path (on every platform, Can crash LMMS!)
• Carla: potential issues when added into Windows build

Here's the plan to fix the issue:

• ZynAddSubFX, .wav export, .pat loading, libsndfile import: Use QFile to get file descriptor and use it
• VST: Use _wfopen and _wopen for Windows build, load library with LoadLibraryW
• DrumSynth: Either rewrite using QFile or use std::stringstream
• MIDI import: Read file with QFile first and then pass istream with the data
• .gig: Hard to work around. Creating a child class of RIFF::File might help, but I expect that it can be easily broken.
• STK: Can't find any workaround.

I'm going to fix first two thing(easier and safer than others) for 1.2 soon. I'll also fix more bugs in 1.2 if the fix is safe.