igorski / MWEngine

Audio engine and DSP library for Android, written in C++ providing low latency performance within a musical context, while providing a Java/Kotlin API. Supports both OpenSL and AAudio.
MIT License
259 stars 45 forks source link

SampleInstrument sometimes produces crackling sound #114

Closed harthorst closed 4 years ago

harthorst commented 4 years ago

I did an upgrade to the newest version of mwengine (thanks for introducing cmake). Now I hear some artifacts which sound like distortions. I used the MWEngine Example app for investigation and added a wav file to the project. I disabled all other sample events. Volume of all synth instrument is set to 0.

It seems that switching the audio driver has an influence to that behavior. Sometimes the sample sounds really clear.

You can find a screen record and also the wav file I used here: https://mega.nz/#F!QgVHBQwa!p75N1xiVv8_o_kn6HQdubw

harthorst commented 4 years ago

One hint: CPU usage seems to be much higher. I'm sure that's related to this problem. On my Xaomi Redmi Note 7 the MWengine example app consumes around 144% CPU even when the player is paused.

igorski commented 4 years ago

I struggle to reproduce this issue.

Just to help me in isolating the root cause, can you confirm that when the application starts the audio stability is fine and that only after changing driver the crackling starts to occur ? If yes, is the behaviour different when you deliberately pause the engine prior to switching driver ?

harthorst commented 4 years ago

Hi Igor. The issue exists from the beginning without changing the audio driver. Did you use my sample? Using the original samples there is no distortion or maybe it is not noticeable. Since MWEngine is used by many apps without this issue it may be a problem of my build system. I had to upgrade ndk to build after upgrading. I'll check that during the weekend and try to reproduce the issue on a different OS.

harthorst commented 4 years ago

I installed the newest Android Studio on a Mac, the issue still exists. I also tested it on Mi Pad 4. I uploaded the MWEngineActivity to the shared folder. Maybe it is related to my sample? In hydrogen it does not cause any trouble.

Maybe this information is helpful: NDK Resolution Outcome: Gradle model version=6.3, NDK version=21.0.6113669

igorski commented 4 years ago

Still working on this, getting some success in reproducing the issue, looks like locking the thread during the buffer queue sometimes times out.

Just a small question, if you check out revision 6a1613b4e5ac5d97aa454389d16c1ad5d91b5bbb can you confirm whether this works without issue ?

harthorst commented 4 years ago

Yes!!! It's much better in that revision. I didn't merge it into my my project but the modified example activity doesn't have that issue anymore. CPU load is much lower too.

igorski commented 4 years ago

Yes!!! It's much better in that revision. I didn't merge it into my my project but the modified example activity doesn't have that issue anymore. CPU load is much lower too.

Good to hear! Please don't merge it, as it is actually an older version of the engine (there is no runtime switching between AAudio and OpenSL here). I was back tracking the changes and think it is down to the way a runtime audio callback is declared. I had noticed it was causing occasional thread lock timeouts which could lead to a buffer under run.

So far my tests with branch issue-114 have shown no under runs on my test devices. May I bother you to see whether this works for you ? (this branch is up to date with all the latest changes including driver switching, so once stable, it will be merged into the master).

harthorst commented 4 years ago

I'm sorry to tell you that the issue occurs in that branch. I again used the ExampleActivity, loaded my wav, created some sample events and threw out all other events. CPU load is higher around 5-6 times.

igorski commented 4 years ago

Ah my bad, I forgot to commit the updated makefile! Just pushed it to the branch. I have updated the makefile to change the linking behaviour of the engine.

harthorst commented 4 years ago

Still the same behavior. When I switch between audio drivers I sometimes get the example app to sound good. When that happens I can see CPU consumption dropping from about 17% to 5% instantly. Can I help you in some way? Providing trace files or trying a different NDK for instance?

One additional information: I used the "Start Recording" button and recorded the output. The wav file also has the corrupted sound. I experimented a little bit: 1) I created a 440hz sine wave, put it into the app and recorded the output (using MWEngine's bouncing). You'll find the output in the upload-folder named "mwengine_output-sine-2.wav". 2) I created a wav file filled with always the same value (211) and did the same as 1) - the output is named "mwengine_output-211.wav". In my understanding mwengine should act like a function and since no volume or effects is applied the output should be equal to the input, right? But it isn't, you clearly can see scratches in the line.

Maybe that helps.

igorski commented 4 years ago

Hi Thomas, still ongoing on this. I was wondering could you supply me with the _mwengineoutput-211.wav file for analysis ? I am very curious to see resulting waveform.

