Still have "fart" noises during transitions when using transport function

[0x0fe]

So, this is not a new issue, and i still can't figure it out nor find a solution, when i use the transport functions, namely Next, Previous and Play/pause, i get digital "fart" noises. I have tried to use the mute, both manually and automatically by seting the mute pin in config, without success. When used manually it doesnt work because i probably cant make the muting at the right time to hide these audio artefacts, when using the automatic mute it does not do anything on this issue.

The fart noises when switching between songs are of variable intensity, proportional to the player volume. when using stop/play the are stronger, but still totally linked to the player volume.

By the sound of it it seems like there is a discontinuity to the data feeding at the end, for example when player.stop() is called there is a silence followed by few more chunk of data and then silence. The fart noise is these few chunk of data/silence most likely. I have tried various methods to correct this, so far without success.

Attached is an audio sample of the problem.

Here is how the transport functions are handled, in this test case directly from the main loop, which also handles player.copy(). Also the mute function (mute pin of the audio PA) is perfectly clean when operated right in the middle of the playback manually, so if i dont have any other option i will have to use that, but i really would prefer to fix the problem at the root, i dont understand what is going on but something is not right with the datas sent to the DAC when usin stop, next and previous.

void key_handler(void){

  if(key1_int){
    printf("Next\n"); 
    player.next();
    while(!digitalRead(KEY1)){player.copy();}
    key1_int=0;    
  } 
  if(key2_int){
    printf("Prev\n");
    player.previous();  
    while(!digitalRead(KEY2)){player.copy();}
    key2_int=0;    
  } 
  if(key3_int){
    printf("Play/Pause\n");
    play_pause();   
    while(!digitalRead(KEY3)){player.copy();}    
    key3_int=0;    
  }          
  if(encoder.isEncoderButtonClicked()){
    keyenc_int=1;
    printf("mute %d\n",mute_flag); 
    mute(mute_flag); 
    mute_flag=1-mute_flag;     
  }
  if(encoder.encoderChanged()){
    dispatchEvent(ENCOD,encoder.readEncoder());     
  }    
}

Audio.zip

[pschatzmann]

I don't know how old your copy of the library is, but did you try just to call player.next(); or player.previous() with the latest version ? When I tested this last, I had the impression that the issue has been resolved

[0x0fe]

i also want to precise that my DAC doesnt make any noise when no data is fed to it, for example if the I2S is not even initlized, also, there is absolutely no problem when the audio files are automatically transitioning from one file to another, it is perfectly silent. The problem only occurse when plyer.next(), player.previous() and player.stop() are used.

i downloaded the library about 4 hours ago from github directly.

[0x0fe]

sadly the issue is still here as you can see in the audio sample.

[0x0fe]

this bit while(!digitalRead(KEY3)){player.copy();} is just a debouncing for the tact switch which continues to copy data, and there is no bouncing of the keys if you wonder.

log:

event[player] key 3
stream prev [7]
event[player] key 3
stream prev [6]
event[player] key 3
stream prev [5]
event[player] key 3
stream prev [4]
event[player] key 1
stream next [3]
event[player] key 1
stream next [4]
event[player] key 1
stream next [5]

[pschatzmann]

It would be interesting if you get the same noise if you use my Event Handler.

[0x0fe]

can you point out an example to use? I can make a test case

[pschatzmann]

https://github.com/pschatzmann/SIDPlayer/blob/main/examples/player/player.ino Just replace the SIDPlayer with an AudioPlayer..

[0x0fe]

ok, i can try with sidplayer too. by the way, is there a way to set mck default pin?

[pschatzmann]

Nop, SIDPlayer is still work in progress and does not work yet. But you can take the navigation logic from it

[0x0fe]

oh, ok, copy that, i use the mp3 player with this handler then, report soon.

[pschatzmann]

If you want to handle the pin_mck via default values in the AudioConfig.h, I suggest that you extend the logic which should be pretty easy to do and then please share it via a Pull Request

[0x0fe]

that is fine, because i set them in the I2S config anyway. Strangely i cannot compile the new test case,

