chmorgan / esp-audio-player

Apache License 2.0
14 stars 5 forks source link

lock != NULL && "Uninitialized lock used" #9

Closed sokolsok closed 8 months ago

sokolsok commented 9 months ago

In my project, I'm reading mp3 files from an SD card and sending them via I2C to an audio amplifier. Everything works fine when I open a file (using fopen()), play it (with audio_player_play()), and then open and play the next one. However, I run into a problem when I try to reopen the same mp3 file a second time. It doesn't matter whether I do this while the playback is ongoing or wait until it's finished.

I always immediately get an error saying, "assert failed: check_lock_nonzero locks.c:300 (lock != NULL && 'Uninitialized lock used')."

Thanks in advance for the help!

image

chmorgan commented 9 months ago

Can you share more of your code? The audio player module should close the file handle when it’s done, so I’m curious if there is something else going on with other code.

Also, can you get a back trace for what is triggering that assertion?

Chris

On Sat, Dec 2, 2023 at 4:06 PM sokolsok @.***> wrote:

In my project, I'm reading mp3 files from an SD card and sending them via I2C to an audio amplifier. Everything works fine when I open a file (using fopen()), play it (with audio_player_play()), and then open and play the next one. However, I run into a problem when I try to reopen the same mp3 file a second time. It doesn't matter whether I do this while the playback is ongoing or wait until it's finished.

I always immediately get an error saying, "assert failed: check_lock_nonzero locks.c:300 (lock != NULL && 'Uninitialized lock used')."

Thanks in advance for the help!

image.png (view on web) https://github.com/chmorgan/esp-audio-player/assets/33319533/fe02ba24-9f03-4d93-b15e-285621d85872

— Reply to this email directly, view it on GitHub https://github.com/chmorgan/esp-audio-player/issues/9, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AEQBRWKDVI3YUIBLFDYHOJ5ZAVCNFSM6AAAAABAEIH6A2VHI2DSMVQWIX3LMV43ASLTON2WKOZSGAZDEMRQGM3DINY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

sokolsok commented 9 months ago

Thank you for your lightning-fast response! :) Sure, here's my code. I've tried to cut out everything that's not directly related to my issue. And of course, I assume I'm doing something wrong during the initialization, but I have no clue what..

Just to clarify: I'm using ESP-IDF and I added your component directly from the IDE. I'm not sure if that has any relevance.

SOURCE:

#include "audio.h"

/* Variables */
static i2s_chan_handle_t tx_chan;

/* Initialization of I2S */
static void i2s_init_std_duplex(void)
{

    i2s_chan_config_t chan_cfg = I2S_CHANNEL_DEFAULT_CONFIG(I2S_NUM_AUTO, I2S_ROLE_MASTER);
    ESP_ERROR_CHECK(i2s_new_channel(&chan_cfg, &tx_chan, NULL));

    i2s_std_config_t std_cfg = {
        .clk_cfg  = I2S_STD_CLK_DEFAULT_CONFIG(TX_FREQ_HZ),
        .slot_cfg = I2S_STD_MSB_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_16BIT, I2S_SLOT_MODE_MONO),
        .gpio_cfg = {
            .mclk = GPIO_NUM_NC,    // some codecs may require mclk signal, this example doesn't need it
            .bclk = STD_BCLK_IO1,
            .ws   = STD_WS_IO1,
            .dout = STD_DOUT_IO1,
            .din  = STD_DOUT_IO1, // In duplex mode, bind output and input to a same gpio can loopback internally
            .invert_flags = {
                .mclk_inv = false,  
                .bclk_inv = false,
                .ws_inv   = false,
            },
        },
    };
    /* Initialize the channels */
    ESP_ERROR_CHECK(i2s_channel_init_std_mode(tx_chan, &std_cfg));
}

static esp_err_t bsp_i2s_write(void * audio_buffer, size_t len, size_t *bytes_written, uint32_t timeout_ms)
{
    esp_err_t ret = ESP_OK;
    ret = i2s_channel_write(tx_chan, (char *)audio_buffer, len, bytes_written, timeout_ms);
    return ret;
}

static esp_err_t audio_mute_function(AUDIO_PLAYER_MUTE_SETTING setting) {
    printf("mute setting %d\r\n", setting);

    return ESP_OK;
}

