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

1. Sound problem since 1.37 (TESTS). 2. Dendy-mode inaccuration #46

Closed
emu-russia opened this Issue Jun 10, 2013 · 16 comments

Comments

Projects
None yet
5 participants
@rdanbrook

This comment has been minimized.

Show comment
Hide comment
@rdanbrook

rdanbrook Jun 12, 2013

Collaborator

I will investigate further.

Collaborator

rdanbrook commented Jun 12, 2013

I will investigate further.

@anikom15

This comment has been minimized.

Show comment
Hide comment
@anikom15

anikom15 Jun 16, 2013

Most likely related to issue #29.

anikom15 commented Jun 16, 2013

Most likely related to issue #29.

@rdanbrook

This comment has been minimized.

Show comment
Hide comment
@rdanbrook

rdanbrook Jun 29, 2013

Collaborator

Okay, I've done a bit of testing. I did notice the same sound problems, and I may be crazy, but it only happens if you open the rom freshly. If you do a reset, the sound problems seem to disappear. I haven't done any recording to test this theory out, but I can plainly hear the problem when I open the rom fresh, and after a reset the problem goes away. If this is truly the case, then it's an even stranger issue than I expected.

I'm not sure if it's related to issue #29, outside of the fact that it's related to the APU.

edit:
I was testing using Windows before. I just tried using RetroArch on Linux, and the problem is even easier to detect, and does not go away with a reset. I have not tested using RetroArch on Windows.

However, running the regular unix shell on Linux results in no problems for me at all. I'm using SDL sound, with PulseAudio running.

Collaborator

rdanbrook commented Jun 29, 2013

Okay, I've done a bit of testing. I did notice the same sound problems, and I may be crazy, but it only happens if you open the rom freshly. If you do a reset, the sound problems seem to disappear. I haven't done any recording to test this theory out, but I can plainly hear the problem when I open the rom fresh, and after a reset the problem goes away. If this is truly the case, then it's an even stranger issue than I expected.

I'm not sure if it's related to issue #29, outside of the fact that it's related to the APU.

edit:
I was testing using Windows before. I just tried using RetroArch on Linux, and the problem is even easier to detect, and does not go away with a reset. I have not tested using RetroArch on Windows.

However, running the regular unix shell on Linux results in no problems for me at all. I'm using SDL sound, with PulseAudio running.

@eugene-s-nesdev

This comment has been minimized.

Show comment
Hide comment
@eugene-s-nesdev

eugene-s-nesdev Jul 30, 2015

Dendy inaccuracy happened after Martin did "favored system" in 1.38:
http://forums.nesdev.com/viewtopic.php?f=3&t=10047&p=113012#p113012

"Power Blade 2 (USA)": scrolling glitches on title movie
"James Bond Jr. (USA)": hang on 1-st level, when you fall into a pit
"Front Line (Japan)", "Aladdin 4 (1995/1996) (Unl)" - screen glitches at 1-st level
"Batman Returns (USA)" - black vertical strip at 1-st level (it happens randomly)

glitches
P.S: I am topicstarter ("emu-russia").

eugene-s-nesdev commented Jul 30, 2015

Dendy inaccuracy happened after Martin did "favored system" in 1.38:
http://forums.nesdev.com/viewtopic.php?f=3&t=10047&p=113012#p113012

"Power Blade 2 (USA)": scrolling glitches on title movie
"James Bond Jr. (USA)": hang on 1-st level, when you fall into a pit
"Front Line (Japan)", "Aladdin 4 (1995/1996) (Unl)" - screen glitches at 1-st level
"Batman Returns (USA)" - black vertical strip at 1-st level (it happens randomly)

glitches
P.S: I am topicstarter ("emu-russia").

@eugene-s-nesdev

This comment has been minimized.

Show comment
Hide comment
@eugene-s-nesdev

eugene-s-nesdev Dec 17, 2015

I've just talked with Marty about this problems:

Me:
Here are 2 major problems, that nobody of new developers cannot solve at this moment:
#46

Martin:
Thanks Eugene, i think i know the answer to that....
During the 12 first seconds, nestopia tries to detect the rate of your soundcard frequency...
Thats why you probably get the "click" and "pop" once the frequency is dynamically adjusted.
Perhaps it would be better to disable that part of the code and use a static value like 44khz, 96khz or whatever was configured in the settings
I should add that this is happens every time you temporarily stop a game, like clicking ön a menu

eugene-s-nesdev commented Dec 17, 2015