Also, what is the average CPU consumption you are currently experiencing ? You mentioned 144 % in your second post and 17 % max in your last. Or is this reading inconsistent ? Are there any logs being sent to logcat when that happens (not from mwengine, but from the system, is there something weird flooding the logcat when you remove the logcat filter ?)

harthorst commented 4 years ago

Hi Igor. The 144% CPU load is what the top command told me on the device. 17% is what android studios profiler shows me. About the wav files: I'm afraid the dents I saw in the 211-File are the beginnings and ends of the sample. I tried to reproduce that setting today and I did not see the audible distortions in the file - so that was a wrong perception. I also tried to reproduce the sine experiment and I also heard a lot of distortion but the sine in the output wav (mwengine_output-sine-14.04.wav) is fine suddenly. But I found a file from 2 days ago which clearly shows distortions (mwengine_output-sine-corrupted.wav). I'll have to spend more time on this. I'll also put a logcat dump into the folder: https://mega.nz/folder/U5cTVQKQ#K8p5xkJkZyCHMyIiKf30Tw

I have a question: do you see similar problems? I'm asking because it should be very easy to reproduce that. Maybe I'm doing something completely wrong. And it can't be my build environment because I tried this on linux and mac with similar results.

igorski commented 4 years ago

Hi Thomas, thanks for the reply and helpful files.

The CPU load difference makes sense, top shows the load relative to a single CPU. Android Studio profiler does that relative to all available cores. The Xaomi Redmi has 8 cores, so 8 cores at 17 % are around 136 % in "top-land". I was afraid there was something horribly wrong there for these large jumps!

The reason for me asking as I am unable to get the distortions you mention, there are unit tests that assert that an input signal is coming out unchanged (or transformed according to expectation, depending on the test) so changes to sequencing stability should be caught. I can imagine that for some reason buffer underruns are occurring, but they must be to the extent that render cycles get skipped while incrementing the sequencer offset (which is strange, while the audible result might crackle at this moment, the recording should be fine, which is what you confirm for _mwengineoutput-sine-14.04.wav).

Then you mention good news : that things are suddenly fine (for the written recording). So I have to ask, did you pull and rebuild the code since the 2 days before ? I have been making changes to this branch (there is another reported case of performance issues which I am tackling in this branch) which provides some performance improvements (though not for the use case of MWEngineActivity, only larger projects, the type that processes 40,000 events). I did fix an issue that could occur for devices where the default buffer size is a multiple of 8 instead of 10. (Would you know what the buffer size is D/MWENGINE reports on sequencer start ? Oh, and what driver the engine starts with by default ?). Additionally I also fixed an issue where upon looping of a sequence, an overlapping event (for instance your use case where a sample is long) would be processed twice (thus doubling the output for the overlapping samples. Oddly enough the last two issues would also have existed in the "stable branch".

Still curious that you don't experience problems with the commit hash I provided you with earlier, the only real change in the code base (apart from runtime AAudio loading) is the C++ and compiler version, which I've restored as I couldn't profile any meaningful changes between their compilation units...

harthorst commented 4 years ago

Hi Igor,

So I have to ask, did you pull and rebuild the code since the 2 days before

To be honest: I don't know. I tried a lot maybe I did a pull, but I'm not sure.

Would you know what the buffer size is D/MWENGINE reports on sequencer start

logcat says: starting native audio rendering thread @ 48000 Hz using 192 samples per buffer

what driver the engine starts with by default

I see a lot of log messages of AAudio, so I guess it's AAudio

I also saw this: 2020-04-16 16:48:21.013 1535-1624/nl.igorski.example D/AAudio: AAudioStream_requestStart(0x7e6ca62080) returned 0 --------- 2020-04-16 16:48:21.013 1535-1648/nl.igorski.example V/MWENGINE: AAudio_IO::Error calculating latency: AAUDIO_ERROR_INVALID_STATE

It looks like selecting OpenSL does not work on my phone. I still see AAudio messages after selecting it.

I uploaded some files for you into the shared folder. Under experiments you'll find 3 folders, the 2 starting with 'build-' are the issue-114-branch and the commit you provided. Each contains an apk, the MWEngine-output and a screen record. You'll also see the commit 6a1613b also has some distortions but much less than issue-114. The third folder contains the sine wav I used for that experiments.

igorski commented 4 years ago

That is a load of excellent news Thomas :)

192 samples confirms that you are now reaping the fruits of the multiples of eight-fix in the issue-114 branch, so that would be a plausible explanation for the lack of distortion in rendered WAV files you mentioned in your previous message (though I still have to check the WAV files you added to the shared folder, thanks for the effort in making these available btw).

I'm also pleased to hear that the default driver is AAudio, in the logcat message you provided I saw that enqueued buffers were of differing buffer sizes, which is something that is impossible using OpenSL (it is not an issue for AAudio as each render will correct for the available samples provided - which is why AAudio is a great improvement over OpenSL - ).

The error for calculating latency is non-blocking, however I will investigate it as I know of no plausible explanation why that would fail.

harthorst commented 4 years ago

Hi Igor. Yes, the outputs look good. Sorry for the false hint. But the sound still is bad (see screenrecord files). I get tons of this message: V/AudioTrack: obtainBuffer(192) returned 192 = 128 + 64 err 0 V/AudioTrack: obtainBuffer(64) returned 64 = 64 + 0 err 0 V/AudioTrack: obtainBuffer(192) returned 192 = 192 + 0 err 0

igorski commented 4 years ago

Hi Thomas, could I bother you to pull the latest changes from this branch and test again. Several improvements have been added and am curious to see whether they affect stability on your device. Would you also be able to provide me with the Android version that is running on your device(s) ?

harthorst commented 4 years ago

Hi Igor. Yes of course. In branch "issue-114" i get compile errors. The file perfutilities.h is missing.

igorski commented 4 years ago

Ah darn, I'm getting sloppy (or you can tell that I'm getting excited). Pushed the file :)

harthorst commented 4 years ago

I can't tell how often that happened to me :) . At the first glance there seems to be an improvement but I only spend 5 minutes in analyzing this. More profund tests will follow. 1) CPU usage seems to be a little bit lower (13% vs 17%). I'll do several restarts to prove this 2) Sound: Cajon sounds way less distorted but there still are interuptions which become clearly audible when I use the sine tone as instrument.

The Redmi Note 7 has Android version: 9 PKQ1.180904.001, Kernel version 4.4.153-perf+

Thank you very much for putting so much effort on this, Igor.

harthorst commented 4 years ago

Hi Igor. After checking more deeply I have to correct myself - there are no glitches anymore. The artifacts I noticed came from the hard beginning and end of the sine tone. After adding fade in and out it sounds good. Now I'm very excited to check this fix in my app. Do you recommend switching now? Thank you so much.

harthorst commented 4 years ago

Sorry for bothering you again. I just found out that after adding effects to the processor chain I hear distortions again. This time you can hear them in the wav file written by MWEngine too.

I put the MWEngine output and screen recorder file in the folder "build-issue-114-4e50bf1". I used the sine.wav as SampleInstrument and added a ReverbSM with default values.

igorski commented 4 years ago

Is it only occurring for the ReverbSM or any processor in general ? (ReverbSM can benefit from some performance improvements as it is the heaviest process in the repo).

harthorst commented 4 years ago

It's not any processor. Limiter for instance seems to be fine. Phaser and Delay are showing the same behavior, also one of my own effects. Changing precision seems to have no effect.

I tried an ancient commit (046df82) in combination with the Delay. It sounds good.

igorski commented 4 years ago

It's not any processor. Limiter for instance seems to be fine. Phaser and Delay are showing the same behavior, also one of my own effects. Changing precision seems to have no effect.

I tried an ancient commit (046df82) in combination with the Delay. It sounds good.

This might be a different issue to the one posed in this issues title ? e.g. if SampledInstrument is now stable again, we could create a follow up for the processors. (Also note that for the commit hash you noted, you were likely using OpenSL as the audio driver whereas the current branches would prefer AAudio, so unless you force OpenSL in your activity (MWEngine.createOutput()) this might not be an entirely accurate observation.

I must say that I just noticed and fixed something dubious which relates to resampling sample playback when playing back a sample that is at a lower sampling rate than the device sample rate (e.g. playing a 44.1 kHz sample on a 48 kHz device) which would cause the sample end to not be calculated correctly. This was actually a problem ever since 046df82... anyways one more factor to deal with choppiness gone!

harthorst commented 4 years ago

fixed something dubious

That fixed it! Now I can use equalizer and reverb without glitches. That are good news. Thank you.

igorski commented 4 years ago

fixed something dubious

That fixed it! Now I can use equalizer and reverb without glitches. That are good news. Thank you.

Makes sense that delay type effects would only repeat the breakup :) Then I will merge this commit as the changeset is already massive. If you do find issues with certain aspects of the engine we can open up a new issue.