tixml locale / Presets don't parse in locales which don't use "." as a decimal separator

[tavasti]

Edit: this Original description and beginning if discussion is on false assumptions of reason. Read more at the end

When running surge in xubuntu 18.04, bit part of the presets do not produce any sound. With failing presets, surges own output meter stays on zero.

Steps to reproduce the behavior:

1. Download xubuntu install media https://xubuntu.org/download
2. Install to new virtual machine
3. On fresly installed VM, run 'sudo apt install ardour qjackctl'
4. Download surge deb, install it and dependencies
5. Start jack.
6. Run ardour, create project with jack, add midi track, add surge to track, set pinnings correct
7. With init-patch you should get sound. Start selecting another patches, and most of them do not produce any sound

Presets that work:
From beginning of Bass presets:
Bass 10, Deep end, Distorted MW, FM Bass 1
From beginning of pads: Ghost pad

Tested with 48k and 44.1k and different Frames/period (256,512,1024)
Have tested with 2 different USB audio interfaces, and also with virtual machines default audio interface.
Same virtual machine with ubuntu studio live dvd, all presets work, so it is verified that this is not hardware problem.

[baconpaul]

To clarify your last comment: ubuntu studio live dvd, a different distribution, you can load surge and play every preset successfully? So these presets are only broken in your xubunutu install?

Any ideas on what's different between the two distributions?

Thank you for the clear report.

[tavasti]

Yes, with ubuntustudio dvd, everything works. Don't have any idea what is difference. I have also one xubuntu machine where surge works, but at least all libraries which surge depends are identical.

Tried also build surge myself, no difference

[baconpaul]

OK so @jsakkine and I just pushed the first version of a headless which actually plays samples to an internal buffer. This will be useful to understand how it works on your system, especially since you can build the synth. So if you don't mind could you do us the following on the broken system

1. Get the latest master from the surge-synthesizer repo.
2. in your surge directory with latest master do this:

./build-linux.sh clean-all
./build-linux.sh build --project=headless
./build/surge-headless

this will start a program running which plays every pad with a C major scale into a buffer and then prints the min and max sample. You should see output like this

cat = Monosynth (smooth)            ; patch = Glisslead                     ; data = [-0.830299 , 0.831995  ] in 175297 samples
cat = Monosynth (smooth)            ; patch = Moogy saw                     ; data = [-0.195033 , 0.207244  ] in 175297 samples
cat = Monosynth (smooth)            ; patch = Photon                        ; data = [-0.749238 , 0.676538  ] in 175297 samples
cat = Monosynth (smooth)            ; patch = Probability                   ; data = [-0.497653 , 0.500113  ] in 175297 samples
cat = Monosynth (smooth)            ; patch = Sinlead                       ; data = [-0.759511 , 0.756525  ] in 175297 samples
cat = Monosynth (smooth)            ; patch = Smoothness World Cup          ; data = [-0.170652 , 0.261902  ] in 175297 samples
cat = Monosynth (smooth)            ; patch = Smoothy Hollow                ; data = [-0.405379 , 0.534643  ] in 175297 samples

So the question is

1. do you see that output
2. are the patches mostly in the [-1,1] range or are some coming in at [0,0]

Thank you!

[tavasti]

There is few with 0-0, but they are really minor.

$ build/surge-headless > headless.log  # did not quit itself, but hit ctrl-c when file stopped growing
$ grep patch headless.log | wc
    477    8513   61056
$ grep patch headless.log  | grep '0.000000.*0.000000' 
cat = Bass                          ; patch = Slow                          ; data = [-0.000000 , 0.000000  ] in 175297 samples
cat = Monosynth                     ; patch = Log Log                       ; data = [-0.000000 , 0.000000  ] in 175297 samples
cat = Monosynth                     ; patch = Radon                         ; data = [-0.000000 , 0.000000  ] in 175297 samples
cat = Monosynth (hard)              ; patch = Resofest 3                    ; data = [-0.000000 , 0.000000  ] in 175297 samples
cat = Pluck                         ; patch = Nice pluck 2                  ; data = [-0.000000 , 0.000000  ] in 175297 samples
cat = Pluck (fast)                  ; patch = Lofipluck                     ; data = [-0.000000 , 0.000000  ] in 175297 samples
cat = Pluck (fast)                  ; patch = Molusk                        ; data = [-0.000000 , 0.000000  ] in 175297 samples

Some are outside [-1,1], does that say something?
cat = Monosynth ; patch = Fundament ; data = [-5.162785 , 6.026954 ] in 175297 samples

[baconpaul]

Yeah I think that outside the -1,1 range is either misconfigured patch or the ‘pop n click’ problem. I’m still investigating

But if most work for you in headless then it is a mystery what causes them to not sound in synth. The control paths aren’t that different. That makes me think something is broken between surge doing a process in the synth and the host transporting it to the control somehow. But I’m not sure how.

Bit of a loss right now. I’ll continue pondering. Than you for testing.

[tavasti]

1. could it be possible that most patches would be producing only pop'n'click, and that is why this test does not reveal anything? What about calculating RMS or average of absolute values?

2. When using those 'broken' patches even surges own level meter does not react.
   Is there some similarities on those patches I said are working?

[baconpaul]

1. Unlikely but good to check - if you want to add rms it’s oretty obvious where it is in arc/headless/main.cpp. Fire a pr in if you do!

2. Haven’t looked for a similarity yet. But the patches work on Mac and work on other linuxen as you pointed out

I have a theory that there’s a bit of ininitialized memory somewhere which is causing things to wedge. Your problem, the other click and pop, then wedge when changing sample rate on Mac. But I haven’t found it yet.

[baconpaul]

Heya I was in there anyway and #721 adds RMS and L1 norms to the headless display. Should push it in the next 20 minutes. When I do can you try again?

Thanks!

[tavasti]

Tested, and RMS or L1 values do not explain anything. Playing and non-playing patches are on same ranges.

[tavasti]

Opening my eyes: on 'broken' patches, Output volume slider is in zero. So next thing is to find out why on certain systems it is zero, or at least fix it not to be zero.

I don't think this is only thing that is somehow weird, but at least towards better.
When putting volume slider up, for example Behemoth and Crush Bass will play only one 'click' on note-on event.

If you push something (can be branch also) I can rebuild & test.

[baconpaul]

OK useful to know. It is not the raw synth it is somewhere in the synth -> plugin -> audio layer.

That “output slider at zero” - that’s the surge output volume slider?

[tavasti]

Yes, on 'Output' section, 'volume' slider, above 'pan' slider. And that value is from patch, so it is individually set for every patch, and for most of the patches it is on zero.

[tavasti]

Looking at Behemoth and Crush Bass presets:

• In addition of zero on volume, on osc selection, oscilators are muted or level zero. No surprise they make no sound

[tavasti]

[esaruoho]

this is what i see on behemoth on macOS:

this is what i see on crushbass on macOS:

[tavasti]

Indeed, your preset makes much more sense :-)
There is also other differences, take a look on Osc 1 shape.

Preset loading has some strangeness

[esaruoho]

@tavasti can ya zip the presets up and attach them so i can attempt to load them just to see if the files themselves are somehow magically corrupted?
i'll send mine now

[esaruoho]

macOS_behemoth_Crushbass.zip

[tavasti]

ubuntu_behemoth_Crushbass.zip

[esaruoho]

tried a diff:

$ diff Behemoth.fxp UbuntuBehemoth.fxp 
[74%, charging, 1:38 left] [Tue Mar 05 16:21:09] [18513mb free/99% full] [~/Documents/Surge] 
$ diff Crush\ bass.fxp UbuntuCrushbass.fxp 
[74%, charging, 1:38 left] [Tue Mar 05 16:21:16] [18507mb free/99% full] [~/Documents/Surge] 

seems like the files have no differences.. if diff is how i figure this thing out, that is :)

[esaruoho]

KaleidoscopeApp also reports the same thing. they're the same file. so something funky going on with Stock Xubuntu :O

[tavasti]

Running gdb on vst plugin load may be tricky? So some debug prints?

Where is preset loading code?

[baconpaul]

Follow from surgesynthesizer::loadpatch downwards

Print and run in Carla single is how I debug Linux mostly yeah

[tavasti]

Loading with carla-single, patches seem to load presets ok (volume not zero), but when loading with carla, mixbus or ardour, they don't.

[baconpaul]

huh. Well that's concerning.

[baconpaul]

have a bit of time this morning. let me build a xubuntu and try and replicate.

"something" is wrong.

[tavasti]

This shows where it goes wrong:

If value is 0.xxxxx it will result 0. So all values below 1 will result 0. Why, don't really know....

diff --git a/libs/xml/tinyxml.cpp b/libs/xml/tinyxml.cpp
index 359f8ca..2a1a567 100644
--- a/libs/xml/tinyxml.cpp
+++ b/libs/xml/tinyxml.cpp
@@ -23,6 +23,7 @@ distribution.
 */
 
 #include <ctype.h>
+#include <iostream>
 #include "tinyxml.h"
 
 #ifdef TIXML_USE_STL
@@ -1126,8 +1127,11 @@ int TiXmlAttribute::QueryIntValue( int* ival ) const
 
 int TiXmlAttribute::QueryDoubleValue( double* dval ) const
 {
-       if ( sscanf( value.c_str(), "%lf", dval ) == 1 )
+  std::cout << "str " << value.c_str() << " ";
+  if ( sscanf( value.c_str(), "%lf", dval ) == 1 ) {
+    std::cout << "res "<< *dval << std::endl;
                return TIXML_SUCCESS;
+  }
        return TIXML_WRONG_TYPE;
 }

[tavasti]

Now I got it, it is locale dependent. I have LC_NUMERIC=fi_FI.UTF-8

[tavasti]

Everything works with:
export LC_NUMERIC=en_US.UTF-8

[baconpaul]

OK that generally didn't work very well. I got a VM working in parallels but couldn't get the resolution off of 800x600
Also I've never used ardour and couldn't make it scan plugins
probably because I couldn't see the UI

Any tips on how to do those steps?

[tavasti]

Forget that xubuntu, most likely any linux with 'export LC_NUMERIC=fi_FI.UTF-8' will fail. And maybe also OSX and Windows also?

[baconpaul]

ooooh that sounds like some very good debugging. What have you found?

[baconpaul]

I'll test OSX now too with that set

[tavasti]

Problem is sscanf( value.c_str(), "%lf", dval )
sscanf depends on locale, and locale where comma is decimal separator, things go wrong.

And same problem is also most likely on saving presets.

[baconpaul]

Right. That makes perfect sense

I don't have fi locale installed on my machines. but this is a very very good find.

[baconpaul]

it's only two places in tinyxml which use sscanf for double values (there's a few which use it for integers and hex but that's all fine).

[baconpaul]

sudo apt-get install language-pack-fi

then this program

#include <iostream>
#include <cstdlib>
#include <stdlib.h>
#include <clocale>

int scanDemo()
{
	double d = 1.23;
	std::string s = "1.23";

	double r;
	sscanf( s.c_str(), "%lf", &r );
	std::cout << "r=" << r << " from sscanf of '" << s << "'" << std::endl;
	r = ::atof(s.c_str());
	std::cout << "r=" << r << " from atof of '" << s << "'" << std::endl;
	char *pEnd;
	r = strtod(s.c_str(), &pEnd);
	std::cout << "r=" << r << " from strtod of '" << s << "'" << std::endl;


	return 0;
}

int main()
{
	std::setlocale(LC_ALL, "en_US.utf8");	
	scanDemo();
	std::setlocale(LC_ALL, "C");
	scanDemo();
	std::setlocale(LC_ALL, "fi_FI.utf8");
	scanDemo();
	return 0;
}

produces this output