static esp_err_t bsp_i2s_reconfig_clk(uint32_t rate, uint32_t bits_cfg, i2s_slot_mode_t ch)
{
    esp_err_t ret = ESP_OK;
    i2s_std_config_t std_cfg = {
        .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(rate),
        .slot_cfg = I2S_STD_PHILIP_SLOT_DEFAULT_CONFIG((i2s_data_bit_width_t)bits_cfg, (i2s_slot_mode_t)ch),
        .gpio_cfg = BSP_I2S_GPIO_CFG,
    };

    ret |= i2s_channel_disable(tx_chan);
    ret |= i2s_channel_reconfig_std_clock(tx_chan, &std_cfg.clk_cfg);
    ret |= i2s_channel_reconfig_std_slot(tx_chan, &std_cfg.slot_cfg);
    ret |= i2s_channel_enable(tx_chan);
    return ret;
}

void AudioTask(void *arg)
{
    esp_err_t ret;

    i2s_init_std_duplex();

    audio_player_config_t config = { .mute_fn = audio_mute_function,
                                     .write_fn = bsp_i2s_write,
                                     .clk_set_fn = bsp_i2s_reconfig_clk,
                                     .priority = 10 };

    ret = audio_player_new(config);
    if (ret == ESP_OK)
        printf("DBG: Audio player cerated successfully\r\n");

    i2s_channel_enable(tx_chan);

    /* Create a task that handles the initialization of an SD card. */ 
    xTaskCreate(SDCardTask, "SDCardTask", 4048, NULL, 1, NULL);
    vTaskDelay(1000 / portTICK_PERIOD_MS);

    printf("DBG: Reading file...\r\n");
    FILE *f = fopen("/sdcard/COB_1A.mp3", "rb");
    if (f == NULL) {
        perror("mp3 opening fail");
    }

    audio_player_play(f);

    vTaskDelay(20000 / portTICK_PERIOD_MS);

    audio_player_play(f); // <----------- this is where the problem occurs

    vTaskDelete(NULL);
}

HEADER:


#ifndef MAIN_AUDIO_H_
#define MAIN_AUDIO_H_

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <inttypes.h>
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "freertos/queue.h"
#include "freertos/semphr.h"
#include "driver/gpio.h"
#include <sys/unistd.h>
#include <sys/stat.h>
#include "audio_player.h"
#include "sd_card.h"
#include "errno.h"
#include "mp3_decoder.h"

#define CONFIG_BSP_I

/* Audio defines */
#define STD_BCLK_IO1        GPIO_NUM_27      // I2S bit clock io number
#define STD_WS_IO1          GPIO_NUM_26      // I2S word select io number
#define STD_DOUT_IO1        GPIO_NUM_25     // I2S data out io number
#define STD_DIN_IO1         GPIO_NUM_25     // I2S data in io number

#define BUFF_SIZE           2048

#define TX_FREQ_HZ          44100

/**
 * @brief ESP-BOX I2S pinout
 *
 * Can be used for i2s_std_gpio_config_t and/or i2s_std_config_t initialization
 */
#define BSP_I2S_GPIO_CFG            \
    {                               \
            .mclk = GPIO_NUM_NC,    \
            .bclk = STD_BCLK_IO1,   \
            .ws   = STD_WS_IO1,     \
            .dout = STD_DOUT_IO1,   \
            .din  = STD_DOUT_IO1,   \
            .invert_flags = {       \
                .mclk_inv = false,  \
                .bclk_inv = false,  \
                .ws_inv   = false,  \
            },                      \
    }

/**
 * @brief Mono Duplex I2S configuration structure
 *
 * This configuration is used by default in bsp_audio_init()
 */
#define BSP_I2S_DUPLEX_MONO_CFG(_sample_rate)                                                         \
    {                                                                                                 \
        .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(_sample_rate),                                          \
        .slot_cfg = I2S_STD_PHILIP_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_16BIT, I2S_SLOT_MODE_MONO), \
        .gpio_cfg = BSP_I2S_GPIO_CFG,                                                                 \
    }