Stream* nextCb(int index);
void initCb(void);
I2SStream i2s();
MP3DecoderHelix decoder;
AudioSourceCallback source(nextCb,initCb);
AudioPlayer player(source,i2s,decoder);
AudioActions actions;
...
  auto cfg = i2s.defaultConfig(TX_MODE);
  cfg.pin_mck = MCK;
  cfg.pin_bck = BCK;
  cfg.pin_ws = WS;
  cfg.pin_data = DAT; 
  i2s.begin(cfg);
  decoder.setNotifyAudioChange(player);

error: no matching function for call to 'audio_tools::AudioPlayer::AudioPlayer(audio_tools::AudioSourceCallback&, audio_tools::I2SStream (&)(), audio_tools::MP3DecoderHelix&)'
 AudioPlayer player(source,i2s,decoder);
BS_FB2_SD_PLAYER_ACTION:184:18: error: request for member 'defaultConfig' in 'i2s', which is of non-class type 'audio_tools::I2SStream()'
   auto cfg = i2s.defaultConfig(TX_MODE);
                  ^~~~~~~~~~~~~
BS_FB2_SD_PLAYER_ACTION:189:7: error: request for member 'begin' in 'i2s', which is of non-class type 'audio_tools::I2SStream()'
   i2s.begin(cfg);

But that is the same as in the other test case, did anything changed in the library in regard to i2s and audioplayer ?

[0x0fe]

no known conversion for argument 2 from 'audio_tools::I2SStream()' to 'audio_tools::AudioStream&'

[pschatzmann]

Just remove the brackets! I2SStream i2s;

[0x0fe]

oh, my bad, i added the MUTE port argument, then removed it and forgot to remove the brackets indeed.

[pschatzmann]

Another thought: have you double checked in your lib directory that you don't have any old copies of the AudioTools library around. If there are multiple copies - even with another name, it is not defined which one will be used

[0x0fe]

yes, i verified that, in fact i zipped the old one as -bak and deleted the directory prior to copy the last from master, also i checked in the build log that the path is correct. In fact on the otehr test case i pass the mute as argument, that is why it could compile fine.

[0x0fe]

sadly, exactly the same result. I attached the audio and the firmware, currently files are in PLAYER/K0015/ this path can be modified line 198. Audio2.zip BS_FB2_SD_PLAYER_ACTION.zip

[pschatzmann]

Hmm, thinking about it, it might be the data in the mp3 codec buffer that might cause this issue. This can be checked if you try if you can reproduce the issue with wav files...

[0x0fe]

oh, ok i can try with wav, i never used wav but i should be able to find a reference and adjust this test case

[0x0fe]

so, i just made another test first, i used the mute manually, and i could solve the issue on stop() fully. Stop() is now perfectly clean and silent as it should, play is fine it never made any noises. Of course that doesnt solve the problem at the root, but at least the function is usable until this is really solved.

void playpause(bool, int, void*) { 
  printf("PP\n");  
  pause_flag=1-pause_flag; 
  if(pause_flag){
    mute(0);
    player.play(); 
  }
  else{
    mute(1);
    player.stop();
  }
}

also, i implemented the same thing on next() and previous(), while it reduced the amplitude of the noises, it doesnt completely solve them, which lead me to think : the noises are actually at the beginning of the next audio playing, rather than at the end of the current one. (or maybe both at the end of current and start of next). Could it be that the function which copies the remaining bytes is called after the next() or previous() function is called?

void previous(bool, int, void*) { printf("Prev\n"); mute(1); player.previous(); }
void next(bool, int, void*) { printf("Next\n"); mute(1); player.next(); }
....
Stream* nextCb(int index){

  if(index<0) return prevCb();  
  if(fileIndex>filesCount-1) return nullptr;
  if(fileIndex<0){fileIndex=0; return nullptr;}
  printf("stream next [%d]\n",fileIndex);
  set_index(fileIndex);
  get_path();
  audioFile = SD_MMC.open(filePath);
  lastFile.close();
  if(fileIndex<filesCount) fileIndex+=1;
  //else { fileIndex=0; return nullptr;}
  mute(0);
  return &audioFile;
}
Stream* prevCb(void){

  if(fileIndex<0){fileIndex=0; return nullptr;}
  if(fileIndex>filesCount-1){fileIndex=0; return nullptr;}
  printf("stream prev [%d]\n",fileIndex);
  set_index(fileIndex);
  get_path();
  audioFile = SD_MMC.open(filePath);
  lastFile.close();
  if(fileIndex>=0) fileIndex-=1;
  //else { fileIndex=0; return nullptr;}
  mute(0);
  return &audioFile;
}

[pschatzmann]

That's my current working assumption as well. If some stale data is in the mp3 buffer it gets played out first just before the new data is coming... Wav has not buffer and sends the data directly to the DAC

[0x0fe]

yes, i will try to bypass this function and see if changes anything

[0x0fe]

So, its not really at the beginning of the next file, but its after the end and fadeout of the current file. By the way, the first blip is the clic of the tact switch, the noise is the second blip. That is with the mute implemented on next() and prev().

Audio3.zip

[0x0fe]

so, because i have a lot of other things to deal quickly on this project i found a quick fix that is good enough until the root cause is more clearly identified and fixed, i used a timer of freertos to disable the mute after 300ms.

Checking at the waveform i noticed that while the file access time varies, the delay between key press and noise is more or less fixed, 300ms is long enough to cover the noise in all cases and short enough to reopen mute before the next stream plays. Of course this is less than ideal but it will be enough to go forward with the rest.

TimerHandle_t mute_Timer;
void mute_timer_cb(void){
  mute(0);
}
mute_Timer = xTimerCreate("muteTimer",pdMS_TO_TICKS(300),pdFALSE,(void*)0,(TimerCallbackFunction_t)(mute_timer_cb));  
Stream* nextCb(int index){

  printf("nextCb index %d\n",index);
  pause_flag=0;
  if(index<0) return prevCb();  
  if(fileIndex>filesCount-1) return nullptr;
  if(fileIndex<0){fileIndex=0; return nullptr;}
  printf("stream next [%d]\n",fileIndex);
  set_index(fileIndex);
  get_path();
  audioFile = SD_MMC.open(filePath);
  lastFile.close();
  if(fileIndex<filesCount) fileIndex+=1;
  //else{ fileIndex=0; return nullptr;}
  xTimerStart(mute_Timer,0);
  return &audioFile;
}
Stream* prevCb(void){

  if(fileIndex<0){fileIndex=0; return nullptr;}
  if(fileIndex>filesCount-1){fileIndex=0; return nullptr;}
  printf("stream prev [%d]\n",fileIndex);
  pause_flag=0;
  set_index(fileIndex);
  get_path();
  audioFile = SD_MMC.open(filePath);
  lastFile.close();
  if(fileIndex>=0) fileIndex-=1;
  //else { fileIndex=0; return nullptr;}
  xTimerStart(mute_Timer,0);
  return &audioFile;
}

[0x0fe]

there is another thing i forgot to mention, i still have this issue with fade.h complaining at first playback, i suspect this is related to my custom callbacks for file handling (due to the index order issue of the default implementation). So the first time it plays, it cannot play because i get the error [E] Fade.h : 271 - setAudioInfo not called I have to press next() or previous() and it will then initilize properly and play. I dont know what i do wrong, my initCb does read the file from filesystem, the selectCb does it as well, which seems wasteful. I havent seen a case where init and select arent called one after the other, so i guess i could avoid opening the file in the initCb, anyway i seem to miss one call to populate transmit the I2C information or something

void initCb(void){

  printf("stream init\n");
  if(!sd_ready) init_sd();
  get_path();
  audioFile=SD_MMC.open(filePath);
}
Stream* selectCb(int id){

  printf("stream select [%d]\n",id);  
  //if(fileIndex+id>=filesCount) return nullptr;
  pause_flag=0;
  set_index(id);
  get_path();
  audioFile = SD_MMC.open(filePath);
  lastFile.close();
  if(fileIndex<filesCount) fileIndex+=1;
  else fileIndex=0;
  mute(0);
  return &audioFile;
}

