Rockbox

Tasklist

FS#6908 - Sansa: avoid audio glitches, vinyl like effect

Attached to Project: Rockbox
Opened by Toni (ahellmann) - Sunday, 25 March 2007, 16:44 GMT
Last edited by Michael Sevakis (MikeS) - Wednesday, 30 May 2007, 14:49 GMT
Task Type Patches
Category Drivers
Status Closed
Assigned To No-one
Operating System Sansa e200
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Workaround to avoid audio glitches (very short audio dropouts) in conjunction with heavy screen update tasks like:
- wps with peakmeter
- fast scrolling lists

This patch also allows flawless frequency scaling (if enabled in the binary).
It maybe also avoid the bug reported in http://www.rockbox.org/tracker/task/6900.

Please test.
This task depends upon

Closed by  Michael Sevakis (MikeS)
Wednesday, 30 May 2007, 14:49 GMT
Reason for closing:  Fixed
Additional comments about closing:  Ah forget it...I actually looked at the patches and they don't actually add any code for that or enable it. :P No complaints about the change...closed.
Comment by Matthias Wientapper (mattzz) - Sunday, 25 March 2007, 20:24 GMT
I applied the patch and have not see the white screen of death ( FS#6900 ) so far.
Comment by Matthias Wientapper (mattzz) - Sunday, 25 March 2007, 20:24 GMT
s/see/seen/
Comment by Paul Louden (Llorean) - Sunday, 25 March 2007, 21:44 GMT
Does this patch include code specifically to fix frequency scaling, or is that merely a side effect of the LCD driver fix?
Comment by Andrew Green (andrewg) - Sunday, 25 March 2007, 22:49 GMT
Thanks so much, the audio 'channel reversal' problem I was having is fixed too. Here is a recording I made before I applied this patch: http://aliant.ath.cx/~andrew/sansa/u2BeautifulDay-SansaDecoding.ogg if you look at the waveform in Audacity you can see how the channels are reversed at about 2:42 and then later on go back to normal.
Comment by Jonathan Gordon (jdgordon) - Monday, 26 March 2007, 01:38 GMT
seems to work... but enabling cpu scaling causes a data abort...
edit: ignore that... you must do a full recompile after enabling cpu frequency :p

works well...
Comment by Toni (ahellmann) - Monday, 26 March 2007, 08:16 GMT
@Llorean: Yes, it is a side effect.
Comment by Toni (ahellmann) - Monday, 26 March 2007, 10:17 GMT
@andrewg: The channel reversion has following reason (to my understanding):
The audio IIS FIFO contains 16 audio samples. After processing 6? samples
the low watermark has been reached and a fiq is initiated to get more data.
This fiq may stall if a cache flush is in work. So the FIFO can run completely empty.
The cpu can only add a left or right audio sample at the time. If the stall happened
when feeding a left sample, but the FIFO expects a right sample as the first sample
after emptying, the channels get reverted.
So disabling the cache_flush() during audio playback solves this problem.
And cache flush is not necessary, because so much data have to be processed, that the
lcd data have been flushed nearly instantly by the audio memory processing.
Comment by Alex (testdasi) - Monday, 26 March 2007, 19:37 GMT
Scroll texts do not appear correctly. A few pixels appear to be in the wrong place on the line. I think it is something wrong with syncing of the pixels in each letter (hope people understand what I mean). I use unifont, with the UniCatcher theme.
Comment by Alex (testdasi) - Monday, 26 March 2007, 19:41 GMT
Music also still turns itself off (like suddenly lower volume then turn if back on normal) every now and then.
Comment by Toni (ahellmann) - Monday, 26 March 2007, 19:51 GMT
@testdasi:
your first comment: Right, the removing of cache_flush() during playback can cause these 'missing pixels'.
You cannot have both glitchless audio and perfect lcd updates with this patch.

your second comment:
Have a look at http://www.rockbox.org/tracker/task/6911. I have noticed this only without #6911 and loud music.
Comment by Toni (ahellmann) - Monday, 26 March 2007, 21:01 GMT
This update avoids scrolling artifacts during music is paused.
During normal playback I could not see/here any audio / screen artifacts.
Comment by Toni (ahellmann) - Monday, 26 March 2007, 22:17 GMT
This time hopefully without bug:
Comment by Rene Peinthor (rp) - Saturday, 31 March 2007, 18:32 GMT
works great for me so far.

good work
Comment by Flake (Flake) - Monday, 02 April 2007, 20:30 GMT
yeah, works great except there's no fade out/in on player starts/being off any more
Comment by Flake (Flake) - Monday, 02 April 2007, 20:34 GMT
sorry, it happens time to time, not always
Comment by Greg Pendergrass (gregpend) - Thursday, 05 April 2007, 16:25 GMT
Is the anti-audio-glitch patch going into current builds anytime soon?
Comment by Paul Louden (Llorean) - Tuesday, 10 April 2007, 04:45 GMT
Attached is the result of an RMAA test. The SNR isn't great (laptop sound card) but I've included my laptop testing itself, it testing a Gigabeat with Rockbox, the Sansa Retail firmware, and the Sansa Rockbox firmware with this patch. You can see for yourself how the results compare: My real concern is that the volume levels don't seem right yet, though.
   RMAA.zip (705.5 KiB)
Comment by Toni (ahellmann) - Tuesday, 10 April 2007, 15:56 GMT
This patch is only meant to avoid the channel swapping and the very small audio dropouts (as in sporadic vinyl like clicking).
Techically spoken: Ensure flawless feeding of the audio fifo with audio samples.
Someone with the AMS specs might have a look at the volume level issue.
Comment by MichaelGiacomelli (saratoga) - Tuesday, 10 April 2007, 18:31 GMT
On the upside, aside from the volume issue, sound output appears identical to the retail firmware.
Comment by Andrew Green (andrewg) - Wednesday, 11 April 2007, 01:59 GMT
I have made a small patch (with help from the datasheet), the DAC gain should be about -7.5 dB which at the time of writing I didn't know.
http://www.rockbox.org/tracker/task/6911
Comment by Andrew Green (andrewg) - Wednesday, 11 April 2007, 02:00 GMT
Oops, I forgot to say that my patch is for the volume 'issue'.
Comment by Flake (Flake) - Thursday, 12 April 2007, 02:33 GMT
glitches anyway appear for the tracks with hight bitrate (>192), one-two per minute
Comment by Alex (testdasi) - Friday, 13 April 2007, 23:50 GMT
It is not working. I think changes on 13 Apr 20:55 to lcd-e200.c is the reason.
Comment by Michael Sevakis (MikeS) - Saturday, 14 April 2007, 03:36 GMT
Yes, the changes will require this patch be resynced.
Comment by Toni (ahellmann) - Tuesday, 17 April 2007, 18:37 GMT
Pheew, I resynced this patch:
The correct order of patching (until it breaks again, if not svned):
1. apply patch http://www.rockbox.org/tracker/task/7036
2. apply this update
Because these two patches include pcm-pp.c and lcd-e200.c the order of patching is important.
After thinking about the importance of both patches, the better solution would have been to update this patch first then the other.
But I am tired of this kind of double/triple work. So everybody is welcome to maintain these patches.
Comment by Michael Sevakis (MikeS) - Wednesday, 18 April 2007, 02:26 GMT
Ehrm...didn't mean to induce frustration by fixing the cache invalidation issues. :\\
Comment by Toni (ahellmann) - Wednesday, 18 April 2007, 05:53 GMT
Ooh no, I am all for your improvements. It is only a pity, that this patch doesn't find it's way into svn (to solve the issue until a better solution is available).
Comment by Rene Peinthor (rp) - Wednesday, 18 April 2007, 12:48 GMT
2 or 3 weeks ago i wrote dan_a a mail about committing this patch, but i think he still has some health problems.
maybe some other developer(barry?) could check the patch and maybe commit it. Or make an reasonable statement, why not to include this patch yet.
Comment by Daniel Ankers (dan_a) - Thursday, 19 April 2007, 07:26 GMT
Hi Toni,
I have some questions about this patch:
1. Why is it no longer necessary to stop and start the DMA? Won't this re-introduce the fade-to-blue bug?
2. Don't you get artifacts on the screen if the cache is not being flushed?

Overall, I'm hesitant to commit something that makes the LCD driver depend on the playback state (though other developers might feel differently.) Have you tried disabling all interrupts while the cache is being flushed? If that is as effective, then it would be a much better approach until the framebuffer is moved to uncached RAM.
Dan
Comment by Barry Wardell (barrywardell) - Thursday, 19 April 2007, 09:43 GMT
I have to agree with dan_a. I haven't committed this myself because I think there should be a better solution to the problem. Ideally, we need to move the framebuffer to uncached RAM. My own attempts at doing this have so far not been successful.

Also, last time I tried the patch, I was still getting graphical glitches during playback.
Comment by Toni (ahellmann) - Thursday, 19 April 2007, 17:06 GMT
dan_a

What I tried:
1. set frequency to fixed 54MHz (near to the minimum speed required for mp3 audio processing)
2. disable interrupts (not fiq) during cache flush
3. stop cop during cache flush
And the result is...: not good!

What I found:
The fiq interrupt is generated, when there are 10 (or less) audio samples left in the fifo. So each fiq call adds 6 new samples.
This theoretically leads to a max time of t=226.75usec to fill the fifo again without any audio glitch.
The cache flush time is dependent on the cpu frequency.
The cache flush does significantly delay the fiq execution (I read a minimum of 7 samples in the fifo @54MHz).
The cache flush time is not constant, but usually takes t=50-70usec @54MHz (memory stalls?).
I had a worst case of t=288usec with frequency scaling enabled. This for sure can lead into problems.
So in my latest tests I could not find the fifo running empty, but still got the clicks and channel swappings.

My conclusion:
A fiq delay of less than 4 audio samples already can get the fifo out of sync.

I will investigate more.
Comment by Toni (ahellmann) - Thursday, 19 April 2007, 17:17 GMT
barrywardell

Did you also get the screen garbage, vertical (and horizontal) splitted screen?
And always cached, even it was supposed to be not cached (with n x 64MB offset).
I have only seen very small graphical glitches during playback. The immense data
transfer during playback 'flushes' the graphical data very soon from cache.
Comment by Barry Wardell (barrywardell) - Saturday, 21 April 2007, 12:54 GMT
The screen garbage was only small bits of the screen while playback was happening, but it was still noticable. Why doe you suggest 64MB offset for non-cached memory? Is it not supposed to be 32MB?
Comment by Toni (ahellmann) - Saturday, 21 April 2007, 17:29 GMT
According to dan_a the cached area is 64MB. But it seems, this is not really verified because of missing specs.

Also I have to correct my findings by this:
- the fifo keeps only 8 audio samples (not 16)
- when the fiq interrupt kicks in, only 2 audio samples are left in the fifo.
- channel swapping and clicks only appear, if the fifo runs completely empty

Another workaround idea is, to sync the flush_icache with the fiq. This allows to finish the flush_icache within 9 audio samples instead of 3 to 9 as in current software. It works quite well here, but gives additional complexity.
Comment by Toni (ahellmann) - Sunday, 22 April 2007, 09:52 GMT
Update to sync with 'battery-optimized-cleaned-update' and bootloader fixes:
- reintroduce dma enabling in lcd_update functions
- remove the conditional flush_icache for the bootloader

The #7036 patch has to be applied first, then this patch can be applied.
Comment by Andre Coleman (drestar23) - Monday, 14 May 2007, 00:25 GMT
Hello i am new to this patching thing so could someone please tell me how to apply this patch???
Comment by Rene Peinthor (rp) - Tuesday, 22 May 2007, 21:07 GMT
i guess this patch can be closed after todays commit from Mike?
Comment by Michael Sevakis (MikeS) - Wednesday, 23 May 2007, 01:09 GMT
I'm hoping so but I kind of like the one that opened it to decide it's fixed on these longstanding issues as well as being notified of any problems with the change first.
Comment by Michael Sevakis (MikeS) - Wednesday, 30 May 2007, 14:44 GMT
I'd like to close any tasks related to Sansa e200 audio glitches since I'm not aware of any complaints or resultant bugs but the material here seems to cover other ground like frequency scaling. Perhaps that should be separated out and moved to a new task before hand? Antonius...you there? Op.?

Loading...