Bug 642 – Built-in generators produce silence after running Nyquist effect when system locale uses comma separator

••• Introduction •••
••• Keywords •••

Audacity 3.0.3 development began 19th April 2021

Audacity Bugzilla

Bug 642 - Built-in generators produce silence after running Nyquist effect when system locale uses comma separator


Summary:	Built-in generators produce silence after running Nyquist effect when system ...

Status:	RESOLVED FIXED

Product:	Audacity
Classification:	Unclassified
Component:	Built-in FX
Version:	2.0.4
Hardware:	PC Windows and Linux

Importance:	P3 Repeatable
Assigned To:	Default Assignee for New Bugs

URL:
Keywords:	nyquist, patch_ready

Depends on:
Blocks:
	Show dependency tree / graph

Reported:	2013-08-04 13:46 UTC by Gale Andrews
Modified:	2018-08-20 11:51 UTC (History)
CC List:	6 users (show)

See Also:
Steps To Reproduce:	1 Ensure Audacity is running in System or German language then exit Audacity. 2 Open Windows Control Panel then choose "Region and Language". Change the format to "German (Germany)". 3 Launch Audacity. 4 Erzeugen > Tongenerator(1) and OK. 5 Effekt > Cross Fade In. 6 Click in the background then Erzeugen > Tongenerator(1) and OK. 7 Effekt > Cross Fade Out. 8 Click in background then Erzeugen > Tongenerator(1) and OK. The generate at Step 8 should fail as absolute silence. If it does not, repeat steps 7 and 8.
Release Note:	GROUP: Effects and Analysis * (Windows and Linux, where system locale uses comma as decimal separator) '''[http://manual.audacityteam.org/o/man/generate_menu.html#generators Built-in tone and noise generators] may produce silence''' if used on audio that has had a [http://manual.audacityteam.org/o/man/effect_menu.html#Nyquist_Effects Nyquist] effect applied to it. '''Workaround:''' Run Audacity in English [http://manual.audacityteam.org/o/man/faq_about_audacity.html#language language], or change the system locale to English and run Audacity in the local language.
First Git SHA:
Group:	---
Workaround:
Closed:	2018-08-20 00:00:00

Attachments
Patch to correct the locale setting for nyquist (909 bytes, patch) 2013-10-03 19:25 UTC, Leland Lucius	Details \| Diff
Make normalize access locale specific decimal separator (480 bytes, patch) 2013-10-07 12:49 UTC, Leland Lucius	Details \| Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Gale Andrews 2013-08-04 13:46:58 UTC

Problem tested and observed with German, French and Italian system locale. 

2.0.0 does the same so not a regression on 2.x.  

1.3.9 appears to be the first release that has this problem. 

Cannot reproduce the problem on Mac.

Comment 1 Leland Lucius 2013-10-03 19:25:37 UTC

Created attachment 398 [details]
Patch to correct the locale setting for nyquist

Prior to invoking nyquist, the locale has to be changed because nyquist understand English formatted numbers and then changed back when it is done.

The setlocale() function is used to do the switching.  The query call that asks what the current locale is returns a pointer to a C library maintained buffer.  There's no guarantee that buffer will be valid from call to call and what was happening is that the call that reset the locale back to the original value was passing in an invalid string and the call was failing, so it never reset the locale back.

Anyway, I changed it to use Widgets flavors of the function and stored the returned value in a wxString.  That gets used in the reset call.

Gale, I don't know how you ever cam up with those steps.  I tried other variants and just gave up because I couldn't find another pattern.  :-)

Leland

Comment 2 Gale Andrews 2013-10-06 22:32:34 UTC

Patch works well for me on Windows and also appears to fix bug 173.  Steve is testing it on Linux.

Comment 3 Steve Daulton 2013-10-07 09:38:01 UTC

(In reply to comment #2)
The patch works for me (tested on Linux Mint).
It also fixes bug 173.

However, most effects, post patch, will not accept a dot as the decimal separator. This may well confuse affected users. I've raised this point on -quality.

Comment 4 Leland Lucius 2013-10-07 12:49:00 UTC

Created attachment 408 [details]
Make normalize access locale specific decimal separator

Comment 5 Steve Daulton 2013-10-07 18:06:17 UTC

(In reply to comment #4)
With "Make normalize access locale specific decimal separator" patch, Normalize now displays a comma as decimal separator (tested on Mint with Deutsch locale).

Comment 6 Gale Andrews 2013-10-08 00:20:18 UTC

Marked "patch_ready" for "Make normalize access locale specific decimal separator" (tested on Windows in German locale).  

"Patch to correct the locale setting for Nyquist" committed in r12655. 

"Time to place first label" and "Label interval" in Analyze > Regular Interval Labels do not accept comma in German locale, but I believe this is a general problem with Text Input Widgets.

So given we're saying in Release Notes changelog that we fixed the comma separator issue, we should IMO have a P3 for comma separator does not work in Text Input Widgets (in respect of the shipped Regular Interval Labels). Or we could give those two controls a temporary slider in Regular Interval Labels, then make the bug P4.

Comment 7 Vaughan Johnson 2013-10-08 00:50:43 UTC

Re: "Patch to correct the locale setting for nyquist" -- does MB's (Markus's) comment above the change need to be updated, or maybe removed? Is the "uselocale()" reference relevant?

Comment 8 Gale Andrews 2013-10-08 02:35:00 UTC

" Make normalize access locale specific decimal separator " committed in r12658.

Comment 9 Leland Lucius 2013-10-08 19:42:12 UTC

(In reply to comment #7)
> Re: "Patch to correct the locale setting for nyquist" -- does MB's (Markus's)
> comment above the change need to be updated, or maybe removed? Is the
> "uselocale()" reference relevant?

It really is still relevant.  For instance, using setlocale() changes the locale for every thread the Audacity has active.  This isn't currently a problem (I don't think) since all of the non-main threads don't really care much about locale.  Except for maybe filenames in DirManager (the only one I can think of just now).

We could switch from wxSetlocale() to wxLocale, but I just had a peek at wxLocale and for all platforms except Windows, setlocale() is used under the covers.  For Windows, SetThreadLocale() is used so it is already thread specific.

Leland

Comment 10 Vaughan Johnson 2013-10-09 01:16:13 UTC

(In reply to comment #9)

>> Re: "Patch to correct the locale setting for nyquist" -- does MB's (Markus's)
>> comment above the change need to be updated, or maybe removed? Is the
>> "uselocale()" reference relevant?
> 
> It really is still relevant. 

Thanks. Added a code comment linking to your Bugzilla comment.