Skip to content
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

AudioInterface: Use 32029/48043 Hz in more places #9055

Merged
merged 2 commits into from Sep 8, 2020

Conversation

JosJuice
Copy link
Member

In particular, I wanted to do change this in AudioInterface::Init so that dumped GC audio doesn't need to have a file split (changing from 32000 Hz to 32029 Hz) when the emulated software initializes the AI registers. I've also made the same change to DI's DTK code.

In particular, I wanted to do change this in
AudioInterface::Init so that dumped GC audio doesn't need
to have a file split (changing from 32000 Hz to 32029 Hz)
when the emulated software initializes the AI registers.
I've also made the same change to DI's DTK code.
@JosJuice JosJuice marked this pull request as draft August 29, 2020 14:30
@JosJuice JosJuice marked this pull request as ready for review August 29, 2020 15:17
@JosJuice
Copy link
Member Author

@iwubcode I had to add another commit to actually get rid of the file split. Could you re-review?

@@ -294,8 +294,11 @@ static u32 AdvanceDTK(u32 maximum_samples, u32* samples_to_process)
static void DTKStreamingCallback(DIInterruptType interrupt_type, const std::vector<u8>& audio_data,
s64 cycles_late)
{
// TODO: Should we use GetAISSampleRate instead of Get48KHzSampleRate?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming you intend to investigate this at a later date?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would involve some kind of hardware testing that I'm not sure how to do. The issue (if it even is an issue) is present in master as well, so I added the comment just for clarity.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment should expand on why we might want to use GetAISSampleRate instead, so anyone else that wants to investigate in some form has all the reasoning behind the TODO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reworded the comment in a way that hopefully should make the reason clearer.

@@ -67,6 +68,12 @@ void InitSoundStream()
g_sound_stream->Init();
}

// Ideally these two calls would be done in AudioInterface::Init so that we don't
// need to have a dependency on AudioInterface here, but this has to be done
// after creating g_sound_stream (above) and starting audio dumping (below)
Copy link
Contributor

@iwubcode iwubcode Aug 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thought is that InitSoundStream() could take the rates? And then HW could expose them via getters and those values could be passed in Core.cpp. Not sure if that's worth it though?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it would work but I'm not sure if it's worth it. Does anyone else have an opinion on this?

@JosJuice JosJuice force-pushed the gc-sample-rate branch 2 times, most recently from ba837ff to e30736d Compare August 29, 2020 19:08
@lioncash lioncash merged commit 8e505ad into dolphin-emu:master Sep 8, 2020
10 checks passed
@JosJuice JosJuice deleted the gc-sample-rate branch September 8, 2020 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants