Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to play_sd_wav/raw with method to use different FS #419

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mjs513
Copy link

@mjs513 mjs513 commented Dec 4, 2021

While playing with MTP/FS Audio we modified the play_sd_wav and play_sd_raw with an addition method based on what @wwatson did for Franks Codec library: https://forum.pjrc.com/threads/68816-Now-for-MP3-with-FS to use different FS as well. So after a bunch of debugging by @KurtE and @frank B we now have all of Frank's codec library working with the T4.1 (+audio shield). With this change we are now able to play wav files on FS device - tested with memboard chips, QSPI, SD Cards and USB. All seem to be working well.

Basically just added a second play method:
bool AudioPlaySdWav::play(FS *fs, const char *filename)
and change using SD.open to
wavfile = fs->open(filename);

nothing else changed.

Don't have any midi equipment or fancy audio stuff to test so fair warning.

@PaulStoffregen
Copy link
Owner

Frank changed his code's license back to GPL, so I can not merge any code which is derived in any way from Frank's work.

@mjs513
Copy link
Author

mjs513 commented Dec 4, 2021

Actually, this change is only to allow use of play_sd_wav and play_sd_raw to use LFS, or SD or USB which is totally independent of Frank's stuff - it was just the impetus and test method. Could easily just have use an example without Frank's codec.

Really don't understand the license issue I guess.

@FrankBoesing
Copy link
Contributor

+if you want to merge the new waveplayer, you can still merge the MIT version from the history. No code change happened since then.

I that really happens, i'll be happy to change the licence back. After that.

@PaulStoffregen
Copy link
Owner

@FrankBoesing - Would you be willing to create a branch within your public repository from the last commit before the license change to GPL?

@FrankBoesing
Copy link
Contributor

@PaulStoffregen
Copy link
Owner

Thanks. I've created a fork.

At this moment my workbench has a complicated USB setup to investigating the USB packet loss issue with MTP, so I probably will not touch any audio stuff for a while.

@KurtE
Copy link
Contributor

KurtE commented Dec 26, 2022

@mjs513 @PaulStoffregen - There is a current thread up on the forum:
https://forum.pjrc.com/threads/71716-recording-playing-back-audio-on-multiple-SD-cards

Where they would like to play back a raw capture file that they may store on different SD cards. Where they have maybe 3 SDs setup.

I know we were playing with this PR and playing media (mostly wave files) stored on other storages.

If I remember correctly, this PR worked fine.
I also showed a minimalistic set of changes on the forum where we can:
pass in a reference to an FS object like:

class AudioPlaySdRaw : public AudioStream
{
public:
	AudioPlaySdRaw(void) : AudioStream(0, NULL) { begin(); }
	void begin(void);
	bool play(const char *filename, FS &fs=SD); 

And likewise in the cpp file change:

bool AudioPlaySdRaw::play(const char *filename, FS &fs)
{
	stop();
#if defined(HAS_KINETIS_SDHC)
	if (!(SIM_SCGC3 & SIM_SCGC3_SDHC)) AudioStartUsingSPI();
#else
	AudioStartUsingSPI();
#endif
	__disable_irq();
	rawfile = fs.open(filename);
	__enable_irq();

where the fs. was SD.

If this way is preferred, would also do it to the other class as well

@h4yn0nnym0u5e
Copy link
Contributor

If this way is preferred, would also do it to the other class as well

This seems a much cleaner approach to me - it's what I've done for my buffered version and it works fine, and makes maintenance of a single play() function much easier.

One further comment ... is there a way of determining whether the FS in use actually needs SPI? As-is, the code assumes it does, but I'd guess (haven't tried it...) that a filesystem on USB Host or littleFS doesn't do so. Not sure about SDIO, either. Are there any drawbacks to leaving AudioStartUsingSPI() in?

KurtE added a commit to KurtE/Audio that referenced this pull request Dec 29, 2022
This is an alternate implementation of
PaulStoffregen#419

It simply adds optional parameter at the end of the play method for the FS which defaults to SD

The other PR, makes a copy of the play method and adds the FS as the first parameter.

EIther way works for me.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants