Skip to content

Switched all audio sample processing to use floats, coded by hselasky, second attempt#660

Closed
corrados wants to merge 34 commits intomasterfrom
integrate_float2
Closed

Switched all audio sample processing to use floats, coded by hselasky, second attempt#660
corrados wants to merge 34 commits intomasterfrom
integrate_float2

Conversation

@corrados
Copy link
Copy Markdown
Contributor

@corrados corrados commented Oct 4, 2020

I merge the previous branch to master but found out that the Windows audio interface gave some ugly clipping. I had to move the master Git pointer hard back to a previous commit to unde the merge. In that current state the code cannot be merged to master and has to be fixed first (I/we have to do some more testing before I merge this code again to the master).

hselasky and others added 30 commits September 25, 2020 21:13
a mix of double and int16_t .

MacOS, Linux and Android already do this, and ASIO also supports it.
Change recording format to be 24-bit WAV, to get the full precision
of the mixed audio!

This patch gives the user the full resolution of the audio device,
both when receiving audio and transmitting audio.

Cleanup LSB/MSB sample processing in ASIO driver while at it.
LSB and MSB indicate little-endian and big-endian data format
presumably.

Signed-off-by: Hans Petter Selasky <hps@selasky.org>
Switch all Jamulus audio sample processing to use floats instead of a mix of double and int16_t
# Conflicts:
#	ChangeLog
#	src/server.cpp
@corrados
Copy link
Copy Markdown
Contributor Author

corrados commented Oct 5, 2020

@hselasky, @bflamig, @pljones I have spent a couple of hours of code style changes, bug fixes, merging on that "float" feature so far. Yesterday I merged the code to master and found out that we have a serious bug. So I had to "hard-reset" the master pointer (which is very ugly, especially for a public Git repo). Since we still have obvious outstanding bugs in the code and we never know what else does not work anymore (e.g. is the jam recorder working correctly, does it make sense to increase the amount of jam recorder data, etc.), I do not want to spend any more time on that. We have much more important tasks for Jamulus to be worked on. The "float" change gives so much work and so little improvement (if any noticable audio improvement at all...).

So I decided not to merge this code in the near/mid term future. Sorry guys...

@hselasky
Copy link
Copy Markdown
Contributor

hselasky commented Oct 5, 2020

@corrados: Sad to hear that. Does this serious bug only affect WIndows?

@bflamig
Copy link
Copy Markdown
Contributor

bflamig commented Oct 5, 2020

Curious under what conditions this bug occurs. A few hours ago I tried out the integrate_float2 branch, with server on Linux and client on Windows and it seemed to work fine with two different audio devices, one with float32 little-endian samples, and one with int32 little-endian samples.

@corrados
Copy link
Copy Markdown
Contributor Author

corrados commented Oct 6, 2020

I am using my Lexion Omega under Windows. I don't know which type it is using. When I play alone with my drums, everything is fine. Just if I connect to an occupied server and many others are playing, I hear these ugly clipping artifacts. I tested the official 3.5.12 version on the same server with the same hardware and I could not hear any clipping artifacts. So the artifacts are clearly generated by the new audio interface code.

@bflamig
Copy link
Copy Markdown
Contributor

bflamig commented Oct 6, 2020 via email

# Conflicts:
#	ChangeLog
#	android/sound.cpp
#	android/sound.h
#	src/channel.h
#	src/server.cpp
@bflamig
Copy link
Copy Markdown
Contributor

bflamig commented Oct 9, 2020

I tried using this floating pt version for band practice tonight, on my Windows machine. Using Reaper as the "audio device", playing both midi drums and mic'ed fiddle. The rest of the band members were on the official 3.5.12 version, as well as the server on a remote Linux box.

I experience no problems at all. The audio was clear. Not able to replicate any clipping problems.

# Conflicts:
#	mac/sound.cpp
#	src/client.cpp
#	src/server.cpp
#	src/server.h
#	src/util.cpp
#	src/util.h
#	windows/sound.cpp
# Conflicts:
#	ChangeLog
#	src/client.cpp
#	windows/sound.h
@corrados
Copy link
Copy Markdown
Contributor Author

A few hours ago I tried out the integrate_float2 branch, with server on Linux and client on Windows and it seemed to work fine with two different audio devices, one with float32 little-endian samples, and one with int32 little-endian samples. [...]
I tried using this floating pt version for band practice tonight, on my Windows machine. [...] I experience no problems at all. The audio was clear. Not able to replicate any clipping problems.

I just checked: My Lexicon Omega uses the ASIOSTInt24LSB type. As far as I understand, your interface is using the ASIOSTFloat32MSB and the other is using ASIOSTInt32LSB, right?