typedef struct {
    char header[3];     /*!< Always "TAG" */
    char title[30];     /*!< Audio title */
    char artist[30];    /*!< Audio artist */
    char album[30];     /*!< Album name */
    char year[4];       /*!< Char array of year */
    char comment[30];   /*!< Extra comment */
    char genre;         /*!< See "https://en.wikipedia.org/wiki/ID3" */
} __attribute__((packed)) mp3_id3_header_v1_t;

typedef struct {
    char header[3];     /*!< Always "ID3" */
    char ver;           /*!< Version, equals to3 if ID3V2.3 */
    char revision;      /*!< Revision, should be 0 */
    char flag;          /*!< Flag byte, use Bit[7..5] only */
    char size[4];       /*!< TAG size */
} __attribute__((packed)) mp3_id3_header_v2_t;

void AudioTask(void *arg);

#endif /* MAIN_AUDIO_H_ */
```_
chmorgan commented 9 months ago

So it looks like the issue is that the file handle is closed upon completion of playback. You’ll have to open another one to play the file again.

On Sat, Dec 2, 2023 at 4:40 PM sokolsok @.***> wrote:

Thank you for your lightning-fast response! :) Sure, here's my code. I've tried to cut out everything that's not directly related to my issue. And of course, I assume I'm doing something wrong during the initialization, but I have no clue what..

Just to clarify: I'm using ESP-IDF and I added your component directly from the IDE. I'm not sure if that has any relevance.

SOURCE: `#include "audio.h"

/ Variables / static i2s_chan_handle_t tx_chan;

static void i2s_init_std_duplex(void) { /* Setp 1: Determine the I2S channel configuration and allocate both channels

  • The default configuration can be generated by the helper macro,
  • it only requires the I2S controller id and I2S role */ i2s_chan_config_t chan_cfg = I2S_CHANNEL_DEFAULT_CONFIG(I2S_NUM_AUTO, I2S_ROLE_MASTER); ESP_ERROR_CHECK(i2s_new_channel(&chan_cfg, &tx_chan, NULL));

/* Step 2: Setting the configurations of standard mode, and initialize tx channel

  • The slot configuration and clock configuration can be generated by the macros
  • These two helper macros is defined in 'i2s_std.h' which can only be used in STD mode.
  • They can help to specify the slot and clock configurations for initialization or re-configuring / i2s_std_config_t std_cfg = { .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(TX_FREQ_HZ), .slot_cfg = I2S_STD_MSB_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_16BIT, I2S_SLOT_MODE_MONO), .gpio_cfg = { .mclk = GPIO_NUM_NC, // some codecs may require mclk signal, this example doesn't need it .bclk = STD_BCLK_IO1, .ws = STD_WS_IO1, .dout = STD_DOUT_IO1, .din = STD_DOUT_IO1, // In duplex mode, bind output and input to a same gpio can loopback internally .invert_flags = { .mclk_inv = false, .bclk_inv = false, .ws_inv = false, }, }, }; / Initialize the channels */ ESP_ERROR_CHECK(i2s_channel_init_std_mode(tx_chan, &std_cfg));

}

static esp_err_t bsp_i2s_write(void audio_buffer, size_t len, size_t bytes_written, uint32_t timeout_ms) { esp_err_t ret = ESP_OK; ret = i2s_channel_write(tx_chan, (char *)audio_buffer, len, bytes_written, timeout_ms); return ret; }

static esp_err_t audio_mute_function(AUDIO_PLAYER_MUTE_SETTING setting) { printf("mute setting %d\r\n", setting);

return ESP_OK;

}

static esp_err_t bsp_i2s_reconfig_clk(uint32_t rate, uint32_t bits_cfg, i2s_slot_mode_t ch) { esp_err_t ret = ESP_OK; i2s_std_config_t std_cfg = { .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(rate), .slot_cfg = I2S_STD_PHILIP_SLOT_DEFAULT_CONFIG((i2s_data_bit_width_t)bits_cfg, (i2s_slot_mode_t)ch), .gpio_cfg = BSP_I2S_GPIO_CFG, };

ret |= i2s_channel_disable(tx_chan); ret |= i2s_channel_reconfig_std_clock(tx_chan, &std_cfg.clk_cfg); ret |= i2s_channel_reconfig_std_slot(tx_chan, &std_cfg.slot_cfg); ret |= i2s_channel_enable(tx_chan); return ret;

}

