This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#12150 - Fully-functional audio mixer
Attached to Project:
Rockbox
Opened by Michael Sevakis (MikeS) - Tuesday, 07 June 2011, 01:35 GMT+2
Last edited by Michael Sevakis (MikeS) - Thursday, 30 June 2011, 09:20 GMT+2
Opened by Michael Sevakis (MikeS) - Tuesday, 07 June 2011, 01:35 GMT+2
Last edited by Michael Sevakis (MikeS) - Thursday, 30 June 2011, 09:20 GMT+2
|
DetailsImplements a multi-channel mixer with fully-independent playback and volume control on each channel.
Long-standing bugs addressed: Voice and keyclick do not work while playback is paused -- fixed Fade on stop/paused does not work on all outputs - fixed Voice and keyclick are as responsive to user input while playing music as when not, which was not previously the case. Additionally, voicing in the audio menus where pcmbuf is running with low-latency is stable. PCM interface for each channel is largely identical to current callback mechanism. The main difference is the function call names and the need for a channel ID. Mixing is done during the mixer's main PCM callback, which calls the callback for all active channels. Double buffering of short mix frames ensures there is enough time to compute the final downmix before hardware FIFO's run out of data. One minor change to PCM drivers is required to implement it so that the next frame may be produced while the current plays. Silence plays for 3 seconds before PCM is actually stopped after all channels report 'no more' in order to prevent popping for keyclick and voice output. Recording workaround in order to be sure PCM is stopped for half-duplex PCM drivers is a bit messy right now but works. Playing PCM from plugins while music is playing should be possible so long as their callbacks aren't very demanding. Same goes for the possibility of voicing/click/beep whle recording where full-duplex PCM is implementable. Yes, as usual it all takes a good bit more RAM to have something nice but it appears a worthwhile trade. 
pcm-live-mixing-19z.patch
(86.7 KiB)
apps/misc.h | 3 apps/beep.c | 118 +++ apps/pcmbuf.c | 319 ++------ apps/action.c | 2 apps/pcmbuf.h | 3 apps/recorder/keyboard.c | 2 apps/plugins/fft/fft.c | 13 apps/gui/wps.c | 16 apps/pcm_mixer.c | 586 +++++++++++++++ apps/playback.c | 20 apps/pcm_mixer.h | 102 ++ apps/voice_thread.c | 257 +++--- apps/plugin.c | 5 apps/voice_thread.h | 5 apps/SOURCES | 4 apps/plugin.h | 10 apps/misc.c | 5 firmware/export/config.h | 5 firmware/export/pcm.h | 9 firmware/target/arm/s5l8700/pcm-s5l8700.c | 16 firmware/target/arm/s5l8702/pcm-s5l8702.c | 2 firmware/target/arm/pnx0101/pcm-pnx0101.c | 12 firmware/target/arm/s3c2440/mini2440/pcm-mini2440.c | 2 firmware/target/arm/s3c2440/gigabeat-fx/pcm-meg-fx.c | 2 firmware/target/arm/s3c2440/gigabeat-fx/wmcodec-meg-fx.c | 2 firmware/target/arm/pcm-telechips.c | 21 firmware/target/arm/as3525/pcm-as3525.c | 8 firmware/target/arm/tms320dm320/mrobe-500/pcm-mr500.c | 2 firmware/target/arm/imx31/gigabeat-s/pcm-gigabeat-s.c | 2 firmware/target/arm/pcm-pp.c | 22 firmware/target/hosted/android/pcm-android.c | 31 firmware/target/hosted/sdl/pcm-sdl.c | 16 firmware/target/hosted/maemo/pcm-gstreamer.c | 2 firmware/target/coldfire/pcm-coldfire.c | 42 - firmware/target/mips/ingenic_jz47xx/pcm-jz4740.c | 1 firmware/pcm.c | 28 36 files changed, 1309 insertions(+), 386 deletions(-) |
This task depends upon
Closed by Michael Sevakis (MikeS)
Thursday, 30 June 2011, 09:20 GMT+2
Reason for closing: Accepted
Additional comments about closing: Committed in r30097
Thursday, 30 June 2011, 09:20 GMT+2
Reason for closing: Accepted
Additional comments about closing: Committed in r30097
Good work - been hoping somebody (including myself) would find time for this over the past 4 years or so.
Have tested on a Windows sim, seems to work in general although voice output is very broken up. It sounds like every other buffer is silence, or maybe it's just not keeping up? It makes no difference whether music is playing or not.
(I got a hunk error on patching firmware/target/arm/s5l8702/pcm-s5l8702.c , but that obviously doesn't affect a sim build.)
Solution: Need to think how to create the buffers gradually regardless of size. :)
So, for now. Not gonna happen except for native targets where we do things (closer to) right instead of making ridiculous assumptions/lazy garbage interfaces.
Maybe a timer to run the mixer would suffice. Still would like to avoid this blowing-up into stupid on account of someone else's.
If I did it right adding the extra call to the target PCM driver or it's one class of target that I have and know is right because I tested it, then yes, it can be run on target.
As an app or sim, things are being a bit sticky but the sim seems to be doing alright for me; I need Steve to check it out but he's gone off and lost intrest apparently.
Still, I would very much appreciate testing testing testing (thank you in advance!) with the other app builds too and samsung-based iPods/Meizu (why do those drivers double-buffer at a low level anyway?). Wouldn't mind getting this in for 3.9 to get rid of some ancient limitations and bugs but I guess time is running short.
ETA: Forgot to mention that it takes care of voice dropping under heavier load which I never bothered to address before. Just needed one more voice frame to queue and a higher minimum priority for the voice thread.
22z seems to work ok for me, patched against r29995. (Well, nearly r29995, I had to modify configure to remove the -fvisibility=hidden option because of my old gcc, but that's a bit OT here.)
@Thomas: That seems very odd. Can you give more details? The changes to pcm-android.c are quite necessary, esp. the provision for callback after a new frame (without that it _won't_ work at all). Just to rule one part out, didn't you say the mutex addition did work without issues? Anything that looks wrong there to you?
@Thomas: It might need some correction to not call pcm_play_dma_started_callback() when pcm_play_get_more_callback() says "no more" since it shouldn't call that unless more is expected.
23 looks ok too.
It might be another Cygwin-ism, but patch always fails on ocm-s5l8702.c...
...
patching file firmware/target/arm/s5l8702/pcm-s5l8702.c
Hunk #1 FAILED at 113.
1 out of 1 hunk FAILED -- saving rejects to file firmware/target/arm/s5l8702/pcm-s5l8702.c.rej
...
@Steve: I'm making the patches from SVN. I don't understand what's going on there to make the patching fail. ??
@Thomas: Excellent. Need to check keyclick, track skip beep and fade on stop/pause since those are also changed with this.
Would it be worth doing a battery benchmark comparison for this patch? (I'm not volunteering as I don't have a working device... just saying...)
Also, this has better coldfire assembly (seems like it should still be a bit better :P ) and divides CPU optimization into some *.c includes to include the necessary inline functions.
1) Easily explainable. On coldfire I got a voice stack overflow one time. Since Coldfire uses any current stack as the interrupt stack, the voice stack is very close to the limit and the patched code will use more stack in the interrupt, perhaps the voice stack should be increased a few bytes. This won't be easily duplicated but can happen.
2) Clip (v1), the keyclick comes out wrong. I don't know why (maybe something with thumb interwork?) Fuze (v2), which uses the same PCM driver works perfectly. With current SVN the situation is a bit better but it's still wrong and 32 samples are missing from the beginning of the keyclick when not playing music. With the patch, doing a USB plug/unplug actually changes the tone of the keyclick until rebooting. Yep, there's some crazy going on here.
If you can cut instruction count in loops by 2/3 by using special CPU features the compiler can't use and mind pipeline requirements better, experience with those SoC it's worth it and almost impossible to botch. The most doubt should be ARMv4 where the fewest saving are made, however, the constructs used are proven in the DSP code. Clipping is fairly free on coldfire and armv6 but expensive on armv4.
forest.spc (SVN / 30z.patch): 54.8/54.5
Numbers for 3G:
The Noise of Carpet.mp3: (SVN / 30z.patch): 39.4/40.1
forest.spc (SVN / 30z.patch): 103.2 / 103.5
FS#12153, which besides giving more stable positioning information, is a better basic approach for lowish-latency DSP but gets rid of hacky things in pcmbuf in a way that works better if this patch is committed first.Opinions?
Of course I want to use the best one the target can use. If they can use the armv6 code with explicit saturation, I guess that's ok. I'm just not sure what's what with the various arm v5 targets. Is that all it's missing is the saturated add and halfword pack? Saturation with or without gain cut on one or more mixdowns is needed.
http://infocenter.arm.com/help/topic/com.arm.doc.qrc0001l/QRC0001_UAL.pdf
Although if this is only going to be used on fading and mixing voice, saving a few cycles on multiplication may not be a huge deal.
From the chart, the instructions in the ARMv6 version that can be used on v5 are 5E instructions:
SMULWy
SMLAWy
Might be good, but not using it for this right now:
SMULxy
SMLAxy
I have debated using a lower resolution for the amplitude to accomodate 16x16 multiplies.
H10-20GB - check
e200 - check
sa9200-check
iPod Video - ORLY?
I give up!
Result: Works like charm as well.
Edit: Test on iPod nano 2G was good, too.
Don't give up! :)
Thanks for testing.
Now, I'll be sure to describe here and elsewhere, such as a mailing list, what is expected in the PCM driver in case any of the changes are discovered to have been incorrect. It's basically the trivial (for the most part) addition of pcm_play_dma_started_callback in one place in each driver.
pcm_play_dma_started callback is expected to be called (it's really inlined) when:
1) A new buffer has been obtained via pcm_play_get_more_callback ... AND
2) After DMA transfer has begun (or if no DMA, the hardware FIFO has just been filled) ... AND
3) Only in the driver's audio callback or ISR, NOT when first starting PCM in pcm_play_dma_start ... AND
3) NOT if pcm_play_get_more_callback returned no buffer (size = 0)