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

Add pcm module. #1255

Merged
merged 4 commits into from Jun 5, 2016
Merged

Add pcm module. #1255

merged 4 commits into from Jun 5, 2016

Conversation

devsaurus
Copy link
Member

After some weeks of implementation and debugging, the pcm module code is now mature enough to give this PR a try. I tried to cover as much of your #1085 feedback as possible in this first iteration.

Modular design

The Lua interface and links to the firmware are kept generic. The back-end hardware is supposed to be plugged in as drivers that need to expose a common set of internal functions to implement the Lua methods:

  • drv:play()
  • drv:record()
  • drv:stop()
  • drv:pause()
  • drv:close()

Drivers are free to support just one or both directions. E.g. the sigma-delta driver can only do playback. Adding other drivers seems feasible IMO, though there might still be driver specific stuff in the general routines which I missed to abstract so far.

Callbacks

They're registered with the drv:on(<identifier>, cb_fun) pattern. That seemed to be the most convenient way.

Recording

Nothing done yet except for avoiding obstacles in the module / driver design. A significant one-off effort will be to code the task which forwards data from the sampling ISR to the Lua callback.

External hardware for sigma-delta

There's some explanation in the module doc on how to convert from digital to analog domain. It requires some soldering / breadboarding skills since there are no off-the-shelf boards for that AFAIK. Cheap amplifiers to further drive 4 Ω and 8 Ω speakers are available from many sources, though.

Pitfalls

Ensure good supply conditions or you'll experience random hiccups when sinking all power into the speaker.

I'm not sure about the real-time characteristics when playing at higher sample rates, this will need further learning. Though I attempted to avoid typical clashes with the SDK guidelines:

  • Short ISR on FRC1 priority. NMI level tends to kill the system once in a while.
  • Schedule data retrieval with the task interface.

A major bottleneck is the spiffs when playing from a file since this blocks further file accesses. And performance-wise during moderate load situations on spiffs. Eg. listing the directory might cause dropouts, but it will not crash.

Streaming from a network source still needs to be demonstrated. I think it's possible but haven't ventured into that (yet).


file.open("jump_8k.u8", "r")

drv = pcm.new(pcm.SD, 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see it mentioned more explicitly that this will fail if the firmware is missing the sigma-delta module.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, that won't fail. The pcm code bases solely on the platform layer where components are linked when referenced. That's independent of user_modules.h where the user interface on Lua layer is tailored.
Users are free to enable the sigma-delta module, though it's neither required nor recommended to access the hardware directly while it's in use by pcm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hhhmm, then can you explain http://stackoverflow.com/q/36324828/131929?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I can just guess that the user had an old state of my pcm branch when he tried initially. pcm.SD was introduced around 12-Apr, not sure when this change finally became visible on github.
Running pcm while having the sigma-delta module disabled definitely works for me. I tried it right now to be sure 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird...you created the PR on April 18th and I answered the SO question on the 21st. The user only tried after that (my assumption). I guess we'll never find out.

Copy link

@Phando Phando Apr 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks man!Can you please make me a new build without sigma-delta so I can test audio without it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your feedback guys, and also for testing this PR.
Please let me know about any issues you encounter while running the new code.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@larkin just gave me a new build using the latest and missing the sigma-delta. PCM works fine and the pcm.SD call doesn't cause an error. Thanks!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devsaurus it's a long shot, but is there any way to control the volume through software?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some interaction between PCM and PWM? I am using PWM on pin 12 and PCM on pin 1. The opposite service fails after I successfully run one.

@devsaurus
Copy link
Member Author

@Phando regarding volume control I could imagine a coarse approach. Like 100%, 50%, 25%, 12.5% etc. Or anything else that doesn't consume significant CPU power.
PWM & PCM: I'm not aware oft obvious dependencies here but I didn't try that combination. Plesse post an example that triggers your issue so I can investigate.

@pjsg
Copy link
Member

pjsg commented May 4, 2016

PWM & PCM: I'm not aware oft obvious dependencies here but I didn't try that combination.

Both of these use the hardware timer. There is only one of these and the current implementation does not allow sharing between modules.

It would be fairly hairy to add a multiplexing layer -- not least because having multiple users would mean that the underlying timer would need to be triggered in single-shot mode. The difficulty is in eliminating any drift. The PWM code goes to a fair amount of effort to make the output be 'right' and not glitch and the frequency be stable.

@devsaurus
Copy link
Member Author

Thanks for the clarification, Philip.
I've added the sharing conflict with pwm to the doc.

@Phando
Copy link

Phando commented May 5, 2016

@pjsg is right. The two libraries don't play nice. If your PWM on full, then you can run the PCM along side the PWM. That said, PWM that is on full is the same as having your pin on gpio.HIGH. Sound is more important than PWM in my project so I wound up sticking with PCM and writing a crude tmr based PWM.

You all rock!

@jmattsson
Copy link
Member

This looks sweet! Merge as soon as the master drop is done?

@devsaurus
Copy link
Member Author

Sure, it's in standby like other PRs until master is updated.
I appreciate the shift as it keeps the feedback window open and allows me to add & verify features. Latest enhancement was the vu callback to propagate peak information back to Lua land.

@marcelstoer
Copy link
Member

Resolve merge conflicts and merge?

@devsaurus
Copy link
Member Author

Yes, but I'll be AFK the next days.

@devsaurus
Copy link
Member Author

Polished PR and final checks done.

@marcelstoer marcelstoer merged commit d416648 into nodemcu:dev Jun 5, 2016
@devsaurus devsaurus deleted the pcm branch June 5, 2016 21:22
@marcelstoer marcelstoer added this to the 1.5.4 milestone Jul 11, 2016
@perigalacticon
Copy link

I know it's the wrong place for this, but could this be ported to Arduino :) ? I was looking for this exact thing.

@RadianM
Copy link

RadianM commented Jan 23, 2018

I've been trying to adapt this module to get fixed frequency operation from the Sigma Delta (312.5KHz) The existing implementation is using PCM with a default prescale of 9 which gives a 125nS pulse (4MHz square wave at 128 data in). This is too fast to output into a typical Mosfet H-bridge (a really great way to drive direct into a low impedance speaker!) and produces a 31.25KHz worst-case audio components at extreme data points (a bit too low for comfort).

I was hoping to offer a PR for a version that provides a more typical fixed frequency PWM which is possible using the sigma_delta_setpwmduty() call yet the implementation here calls platform_sigma_delta_set_target(). This particular platform call is decorated with ICACHE_RAM_ATTR so works with the PCM interrupt code.

My approach was therefore to add the ICACHE_RAM_ATTR to platform_sigma_delta_set_pwmduty() in platform.c and call it from drv_sigma_delta.c instead of platform_sigma_delta_set_target(). So far so good, except - data points 127 & 129 (only!) produce a high pitched whistle. Despite all my efforts I can't figure out why this should be. Other than that, the output is "proper" PWM at a perfect frequency for class-D operation.

Back to the motivation... (because this is a trick people may be unaware of)... a cheap dual (complementary) mosfet bridge driver such as MCP1405 can deliver several Watts into a low-impedance speaker wired between its two outputs when powered from 5V (or more). Both inputs are tied together back to the PWM GPIO pin. At 128 data input (silence) the current consumption is just a couple of mA which is great for battery powered noise generators! I do therefore hope I can pick up some discussion here if I can.

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.

None yet

8 participants