void AudioTask(void

arg) { esp_err_t ret; size_t f_out = 61024;

const char *test = MOUNT_POINT"/test.mp3";

i2s_init_std_duplex();

audio_player_config_t config = { .mute_fn = audio_mute_function, .write_fn = bsp_i2s_write, .clk_set_fn = bsp_i2s_reconfig_clk, .priority = 10 };

ret = audio_player_new(config); if (ret == ESP_OK) printf("DBG: Audio player cerated successfully\r\n");

i2s_channel_enable(tx_chan);

/ Create a task that handles the initialization of an SD card. / xTaskCreate(SDCardTask, "SDCardTask", 4048, NULL, 1, NULL); vTaskDelay(1000 / portTICK_PERIOD_MS);

printf("DBG: Reading file...\r\n"); FILE *f = fopen("/sdcard/COB_1A.mp3", "rb"); if (f == NULL) { perror("mp3 opening fail"); }

audio_player_play(f);

vTaskDelay(20000 / portTICK_PERIOD_MS);

audio_player_play(f); // <----------- this is where the problem occurs

vTaskDelete(NULL);

}`

HEADER: `#ifndef MAIN_AUDIOH

define MAIN_AUDIOH

include

include

include

include

include "freertos/FreeRTOS.h"

include "freertos/task.h"

include "freertos/queue.h"

include "freertos/semphr.h"

include "driver/gpio.h"

include <sys/unistd.h>

include <sys/stat.h>

include "audio_player.h"

include "sd_card.h"

include "errno.h"

include "mp3_decoder.h"

define CONFIG_BSP_I

/ Audio defines /

define STD_BCLK_IO1 GPIO_NUM_27 // I2S bit clock io number

define STD_WS_IO1 GPIO_NUM_26 // I2S word select io number

define STD_DOUT_IO1 GPIO_NUM_25 // I2S data out io number

define STD_DIN_IO1 GPIO_NUM_25 // I2S data in io number

define BUFF_SIZE 2048

define TX_FREQ_HZ 44100

/**

  • @brief https://github.com/brief ESP-BOX I2S pinout
  • Can be used for i2s_std_gpio_config_t and/or i2s_std_config_t initialization */

    define BSP_I2S_GPIO_CFG

    { .mclk = GPIO_NUM_NC, .bclk = STD_BCLK_IO1, .ws = STD_WS_IO1, .dout = STD_DOUT_IO1, .din = STD_DOUT_IO1, .invert_flags = { .mclk_inv = false, .bclk_inv = false, .ws_inv = false, }, }

/**

  • @brief https://github.com/brief Mono Duplex I2S configuration structure
  • This configuration is used by default in bsp_audio_init() */

    define BSP_I2S_DUPLEX_MONO_CFG(_sample_rate)

    { .clk_cfg = I2S_STD_CLK_DEFAULT_CONFIG(_sample_rate), .slot_cfg = I2S_STD_PHILIP_SLOT_DEFAULT_CONFIG(I2S_DATA_BIT_WIDTH_16BIT, I2S_SLOT_MODE_MONO), .gpio_cfg = BSP_I2S_GPIO_CFG, }

typedef struct { char header[3]; /!< Always "TAG" / char title[30]; /!< Audio title / char artist[30]; /!< Audio artist / char album[30]; /!< Album name / char year[4]; /!< Char array of year / char comment[30]; /!< Extra comment / char genre; /!< See "https://en.wikipedia.org/wiki/ID3" / } attribute((packed)) mp3_id3_header_v1_t;

typedef struct { char header[3]; /!< Always "ID3" / char ver; /!< Version, equals to3 if ID3V2.3 / char revision; /!< Revision, should be 0 / char flag; /!< Flag byte, use Bit[7..5] only / char size[4]; /!< TAG size / } attribute((packed)) mp3_id3_header_v2_t;

void AudioTask(void *arg);

