Does ESP-ADF support c++ (AUD-5323)

[tank104]

Environment

Audio development kit: [ESP32-Dev]
[Required] Module or chip used: ESP32-WROVER-E
[Required] IDF version: v5.2.1
[Required] ADF version: v2.6-86-g1a03fe6acd esp-id
Build system: CMake
[Required] Running log: All logs from power-on to problem recurrence
Compiler version : that command doesn't work
Operating system: Windows
(Windows only) Environment type: ESP Command Prompt
Using an IDE?: PlatformIO & VSCode
Power supply: USB

Problem Description

If I change sample app:
https:/espressif/esp-adf/blob/master/examples/player/pipeline_spiffs_mp3/main/play_spiffs_mp3_example.c

to be a cpp file and change main to: extern "C" void app_main(void)

I get a couple of compile errors:

components/esp-adf-libs/esp_codec/include/codec/filter_resample.h:65:5: error: designator order for field 'rsp_filter_cfg_t::dest_rate' does not match declaration order in 'rsp_filter_cfg_t'

components/audio_stream/include/i2s_stream.h:232:1: error: designator order for field 'i2s_stream_cfg_t::transmit_mode' does not match declaration order in 'i2s_stream_cfg_t'

Expected Behavior

Compiles code and works

Actual Behavior

Errors

Steps to Reproduce

Change main.c to main.cpp and extern "C" void app_main(void)

My first question is that am I doing this wrong - is only C supported when using ADF?
I could probably try and fix those files, but don't want to if its just not supported.

[tank104]

I found I can fix by declaring those two issues in my main.cpp like below. (just changing order of a couple of properties)

However question still stands whether this library is designed with cpp in mind?

#define I2S_STREAM_CFG_DEFAULT_CPP() I2S_STREAM_CFG_DEFAULT_WITH_PARA_CPP(I2S_NUM_0, 44100, I2S_DATA_BIT_WIDTH_16BIT, AUDIO_STREAM_WRITER)

#define I2S_STREAM_CFG_DEFAULT_WITH_PARA_CPP(port, rate, bits, stream_type)  {  \
    .type = stream_type,                                                        \
    .transmit_mode = I2S_COMM_MODE_STD,                                         \
    .chan_cfg = {                                                               \
        .id = port,                                                             \
        .role = I2S_ROLE_MASTER,                                                \
        .dma_desc_num = 3,                                                      \
        .dma_frame_num = 312,                                                   \
        .auto_clear = true,                                                     \
    },                                                                          \
    .std_cfg = {                                                                \
        .clk_cfg  = I2S_STD_CLK_DEFAULT_CONFIG(rate),                           \
        .slot_cfg = I2S_STD_PHILIPS_SLOT_DEFAULT_CONFIG(bits, I2S_SLOT_MODE_STEREO),  \
        .gpio_cfg = {                                                           \
            .invert_flags = {                                                   \
                .mclk_inv = false,                                              \
                .bclk_inv = false,                                              \
            },                                                                  \
        },                                                                      \
    },                                                                          \
    .expand_src_bits = I2S_DATA_BIT_WIDTH_16BIT,                                \
    .use_alc = false,                                                           \
    .volume = 0,                                                                \
    .out_rb_size = I2S_STREAM_RINGBUFFER_SIZE,                                  \
    .task_stack = I2S_STREAM_TASK_STACK,                                        \
    .task_core = I2S_STREAM_TASK_CORE,                                          \
    .task_prio = I2S_STREAM_TASK_PRIO,                                          \
    .stack_in_ext = false,                                                      \
    .multi_out_num = 0,                                                         \
    .uninstall_drv = true,                                                      \
    .need_expand = false,                                                       \
    .buffer_len = I2S_STREAM_BUF_SIZE,                                          \
  }

#define DEFAULT_RESAMPLE_FILTER_CONFIG_CPP()      \
    {                                             \
      .src_rate = 44100,                          \
      .src_ch = 2,                                \
      .dest_rate = 48000,                         \
      .dest_bits = 16,                            \
      .dest_ch = 2,                               \
      .src_bits = 16,                             \
      .mode = RESAMPLE_DECODE_MODE,               \
      .max_indata_bytes = RSP_FILTER_BUFFER_BYTE, \
      .out_len_bytes = RSP_FILTER_BUFFER_BYTE,    \
      .type = ESP_RESAMPLE_TYPE_AUTO,             \
      .complexity = 2,                            \
      .down_ch_idx = 0,                           \
      .prefer_flag = ESP_RSP_PREFER_TYPE_SPEED,   \
      .out_rb_size = RSP_FILTER_RINGBUFFER_SIZE,  \
      .task_stack = RSP_FILTER_TASK_STACK,        \
      .task_core = RSP_FILTER_TASK_CORE,          \
      .task_prio = RSP_FILTER_TASK_PRIO,          \
      .stack_in_ext = true,                       \
    }

[jason-mao]

@tank104 Thanks for your feedback. To be honest, we support C++ in theory( we declare the C function extern "C"), but there is no such example. I think we can add a C++ example to check that.

[tank104]

Is it worth me doing a PR that changes the order to fix this for CPP? Or there will be so many of these issues that I am best to do my fixes locally?

[jason-mao]

@tank104 I'd appreciate it if you could create a PR.

[tank104]

Pull request created: #1192

[tank104]

And espressif/esp-adf-libs#31

[X-Ryl669]

Thanks for your PR.

BTW, using macros to initialize structures is poor coding technique (either in C or C++). This is because, as you've seen, as soon as one modify the structure, and forget to update the macro (like when ADF went from 2.4 to 2.5 and from 2.5 to 2.6), then this break code in subtle and often impossible to debug ways.

The C compiler won't tell you anything about a non initialized member so your code that was previously working is now broken and you have no clue why. At least, the C++ compiler will warn you about a misordered declaration. Yet, any forgotten member will be initialized to 0 and this can be catastrophic (think of the new task_stack member in 2.6 to understand why).

A good coding practice, IMHO would be to have "construction" function, as static inline so the compiler can inline them anyway, like this:

// sorry, I'm lazy to figure out what A, B, C, D types are
static inline i2s_stream_cfg_t I2S_STREAM_CFG_DEFAULT_WITH_PARA_CPP(A port, B rate, C bits, D stream_type)  {
  i2s_stream_cfg_t cfg;
  cfg.type = stream_type;                                                  
  cfg.transmit_mode = I2S_COMM_MODE_STD;
[...]
   return cfg;
}

You can name it the same as the previous macro to avoid breaking user code. At least, it'll check the parameter type (the macro doesn't) and will work in both C and C++ compiler.

Building this code gives the exact same binary code as the macro version when optimization are on and is debuggable.

[tank104]

Got to be honest - coming back to c++ world after 15 years in c# world, so I don't feel I can offer opinions just yet - although what your saying sounds like a lot of sense, I don't feel comfortable making those changes yet.