search

MJoergen / C64MEGA65

Commodore 64 core for the MEGA65 based on the MiSTer FPGA C64 core
GNU General Public License v3.0
30 stars 4 forks source link

Atrocious HDMI audio #116

Closed sy2002 closed 10 months ago

sy2002 commented 1 year ago

When I tested Eye of the Beholder to double-check if my .crt loading regression is "really really fixed", I stumbled over strange background noises in the HDMI audio stream. It was subtle, not super prominent, and I ignored it at first.

But then I saw this commit 2b7dd65120d99212890179ee11eeb8fc14f51104 and this commit message:

M2M: Refactor audio clock speed

Replace the 30 MHz clock signal with a 12.288 MHz clock.
The 30 MHz valus is completely arbitrary, whereas 12.288 MHz is exactly
256 times the audio sample rate (48 kHz).

So my working assumption is, that something went wrong here.

I then did some more tests: The subtle distortion that I hear in Eye of the Beholder is for some reason 100x amplified when I listen to the song "Excalibur" or "Game of Thrones": I recorded the issue, listen to "Excalibur" here: excalibur.zip

Maybe the reason for the amplification of the problem is that Eye of the Beholder is Mono (one SID) and the two songs "Excalibur" and "Game of Thrones" are Stereo? But independent from Mono and Stereo: The problem is always there. Just different degrees of amplifications.

I hope you can reproduce this (i.e. the speakers of your monitor are sensible enough; my HiFi for sure is ;-)), so that you can test the bugfixing progress independently from me. But of course I am here to support and to test in case you can't. It just takes a bit longer currently due to my extreme end-of-year business.

Reproduction steps:

  1. Download this music disk
  2. Mount it and LOAD "*",8,1
  3. Choose Dual Sid, D420 in the music player's startup menu
  4. Configure the OSM to Stereo SID by choosing "L:8580 R:8580" D420
  5. Audio Improvements On
  6. Play Excalibur

What I find interesting is, that "Audio Improvements" amplify the problem.

Closing thoughts:

sy2002 commented 1 year ago

Update: I built and tested the very commit before the audio refactoring (c5f2d0abedd56dee54fea25a19282ca5071dde66) and there everything is OK, so it seems indeed that the refactoring introduced a problem.

MJoergen commented 1 year ago

Thank you for the detailed report. I've tried to reproduce and I have some doubts.

  1. On the analog output I hear no distortion.
  2. On my HDMI monitor I hear no distortion.

However, when I do a recording from the analog output to my desktop computer (via a 3.5 mm audio cable) and record using Audacity I notice a difference between the left and right channels. The left channel sounds great, but the right channel is indeed distorted.

I tried the same setup using commit c5f2d0a, but same result, i.e. left channel sounds great and right channel is heavily distorted.

So, could you please confirm that you hear a difference between the left and right channels? I would like to determine whether my observation is just an artifact in my setup.

sy2002 commented 1 year ago

OK - just to make sure we have the same setup: you have "Improve Audio = ON"? Because this seems to amplify the effect and maybe the tube-amplifier and the HiFi speakers I use are more sensitive to this kind of stuff than your equipment?

Unfortunately I have no analog audio recording abilities at this moment, so I am exclusively testing HDMI audio.

I re-tested, here is the result (using the music disk I provided to you above):

Commit c5f2d0abedd56dee54fea25a19282ca5071dde66 (the one before you refactored):

So this is the proof that the problem seems to be somewhere else and not directly related to the "refactor commit 2b7dd65120d99212890179ee11eeb8fc14f51104

WOW - this one is interesting!

MJoergen commented 1 year ago

Interesting indeed. When I find time (within a few days I hope) I will attempt to recreate the tests you have described, to see whether I can reproduce your observations.

sy2002 commented 1 year ago

Update on this statement from above:

What I find interesting is, that "Audio Improvements" amplify the problem.

