audiocore-mp3-decoder #12
Reference in New Issue
Block a user
Delete Branch "audiocore-mp3-decoder"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implement mp3 decoding in audiocore module on core1 using the helix_mp3 fixed point MP3 decoder library.
The audiocore module and C module should be complete, the higher-level mp3player module is still WIP.
da48e7546fto3a0784c863WIP: audiocore-mp3-decoderto audiocore-mp3-decoderJust a few questions, and one irrelevant typo.
@@ -27,0 +55,4 @@uint32_t *buf;if (multicore_fifo_rvalid()) {cmd = multicore_fifo_pop_blocking();switch (cmd) {That#s not something we need to do now, but we definitely need to document the IPC protocol. I am getting lwIP vibes here ^^
There is some "documentation" in audiocore.h where the commands are defined.
@@ -27,0 +87,4 @@i2s_play(samplerate);playing = true;}volume_adjust((int16_t *)buf, 2304, current_volume);2304 should probably be a constant (I think this is the second time I saw it in our codebase)
@@ -86,0 +89,4 @@// FLUSH - signals end of file and stop decoding when buffer empty - no arguments - return 0 when decoding is finished#define AUDIOCORE_CMD_FLUSH 0xdeadc0dd#define AUDIOCORE_MAX_VOLUME 0x8000uStumbled across this because of
volume_adjust()and the 15 bit right shift.I am only a little bit confused, so here's a stupid question: do we implement an efficient 'clamp' here?
The volume adjustment is a kind of fixed point multiplication right now. We multiply our signed 16 bit sample with a 16 bit multiplier and then discard the lower 15 bits. So AUDIOCORE_MAX_VOLUME is in effect a multiply by 1, the PCM samples stay the same. And any lower volume reduces the amplitude. The code can currently not increase the volume, which saves us the trouble of having to deal with clamping and such.
Do you think it is necessary to be able to increase the volume here?
@@ -80,0 +147,4 @@i2s_context.cur_playing = 0;memset(i2s_context.dma_buf, 0, sizeof(i2s_context.dma_buf[0][0]) * AUDIO_BUFS * I2S_DMA_BUF_SIZE);i2s_max98357_program_init(audiocore_pio, i2s_context.pio_sm, i2s_context.pio_program_offset, i2s_context.out_pin,i2s_context.sideset_base, samplerate);Ah, I was already wondering why i2s needs to know about the samplerate. Check 👍
@@ -81,3 +157,4 @@{pio_sm_set_enabled(audiocore_pio, i2s_context.pio_sm, false);dma_channel_set_irq1_enabled(i2s_context.dma_ch, false);dma_channel_abort(i2s_context.dma_ch);Shouldn't be dma_channel_abort() and irq_remove_handler() be the other way round because of E13?Ah, bullshit, line 83 takes care about that... Check 👍
@@ -63,2 +99,3 @@if (to_copy > 0) {audiocore_audio_buffer_put(bufinfo.buf, to_copy);audiocore_buffer_put(bufinfo.buf, to_copy);__sev();Aaah, ARM Compiler manual says this is "Set Event" - not Schienenersatzverkehr...
@@ -95,0 +143,4 @@if (volume < 0 || volume > 255)mp_raise_ValueError("volume out of range");multicore_fifo_push_blocking(AUDIOCORE_CMD_SET_VOLUME);multicore_fifo_push_blocking(AUDIOCORE_MAX_VOLUME * volume / 255);Partly complements my question from above.
The volume setting is something I would like to know more about. I am a bit tired right now, let's talk about this some later...
See #12 (comment)
@@ -0,0 +29,4 @@}}static size_t __time_critical_func(mp3_get_continous_data)(unsigned char **readptr)continuous
3a0784c863to1385eee85c