MusicPlayerDaemon / MPD

Music Player Daemon
https://www.musicpd.org/
GNU General Public License v2.0
2.18k stars 352 forks source link

DSD on ARM is broken #469

Closed Wang-Yue closed 5 years ago

Wang-Yue commented 5 years ago

Hi, Max. I'm using MPD to play DSD (either DoP or Native). I run into an issue that with the same Linux kernel version, same MPD version, and same DSD capable DAC, the sound is very different when running MPD on my Raspberry Pi (arm based) and PC (x64 based). PCM is not affected. On ARM the DAC has a constant rustle sound. There're various bit manipulations happens in processing DSD music, which leads me to question whether ARM and x64 would feed the same data to a DAC. Is there some way I can easily output the data stream into a file and compare between the two?

Wang-Yue commented 5 years ago

I made some progress and changed the alsa output code (both are based on 0.21.4 code):

diff --git a/src/output/plugins/AlsaOutputPlugin.cxx b/src/output/plugins/AlsaOutputPlugin.cxx
index 8f32cf8d7..20f436ac2 100644
--- a/src/output/plugins/AlsaOutputPlugin.cxx
+++ b/src/output/plugins/AlsaOutputPlugin.cxx
@@ -866,6 +866,11 @@ AlsaOutput::Close() noexcept
    delete[] silence;
 }

+#include <iostream>
+#include <fstream>
+using namespace std;
+ofstream myfile("example.bin", ios::out | ios::app | ios::binary); 
+
 size_t
 AlsaOutput::Play(const void *chunk, size_t size)
 {
@@ -890,8 +895,10 @@ AlsaOutput::Play(const void *chunk, size_t size)

        size_t bytes_written = ring_buffer->push((const uint8_t *)e.data,
                             e.size);
-       if (bytes_written > 0)
+       if (bytes_written > 0) {
+           myfile.write((const char *)e.data, bytes_written);
            return pcm_export->CalcSourceSize(bytes_written);
+       }

        /* now that the ring_buffer is full, we can activate
           the socket handlers to trigger the first

and run on the same audio file with dop. The output file differs. They are same from the beginning but after a few megabytes they start to differ a lot. Even their final size doesn't match. ARM file size is a little bit smaller than the x86 output.

-rw-r--r-- 1 yue yue 137953280 Jan 27 16:22 example.bin.x86
-rw-r--r-- 1 yue yue 137816832 Jan 27 16:35 example.bin.arm

that is so weird. @MaxKellermann any thoughts you can share?

Anyway the above proved that there is a bug on arm processors.

Wang-Yue commented 5 years ago

v0.20.23 works fine. things breaks after v0.21. Very likely an endianness bug in the alsa output.

MaxKellermann commented 5 years ago

Endianess? ARM is little-endian just like x86.

Wang-Yue commented 5 years ago

My bad. But starting from v0.21 things are broken for ARM & DSD (either DoP or Native) only. x86 and arm generate different buffer files when applying the patch above. Even their dump file size differ.

MaxKellermann commented 5 years ago

Did the ALSA plugin insert silence because not enough data was delivered? This is not logged - you could set a breakpoint in the plugin.

Wang-Yue commented 5 years ago

I don't know --- I would say this is probably not the real cause.

The situation is, my pc plays dsd files nicely, while raspberry pi doesn't. If extra silence is causing issue, it should make raspberry pi dump file bigger. But in the above test, raspberry pi dump file is smaller than the pc one.

and I assume silence is something with a pattern (such as 0000). The place that starts to differ does not show that pattern.

MaxKellermann commented 5 years ago

Actually, 0x69 is the DSD silence pattern, see src/pcm/Traits.hxx. Do MPD's unit tests succeed? MPD has one unit test for DoP export. What would be really helpful is a git bisect, though I understand building MPD is extremely slow .... can you cross-compile or use distcc?

Wang-Yue commented 5 years ago

Umm.. I do see a lot of 0x69 there.

Could you let me know how to perform the DoP unit test? I can let you know whether it succeed for v0.20.23 and v0.21 or not.

MaxKellermann commented 5 years ago

Configure with -Dtest=true and then run ninja test

Wang-Yue commented 5 years ago

ran out of memory a few times when building the test but I managed to get this done.

congratulations, it passes all tests

 1/13 TestUtil                                OK       0.10 s 
 2/13 TestRewindInputStream                   OK       0.10 s 
 3/13 test_mixramp                            OK       0.08 s 
 4/13 test_protocol                           OK       0.08 s 
 5/13 test_queue_priority                     OK       0.03 s 
 6/13 TestFs                                  OK       0.05 s 
 7/13 TestIcu                                 OK       0.09 s 
 8/13 test_translate_song                     OK       0.05 s 
 9/13 test_icy_parser                         OK       0.03 s 
10/13 test_archive                            OK       0.08 s 
11/13 test_archive_bzip2                      OK       1.79 s 
12/13 test_archive_zzip                       OK       1.78 s 
13/13 test_pcm                                OK       0.05 s 

OK:        13
FAIL:       0
SKIP:       0
TIMEOUT:    0

but clearly this only means the test does not cover everything.

As I said the two dump files started to differ after probably a megabyte (or a few hundred KB). with a small fake sample you won't see it fail.

You can try by downloading the smallest dsd file on http://www.2l.no/hires/, for instance, http://www.lindberg.no/hires/test/2L-125_stereo-2822k-1b_04.dsf.zip , and then set the output to either DoP or native DSD. On Raspberry Pi this is broken to the point that you can hear the noise very clearly. The dump file size is different too.

This is not caused by a performance issue. MPD v0.20.23 on Raspberry Pi is capable of playing DSD256 without any issue (with a <10% low CPU utilization and sufficient USB trasport speed), , so it's pretty unlikely that the above DSD64 file would fail.

Even if you fill the period buffer with silence, it shouldn't affect the ring_buffer which is what I am copying above. ring_buffer should fill with exactly the same DSD data on both systems, but they are not right now.

MaxKellermann commented 5 years ago

Now that you've compiled the tests - please run test/run_decoder to run only the decoder plugin and see if its output differs.

Wang-Yue commented 5 years ago

Compared the two files generated on PC and RPi. They are exactly the same.

MaxKellermann commented 5 years ago

Sigh. That means it gets more complicated; the data gets altered somewhere later in the chain between decoder and output.

Wang-Yue commented 5 years ago

Let me know what I can do to help if needed. I can apply patches and run tests on both machines if needed.

coroner21 commented 5 years ago

Hi @Wang-Yue, could you tell what sample format mpd using in your case on RPi for DSD native playback? I found that I have audible distortion on a beagle bone black when DSD_U32LE is used, however with DSD_U16LE everything works fine. So far I suspected this to be an issue with the ALSA sound driver but might as well be an mpd problem...

Wang-Yue commented 5 years ago

@coroner21 , My DAC only supports DSD_U32 for native DSD. It's not an ALSA driver issue as mpd v0.20.23 has no problem playing that while 0.21 failed. The new alsa plugin also dump different data compared to the previous one. The issue happens on both DoP and Native.

borine commented 5 years ago

@Wang-Yue - this is a long shot, but if you have time you could try an experimental fix I have for a different alsa device - it makes a slight change to the way mpd handles output to alsa devices. It's in a branch called alsa-revents in my mpd fork here: https://github.com/borine/MPD/tree/alsa-revents I don't have any DSD or DoP devices, so can't say whether this is likely to make any difference to your issue - i'm just curious to see if it helps.

timpani2012 commented 5 years ago

@borine Dear borine, I used I2S of Raspberry Pi with ES9038Q2M DAC to test your mpd fork https://github.com/borine/MPD/tree/alsa-revents. The click sound of DSD still exists. Very strange, for the Raspberry Pi, if you use a 64-bit OS instead of a 32-bit's, The version 0.21.x is no click sound.

Wang-Yue commented 5 years ago

@borine I will test in the coming week when I have access to the hardware

@timpani2012 it's not clicking. it's noise. clicking is another issue, see (https://github.com/raspberrypi/linux/issues/2215). and the noise only happens on 0.21.x.

Wang-Yue commented 5 years ago

@borine your patch doesn't work

coroner21 commented 5 years ago

Anyone working on this? As mentioned I found that when DSD_U32 is used the issue is there (distorted sound) whereas with DSD_U16 DSD playback is no problem at all. Any difference in the data being processed from input to output when the stream is split into 16bit chunks as opposed to 32bit? This should give hopefully an idea how to address this problem (which I can work around by hard coding the DSD sample width to 16bit on my BBB-based DSD).

MaxKellermann commented 5 years ago

No, nobody is working on this.

franz159 commented 5 years ago

I noticed the issue with my Khadas Tone Board using Moode. least now I know the root cause, and continue using mpd 0.20.x

coroner21 commented 5 years ago

No, nobody is working on this.

Well I could try to help if you would give at least something away regarding how to debug this... I would think the fact that DSD_U16 works just fine whereas DSD_U32 is not should point somewhere, no?

Wang-Yue commented 5 years ago

@coroner21 the result already diverges in the function described here https://github.com/MusicPlayerDaemon/MPD/issues/469#issuecomment-457970244

You can walk up the stack and see where it differs.

TheOldPresbyope commented 5 years ago

@Wang-Yue

I am using a Khadas Tone Board (XMOS USB-bridge to ​ESS ES9038Q2M DAC) driven by a Raspberry Pi 3B+ running raspbian stretch on kernel 4.19.46-v7+. The Khadas firmware is v1.04 which apparently is the latest available.

With MPD 0.20.23 I get clean audio out playing DSD64 files as native DSD. (I'm not using DoP although the board supports it.). Whatever noise floor there is is inaudible to me.

Example ALSA params during playback:

cat /proc/asound/card1/pcm0p/sub0/hw_params
access: RW_INTERLEAVED
format: DSD_U32_BE
subformat: STD
channels: 2
rate: 88200 (88200/1)
period_size: 11025
buffer_size: 44100

With MPD 0.21.x (I'm at 0.21.8 at the moment but true too for 0.21.3) there is a noticeable noise floor during playback---a constant low-level "frying" noise.

I don't know if this is what you meant by "a constant rustle sound."

However, you should know I spent the last two days doing the git bisect that MaxK suggested early in this thread. I started with MPD 0.20.23 as the "good" commit and MPD 0.21.3 as the "bad" commit. There are 1551 commits separating them, which using git bisect cost me 11 build/test cycles to zero in on the "first bad commit".

git bisect bad
a06bf388d968279808009d4538def57e78b4bee7 is the first bad commit
commit a06bf388d968279808009d4538def57e78b4bee7
Author: Max Kellermann <max@musicpd.org>
Date:   Sat Dec 30 17:42:43 2017 +0100

    MusicChunk: make the struct size exactly 4096

:040000 040000 cd2e798e523da89e9a19e21f6582b6b2ec62974c 0a9027ec474cfbd05c894ff766758c5339dcf8cf M  src

This means the last "good" commit in my case is

commit de0c3e717e82508499ae572cfdf7423a3e36259b
Author: Max Kellermann <max@musicpd.org>
Date:   Sat Dec 30 17:47:08 2017 +0100

    MusicChunk: split struct MusicChunkInfo from struct MusicChunk

I am eager to find out if you find this is also true in your case or if our two issues stem from different causes.

Wang-Yue commented 5 years ago

I don't know if this is what you meant by "a constant rustle sound."

right.

I am using a Khadas Tone Board (XMOS USB-bridge to ​ESS ES9038Q2M DAC) driven by a Raspberry Pi 3B+ running raspbian stretch on kernel 4.19.46-v7+. The Khadas firmware is v1.04 which apparently is the latest available.

I have that board as well:-)

Does RPi 3B+ have this drop out problem? See https://github.com/raspberrypi/linux/issues/2215 and https://github.com/raspberrypi/linux/issues/2339. I have a RPi 3B and it's almost unusable. Testing with high bitrate files (say DSD256) could easily verify that.

I am eager to find out if you find this is also true in your case or if our two issues stem from different causes.

a06bf388d968279808009d4538def57e78b4bee7 is a bad commit that caused known problem. See https://github.com/MusicPlayerDaemon/MPD/issues/233. I'm not sure if the later fix (https://github.com/MusicPlayerDaemon/MPD/commit/a2340c313f49d45abf3ade4645264e45c54918c7) could fix it completely. I'm not familiar with the alsa code. I would suggest you patch in the latter commit, and see if the problem is still there. if it is, @MaxKellermann is probably the most knowledgable person to make a fix.

Thanks for doing the bisect, by the way. It was too painful to do that on my RPi.

TheOldPresbyope commented 5 years ago

Thanks for your quick response.

  1. I'm not suffering the drop outs described in the referenced issues. I have only DSD64 files at hand.

  2. The fix you mention (a2340c3) was committed a month before the release of MPD 0.21.8. I experience the noise with MPD 0.21.8.

At this point I'm out of ideas. I don't have other DSD-capable DACs with which to compare. I can try building current MPD on an x86 box to see if I can repro your experience, but building my player on a different architecture would require changes to my plans.

TheOldPresbyope commented 5 years ago

Oops. I neglected to reiterate that I'm not using DoP.

Wang-Yue commented 5 years ago

I'm not suffering the drop outs described in the referenced issues. I have only DSD64 files at hand.

That's great news. Maybe I should consider purchasing a 3B+ then...

DSD256 test file is freely available at http://www.2l.no/hires/.

The fix you mention (a2340c3) was committed a month before the release of MPD 0.21.8. I experience the noise with MPD 0.21.8.

a2340c3 was meant to solve the bug a06bf38 introduces. I would recommend you checkout a06bf38, and cherry pick a2340c3 on top of that directly, and see if the problem still occurs.

If it still occurs, (most likely you'll run into this situation.) it means a2340c3 does not solve the bug a06bf38 introduced entirely.

If problem disappears, it means something after a06bf38 but before MPD 0.21.8 is causing a new issue, and has the same broken behavior as a06bf38.

Oops. I neglected to reiterate that I'm not using DoP. I can try building current MPD on an x86 box to see if I can repro your experience,

The issue happens on both dop and native dsd for arm . simply testing DoP is sufficient to repro the bug.

x86 is not affected. no need to try x86 as I already tried that in the past when reporting the bug

starmood commented 5 years ago

Let me provide some information I have. I had got this kind of noise issue from last year, before 0.21 official releasing. Because there was another special branch was always based on latest commit, not sure if you know that one. And I had tried 2 I2S boards with PI, one is 9038q2m, anther is Hifiberry digi and using spdif to external DAC, both has problem. I am not expert on this, so can only provide some information, hope it’s useful for someone who can fix it eventually.

MaxKellermann commented 5 years ago

I wonder if this is yet another rounding bug which was revealed by the 4k-chunk commit. So far, you compared ARM32 with x86-64. This may not be an ARM-specific issue after all, but a 32 bit vs 64 bit. On 64 bit, pointers are larger, and the struct layout is different, which may just happen to not trigger the bug. Did anybody test with ARM64 (Aarch64) or x86-32?

Wang-Yue commented 5 years ago

Did anybody test with ARM64 (Aarch64) or x86-32?

Umm... I don't have either environment avaiable.

Maybe someone can try compiling a 32bit MPD, and link to 32bit Linux libraries, and run on 64bit Linux on X64. Maybe that's the easiest way to repro this.

ghost commented 5 years ago

I wonder if this is yet another rounding bug which was revealed by the 4k-chunk commit. So far, you compared ARM32 with x86-64. This may not be an ARM-specific issue after all, but a 32 bit vs 64 bit. On 64 bit, pointers are larger, and the struct layout is different, which may just happen to not trigger the bug. Did anybody test with ARM64 (Aarch64) or x86-32?

I just tested with latest self-built MPD:

# mpd -V

Music Player Daemon 0.21.10 (0.21.10)

My board is an Odroid C2 (Aarch64) and my OS is ArchlinuxARM. My dac is a diy 9018k2m with a diyinhk xmos USB which supports both DoP and native DSD.

- Test no. 1 - native DSD

# cat /proc/asound/card*/pcm*p/sub*/hw_params

access: RW_INTERLEAVED format: DSD_U32_BE subformat: STD channels: 2 rate: 88200 (88200/1) period_size: 11025 buffer_size: 44100

- Test no. 2 - DoP

# cat /proc/asound/card*/pcm*p/sub*/hw_params

access: RW_INTERLEAVED format: S32_LE subformat: STD channels: 2 rate: 176400 (176400/1) period_size: 22050 buffer_size: 88200

I listened to various DSD64 files without any issue at all with both native DSD and DoP.

montaintop commented 5 years ago

tested on tinkerboard S, ARMHF, noise is there , both DOP and DSD native.

timpani2012 commented 5 years ago

@MaxKellermann @Wang-Yue I tested the new version 0.21.11 MPD, It has solved the DSD playback noise problem on the Raspberry Pi 32-bit system. Thank you,MaxKellermann and Wang-Yue. thanks for all.

MaxKellermann commented 5 years ago

@timpani2012 I just removed the workaround from the "master" branch, because the master branch now has proper fixes. Can you please test that, too?

timpani2012 commented 5 years ago

@MaxKellermann The latest MPD you made today, it plays DSD OK.

MaxKellermann commented 5 years ago

Thanks, this means we now have a proper general solution for this kind of problem.

timpani2012 commented 5 years ago

Dear Max KellermannSorry, about DSD playing, there seems to be something wrong with the new MPD version. I'll do the test again.Best regardsXiao----- 原始邮件 ----- 发件人:Max Kellermann notifications@github.com 收件人:MusicPlayerDaemon/MPD MPD@noreply.github.com 抄送人:timpani2012 xiaoqingyong@sina.com, Mention mention@noreply.github.com 主题:Re: [MusicPlayerDaemon/MPD] DSD on ARM is broken (#469) 日期:2019年06月18日 17点51分

@timpani2012 I just removed the workaround from the "master" branch, because the master branch now has proper fixes. Can you please test that, too?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

kiwi-ed commented 5 years ago

Hi, I have the same issue on and ARM and USB DAC ( Chord Hugo ). Is there a repo link to 0.21.11 so I too can try? For me as well 0.20.x was fine, no constant crackle, frying, sizzling noise - it was clean.

Kernel 4.9.88 Yocto aplay: version 1.1.5

timpani2012 commented 5 years ago

Hi, @MaxKellermann

Using your version 17062019 with Raspberry PI 3B+, 32bit OS: Playing DSD with USB AUDO, the problem is that using DoP mode cannot play,but using native mode, it plays well, no noise! Using your version 18062019 with Raspberry PI 3B+, 32bit OS: Playing DSD with USB AUDO, all mode DoP and Native cannot play.

The I2S mode with DoP mode, same as above.

timpani2012 commented 5 years ago

I think the noise problem that has been bothering for a long time has been solved, the problem that DSD can't play is not a big problem.

starmood commented 5 years ago

Tested with last commit on 32bit raspbian. When playing with DOP, DAC cannot lock the signal. MPD is 100%CPU loading.and I even cannot stop MPD, I can only kill -9. Playing with normal PCM is OK

timpani2012 commented 5 years ago

@MaxKellermann, @Wang-Yue Version 0.21.11, The DSD playback problem has been solved. Thank you very much.

ilyxa commented 5 years ago

aarch64 (RPi3B):

[ilyxa@kangaroo ~]$ uname -a
Linux kangaroo 5.1.16-1-ARCH #1 SMP Wed Jul 3 20:40:57 MDT 2019 aarch64 GNU/Linux

mpd:

[ilyxa@kangaroo ~]$ mpd -V | head -1
Music Player Daemon 0.22~git (v0.21.11-433-g326c6ae61)

DSD playback still broken (garbled), both native and DoP.

ilyxa commented 5 years ago

DoP:

[ilyxa@kangaroo ~]$ cat /proc/asound/HIFIRef/pcm0p/sub0/hw_params 
access: RW_INTERLEAVED
format: S32_LE
subformat: STD
channels: 2
rate: 352800 (352800/1)
period_size: 32768
buffer_size: 131072

alsa_output: opened hw:2,0 type=HW
alsa_output: buffer: size=90..131072 time=255..371520
alsa_output: period: size=45..65536 time=127..185760
alsa_output: default period_time = buffer_time/4 = 371519/4 = 92879
alsa_output: format=S32_LE (Signed 32 bit Little Endian)
alsa_output: buffer_size=131072 period_size=32768
alsa_output: DoP (DSD over PCM) enabled
output: opened "DSD1796" (alsa) audio_format=dsd128:2

Native:

[ilyxa@kangaroo ~]$ cat /proc/asound/HIFIRef/pcm0p/sub0/hw_params 
access: RW_INTERLEAVED
format: DSD_U32_BE
subformat: STD
channels: 2
rate: 176400 (176400/1)
period_size: 22050
buffer_size: 88200

decoder: audio_format=dsd128:2, seekable=true
alsa_output: opened hw:2,0 type=HW
alsa_output: buffer: size=46..131072 time=260..743039
alsa_output: period: size=23..65536 time=130..371520
alsa_output: default period_time = buffer_time/4 = 500000/4 = 125000
alsa_output: format=DSD_U32_BE (Direct Stream Digital, 4-byte (x32), big endian, oldest bits in MSB)
alsa_output: buffer_size=88200 period_size=22050
output: opened "DSD1796" (alsa) audio_format=dsd128:2
ilyxa commented 5 years ago

Max, I can't confirm that this was mpd problem, I'll got a lot of errors:

alsa_output: Underrun on ALSA device "hw:2,0"
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun
alsa_output: Decoder is too slow; playing silence to avoid xrun

with high interrupts level. This is possible wi-fi problem on RaspberryPi (throughput is more than enough, latency is not a problem, system 95% idle but I still get a same problem not only with DSD but with ordinary PCM streams too).

kiwi-ed commented 5 years ago

Hi,

Version 0.21.11 on the few DSDs I've tested is fine. ARM iMx7d and USB DAC ( Chord Hugo ). On a DSD ~%4-17 CPU, ALSA 1.1.5.

Thanks.

MaxKellermann commented 5 years ago

OK, so this is fixed after all, and the remaining problem was just ring buffer xruns.