calling player.play() after player.begin()

stream init
[I] MetaDataID3.h : 564 - virtual void audio_tools::MetaDataID3::begin()
stream select [0]
[I] AudioCopy.h : 71 - buffer_size=1024
Audio : ready
[E] Fade.h : 271 - setAudioInfo not called
[E] Fade.h : 271 - setAudioInfo not called
[E] Fade.h : 271 - setAudioInfo not called
[E] Fade.h : 271 - setAudioInfo not called
[E] Fade.h : 271 - setAudioInfo not called
[E] Fade.h : 271 - setAudioInfo not called

calling player.previous(), still an error, calling player.next() finally the i2s parameters are passed.

Prev
[I] AudioPlayer.h : 484 - void audio_tools::AudioPlayer::writeEnd()
[E] Fade.h : 271 - setAudioInfo not called
nextCb index -1
stream prev [1]
[I] AudioEncoded.h : 328 - virtual void audio_tools::EncodedAudioPrint::end()
[E] Fade.h : 271 - setAudioInfo not called
[I] MetaDataID3.h : 570 - virtual void audio_tools::MetaDataID3::end()
[I] AudioPlayer.h : 196 - reset codec
[I] MetaDataID3.h : 564 - virtual void audio_tools::MetaDataID3::begin()
[I] AudioCopy.h : 71 - buffer_size=1024
[I] AudioPlayer.h : 234 - sample_rate: 44100
[I] AudioPlayer.h : 235 - bits_per_sample: 16
[I] AudioPlayer.h : 236 - channels: 2
[I] VolumeStream.h : 183 - setVolume: 0.500000
[I] VolumeStream.h : 183 - setVolume: 0.500000
[I] AudioTypes.h : 63 - sample_rate: 44100
[I] AudioTypes.h : 64 - channels: 2
[I] AudioTypes.h : 65 - bits_per_sample: 16
[I] I2SStream.h : 69 - virtual void audio_tools::I2SStream::setAudioInfo(audio_tools::AudioBaseInfo)
[I] AudioTypes.h : 63 - sample_rate: 44100
[I] AudioTypes.h : 64 - channels: 2
[I] AudioTypes.h : 65 - bits_per_sample: 16
[I] Fade.h : 109 - fade in 1152 frames from volume 0.000000

[pschatzmann]

Did you try to call setAudioInfo() on the player to make sure that it has some information. I think the only importent information is the number of channels which is usually. Usually the decoder should do this automatically with the actual information when a new mp3 is played

[0x0fe]

i dont call it manually, i have to find how i can do this, yes it is done automatically in some case, but for some reason it doesnt for first playback.

[pschatzmann]

out of my head: AudioBaseInfo info; info.sample_rate = 44100; info.channels = 2; info.bits_per_sample 16; player.setAudioInfo(info);

[0x0fe]

so that is what i do at the init decoder.setNotifyAudioChange(player); and when i try to do player.setAudioInfo(); in selectCb, oif course it complain that it is missing argument, which i ma not sure how to get.

AudioBaseInfo info; info.sample_rate = 44100; info.channels = 2; info.bits_per_sample 16; player.setAudioInfo(info);

ah i see, directly hardcoding it

[pschatzmann]

The decoder.setNotifyAudioChange(player); is automatically called in the constructor of the AudioPlayer

[0x0fe]

i see, so no need to set explicitely. Anyway, it doesnt work all the time, but i am hapy to harcode that for now.

[pschatzmann]

I thought I start with the easy thing and tested the starting and stopping: using this example.

Zoomin in:

This is working perfectly!

[pschatzmann]

For the next function, I am getting the following

This beginning of the the next song looks to me as if it contains the finaly piece from the prior song, so I assume that this must come from the mp3 codec

[pschatzmann]

After adding the logic to restart the codec this issue seems to be resolved for me as well:

I have just the gut feeling that your issues might be related to your hardware...

[pschatzmann]

One last question: maybe your DAC is reacting badly if it stops to receive any data. Did you try to set player.setSilenceOnInactive (true)

[0x0fe]

DAC is reacting badly if it stops to receive any data

As i told the DAC has no problem with no data input, it is silent when the I2S is not initilized and also silent when its initialized but there is no audio datas sent. Besides, at the transition between songs (automatic) there is no such issue, it stays perfectly silent.

using this example.

Does the example use SD_MMC and a DAC? i thought audiokit uses a codec.

I have just the gut feeling that your issues might be related to your hardware.

That is unlikely, the sound we ear is chunks of the audio file, that is not going to be magically produced by the DAC, it has to be sent by the ESP32S3 one way or another. Yes setSilenceOnInactive has been used before without any difference.

[0x0fe]

After adding the logic to restart the codec this issue seems to be resolved for me as well:

Can you point me to this change? i wwould like to try it too

[pschatzmann]

The AudioKit has a codec which needs to be initialilized in the beginning, but the audio communication is done with I2S like in the I2SStream. So there should be no difference when the playing has started. Here is the change: https://github.com/pschatzmann/arduino-audio-tools/commit/657d71db9078acdf4d1726281306db0d17b32690

[0x0fe]

but the audio communication is done with I2S like in the I2SStream

yes, i see, maybe it has to do with the SD_MMC, furthermore that i have a custom implementation of the callbacks due to the index issue.

[pschatzmann]

If you are not happy with the standard navigation logic, the cleanest way would be to provide your own subclass of AudioSource

[0x0fe]

would it change anything compared to using the custom callbacks?

AudioSourceCallback source(nextCb,initCb);
source.setCallbackSelectStream(selectCb);

The reason i use these callbacks is because the VFS arduino/espressif implementation has troubles to index files in the correct order in some corner cases (when filename contains leading zeros). With my current implementation it is fixed, as a side effect it provides an endOfPLayback callback which i need to notify another task.

If there is an advantage to use a custom class i will do it, but if it is purely aestethic i prefer 3 callbacks.

Regarding this noise issue, could it be caused by these 3 callbacks? I assume this is implemented more or less in the same way inside the library, minus using qsort for determining the index of a given file.

[pschatzmann]

Nop, I did not realize that you already use AudioSourceCallback: in the end it's the same thing

[pschatzmann]

Right now I am running out of options: after all it is really strange that I can't reproduce this!

[0x0fe]

yes, i will test your fix first, and see if it changes anything

[0x0fe]

Here is the change: https://github.com/pschatzmann/arduino-audio-tools/commit/657d71db9078acdf4d1726281306db0d17b32690 So, i just tested that, disabling the muting, it did not solve the issue. Later when i have more time i will hook up both the logic analyzer and oscilloscope to make sync captures of I2S lines, VCC lines and speaker lines to see what is going on exactly. For now the mute trick is good enoug.

[pschatzmann]

I assume that your recording was for a start/stop. If you look the the following implementation:

        /// The same like start() / stop()
        virtual void setActive(bool isActive){
            if (isActive){
                fade.setFadeInActive(true); 
            } else {
                fade.setFadeOutActive(true); 
                copier.copy();
                writeSilence(2048);
            }            
            active = isActive;
        }

I assume that your noise is just after the writeSilence(). You can play with the length of the silence to confirm my assumption. This would be the transition when I2S stops to receive any data.

If you check in the Board Manager. What ESP32 Version are you using ?

[pschatzmann]

I am closing this issue since I can't reproduce it!

[0x0fe]

yes, this issue may be related to the ESP32S3, or the combination of ESP32S3 and the specific DAC i use, which requires MCLK in addition to BCLK and WS. One thing i noticed recently when doing some tests, when i change the samplerate with i2s_set_sample_rates and if I2S_AUTO_CLEAR is set tu true I get this "fart" noise, however if I2S_AUTO_CLEAR is false i get the current buffer played in loop 2 or 3 times instead, but no digital fart. So it seems to be related to the I2S / DMA buffering handling maybe.

[pschatzmann]

Maybe we should consider to switch from the old I2S API to the new one...