Actually, there are situations where "Audio Improvements" hides / covers the problem and there are situations where "Audio Improvements" emphasizes the problem.

sy2002 commented 12 months ago

@MJoergen As you asked for, I did a kind of "binary search" to narrow down the commit where the sound bug was introduced. My challenge is that one build takes about 47mins... so for now I can only offer two results for now:

✅ Alpha 3: Audio OK. Commit #c099921, remember Alpha 3 introduced: "Reconfigurable (and completely refactored) MMCM video clock, refactored reset, ASCAL reduced burst length that might fix some rare REU bugs"

:x: 3030cb7d9b15344e445de32b28edc8bba2e52c35 "Fix HDMI audio microcuts": I was able to reproduce the problem with the song Excalibur and Odyssey and Game of Thrones, but only after switching back and forth between display modes

MJoergen commented 12 months ago

Thank you so much. This is already very helpful information.

There have been one other HDMI audio related fix (specifically fixing the problem on my monitor). So to test whether this is the cause, please try with these two bit-files, generated from two close commits.

mega65_r3_17f06ab.bit.gz mega65_r3_18d9349.bit.gz

sy2002 commented 12 months ago

✅ mega65_r3_17f06ab.bit: Sound OK - was not able to reproduce the problem. ✅ mega65_r3_18d9349.bit: Sound OK - was not able to reproduce the problem.

MJoergen commented 12 months ago

Your results narrow down the list of commits to less than ten. Most of these commits are harmless, but one commit (3030cb7) does indeed touch the HDMI audio. So this is likely the culprit.

Just to be completely sure, I've completed a build based on the very previous commit: mega65_r3_1981141.bit.gz

EDIT: I've made two additional builds I would like tested as well: mega65_r3_0fed2c6.bit.gz mega65_r3_f8cbef0.bit.gz

When you find time, please try this build.

sy2002 commented 12 months ago

Thanks @MJoergen - I will not be able to test this in the next 7 days or so. Hopefully you can reproduce it.

sy2002 commented 11 months ago

@MJoergen here are the test results:

✅ mega65_r3_1981141.bit: Sound OK - was not able to reproduce the problem. ✅ mega65_r3_0fed2c6.bit: Sound OK - was not able to reproduce the problem. ✅ mega65_r3_f8cbef0.bit: Sound OK - was not able to reproduce the problem.

sy2002 commented 11 months ago

@MJoergen on Skype you wrote:

Perhaps you could spend a few minutes to reproduce the issue on the first image reported (3030cb7). You've previously tested this image, and shown the audio fails on that image.

❌ 3030cb7 - Sound not OK - I was able to reproduce the problem multiple times. using 3030cb7.

But what I find stunning is that sometimes it takes like 30min of playing around with the core and switching back and forth screen resolutions and stuff to reproduce it and then the problem is only very faintly audible. But then, sometimes it happens DIRECTLY after I use the m65 -q mega65_r3.bit command line to program the FPGA and play the first song. And in this case, it is not faintly audible it is loud, ugly, atrocious. Listen to this recording of a situation, where the problem directly arose after I programmed the FPGA: audibug-3030cb7.zip "Audio Improvements ON" is emphasizing the problem but it is also audible when OFF.

So in short:

1) Sometimes it takes 30min to reproduce and I need to switch back and forth the screen resolution multiple times and even then I can only faintly hear the bug.

2) Sometimes the problem is IMMEDIATELY audible after programming the FPGA via m65 and THEN it is not faintly but super loud as you can hear: audibug-3030cb7.zip

3) I did some experiments: It takes 7 to 10 times of re-programming the FPGA via m65 to provoke the "loud" version of the problem as described in (2). It seems to be easier to reproduce the problem by just re-programming the FPGA while it stays in the 720p @ 50 Hz mode all the time (instead of trying this strenuous manual switching of screen-resolutions).

Disclaimer: Everything I wrote so far is about commit 3030cb7.

