espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.75k stars 7.44k forks source link

ESP_I2S playMP3() - playback problem #10469

Closed toybuilder closed 1 month ago

toybuilder commented 1 month ago

Board

ESP32 dev module

Device Description

ESP32-WROOM on custom board that has IIS decoder on board.

Hardware Configuration

N/A

Version

latest master (checkout manually)

IDE Name

Arduino

Operating System

Windows 11

Flash frequency

80MHz

PSRAM enabled

yes

Upload speed

921600

Description

When playing back MP3 files, some MP3 files will play, others will not. It might be related to the handling of ID3 tag as described in https://github.com/adafruit/Adafruit_MP3/issues/5 ?

Sample "bad" mp3 file at https://github.com/toybuilder/esp-arduino-i2s-audio/blob/main/nogood.mp3

Hardware is fine, as playback of other MP3 files work as expected.

Sketch

#include <ESP_I2S.h>
// Simple example to play an MP3 audio
// Using Arduino esp32 support version 3.0.5

// ESP-I2S support:
// see https://docs.espressif.com/projects/arduino-esp32/en/latest/api/i2s.html#sample-code
// see source at https://github.com/espressif/arduino-esp32/tree/master/libraries/ESP_I2S/src
I2SClass I2S;

// Using Arduino INCBIN library to embed the MP3 file
// See INCBIN documentation about the location of the audio file. https://reference.arduino.cc/reference/en/libraries/incbin/
// Easiest is to use absolute path for the audio filename
#include "incbin.h"
INCBIN (Sound,"coin-recieved-230517.mp3"); // replace filename with your MP3 file

// Note: Not all mp3 files work.  Specifically, MP3 recorded using Windows Sound Recorder would not play in my testing.
// Also, be careful about the size of the sound file, as it will take space out of the heap to store the data
// These short sound files were tested and worked:
// https://pixabay.com/users/brvhrtz-33128829/ stab-f-01-brvhrtz-224599.mp3
// https://pixabay.com/users/ribhavagrawal-39286533/  coin-recieved-230517.mp3

void play_audio() {
  I2S.setPins(23, 4, 33, -1, -1); //SCK, WS, SDOUT, SDIN, MCLK
  I2S.begin(I2S_MODE_STD, 16000, I2S_DATA_BIT_WIDTH_16BIT, I2S_SLOT_MODE_STEREO);
  I2S.playMP3((uint8_t*)gSoundData,gSoundSize);
  I2S.end();
}

Debug Message

none.  

playMP3 returns false for problem MP3 files

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

toybuilder commented 1 month ago

I filed this issue, as other people trying to get this working might get thrown off by this issue. I got lucky and the first MP3 file I tried worked. It was when I tried to use other MP3 files that I went down the rabbit hole. If someone tried with a "bad" MP3 file to begin with, they could have a much harder time.

me-no-dev commented 1 month ago

Can you provide more information about the different MP3 files you have tried? Bitrate, etc.

toybuilder commented 1 month ago

On https://github.com/toybuilder/esp-arduino-i2s-audio/, I checked in good.mp3 and nogood.mp3 nogood.mp3 (192kbps) was recorded with Windows Sound Recorder. It doesn't work. That file was then re-encoded into good.mp3 (225kbps) by using an online conversion tool (https://restream.io/tools/mp3-converter).

Some other files that I tried and worked were 256kbps mp3's downloaded from pixabay.com. The Windows Sound Recorder files that fall failed were between 96kbps to 192kbps.

It occurred to me that I should just try removing the ID3 tag -- following the instruction found at https://www.frightideas.com/blog/post/removing-id3-tags, I made a copy of nogood.mp3 without ID3 tag (or other properties) and that worked. I checked that into the github repo that I mentioned earlier.

me-no-dev commented 1 month ago

For MP3 playback we use this ESP-IDF component. Maybe you can post an issue there with your findings and they can do the necessary steps to update the code to play the bad mp3

toybuilder commented 1 month ago

I think the problem is in ESP_I2S.cpp itself.

If you look at the reference playback code, err=MP3Decode(...) does not result in an early exit of the decoding:

https://github.com/chmorgan/libhelix-mp3/blob/fb0c06b5cb964c50706c08b0c3bc6615f08d0ffe/testwrap/main.c#L156C3-L175C4 (specifically: https://github.com/chmorgan/libhelix-mp3/blob/fb0c06b5cb964c50706c08b0c3bc6615f08d0ffe/testwrap/main.c#L162)

but in ESP_I2S.cpp, it just exits prematurely:

https://github.com/espressif/arduino-esp32/blob/f8e03cfaac337731ffaad0a7af0d45e91adab1a4/libraries/ESP_I2S/src/ESP_I2S.cpp#L1033C5-L1037C20

(Disregard this comment. I now think it is wrong.)

toybuilder commented 1 month ago

I realized that I can re-build my program under Arduino with Core Debug Level: Verbose logging turned on, and I now see that the problem is err -6 (ERR_MP3_INVALID_FRAMEHEADER).

I will file the issue there. Thank you for pointing me to the libhelix-mp3 component.

toybuilder commented 1 month ago

It looks like I actually was on the right track originally... The issue is that MP3Decode will return when an ID3 tag contains bytes that look like a sync pattern for an MP3 frame, which does not result in a proper decoding of an MP3 frame header. MP3Decode then needs to be called again to resume searching for the next sync pattern.

I think maybe adding a note in the documentation for playMP3() is needed, informing the reader to remove ID3 tags from the source data AND/OR to properly handle skipping over the ID3 data.

toybuilder commented 1 month ago

FWIW, I was able to get it to work by making the following changes to ESP_I2S.cpp:

    if (err) {
      log_e("Decode ERROR: %d", err);

changes below

      if (bytesAvailable) {
        readPtr++;
        bytesAvailable--;
      }
      //MP3FreeDecoder(decoder);
      //return false;

changes above

    } else {
      MP3GetLastFrameInfo(decoder, &frameInfo);
      configureTX(frameInfo.samprate, (i2s_data_bit_width_t)frameInfo.bitsPerSample, (i2s_slot_mode_t)frameInfo.nChans);
      write((uint8_t *)outBuf, (size_t)((frameInfo.bitsPerSample / 8) * frameInfo.outputSamps));
    }
  } while (true);
VojtechBartoska commented 1 month ago

@toybuilder Can I close this as asnwered?

toybuilder commented 1 month ago

I guess so. At least if others encounter the same issue, we've at least described what the problem and workaround are.