A2DP sink demo reconnect bug

[Elrir94]

Describe the bug

In the a2dp_sink_demo example built for the pico2W there is no more audio, after reconnecting.

Expected behavior
Audio working after reconnecting to the pico.

HCI Packet Logs
Packets arrives but are not saved in the SBC ring buffer. In fact I logged them and they arrive but I don't have a dump now.

Environment: (please complete the following information):

• btstack version 1.6.2 for the latest pico-sdk 2.1.1
• Pico 2 W with onboard controller
• Pixel 8A

Additional context
From the concise description when I follow these steps:
1)I connect to the pico with my phone
2)I listen to some music, it works really good!
3)I disconnect and then reconnect
4)and finally I restart the music,
The first thing happened was that the pico panicked because it cannot claim again the same pio SM, so i modified the code of the audio_i2s.c to have a deinit function that is something like this:

void audio_i2s_deinit() {
   audio_i2s_set_enabled(false);
   pio_remove_program_and_unclaim_sm(&audio_i2s_program,audio_pio,shared_state.pio_sm,shared_state.offset);
   dma_irqn_set_channel_enabled(PICO_AUDIO_I2S_DMA_IRQ, shared_state.dma_channel, 0);
   irq_set_enabled(DMA_IRQ_0 + PICO_AUDIO_I2S_DMA_IRQ, 0);
   user_irq_unclaim(DMA_IRQ_0 + PICO_AUDIO_I2S_DMA_IRQ);
   added_irq = true;
   dma_channel_unclaim(shared_state.dma_channel);
} 

and i added that code to
https://github.com/raspberrypi/pico-examples/blob/84e8d489ca321a4be90ee49e36dc29e5c645da08/pico_w/bt/btstack_audio_pico.c#L192

static void btstack_audio_pico_sink_close(void){
    // stop stream if needed
    if (btstack_audio_pico_sink_active){
        btstack_audio_pico_sink_stop_stream();
    }
    audio_i2s_deinit();
}

so now it works, i can disconnect and reconnect and it doesn't panic anymore but I can't hear any audio and on the console I receive the "Error storing samples in SBC ring buffer!!!" over and over. It seems to have something to do with buffers, in fact using the debugger i found out that the code listed below (in the same file btstack_audio_pico.c) always breaks the loop and doesn't fill the audio_buffer.

static void btstack_audio_pico_sink_fill_buffers(void){
    while (true){
        audio_buffer_t * audio_buffer = take_audio_buffer(btstack_audio_pico_audio_buffer_pool, false);
        if (audio_buffer == NULL){
            break; // BREAKS HERE
        }

        int16_t * buffer16 = (int16_t *) audio_buffer->buffer->bytes;
        (*playback_callback)(buffer16, audio_buffer->max_sample_count);

        // duplicate samples for mono
        if (btstack_audio_pico_channel_count == 1){
            int16_t i;
            for (i = SAMPLES_PER_BUFFER - 1 ; i >= 0; i--){
                buffer16[2*i  ] = buffer16[i];
                buffer16[2*i+1] = buffer16[i];
            }
        }

        audio_buffer->sample_count = audio_buffer->max_sample_count;
        give_audio_buffer(btstack_audio_pico_audio_buffer_pool, audio_buffer);
    }
}

Any help would be greatly appreciated.
Thanks.
Mattia.

[mringwal]

btstack_audio_pico_sink_fill_buffers implements a polling logic, so it's plausible/expected that the call to take_audio_buffer returns NULL - but it shouldn't return NULL always.

With the original code, init_audio is only called once (as btstack_audio_pico_audio_buffer_pool is never cleared). Could you try to figure out, why init_audio is called twice for you (which then would cause the panic, as the PIO SM is already set up)?

Alternatively, it might be good to test the open/start/stop/close sequence, e.g. with a timer and the mod-player or the sine_player demos.

[Elrir94]

1)Yes, with the original code, init_audio it's called every time you connect a bluetooth audio source and play some music. If you disconnect after that and then finally reconnect and try to play music again it panics only the second time you play the music. That's because after the connection is released and the stream is closed via this function :