This leads to the following question I have for you - maybe we can do "debugging by thinking":

Can it be that this is some kind of an "FPGA logic initialization" bug?

You also wrote on Skype:

Of the three new images, I expect two of them to work, but I do expect f8cbef0 to fail in the same way as 3030cb7.

❌ f8cbef0 - Sound not OK - Yes, this time I was able to use my newly discovered "re-program the FPGA" technique to reproduce the problem, so f8cbef0 failed, too (in contrast to my claim above, that it works). In this specific instance, it happened directly after the initial programming. No re-programming needed this time.

So this is what you expected. Now let's try the re-prgram technique on the two others that you do expect to be stable.

✅ 1981141 - Sound OK - was not able to reproduce after 12 attempts of re-programming ✅ 0fed2c6 - Sound OK - was not able to reproduce after 12 attempts of re-programming

I also tested your very latest commit 12a36d9 in develop from December 20, which does not have branch "mfj_fix_issue_116" included:

✅ 12a36d9 - Sound OK - was not able to reproduce after 12 attempts of re-programming

Is this expected? It does not include the fixes from the specialized branch, but 1981141 does also not include these fixes and works, too.

I think we would be able to close this issue as everything seems to work with the latest commit 12a36d9 and also with the "remove HDMI text messaging" commit 0fed2c6. But do we have any explanation why the bug went away?

MJoergen commented 11 months ago

Thank you for your detailed testing and reporting. That the latest commit 12a36d9 works is indeed unexpected!

Between the commits f8cbef0 (from December 4th) and 12a36d9 (from December 20th) there are only approximately ten other commits. And looking at the file difference statistics shows the following:

mike@mike-desktop2 ~/git/MJoergen/C64MEGA65 (develop=) $ git diff --stat f8cbef0 12a36d9 
 CORE/CORE-R3.xpr                              |   6 +-
 CORE/CORE-R4.xpr                              |   6 +-
 CORE/CORE-R5.xpr                              |   6 +-
 CORE/vhdl/config.vhd                          |  10 +--
 CORE/vhdl/main.vhd                            |  20 +++--
 CORE/vhdl/prg_loader.vhd                      | 194 ++++++++++++++++++++++------------------------
 M2M/vhdl/controllers/M65/audio.vhd            |   8 +-
 M2M/vhdl/framework.vhd                        |  26 +++++--
 M2M/vhdl/i2c/cpu_to_i2c_master.vhd            |  67 +++++++++++++---
 M2M/vhdl/i2c/i2c_controller.vhd               |  11 ++-
 M2M/vhdl/i2c/i2c_master.vhd                   |   9 +++
 M2M/vhdl/i2c/rtc.vhd                          |  95 -----------------------
 M2M/vhdl/i2c/rtc_controller.vhd               | 251 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 M2M/vhdl/i2c/rtc_master.vhd                   | 323 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 M2M/vhdl/i2c/rtc_reader.vhd                   | 190 ---------------------------------------------
 M2M/vhdl/i2c/{rtc_i2c.vhd => rtc_wrapper.vhd} |  59 ++++++++------
 M2M/vhdl/qnice_arbit.vhd                      |   6 +-
 M2M/vhdl/top_mega65-r3.vhd                    |  14 ++--
 M2M/vhdl/top_mega65-r4.vhd                    |  17 ++--
 M2M/vhdl/top_mega65-r5.vhd                    |  23 +++---
 doc/inofficial.md                             |   1 +
 21 files changed, 861 insertions(+), 481 deletions(-)

Nothing in the above list can explain the sudden disappearance of the HDMI audio bug - nothing is related to HDMI audio. The file M2M/vhdl/controllers/M65/audio.vhd is used for the audio DAC.

I do still believe this HDMI audio bug is related to upgrading from Tyto 1 to Tyto 2. This follows from the commit 3030cb7 where Tyto 2 is introduced, while the very previous commit 1981141 does not have the HDMI audio bug.

Originally, we upgraded from Tyto 1 to Tyto 2 to solve the problem of "micro cuts" (see issue #13). However, this same upgrade seems to have a side effect of worsening the HDMI audio.

So we are at a cross-roads: Should we keep Tyto 2 and hope that the HDMI audio bug won't reappear? Or should we downgrade to Tyto 1 and potentially re-introduce the "micro cuts"?

I can add that comparing the difference between Tyto 1 and Tyto 2 can be done with the command git diff -w 0fed2c6 12a36d9 M2M/vhdl/controllers/HDMI/vga_to_hdmi.vhd. The differences seem to be limited to:

So it is entirely possible that the second bullet above may be the real culprit ?!

sy2002 commented 10 months ago

@MJoergen Here is an update:

The latest Alpha 5 release, which does not have the branch mfj_fix_issue_116 included did indeed fail. Interestingly enough upon the first power-on of the MEGA65. Here is a recording how it sounds: audibug-3030cb7.zip

This new is kind of "good" news, since a few days ago I ran into the strange situation, that I was not able to reproduce the bug with this commit 12a36d9490bf80dee237d5707cbd1774eb45b201 which is similar to "Alpha 5", see here: https://github.com/MJoergen/C64MEGA65/issues/116#issuecomment-1872986194 - and this also confused you. This is now a proof that 12 re-tries might not always be enough to reproduce the problem. And that sometimes it "just happens" when you power-on the MEGA65 (I have flashed Alpha 5 to run when I power on the machine).

Might this problem be temperature related? My MEGA65 was ice-cold. It was cold in the room over night.

More findings:

  1. A LONG reset does not mitigate the problem. If it is there, it is here to stay...

  2. A power cycle removes the problem

Since I was not (so far) able to reproduce the problem with the branch mfj_fix_issue_116, I will now merge this branch to develop and make an Alpha 6 for us, which will also contain MiSTer's drive-switching-fix. We can then, before releasing an RC1, test a bit by ourselves (including @paich64) if we can reproduce the bug in Alpha 6.

paich64 commented 10 months ago

The following actions have been repeated 40 times without encountering the Mentionned Audio Issue

On PC (40 times): ./m65 -q /home/paich64/tools/mega65tools/c64core/04_audio_issue/C64MEGA65-V5.1A6-R3.bit

On Mega65(40 times) C64 core which has just started and which is configured with

execute the following steps:

Result : Audio issue never occured

paich64 commented 10 months ago

@sy2002 @MJoergen in addition to my previous update (and as discussed during our call) :

Before the swap disk issue was fixed

Booze Design 2001 Royal Arte 100% (Mister) KO (Mega65) KO <=====

Plush 1997 +H2K (Mister) KO (Mega65) OK

Shape 2017 The Shores of Reflection (Mister) OK (Mega65) KO

Resource 2001 Soiled Legacy (Mister) KO (Mega65) OK

Coma 1997 Void (Mister) KO (Mega65) OK

After the swap disk issue was fixed

Booze Design 2001 Royal Arte 100% (Mister) OK (Mega65) OK <=====

Plush 1997 +H2K (Mister) OK (Mega65) OK

Shape 2017 The Shores of Reflection (Mister) OK (Mega65) KO

Resource 2001 Soiled Legacy (Mister) OK (Mega65) OK

Coma 1997 Void (Mister) OK (Mega65) OK

Mega65 was better than the Mister. And the fix

Anyway the fix is obviously to be kept as it fixed "Royal Arte" and running other demos does not seem (so far) to have introduced disk swap regressions.

Other issues fixed in https://github.com/MiSTer-devel/C64_MiSTer/issues/160

sy2002 commented 10 months ago

@paich64 Here is Alpha 7 C64MEGA65-V5.1A7.zip

paich64 commented 10 months ago

@sy2002

paich64 commented 10 months ago

@sy2002 @MJoergen , Unfortunately after having put the core under "pressure", the Audio bug occured after the 142nd execution of "Excalibur", using V5.1A7 ..... I also confirm it's only HDMI. The Mega65 analog audio output is not impacted.

paich64 commented 10 months ago

@sy2002 @MJoergen the audio issue captured : https://youtu.be/6-1ZefC8ObM

paich64 commented 10 months ago

additional comments :

Maybe we should consider testing using 720p ( a 80 iterations run) and the using 576p (a 80 iteration run). We may come to the conclusion that it's only occuring using 720p (just a guess ...)

sy2002 commented 10 months ago

@paich64 WOW - Thank you for this incredibly thorough testing cycle! You have proven a ton of stamina.

paich64 commented 10 months ago

@sy2002, @tonton tested and confirmed the random micro-cuts of sound is still fixed with this Alpha 7

sy2002 commented 10 months ago

@paich64 We know that this is a huge ask. Sorry for that! Can you please re-test using this Alpha 8 version: C64MEGA65-V5.1A8.zip Feel free to use 720p - maybe it provokes the issue faster.

paich64 commented 10 months ago

@paich64 We know that this is a huge ask. Sorry for that! Can you please re-test using this Alpha 8 version: C64MEGA65-V5.1A8.zip Feel free to use 720p - maybe it provokes the issue faster.

I want to be in the exact same conditions (having enough time ahead of me to run as many iterations as possible). Because of a job constraint i will do it tomorrow starting from the end of afternoon ;)

paich64 commented 10 months ago

@sy2002 I ran a new bench session with V5.1A8 and unfortunately it failed again after 31 attempts.

It seems to happen faster when indeed using 720p.

sy2002 commented 10 months ago

@paich64 THANK YOU! We highly appreciate your dedication and your support!

paich64 commented 10 months ago

We must fix it, so whatever it takes in terms of testing, please ask :)

Le mer. 10 janv. 2024 à 08:48, sy2002 @.***> a écrit :

@paich64 https://github.com/paich64 THANK YOU! We highly appreciate your dedication and your support!

— Reply to this email directly, view it on GitHub https://github.com/MJoergen/C64MEGA65/issues/116#issuecomment-1884343790, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABROXOCADZO4ABVXQB3JLH3YNZBWRAVCNFSM6AAAAAA723EC4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBUGM2DGNZZGA . You are receiving this because you were mentioned.Message ID: @.***>

sy2002 commented 10 months ago

@paich64 Please perform 150 (or more) tests with this Alpha 9 version: C64MEGA65-V5.1A9.zip

Adam Barnes (@amb5l ) found a connection between the clock speed/resolution and the problem, which is why I encourage you to continue to test using 720p (which lead to faster failure in past). If you manage to do 150 tries without the problem happening then we would consider it as resolved.

FYI, if you're interested in the details, here is a quote from Adam: "I have a theory. PCM sample pairs are latched into signals called iec_ by the PCM process which is clocked by pcm_clk (qualified by pcm_clken). iec_req is asserted for each PCM sample pair. The way this is done has changed recently - it is now generated by the P_IECREQ process - but the timing is the same. The iec signals pass through a CDC synchroniser so they can be safely sampled in the vga_clk domain. This sychroniser has 2 stages. So does the synchroniser for iec_req -> vga_iec_en. This means there is a chance that vga_iec_en could be asserted while the signals it is supposed to qualify are still updating. This chance will vary as the phase relationship between the PCM and VGA clock domains changes. Which will happen when there are resolution/clock changes. See iec_req_s, vga_iec_en and iec_ack for changes. Basically vga_iecen is delayed by a couple of clocks to allow all the iec* synchroniser outputs from SYNC3 to settle."

sy2002 commented 10 months ago

Copying the results from @paich64 's test from Skype to here:

grafik

200 successful tests using 720p

We consider this bug fixed.