pine64 / OpenPineBuds

Community maintained firmware for PineBuds Pro
148 stars 18 forks source link

What do to with the CN sound files? #44

Closed ThatcherC closed 1 year ago

ThatcherC commented 1 year ago

In 4166b13d5402033c5948004259c03e5fdd5c7d40 we merged #28 which replaces the old TXT sound files in config/\_default\_cfg\_src\_/res/en with MP3s to make playing around with custom sounds easier. There are still some TXT sound files in config/_default_cfg_src_/res/cn though. I assume they're Chinese language versions of all the same sounds.

What would we like do with those? I feel like they shouldn't be left at TXT files, and instead should be either:

  1. Converted to MP3s and kept in place (using the same process used for the EN sound files), or
  2. Converted to MP3s and preserve, but largely removed from the code.

I like option 2, because it would allow us to remove about 35 instances of a pattern that looks like this in main/services/bt_app/app_media_player.cpp:

https://github.com/pine64/OpenPineBuds/blob/e72390463dcb9105f9d86353f509e87458ecb32b/services/bt_app/app_media_player.cpp#L521-L526

If we keep the EN and CN sounds files as MP3 without changing the code, we're stuck dealing with this repeated language check all the time. I think a better system would be to convert the CN sounds, an then add some kind of Makefile switch that either builds with the EN or CN sounds, instead of all this runtime language checking. This strategy would also make it much easier to add additional language! We could add folders like res/es or res/fr or res/jp for additional languages without having to add additional runtime language checks in the code. With this kind of change in place, the code above would look like

g_app_audio_data =  (U8 *) SOUND_REFUSE;
g_app_audio_length = SOUND_REFUSE_len;

which is shorter and doesn't explicitly reference any particular language.

Does this sound like a good change? I'd be happy to put together the changes if so!

Ralim commented 1 year ago

I think we should go for option (2).

I think a better system would be to convert the CN sounds, an then add some kind of Makefile switch that either builds with the EN or CN sounds

I 1000% agree with this.

I'm also happy if we just drop the cn sounds entirely for now 🤷🏼

ThatcherC commented 1 year ago

Addressed in https://github.com/pine64/OpenPineBuds/pull/46!