I've just talked with Marty about this problems:

Me:
Here are 2 major problems, that nobody of new developers cannot solve at this moment:
#46

Martin:
Thanks Eugene, i think i know the answer to that....
During the 12 first seconds, nestopia tries to detect the rate of your soundcard frequency...
Thats why you probably get the "click" and "pop" once the frequency is dynamically adjusted.
Perhaps it would be better to disable that part of the code and use a static value like 44khz, 96khz or whatever was configured in the settings
I should add that this is happens every time you temporarily stop a game, like clicking ön a menu

@rdanbrook

This comment has been minimized.

Show comment
Hide comment
@rdanbrook

rdanbrook Dec 17, 2015

Collaborator

It's great that you have been able to communicate with Marty about this issue. I wouldn't mind having access to him somtimes :p.

It makes sense that dynamically adjusting the sample rate would cause this problem. I'm not sure if Marty was referring to the win32 specific code or the core though. I will take a look.

https://github.com/rdanbrook/nestopia/blob/master/source/core/NstApu.cpp#L429
Maybe it's this line. I'll play around as soon as I have time.

Collaborator

rdanbrook commented Dec 17, 2015

It's great that you have been able to communicate with Marty about this issue. I wouldn't mind having access to him somtimes :p.

It makes sense that dynamically adjusting the sample rate would cause this problem. I'm not sure if Marty was referring to the win32 specific code or the core though. I will take a look.

https://github.com/rdanbrook/nestopia/blob/master/source/core/NstApu.cpp#L429
Maybe it's this line. I'll play around as soon as I have time.

@eugene-s-nesdev

This comment has been minimized.

Show comment
Hide comment
@eugene-s-nesdev

eugene-s-nesdev Dec 17, 2015

[quote]I'm not sure if Marty was referring to the win32 specific code or the core though. I will take a look.[/quote]

In theory, it must be core specific code.
I've compiled original 1.40 with linux overlay H (by R.Bannister) for ubuntu 10.04 a couple years ago,
and it also have same sound problems.

eugene-s-nesdev commented Dec 17, 2015

[quote]I'm not sure if Marty was referring to the win32 specific code or the core though. I will take a look.[/quote]

In theory, it must be core specific code.
I've compiled original 1.40 with linux overlay H (by R.Bannister) for ubuntu 10.04 a couple years ago,
and it also have same sound problems.

@rdanbrook

This comment has been minimized.

Show comment
Hide comment
@rdanbrook

rdanbrook Dec 18, 2015

Collaborator

Okay, I've found the source of the problem. It's now just a matter of finding the correct way to fix it. The Apu::Synchronizer::Clock function seems to be where the issue is taking place, and if I do printf debugging I can see things corresponding to the blip in the audio.

I committed a change (commented out some code only), let's see if any guinea pigs want to test that out and see how it goes.

Collaborator

rdanbrook commented Dec 18, 2015

Okay, I've found the source of the problem. It's now just a matter of finding the correct way to fix it. The Apu::Synchronizer::Clock function seems to be where the issue is taking place, and if I do printf debugging I can see things corresponding to the blip in the audio.

I committed a change (commented out some code only), let's see if any guinea pigs want to test that out and see how it goes.

@rdanbrook

This comment has been minimized.

Show comment
Hide comment
@rdanbrook

rdanbrook Dec 18, 2015

Collaborator

It appears the problem is fixed, but I did experience some weirdness on OS X, so I'll still look deeper at this.

The Dendy stuff is still messed up though so I'm leaving the issue open. Can't wait to close this one.

Collaborator

rdanbrook commented Dec 18, 2015

It appears the problem is fixed, but I did experience some weirdness on OS X, so I'll still look deeper at this.

The Dendy stuff is still messed up though so I'm leaving the issue open. Can't wait to close this one.

@eugene-s-nesdev

This comment has been minimized.

Show comment
Hide comment
@eugene-s-nesdev

eugene-s-nesdev Dec 18, 2015

Yes, it was really major issue!
http://forums.nesdev.com/viewtopic.php?p=160868#p160868
Try to test this patch well for make sure bug 100% gone

eugene-s-nesdev commented Dec 18, 2015

Yes, it was really major issue!
http://forums.nesdev.com/viewtopic.php?p=160868#p160868
Try to test this patch well for make sure bug 100% gone

@emu-russia

This comment has been minimized.

Show comment
Hide comment
@emu-russia

emu-russia Jun 10, 2016

Okay.
After 2 days of HARD debugging, FHorse has FIXED all those bugs in dendy mode. Now all works like charm
Source of problem was the flag of VBLANK and the NMI were set prior of sleep scanlines (in new part of code that was added since 1.38)
Here is the patch FHorse did for 1.46.2, because you've dropped out all lower than OpenGL 3.2 since 1.47 (i see only black screen on i7-2600K/GMA HD3000, that has OGL3.1 only, why you did it ???)

Can you also disable these 2 lines in NstApu.cpp to do triangle fix in dendy mode right way?
http://savepic.su/7245317.png

https://github.com/rdanbrook/nestopia/blob/master/source/core/NstApu.cpp
Lastest source shows me it 1595 and 1596 lines for now.

Also i have a request about region selectors. For now it completely broken.
Original Nestopia has 2 region selectors.
First was main, named "Region selector", which contain "auto/NTSC/PAL" modes, and it was able to FORCE select mode.
Second was slave, named "Favored system", which contain "NES NTSC / NES PAL / Famicom and Dendy" modes, and it parameters were linked to main selector.
For example,
If you want to set dendy mode, go to preferences and select dendy in "favored system" settings.
Then you need to close emulator. After start it again, PAL mode in region selector will work like dendy.
If you need to return PAL back, you do the full cycle all over again.

But you're kicked out MAIN Region selector since 1.46 in linux version, and for now we got strange behaviour things.
On my machine all "NES NTSC / NES PAL / Famicom and Dendy" works like NTSC
On FHorse machine only NTSC and Dendy works, but he cannot get true PAL.
We tried vanilla 1.46.2, of course, with Tepples's overclock demo, that i've linked here:
#143

This is awful.

1.46.2_Dendy_Mode_Timing_Fix.zip

emu-russia commented Jun 10, 2016

Okay.
After 2 days of HARD debugging, FHorse has FIXED all those bugs in dendy mode. Now all works like charm
Source of problem was the flag of VBLANK and the NMI were set prior of sleep scanlines (in new part of code that was added since 1.38)
Here is the patch FHorse did for 1.46.2, because you've dropped out all lower than OpenGL 3.2 since 1.47 (i see only black screen on i7-2600K/GMA HD3000, that has OGL3.1 only, why you did it ???)

Can you also disable these 2 lines in NstApu.cpp to do triangle fix in dendy mode right way?
http://savepic.su/7245317.png

https://github.com/rdanbrook/nestopia/blob/master/source/core/NstApu.cpp
Lastest source shows me it 1595 and 1596 lines for now.

Also i have a request about region selectors. For now it completely broken.
Original Nestopia has 2 region selectors.
First was main, named "Region selector", which contain "auto/NTSC/PAL" modes, and it was able to FORCE select mode.
Second was slave, named "Favored system", which contain "NES NTSC / NES PAL / Famicom and Dendy" modes, and it parameters were linked to main selector.
For example,
If you want to set dendy mode, go to preferences and select dendy in "favored system" settings.
Then you need to close emulator. After start it again, PAL mode in region selector will work like dendy.
If you need to return PAL back, you do the full cycle all over again.

But you're kicked out MAIN Region selector since 1.46 in linux version, and for now we got strange behaviour things.
On my machine all "NES NTSC / NES PAL / Famicom and Dendy" works like NTSC
On FHorse machine only NTSC and Dendy works, but he cannot get true PAL.
We tried vanilla 1.46.2, of course, with Tepples's overclock demo, that i've linked here:
#143

This is awful.

1.46.2_Dendy_Mode_Timing_Fix.zip

@rdanbrook

This comment has been minimized.

Show comment
Hide comment
@rdanbrook

rdanbrook Jun 11, 2016

Collaborator

Are there any detailed logs/forum topics describing how FHorse got to these conclusions? I will merge the patch, but I want to understand exactly what I'm merging first.

As for OpenGL, it has been moved to 3.2 to make it possible to render to the GTK+ OpenGL widget in the future instead of using dirty hacks that are windowing system specific. This will be required to run on Wayland. There should be a way to use software fallback if your video driver does not support modern OpenGL.

I will take another look at the region selector.

Collaborator

rdanbrook commented Jun 11, 2016

Are there any detailed logs/forum topics describing how FHorse got to these conclusions? I will merge the patch, but I want to understand exactly what I'm merging first.

As for OpenGL, it has been moved to 3.2 to make it possible to render to the GTK+ OpenGL widget in the future instead of using dirty hacks that are windowing system specific. This will be required to run on Wayland. There should be a way to use software fallback if your video driver does not support modern OpenGL.

I will take another look at the region selector.

@eugene-s-nesdev

This comment has been minimized.

Show comment
Hide comment
@eugene-s-nesdev

eugene-s-nesdev Jun 11, 2016

I will show him this topic, try to tell here.

eugene-s-nesdev commented Jun 11, 2016

I will show him this topic, try to tell here.

@punesemu

This comment has been minimized.

Show comment
Hide comment
@punesemu

punesemu Jun 11, 2016

Watching routine Ppu::Run you can easily see that the flag of VBLANK and the NMI are performed to cycles.hClock 681 (HCLOCK_VBLANK_0), 682 (HCLOCK_VBLANK_1) and 684 (HCLOCK_VBLANK_2) that is virtually one scanline after the VACTIVE (240) scanlines. This is fine for PPU_RP2C02 (NTSC) and PPU_RP2C07 (PAL) but not for PPU_DENDY that needs another 50 sleep scanlines. What I did was nothing more than adding these 50 scanlines first of the HCLOCK_VBLANK_0 that are performed only when the variable (ssleep >= 0) and this is true only in the case of PPU_DENDY. This way I left intact the logic with which the routine work for NTSC and PAL, intervening only for Dendy mode because ssleep will always be -1 for PPU_RP2C02 and PPU_RP2C07.
I hope that I was able to explain well.

punesemu commented Jun 11, 2016

Watching routine Ppu::Run you can easily see that the flag of VBLANK and the NMI are performed to cycles.hClock 681 (HCLOCK_VBLANK_0), 682 (HCLOCK_VBLANK_1) and 684 (HCLOCK_VBLANK_2) that is virtually one scanline after the VACTIVE (240) scanlines. This is fine for PPU_RP2C02 (NTSC) and PPU_RP2C07 (PAL) but not for PPU_DENDY that needs another 50 sleep scanlines. What I did was nothing more than adding these 50 scanlines first of the HCLOCK_VBLANK_0 that are performed only when the variable (ssleep >= 0) and this is true only in the case of PPU_DENDY. This way I left intact the logic with which the routine work for NTSC and PAL, intervening only for Dendy mode because ssleep will always be -1 for PPU_RP2C02 and PPU_RP2C07.
I hope that I was able to explain well.

@eugene-s-nesdev

This comment has been minimized.

Show comment
Hide comment
@eugene-s-nesdev

eugene-s-nesdev Jun 12, 2016

  1. Sorry again. This is FINAL patch to fix triangle (made for 1.46.2).
    It completely removes all unused stuff.
    1.46.2_Dendy_Triangle_Fix.patch.zip

  2. How do you think, it's need to revert changes that was made in 1.45,
    before correct solution from Marty that fixed sound issues was found?

1.45 changelog:

  • Modified default sound settings to avoid desync

eugene-s-nesdev commented Jun 12, 2016

  1. Sorry again. This is FINAL patch to fix triangle (made for 1.46.2).
    It completely removes all unused stuff.
    1.46.2_Dendy_Triangle_Fix.patch.zip

  2. How do you think, it's need to revert changes that was made in 1.45,
    before correct solution from Marty that fixed sound issues was found?

1.45 changelog:

  • Modified default sound settings to avoid desync
@rdanbrook

This comment has been minimized.

Show comment
Hide comment
@rdanbrook

rdanbrook Jun 12, 2016

Collaborator

Okay, all patches are applied, and I fixed the region selector in the cross-platform port.

The change in 1.45 was just a change for the default settings on the legacy win32 release. There's not a lot of reason to bother changing it back, it is probably better the way it is now anyway.

I believe this means we can finally close this topic!

Thanks everyone for collaborating to make this a better emulator.

Collaborator

rdanbrook commented Jun 12, 2016

Okay, all patches are applied, and I fixed the region selector in the cross-platform port.

The change in 1.45 was just a change for the default settings on the legacy win32 release. There's not a lot of reason to bother changing it back, it is probably better the way it is now anyway.

I believe this means we can finally close this topic!

Thanks everyone for collaborating to make this a better emulator.

@rdanbrook rdanbrook closed this Jun 12, 2016

rdanbrook added a commit that referenced this issue Jan 28, 2017

Merge pull request #46 from Monroe88/palettes3
(libretro) Add updated FirebrandX palettes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment