Nyquist effects overflow at > 2^31 samples

[petersampsonaudacity]

Describe the bug
Nyquist effects may not work if used on extremely long selections''' containing more than 2^31 samples (just over 13.5 hours at 44100 Hz).

Originally logged on Bugzilla as P3 439 in 2011

To Reproduce
Steps to reproduce the behavior:
Example 1.
Make a selection of 14 hours duration at 441000 Hz and enter either of the following commands into the Nyquist Prompt:
(get-duration 1)
(print len)

Example 2.
Generate 14 hours of silence at 44.1 kHz then run
(s-max s (const 0.5))

Example 3.
Just run delay.ny on a 14 hour 44100 Hz tone.

Example 4.
Evaluate (snd-play (s-rest (* 15 60 60))). SND-PLAY doesn't actually play anything; it's there for benchmarking audio computation and all it does is retrieve and discard samples, so it's much faster than writing samples to disk. This expression crashes Nyquist directly.

Expected behavior
Nyquist effects should work if used on extremely long selections.

Screenshots

Additional information (please complete the following information):

• OS: all Os
• Version: originally reported in 2011 - but remain extant ten years later with 3.0.4 and alpha 3.1.0
• (if relevant) Sound Device [e.g. Windows WASAPI, USB Sound card]

Additional context
Roger wrote on Bugzilla
I think the problem here is internal to Nyquist. Does anyone actually apply effects to 14+ hours of 44.1K audio, or is this just a what-if scenario? An ideal fix would probably be to bump the sample counts to 64 bits in Nyquist, but I think the most practical thing is to compute when overflow is going to occur and just stop and raise an error in Nyquist to avoid crashing. I will look at this on the Nyquist (upstream) side.

Steve replied

Surprisingly we do see people wanting to process very long recordings for some very unlikely reasons, for example (an actual example from the Audacity forum) for marine biology research (hydrophone recordings of whales). I'd be happy to see this demoted to a "won't fix" if it were not for the crashing (which I guess would be very frustrating if you'd been waiting for a 14 hour file to import). If the crashing can be prevented, then the workaround of processing shorter sections is both simple and obvious. The problem at the moment is that it is not possible to write checks into Nyquist plug-ins against this, because the checks also fail at 2^31 samples.

Steve later wrote in Feb21

When Audacity 3.0.0 has been released we can remove the NYQ_MAX_LEN restriction, close this bug, and then log anything that breaks.

See the Bugzilla bugthred for further discussion:
https://bugzilla.audacityteam.org/show_bug.cgi?id=439

[LWinterberg]

@SteveDaulton are you going to remove the NYQ_MAX_LEN restriction or can this be closed?

[SteveDaulton]

@SteveDaulton are you going to remove the NYQ_MAX_LEN restriction or can this be closed?

It looks like Paul-Licameli has already done that here:
1189cfd
but not updated the code comments.

Perhaps you could ask him to update the code comments around that change, and if he thinks it appropriate, perhaps

            if (mCurLen > NYQ_MAX_LEN) {
               float hours = (float)NYQ_MAX_LEN / (44100 * 60 * 60);
               const auto message =
                  XO(
"Selection too long for Nyquist code.\nMaximum allowed selection is %ld samples\n(about %.1f hours at 44100 Hz sample rate).")
                     .Format((long)NYQ_MAX_LEN, hours);
               Effect::MessageBox(
                  message,
                  wxOK | wxCENTRE,
                  XO("Nyquist Error") );
               if (!mProjectChanged)
                  em.SetSkipStateFlag(true);
               return false;
            }

could be removed and (optionally) replaced with a simple assert:
wxASSERT(mCurLen <= NYQ_MAX_LEN);

[LWinterberg]

Paul changed it to be std::numeric_limits<long>::max(), which is identical to the hardcoded number that was there before.

[crsib]

long is a really bad type though. It is at least 32 bits.

On Windows sizeof(long) == 4 (both x32 and x64), on Unix systems it usually has the pointer size.

[crsib]

But you know, when long is not long enough you can always use long long, which can be longer than long

[LWinterberg]

judging by #1734 (comment) it seems like increasing the limit won't do much anyway as long as nyquist is limited to 2GB

[SteveDaulton]

long is a really bad type though. It is at least 32 bits.

Indeed.
'long' is not "the same as" the previously hard coded value of 2147483647, though it may be equivalent on some (old?) systems.

According to https://en.cppreference.com/w/cpp/types/climits
LONG_MAX = 9223372036854775807
I don't know how common it is for 'long' to be 2147483647, but if that's common, then as crsib hinted, perhaps better to change it to 'long long'.

[SteveDaulton]

judging by #1734 (comment) it seems like increasing the limit won't do much anyway as long as nyquist is limited to 2GB

Nyquist can process audio tracks that far exceed 2 GB. The 2 GB limit only applies to scripts that hold the entire track in RAM. Whenever possible, Nyquist programmers should ensure that samples are released as they are processed. There are some cases where that is not possible, which is why mMaxLen is still required.

[crsib]

I don't know how common

Well, Windows is the most popular OS even if mobile OSes are considered. I think there is a long fun story of Microsoft porting WinAPI to x64 which we will never know :-)

https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models

This was fixed in C99 (which MS never really supported) and C++11 using the intn_t notion

[SteveDaulton]

@crsib, would changing
#define NYQ_MAX_LEN (std::numeric_limits<long>::max())
to
#define NYQ_MAX_LEN (std::numeric_limits<long long>::max())
work in 32-bit builds (on 32-bit systems)?

[crsib]

I'm not very familiar with Nyquist API.

It is safe to assume that sizeof(long long) == 8 on all supported platforms.

[SteveDaulton]

Nyquist itself should be good up to about 2^58 samples (about 200 centuries @ 44100 Hz ?) without sample count overflow, which is big enough that we can safely leave it up to Nyquist's error checking. Nyquist now has checks built in to prevent excessive amounts of audio data being held in RAM (such as "MAX-AUDIO-MEM", which has a default of around 1 billion samples). Normally our NYQ_MAX_LEN limit should just keep out of the way and let Nyquist do its thing. mMax should be initialised to a big enough value that it keeps out of the way unless a plug-in specifically needs to limit the number of samples to a smaller value, which it can do via the ;maxlen header. I think pull request #3051 does what is required.

[Paul-Licameli]

If the template argument of numeric_limits is changed, make it one of: https://en.cppreference.com/w/cpp/types/integer

[SteveDaulton]

If you don't like my fix Paul, feel free to modify it.

[SteveDaulton]

As you guys now know what needs to be done, please fix it. Thank you.

[Paul-Licameli]

If you don't like my fix Paul, feel free to modify it.

I was just agreeing with @crsib that modern C++ standardizes names for numerical types of really specific bit widths, unlike old types like int long and long long for which it only specifies lower bounds and an ordering of types by width.

So if the intention is 64 bits, better to say int64_t.

The link I gave says those typedefs are optional actually, but I'm sure we can use them on all the compilers that matter.

[SteveDaulton]

The link I gave says those typedefs are optional actually, but I'm sure we can use them on all the compilers that matter.

What's the problem?

[SteveDaulton]

Following up on a forum user's post: Any chance that this will be fixed in Audacity 3.2.0?

[SteveDaulton]

Any chance this will be fixed for 3.4.0?

[SteveDaulton]

The "Label Sounds" and "Noise Gate" effects are still limited to 2^31 samples. This is fixed in #4494