esp-box icon indicating copy to clipboard operation
esp-box copied to clipboard

Audio component

Open chmorgan opened this issue 3 years ago • 15 comments

@tore-espressif here is the updated PR that uses external components and renames the playlist component to I hope better reflect what the component does.

This includes all of the fixes from the previous PR review.

Thoughts on next steps?

chmorgan avatar Aug 26 '22 00:08 chmorgan

You will need an API token for uploading the components. I can send it to you in private message

tore-espressif avatar Aug 29 '22 16:08 tore-espressif

@tore-espressif sure, you can email to me via [email protected], or if you prefer signal or some other encrypted means I can give you my mobile.

chmorgan avatar Aug 29 '22 17:08 chmorgan

@tore-espressif just read your message here, let me get those files in place and get things working/building and ready for the secret.

chmorgan avatar Aug 29 '22 17:08 chmorgan

@tore-espressif switched to fixed versions. It's been a journey but this looks like a good first set of changes to get in.

chmorgan avatar Sep 01 '22 16:09 chmorgan

@Real-YuZhe @leeebo @wujiangang PTAL (related code reviews #30 )

tore-espressif avatar Sep 02 '22 08:09 tore-espressif

@chmorgan ,hello: I tested and found the following situations: when the playback started, or the playback resume from pause, there is no sound from the speaker; I must click the button "vol +/-" to recovery it.

I notice that,you'll do disable mute after function "aplay_file(i, audio_event.fp)" i->config.mute_fn(AUDIO_PLAYER_MUTE); es8311_write_reg(ES8311_DAC_REG32, 0x00);

the vol register(ES8311_DAC_REG32) will be cleared, need to do some improvement?

espressif2022 avatar Sep 05 '22 11:09 espressif2022

This should have been addressed in the mute function, the original volume should be restored there when exiting mute. Maybe a change was lost somewhere.

Were you testing with mp3 demo or factory demo in the esp-box repository?

Chris

On Mon, Sep 5, 2022 at 7:48 AM espressif2022 @.***> wrote:

@chmorgan https://github.com/chmorgan ,hello: I tested and found the following situations: when the playback started, or the playback resume from pause, there is no sound from the speaker; I must click the button "vol +/-" to recovery it.

I notice that,you'll do disable mute after function "aplay_file(i, audio_event.fp)" i->config.mute_fn(AUDIO_PLAYER_MUTE); es8311_write_reg(ES8311_DAC_REG32, 0x00);

the vol register(ES8311_DAC_REG32) will be cleared, need to do some improvement?

— Reply to this email directly, view it on GitHub https://github.com/espressif/esp-box/pull/42#issuecomment-1236896188, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AFAZTMWVBFE3JY6HBDV4XMY7ANCNFSM57U6BCPA . You are receiving this because you were mentioned.Message ID: @.***>

chmorgan avatar Sep 05 '22 15:09 chmorgan

This should have been addressed in the mute function, the original volume should be restored there when exiting mute. Maybe a change was lost somewhere. Were you testing with mp3 demo or factory demo in the esp-box repository? Chris On Mon, Sep 5, 2022 at 7:48 AM espressif2022 @.> wrote: @chmorgan https://github.com/chmorgan ,hello: I tested and found the following situations: when the playback started, or the playback resume from pause, there is no sound from the speaker; I must click the button "vol +/-" to recovery it. I notice that,you'll do disable mute after function "aplay_file(i, audio_event.fp)" i->config.mute_fn(AUDIO_PLAYER_MUTE); es8311_write_reg(ES8311_DAC_REG32, 0x00); the vol register(ES8311_DAC_REG32) will be cleared, need to do some improvement? — Reply to this email directly, view it on GitHub <#42 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AFAZTMWVBFE3JY6HBDV4XMY7ANCNFSM57U6BCPA . You are receiving this because you were mentioned.Message ID: @.> I'm testing with factory demo; I've check it, mp3 demo did the operation "es8311_codec_set_voice_volume" I lost this part of code in factory demo?

I ported the function "audio_mute_function" to the factory demo, and found another situation: after SR wakeword detected ”Hi ESP“, audio is silent too, the volume register doesn't seem to be setted; if I play a song first, it'll ok;

espressif2022 avatar Sep 06 '22 02:09 espressif2022

@espressif2022 hmm. I fixed a minor nit with the global static, moved it into the mute function, and applied the mp3_demo mute function to the factory_demo (as it looks like you did in testing.) That's what is pushed up right now.

On the "Hi esp" issue, thoughts on how to resolve? The intent of muting is to eliminate noise, clicks, pops etc, that can occur when transitioning to/from playback. I'm still seeing these here however, when playing/pausing and I'm still unsure of the origin of that noise.

We could remove the muting functionality from the mute handler entirely. That would keep the codec unmuted (and thus not adjust REG32 to zero like the es8311 set voice mute function does. We could also have other audio playback unmute/play/mute.

Thoughts on the best approach? and I do want to resolve the clicking issue, its on the list of things to look at at some point.

chmorgan avatar Sep 07 '22 13:09 chmorgan

@chmorgan the the noise of playback, when playing/pausing; do i2s_zero_dma_buffer(I2S_NUM_0) before set_state(i, AUDIO_PLAYER_STATE_PAUSE) may work ; I have tested, you can also try to see if it solves the problem you describe;

espressif2022 avatar Sep 08 '22 06:09 espressif2022

@espressif2022 hmm. Yeah that zero dma was in place before and it didn't address the issue, at the same time I'm not sure the mp3 decode was properly writing data to the buffer.

Mind if I revisit the clicking at a later time? This PR plus components is the culmination of a few months of outstanding work. If you feel this PR and components are moving the project/ecosystem forward I'd like to get that in place and iterate on top of that work with future PRs.

chmorgan avatar Sep 08 '22 12:09 chmorgan

@espressif2022 hmm. Yeah that zero dma was in place before and it didn't address the issue, at the same time I'm not sure the mp3 decode was properly writing data to the buffer.

Mind if I revisit the clicking at a later time? This PR plus components is the culmination of a few months of outstanding work. If you feel this PR and components are moving the project/ecosystem forward I'd like to get that in place and iterate on top of that work with future PRs.

Ok, I will submit to our internal gitlab in the next few days. This really took a lot of time, I'm very sorry

espressif2022 avatar Sep 08 '22 13:09 espressif2022

@chmorgan ,hello! We have merged into gitbub last week. You can confirm it when you have time. If it is ok, it will be closed. Thanks!

espressif2022 avatar Sep 26 '22 03:09 espressif2022

Oh ok. I just saw the commits, thank you! Sorry about the stuff that had to be fixed. I also saw the lvgl 8.3 pr was merged. The plan is to switch that to a component in the near future as well.

On Sun, Sep 25, 2022 at 11:18 PM espressif2022 @.***> wrote:

@chmorgan https://github.com/chmorgan ,hello! We have merged into gitbub last week. You can confirm it when we have time. If it is ok, it will be closed. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/espressif/esp-box/pull/42#issuecomment-1257420717, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AHBFHQ4IPDAFLM3VHDWAEIZ5ANCNFSM57U6BCPA . You are receiving this because you were mentioned.Message ID: @.***>

chmorgan avatar Sep 26 '22 17:09 chmorgan

Oh ok. I just saw the commits, thank you! Sorry about the stuff that had to be fixed. I also saw the lvgl 8.3 pr was merged. The plan is to switch that to a component in the near future as well. On Sun, Sep 25, 2022 at 11:18 PM espressif2022 @.> wrote: @chmorgan https://github.com/chmorgan ,hello! We have merged into gitbub last week. You can confirm it when we have time. If it is ok, it will be closed. Thanks! — Reply to this email directly, view it on GitHub <#42 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJH4AHBFHQ4IPDAFLM3VHDWAEIZ5ANCNFSM57U6BCPA . You are receiving this because you were mentioned.Message ID: @.>

Thanks for your work. Lvgl 8.3 adds a lot of features, which are pretty good.

espressif2022 avatar Sep 27 '22 03:09 espressif2022