static void media_processing_close(void){
    if (!media_initialized) return;
    media_initialized = 0;
    audio_stream_started = 0;
    sbc_frame_size = 0;

    // stop audio playback
    const btstack_audio_sink_t * audio = btstack_audio_sink_get_instance();
    if (audio){
        printf("close stream\n");
        audio->close();
    }
}

that calls static void btstack_audio_pico_sink_close(void) in which audio is stopped but the PIO is never unclaimed, so I added the code I mentioned before and it doesn't panics anymore. First problem solved.

2)Second problem is that there is no audio after you do the steps i listed above. Regarding the buffer I dug deeper with the debugger and the strange behaviour is : initially data are written into the ring buffer, for about 1904 Bytes but then the function:

static void playback_handler(int16_t * buffer, uint16_t num_audio_frames){
    
    // called from lower-layer but guaranteed to be on main thread
    if (sbc_frame_size == 0){
        memset(buffer, 0, num_audio_frames * BYTES_PER_FRAME);
        return;
    }

    // first fill from resampled audio
    uint32_t bytes_read;
    btstack_ring_buffer_read(&decoded_audio_ring_buffer, (uint8_t *) buffer, num_audio_frames * BYTES_PER_FRAME, &bytes_read);
    buffer          += bytes_read / NUM_CHANNELS;
    num_audio_frames   -= bytes_read / BYTES_PER_FRAME;

    // then start decoding sbc frames using request_* globals
    request_buffer = buffer;
    request_frames = num_audio_frames;
    while (request_frames && btstack_ring_buffer_bytes_available(&sbc_frame_ring_buffer) >= sbc_frame_size) {
        // decode frame
        uint8_t sbc_frame[MAX_SBC_FRAME_SIZE];
        btstack_ring_buffer_read(&sbc_frame_ring_buffer, sbc_frame, sbc_frame_size, &bytes_read);
        btstack_sbc_decoder_process_data(&state, 0, sbc_frame, sbc_frame_size);
        //printf("bytes_read = %d\n", bytes_read);
    }
}

doesn't get called anymore and the program outputs : "Error storing samples in SBC ring buffer!!!" to the serial monitor. By looking at the returned code of the function btstack_ring_buffer_write it is ERROR_CODE_MEMORY_CAPACITY_EXCEEDED.

I noticed that the problem regards the buffers in particular the ones regarding the audio which are implemented in this file https://github.com/raspberrypi/pico-extras/blob/4ccfef6fe068bbae8106d0d1f072c7f997472040/src/common/pico_audio/audio.cpp like a producer-consumer with semaphores to sync the access to the shared buffer. Maybe the problem is there, what do you think?
I can't grasp quite well all the code in the audio.cpp and audio_i2s.c files because they are all undocumented.

Anyway thanks for the help!
Edit:
3)Tomorrow I'm going to test the mod_player demo.

[mringwal]

I'm still confused. In btstack_audio_pico_sink_init, btstack_audio_pico_audio_buffer_pool is set with the return value from init_audio. Assuming that the return value isn't NULL, init_audio should never be called again.

How/why/where do you think btstack_audio_pico_audio_buffer_pool is set back to NULL?

I'd suggest to follow the quick & dirty approach, which does not try to free the PIO SMs and I2S, and just tries to re-enable I2S playback after the reconnect. (Yes, it would be better to fully release all resources, but that requires a better understanding)

[Elrir94]

Ok the quick and dirty approach works! In the media_processing_close

static void media_processing_close(void){
    if (!media_initialized) return;
    //media_initialized = 0; <--------don't clear this flag so it doesn't initialize again after reconnect!
    audio_stream_started = 0;
    sbc_frame_size = 0;

    // stop audio playback
    const btstack_audio_sink_t * audio = btstack_audio_sink_get_instance();
    if (audio){
        printf("close stream\n");
        audio->close();
    }
}

Thank you!
When I'll have some sort of documentation and a better understanding of the pico audio I'll try to free the resource.