pschatzmann / ESP32-A2DP

A Simple ESP32 Bluetooth A2DP Library (to implement a Music Receiver or Sender) that supports Arduino, PlatformIO and Espressif IDF
Apache License 2.0
1.65k stars 270 forks source link

Very noising sound (on M5Stack) #7

Closed morgan-wild closed 3 years ago

morgan-wild commented 3 years ago

Hi and thank you for your really nice job.

I'm trying the sketch to stream data from bluetooth directly to the internal DAC of my M5Stack (which includes everything for audio). But the sound is loud and not clean at all.

I tried to play with the i2s config but the values you defined looks good.

Any idea? Regards.

AlixANNERAUD commented 3 years ago

Hi, I also have exactly the same issue on a NodeMCU32-S and internal dac. It sounds saturated and noisy. Any idea ?

pschatzmann commented 3 years ago

Hallo,

I am not really an expert but my understanding is that the internal DAC is not really suited to generate quality sound. You would really need to connect your device to some audio DAC via I2S if you want some decent quality.

It might still be good enough if your requirements are pretty low or you just want to try things out.

The issues are coming from 2 facts:

Kind regards Phil

On 26 Oct 2020, at 12:07, Alix ANNERAUD notifications@github.com wrote:

Hi, I also have exactly the same issue on a NodeMCU32-S and internal dac. It sounds saturated and noisy. Any idea ?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

AlixANNERAUD commented 3 years ago

Hello, Personnaly, when I use the internal DAC with wav files and the ULP coprocessor (with bitluni ulp sound examples), I can get a pretty decent quality. So I think there's a way to improve that. But I am not an expert of the internet DAC. Maybe it's due to the i2s configuration ? Anyway, I will try it with an external DAC. Thank you for your work ! Regards.

pschatzmann commented 3 years ago

Yes you can try to increase the dma_buf_count or dma_buf_len. Are you sure that your sample rate of 88200 is correct - usually for Audio CD Quality this is 44100 ? I also doubt that you can process that many samples per second…

On 1 Nov 2020, at 01:29, Alix ANNERAUD notifications@github.com wrote:

Hello, Personnaly, when I use the internal DAC with wav files and the ULP coprocessor (with bitluni ulp sound examples), I can get a pretty decent quality. So I think there's a way to improve that. But I am not an expert of the internet DAC. Maybe it's due to the i2s configuration ? Anyway, I will try it with an external DAC. Thank you for your work ! Regards.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

thinkcz commented 3 years ago

I think I found the reason, the problem with internal DACs seems to be in signed/unsigned issue. Internal DAC is 8bit unsigned value whereas data coming through BT is always 16bit signed. This ugly hack in /src/BluetoothA2DPSink.cpp fixed it for me. It basically takes int16_t from input buffer and convert it to uint16_t output then write it to internal DAC. But it is really hack if you use it won't behave correctly when external DAC is used, it needs to be fixed properly.

void  BluetoothA2DPSink::audio_data_callback(const uint8_t *data, uint32_t len) {
   ESP_LOGD(BT_AV_TAG, "%s", __func__);

    static uint16_t voldata[16384];
    for (int i=0;i<len/2;i++) {

        int16_t a = data[i*2] | data[i*2+1]<<8;

        uint16_t correction  = a + 0x8000;
        voldata[i]=correction;

    }

   size_t i2s_bytes_written;
   if (i2s_write(i2s_port,(void*) voldata, len, &i2s_bytes_written, portMAX_DELAY)!=ESP_OK){
      ESP_LOGE(BT_AV_TAG, "i2s_write has failed");    
   }

   if (i2s_bytes_written<len){
      ESP_LOGE(BT_AV_TAG, "Timeout: not all bytes were written to I2S");
   } 

   if (data_received!=NULL){
      data_received();
   }
}
thinkcz commented 3 years ago

We would need to distinguish based on I2S config and use this hack only for I2S_MODE_DAC_BUILT_IN flag. Also, the temp buffer is unreasonably big now.

pschatzmann commented 3 years ago

Cool idea: Is there any reason why you add your own copy of voldata and you don't do the correction directly on the original data ?

My current thought is to add some additional parameters to the constructor of the class which would then trigger this functionality.

thinkcz commented 3 years ago

Well, as I said, it's ugly hack ;) and as the data is const on the input to the method I didn't want to mess with it;). regarding the switch maybe we can just simply check for the flag I2S_MODE_DAC_BUILT_IN in this->i2s_config as the conversion is only needed if this flag for I2S config is present. But maybe even the additional parameter to the constructor would work. Let me try to implement it in a nicer way and send to you pull request.

pschatzmann commented 3 years ago

Cool. I agree, checking for the I2S_MODE_DAC_BUILT_IN is even better...

morgan-wild commented 3 years ago

Thank you @thinkcz for your help and @pschatzmann for your reactivity!

That's true the internal 8 bit DAC is not really suitable for audio, so today I bought an external DAC (PCM5102) and I will test this library with it ;)

But before that, I will wait for the @thinkcz 's pull request!

thinkcz commented 3 years ago

It's there, ready to be merged.

pschatzmann commented 3 years ago

The correction has been merged. Thanks a lot to Martin!

morgan-wild commented 3 years ago

Thank you! The sounds looks very better but is still dirty (a constant noise in the background I can't explain). I hope it will disappear with an external DAC!