r=1.23 from sscanf of '1.23'
r=1.23 from atof of '1.23'
r=1.23 from strtod of '1.23'
r=1.23 from sscanf of '1.23'
r=1.23 from atof of '1.23'
r=1.23 from strtod of '1.23'
r=1 from sscanf of '1.23'
r=1 from atof of '1.23'
r=1 from strtod of '1.23'

There is no std library function which uses locale("C") by default.

So I think we need to go to those sscanf and sprintf calls and do a setlocale("C") around them

but I'm out of my depth here. I have not done a lot of locale specific programming. Your thoughts?

[baconpaul]

here's what a solution looks like

paul@ubuntu:~/lcl$ more < main.cpp 
#include <iostream>
#include <cstdlib>
#include <stdlib.h>
#include <clocale>
#include <sstream>

int scanDemo()
{
	double d = 1.23;
	std::string s = "1.23";

	std::stringstream dataStream;
	dataStream.imbue(std::locale("C"));
	dataStream << s;
	double r;
	dataStream >> r;
	std::cout << "R=" << r << std::endl;

	return 0;
}

int main()
{
	std::setlocale(LC_ALL, "en_US.utf8");	
	scanDemo();
	std::setlocale(LC_ALL, "C");
	scanDemo();
	std::setlocale(LC_ALL, "fi_FI.utf8");
	scanDemo();
	return 0;
}
paul@ubuntu:~/lcl$ ./a.out 
R=1.23
R=1.23
R=1.23

[baconpaul]

So @tavasti do you think you can use that stream imbued with locale change to modify the sscanf and sprintf in the tinyxml lib?

I confirmed it works on macOS and linux both. We can check windows if you get a PR together.

[baconpaul]

And great find! Thank you! I would (obviously) have never found that!!

[tavasti]

My C++ is badly rusted while not used in 15 years, but with that clear example I should be capable to do it.

[baconpaul]

OK! If you get stuck I can bang it out in the pipeline also. Not that hard. But if you do get it together that would be wonderful.

Give me a minute and I'll give you a crystal clear template. Sound good?

[tavasti]

Sounds good. Busy right now, but I expect this to get this done in next 48 hours.

[baconpaul]

OK here's a great sample to follow

#include <iostream>
#include <cstdlib>
#include <stdlib.h>
#include <clocale>
#include <sstream>
#include <string.h>

double sscanfDoubleReplacement( const char* c )
{
	std::stringstream dataStream;
	dataStream.imbue(std::locale::classic());
	dataStream << c;
	double r;
	dataStream >> r;

	return r;
}

void sprintfDoubleReplacement( double d, char* c, int n )
{
	std::stringstream dataStream;
	dataStream.imbue(std::locale::classic());
	dataStream << d;
	std::string s;
	dataStream >> s;
	strncpy( c, s.c_str(), n );

	return;
}

void scanDemo()
{
	double kindaPi = 3.14;
	std::string eStr = "2.718";

	double sd;
	sscanf( eStr.c_str(), "%lf", &sd );
	std::cout << "sscanf=" << sd << "  replacement=" << sscanfDoubleReplacement( eStr.c_str() ) << std::endl;

	char bufA[1024], bufB[1024];
	sprintf( bufA, "%lf", kindaPi );
	sprintfDoubleReplacement( kindaPi, bufB, 1024 );
	std::cout << "sprintf=" << bufA << " replacement=" << bufB << std::endl;
}

int main()
{
	std::setlocale(LC_ALL, "en_US.utf8");	
	scanDemo();
	std::setlocale(LC_ALL, "fi_FI.utf8");
	scanDemo();
	return 0;
}

which gives this output for me

paul@ubuntu:~/lcl$ ./a.out 
sscanf=2.718  replacement=2.718
sprintf=3.140000 replacement=3.14
sscanf=2  replacement=2.718
sprintf=3,140000 replacement=3.14

[baconpaul]

And I see 4 printfs and 2 scanf in libs/tinyxml which need changing. Thanks!

[tavasti]

PR created. Replacement of sprintf in tinyxml.cpp are not tested, because I don't know even if they are used.

Preset writing sprintf is in src/common/Parameter.cpp