endif / MAIN_AUDIOH /`

— Reply to this email directly, view it on GitHub https://github.com/chmorgan/esp-audio-player/issues/9#issuecomment-1837260030, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AGE6YCVJVDDOXM7OLTYHON5TAVCNFSM6AAAAABAEIH6A2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZXGI3DAMBTGA . You are receiving this because you commented.Message ID: @.***>

sokolsok commented 9 months ago

You're totally right! Reopening the file handle fixes the issue spot on. Thanks a bunch for your help, and kudos for an absolutely amazing component! :)

Got another question, not exactly about your component but more about audio in general. Since you definitely have way more experience with audio (mine is practically nonexistent), maybe you could help me out. I'm building something like a piano, and I want several .mp3 files to overlap (like when someone presses multiple keys rapidly). Do you think this requires tweaking the mp3 decoder or more the audio player itself? If you could point me in the right direction, I'd be super grateful.

chmorgan commented 9 months ago

That’s an interesting use case.

The audio player component doesn’t lend itself to this kind of usage. Consider that it’s trying to do as much mp3 decoding as possible, to stay ahead of the playback output, but it’s also not working synchronously because the audio system in the esp-idf isn’t asking for audio, its receiving audio that is pushed into it.

Ideally what you’d want is a system by-which you’d track the position in any active note being played, and you’d ask each of these notes for say 512 bytes of audio data. Notes would output up to 512 bytes, stopping if they were done playing.

Your code would, for every sample to be output, sum up all of the notes at that sample index and divide by the number of samples, and that’s the value you’d output. In this manner you’d be mixing the notes in on a per-sample basis. That output buffer is what goes to the i2s device for output.

You could probably use the audio player audio_player_write_fn to do this, but you’ll get data in odd mismatched chunks and need to do some kind of buffering to realign, and your write function would have to use some OS blocking mechanism to keep the audio player suspended while you handled synchronizing things. For sure its doable.

Plus each audio player is going to use a non-trivial amount of ram so if you needed say 16 simultaneous audio players that might take a lot of memory. Feels like the amount of memory used by audio_player_new() would be helpful in the readme or header since I can’t recall and for my single player use case it didn’t matter much, had lots of ram available.

The there is the option to use a pool of audio players, such that anytime you play a note you decode it as quickly as possible into a buffer (assuming you can buffer that much data in terms of ram), and then reuse that player instance for the next audio prompt. That limits the number of audio players but increases the amount of memory required to hold the temporary data. This probably isn’t as good of an approach as the one-per-simultaneous note approach.

You could use wav files, but they are quite large. Advantage there is they are really easy to decode and decoding uses basically zero memory vs. mp3 with a ton of temporary buffers, internal tables etc.

But yeah, if we knew the memory used by audio_player_new(), that would determine how many simultaneous notes you might fit into memory, then the next approach would be to put together the code to synchronize them. It’s definitely doable, and an interesting thought experiment to figure out how to do so, but definitely some work.

Chris

On Dec 2, 2023, at 5:22 PM, sokolsok @.***> wrote:

You're totally right! Reopening the file handle fixes the issue spot on. Thanks a bunch for your help, and kudos for an absolutely amazing component! :)

Got another question, not exactly about your component but more about audio in general. Since you definitely have way more experience with audio (mine is practically nonexistent), maybe you could help me out. I'm building something like a piano, and I want several .mp3 files to overlap (like when someone presses multiple keys rapidly). Do you think this requires tweaking the mp3 decoder or more the audio player itself? If you could point me in the right direction, I'd be super grateful.

— Reply to this email directly, view it on GitHub https://github.com/chmorgan/esp-audio-player/issues/9#issuecomment-1837268109, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AD7ULWWJJ33JSQRFY3YHOS3HAVCNFSM6AAAAABAEIH6A2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZXGI3DQMJQHE. You are receiving this because you commented.

sokolsok commented 9 months ago

Thanks a ton for your thorough explanation! It's always great to hear from someone who knows their stuff :)

You're right, though - such an approach would indeed require quite a bit of resources. I hadn't even thought about how much RAM it could take up...

But since I'm using an SD card, I'm not exactly short on space. I don't need to store all the samples on the internal FLASH. So, I I could use WAV files if it really simplifies (in terms of resource use) the decoding process. Especially since your component supports WAV files. In this case, do you think there's a simpler way to sum up each sample?

P.S. I'm assuming I could do the same for volume up/down, right? Just by adding or subtracting some value from each sample?

I've got another thing on my mind.... After decoding WAV (or mp3) files, we create a data buffer, like 16-bit for each sample. Could we prepare these buffers for each audio file in advance and save them on the SD card as separate files? Then, during the normal operation of the device, we could directly use them. It would be easier to perform any operations like summing, adding, or subtracting. What do you think about this? Do you reckon it's a good direction to go in?

And once again, thank you for your help! Sebastian

chmorgan commented 9 months ago

Yeah I’m probably off on how to mix them together. I’m sure some google searches will turn up how to do it. I’d write some tests to hear how it sounds.

For volume you really want to scale the value, not truncate it.

For sure I’d try first with the wav files since you have the space. It’s totally doable with the mp3s too but unlike file handles, audio players have a lot more overhead :-)

Your last idea is a wav file without the wav file header I think. The wav file is the raw audio with a header. It’s better to keep it in wav format so you know the bit rates and other info. I don’t see many good reasons to keep them entirely raw (eg without the wav header) since you’ll likely wish you had that header at some point.

Let me know how you make out. I’m interested in how it sounds :-)

On Sun, Dec 3, 2023 at 4:33 AM sokolsok @.***> wrote:

Thanks a ton for your thorough explanation! It's always great to hear from someone who knows their stuff :)

You're right, though - such an approach would indeed require quite a bit of resources. I hadn't even thought about how much RAM it could take up...

But since I'm using an SD card, I'm not exactly short on space. I don't need to store all the samples on the internal FLASH. So, I I could use WAV files if it really simplifies (in terms of resource use) the decoding process. Especially since your component supports WAV files. In this case, do you think there's a simpler way to sum up each sample?

P.S. I'm assuming I could do the same for volume up/down, right? Just by adding or subtracting some value from each sample?

I've got another thing on my mind.... After decoding WAV (or mp3) files, we create a data buffer, like 16-bit for each sample. Could we prepare these buffers for each audio file in advance and save them on the SD card as separate files? Then, during the normal operation of the device, we could directly use them. It would be easier to perform any operations like summing, adding, or subtracting. What do you think about this? Do you reckon it's a good direction to go in?

And once again, thank you for your help! Sebastian

— Reply to this email directly, view it on GitHub https://github.com/chmorgan/esp-audio-player/issues/9#issuecomment-1837423566, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AHATFII63POAREOQPLYHRBNJAVCNFSM6AAAAABAEIH6A2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZXGQZDGNJWGY . You are receiving this because you commented.Message ID: @.***>

sokolsok commented 9 months ago

Absolutely, you're right on about WAV files and my (not so bright) idea of converting them to a raw buffer. I didn't think about how an uncompressed WAV file is exactly that - a buffer of 16-bit data.

Like I mentioned, this is my first crack at audio and it's not exactly intuitive for me.

Regarding how it'll eventually sound, you can check it out here: youtube.com/@SmartSolutionsForHome I've got a modest channel on YT where I post my projects. This one is called: PetProdigy and it'll be my next video. PP23

P.S. If you don't mind, I'd love to give a shoutout on my video to your amazing component! :)

chmorgan commented 9 months ago

Of course, always looking to spread the word on useful tools.

It’s used in the esp-box example repo as well, and my day job is using it for a couple of devices.

If you get something posted drop me a link and I’ll see how it sounds.

Let me know if you have any other questions!

Chris

On Sun, Dec 3, 2023 at 8:57 AM sokolsok @.***> wrote:

Absolutely, you're right on about WAV files and my (not so bright) idea of converting them to a raw buffer. I didn't think about how an uncompressed WAV file is exactly that - a buffer of 16-bit data.

Like I mentioned, this is my first crack at audio and it's not exactly intuitive for me.

Regarding how it'll eventually sound, you can check it out here: [ @.** I've got a modest channel on YT where I post my projects. This one is called: PetProdigy* and it'll be my next video. PP23.JPG (view on web) https://github.com/chmorgan/esp-audio-player/assets/33319533/cac09dba-eba0-4108-a6ac-fbb79ca78ab9

P.S. If you don't mind, I'd love to give a shoutout on my video to your amazing component! :)

— Reply to this email directly, view it on GitHub https://github.com/chmorgan/esp-audio-player/issues/9#issuecomment-1837488151, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4ABVGS43MSVR54QSTNDYHSALNAVCNFSM6AAAAABAEIH6A2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZXGQ4DQMJVGE . You are receiving this because you commented.Message ID: @.***>

chmorgan commented 8 months ago

Closing as it looks like this issue has been resolved. Please stay in touch if you have any other questions or comments.

sokolsok commented 7 months ago

Hey Chris, I've been diving deeper into the WAV file mixing topic (while juggling other aspects of this project on the side). But eventually, I had to tackle this toughest nut to crack. I've been wrestling with it for a while now and, I won't lie, I'm hitting a few snags. If you could spare a moment to glance over my project (https://github.com/sokolsok/PetPiano/tree/simple-mode-v2), I'd be incredibly grateful.

So, I copied your library locally to my project for easier editing. I axed everything related to mp3, as I'll only be playing WAV files since they're simpler to handle. In the "main" directory, in the "audio.c" file, I've got the whole player initialization. Your library is in the "audio_player" folder.

The working principle I'm aiming for is this: in a separate task in "keyboard.c", when a key is pressed, it triggers the function "play(some_wav)", which plays a sound. Ps. All files are identical, i.e., mono, 44100kHz, 16bit, and 10s long. This works flawlessly. Next, I'd like it so that when another key is pressed while the previous sound is still playing, it mixes with it instead of interrupting the previous sound and starting anew. And ideally, this should work for any number of keys, but mixing two is bear minimum goal ;)

I thought about doing it this way: have some additional output buffer (let's call it buff_out[]), which ultimately gets sent to I2S. When play() is called, the entire buffer from the file would be copied to this buff_out[] and then played, adding an extra index that tracks how much has already been played. Then, when a key is pressed again, I would reassign the raw WAV file values to the buff_out[] buffer, but this time I could add the rest of the buffer that remained from the previous file that was playing.

Pseudo code:

           `//When nothing is currently playing,
            if(j == 0)
        {
            for(i = 0; i<10; i++)
                    out_buff[i] =  wav_buff[i];
        }
            //When something is already playing.
        else
        {
            for(i = 0; i<10; i++)
            {
                if((j + i) < 10)
                    out_buff[i] =  out_buff[j + i] + wav_buff[i];
                else
                    out_buff[i] =  wav_buff[i];
            }
        }

        j=0;`

I'm not sure if I've made this clear. The problem is, in my case, the entire file is divided into 383 buffers of 2304 bytes each. I'm not sure where in your library I could physically "get to" this buffer and copy it to buff_out[]. Another thing, as far as I understand, it reads these 2304 bytes from the file on the fly, plays them, clears the buffer, and then loads the next 2304 bytes. So the file is always open, right? Would I need to first copy the entire WAV file buffer to some temporary buffer?

Or maybe you think I should approach this differently altogether? Thanks in advance for any advice :)

chmorgan commented 7 months ago

Hello again!

So what I’d look at is creating a function that will receive the wav output i2s data. Effectively you want to receive data from the decoder up to say N bytes, from each separate file, and then mix those samples together into N output bytes you send via i2s.

Your “output function” can receive data and then wait on an event to block the decoder until you need more data. This will suspend the thread/decoder and your final output mixer thread will release the decoders when more data is needed and wait on the signal before processing the data in each output decoders buffer (the one where you’d store the decoded data that would normally be sent directly to i2s)

The smaller the chunks are the closer in time you’ll synchronize output between intended output time and actual time. Like if your chunks are large you’ll want to output at time T but you’ll be delayed by what ever the sample size is, in terms of time.

On Sun, Jan 14, 2024 at 11:12 AM sokolsok @.***> wrote:

Hey Chris, I've been diving deeper into the WAV file mixing topic (while juggling other aspects of this project on the side). But eventually, I had to tackle this toughest nut to crack. I've been wrestling with it for a while now and, I won't lie, I'm hitting a few snags. If you could spare a moment to glance over my project ( https://github.com/sokolsok/PetPiano/tree/simple-mode-v2), I'd be incredibly grateful.

So, I copied your library locally to my project for easier editing. I axed everything related to mp3, as I'll only be playing WAV files since they're simpler to handle. In the "main" directory, in the "audio.c" file, I've got the whole player initialization. Your library is in the "audio_player" folder.

The working principle I'm aiming for is this: in a separate task in "keyboard.c", when a key is pressed, it triggers the function "play(some_wav)", which plays a sound. Ps. All files are identical, i.e., mono, 44100kHz, 16bit, and 10s long. This works flawlessly. Next, I'd like it so that when another key is pressed while the previous sound is still playing, it mixes with it instead of interrupting the previous sound and starting anew. And ideally, this should work for any number of keys, but mixing two is bear minimum goal ;)

I thought about doing it this way: have some additional output buffer (let's call it buff_out[]), which ultimately gets sent to I2S. When play() is called, the entire buffer from the file would be copied to this buff_out[] and then played, adding an extra index that tracks how much has already been played. Then, when a key is pressed again, I would reassign the raw WAV file values to the buff_out[] buffer, but this time I could add the rest of the buffer that remained from the previous file that was playing.

Pseudo code: ` //When nothing is currently playing, if(j == 0) { for(i = 0; i<10; i++) out_buff[i] = wav_buff[i]; } //When something is already playing. else { for(i = 0; i<10; i++) { if((j + i) < 10) out_buff[i] = out_buff[j + i] + wav_buff[i]; else out_buff[i] = wav_buff[i]; } }

  j=0;`

I'm not sure if I've made this clear. The problem is, in my case, the entire file is divided into 383 buffers of 2304 bytes each. I'm not sure where in your library I could physically "get to" this buffer and copy it to buff_out[]. Another thing, as far as I understand, it reads these 2304 bytes from the file on the fly, plays them, clears the buffer, and then loads the next 2304 bytes. So the file is always open, right? Would I need to first copy the entire WAV file buffer to some temporary buffer?

Or maybe you think I should approach this differently altogether? Thanks in advance for any advice :)

— Reply to this email directly, view it on GitHub https://github.com/chmorgan/esp-audio-player/issues/9#issuecomment-1890994198, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AAOWPG27XIKIJU6S2TYOP7WZAVCNFSM6AAAAABAEIH6A2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJQHE4TIMJZHA . You are receiving this because you modified the open/close state.Message ID: @.***>

sokolsok commented 7 months ago

Thank you so much for your response! I really appreciate it :)

It seems like I'm heading in that direction. I want to create a function that will capture the new chunk and merge it with the one currently playing. However, I have two problems with this: where in your library do you think is the best place for this? In the "esp_err_t aplay_file(audio_instance_t i, FILE fp)" function? The second issue is that there are 383 chunks per sound. When opening a file, would I need to copy the entire buffer into some local buffer? Because from what I understood from your library, the next chunks are read continuously, which saves a lot of memory. Am I misunderstanding something?

Regarding the delay, sure. The larger the chunk, the greater the delay will be, but I don't have any strict requirements here :)

Thanks again for the help!

chmorgan commented 7 months ago

You can use the write function to pass in your handler:

audio_player_write_fn write_fn;

You’d need one decoder per file playing.

The decoder decodes as quickly as the write_fn accepts data. So if you make a write function that only accepts so much data and then blocks, waiting on an event, you can then take action in another thread to mix the data together and pass it to the real i2s device.

On Tue, Jan 16, 2024 at 5:51 PM sokolsok @.***> wrote:

Thank you so much for your response! I really appreciate it :)

It seems like I'm heading in that direction. I want to create a function that will capture the new chunk and merge it with the one currently playing. However, I have two problems with this: where in your library do you think is the best place for this? In the "esp_err_t aplay_file(audio_instance_t i, FILE fp)" function? The second issue is that there are 383 chunks per sound. When opening a file, would I need to copy the entire buffer into some local buffer? Because from what I understood from your library, the next chunks are read continuously, which saves a lot of memory. Am I misunderstanding something?

Regarding the delay, sure. The larger the chunk, the greater the delay will be, but I don't have any strict requirements here :)

Thanks again for the help!

— Reply to this email directly, view it on GitHub https://github.com/chmorgan/esp-audio-player/issues/9#issuecomment-1894647935, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AC6S2ONTQQ3PFA5QBLYO4AADAVCNFSM6AAAAABAEIH6A2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJUGY2DOOJTGU . You are receiving this because you modified the open/close state.Message ID: @.***>