@bflamig
Copy link
Copy Markdown
Contributor

bflamig commented Oct 10, 2020

UPDATE: My original comment was wrong!

@corrados: Reapers on Window uses the ASIOTInt32LSB and the ASIOSFloat32LSB (not MSB) types. Since I only tested the new software on Windows as a client, these are the only two types I've tested so far. I haven't tried the software as a Windows jamulus server.

So perhaps the 24 bit stuff is broken somehow.

@hselasky
Copy link
Copy Markdown
Contributor

@corrados : Hi, I wonder if the clipping issue has something to do with the Windows compiler padding the sample structures, so that the 3-bytes become 4-bytes under Windows.

Can you try enclosing all the sampleXXX structs using:

#pragma pack(push,1)
#pragma pack(pop)

@bflamig
Copy link
Copy Markdown
Contributor

bflamig commented Oct 12, 2020

Just tested the latest integrate_float2 code running on a Linux server, and windows client, with four other clients running official Jamulus client software. Everything worked fine. This is with 32 bit little-endian samples on my end as before.

It sounds clearer, but probably wishful thinking.

@corrados
Copy link
Copy Markdown
Contributor Author

This is with 32 bit little-endian samples on my end as before.

I think you can test all the possible types with your audio interface, too. Since we have all conversions already available (from/to float from any type). So simply use your interface (e.g. 32 bit) and let it convert to float (like it is implemented right now). But then simply use your conversion function to convert that float signal to, e.g., 24 bit and immediately convert it back to float and use that float signal for input for Jamulus. Then you should (hopefully) hear the clipping which I do with my audio interface.

@hselasky
Copy link
Copy Markdown
Contributor

@corrados: I wrote a small test program using clang:

        uint8_t test[3];
        sample24MSB *p = (sample24MSB *)test;

        p->Put(-1.0f); printf("0x%x 0x%x 0x%x\n", test[0], test[1], test[2]);
        p->Put(1.0f); printf("0x%x 0x%x 0x%x\n", test[0], test[1], test[2]);

        printf("%zd\n", sizeof(sample24MSB));

And it correctly outputs:

0x80 0x0 0x0
0x7f 0xff 0xff
3

I know some devices do not support the full range they report, and I suggest that the float clipping reserves a dB for audio processing in the sound card:

diff --git a/src/util.h b/src/util.h
index 1f65fdea..4122742d 100644
--- a/src/util.h
+++ b/src/util.h
@@ -77,7 +77,8 @@ class CClient;  // forward declaration of CClient
 // range check audio samples
 static inline float ClipFloat ( const float fInput )
 {
-    return qBound ( -1.0f, fInput, 1.0f );
+    static constexpr float limit = 1.0f - (1.0f / 10.0f);
+    return qBound ( -limit, fInput, limit );
 }
 
 // debug error handling

@hselasky
Copy link
Copy Markdown
Contributor

@corrados : I think the casting to/from int16_t avoids hitting -1.0f with the old code!

@hselasky
Copy link
Copy Markdown
Contributor

@corrados : I've imported a simple audio compressor from ZynAddSubFX . It should fix the clipping issue!

#672

@bflamig
Copy link
Copy Markdown
Contributor

bflamig commented Oct 12, 2020

@hselasky: Your little example works because it's just one sample. (It is in danger of overwriting memory).

I do believe that in the real code that the sample24LSB and sample24MSB structures need to have packing enforced. They almost assuredly will be represented as 4 bytes and cause all sorts of problems when the buffers are supposed to be pointers to 24 bit (3 byte) samples. I could surely understand the nasty audio output.

So that's almost assuredly what the problem is. But I haven't tested that. It doesn't explain why the existing code sometimes worked for @corrados when he was just running Jamulus by himself.

@bflamig
Copy link
Copy Markdown
Contributor

bflamig commented Oct 14, 2020

UPDATE: I was wrong about the packing of those structures, at least in MSVC 2019.

sizeof(sample32LSB) = 3 and sizeof(sample32MSB) = 3, regardless of what packing pragma you use.

And if, for example, you were to have an array of 5, e g:

sample32LSB foo[5];

then

sizeof(foo) = 15

according to the MSVC 2019 compiler.

So it would appear that structure padding is not an issue here.

@corrados
Copy link
Copy Markdown
Contributor Author

The branch https://github.com/corrados/jamulus/tree/integrate_float2 is already in the Jamulus Git repo. So there is no need for a Pull Request here. As soon as I continue working on that topic, I'll update the branch and simply merge the code into the Git master branch. I'll close this Pull Request therefore now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants