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

Add dsp rom hashes to movie header. #248

Merged
merged 1 commit into from Jun 8, 2014

Conversation

RachelBryk
Copy link
Member

Also fix a random typo.

Also fix a random typo.
@@ -74,7 +74,7 @@ static bool LoadRom(const std::string& fname, int size_in_words, u16 *rom)
return false;
}

// Returns false iff the hash fails and the user hits "Yes"
// Returns false if the hash fails and the user hits "Yes"

This comment was marked as off-topic.

This comment was marked as off-topic.

@Sonicadvance1
Copy link
Contributor

LGTM

@magumagu
Copy link
Contributor

@dolphin-emu-bot rebuild

@shuffle2
Copy link
Contributor

This PR grows old! ...poking @RachelBryk and other interested parties.

@RachelBryk
Copy link
Member Author

What did we decide to do/not do for this?

@neobrain
Copy link
Member

neobrain commented Jun 6, 2014

After a short discussion with @delroth , it seems that we indeed do want to have something like this.

I'm a bit concerned about two issues though:
* We don't have any good way to have both the homebrew and the official DSP roms "installed" (minor one)
* What's the current behavior in case the DSP rom hashes don't match? I don't see this commit adding any custom error message, so this might end up being highly confusing to resolve for an end user.

@RachelBryk
Copy link
Member Author

It doesn't do anything if the hashes mismatch, the idea is just to have the information available, so it can be read with another tool made to read the dtm file. It isn't a critical error, so i'm not sure if it is necessary to alert the user at all. It can cause desyncs though, so if a movie doesn't sync, it's useful to know which rom was used, in case that is the reason. I don't think this is confusing at all, because tasers are not typical users, and would be accustomed to using a hex editor to read the file anyway. The same thing is done with video backend, where it's saved to the header, but never read, because in most cases it doesn't affect sync (but it can in rare cases so it's useful to have the information available).

@neobrain
Copy link
Member

neobrain commented Jun 8, 2014

Sounds good then.

@dolphin-emu-bot rebuild

neobrain added a commit that referenced this pull request Jun 8, 2014
Add DSP rom hashes to movie header.
@neobrain neobrain merged commit 489534b into dolphin-emu:master Jun 8, 2014
@RachelBryk RachelBryk deleted the dsp-rom-hash branch July 6, 2014 06:17
Joern-P pushed a commit to Joern-P/dolphin that referenced this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants