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

Custom BGM is fixed #5829

Merged
merged 1 commit into from
Apr 10, 2014
Merged

Custom BGM is fixed #5829

merged 1 commit into from
Apr 10, 2014

Conversation

kaienfr
Copy link
Contributor

@kaienfr kaienfr commented Apr 10, 2014

Based on this commit #5828 on improvements to sceMp3
Custom BGM can be update now. Test on "Miku" is just all right :)

Based on this commit hrydgard#5828 on improvements to sceMp3
Custom BGM can be update now. Test on "Miku" is just all right :)
@kaienfr kaienfr mentioned this pull request Apr 10, 2014
@kaienfr
Copy link
Contributor Author

kaienfr commented Apr 10, 2014

Add a new audio hack menu in #5828 we can chose to active or not the new setting to prevent possibly other unknown sound issues.

@hrydgard
Copy link
Owner

As previously discussed, a partial memcpy is not the correct way to shorten filenames according to FAT32 rules - but on the other hand this is really rarely-used functionality and it hardly matters - the game will just use the short name as an internal identifier anyway. So I'll let it slide for now and possibly revisit later.

hrydgard added a commit that referenced this pull request Apr 10, 2014
@hrydgard hrydgard merged commit 6fe617d into hrydgard:master Apr 10, 2014
@kaienfr kaienfr deleted the new_Custom_BGM branch April 10, 2014 10:37
@kaienfr
Copy link
Contributor Author

kaienfr commented Apr 10, 2014

Ok, let's look and see others feedback if we need something more to do.

@unknownbrackets
Copy link
Collaborator

Well, and the strncpy() still copies 256 bytes always. I hate it when there's comments like this:

// Copy 10 bytes.
memcpy(dst, src, 20);

Which is exactly how this looks to me.

-[Unknown]

@kaienfr
Copy link
Contributor Author

kaienfr commented Apr 11, 2014

We could copy in a right order, copy the short part before the long part.
for example, we copy the 8.3 name first even it occupied 256 bytes (anyway it will not longer than the length of sortname+full path), then we copy the full path from the position we need.

@hrydgard
Copy link
Owner

Agh, forgot the extralong strncpy thing. That's the danger of re-opening pullrqs, losing comments...

@Fei-Lix
Copy link

Fei-Lix commented Apr 13, 2014

there is one of my mp3 was Star Story BGM shows this message:
npjh50465_00001
also other custom BGM gives no sounds like Kagerou Daze BGM, I'll provide these samples here (includes Edit data + MP3 of Star Story and Kagerou Daze): mediafire.com/?bluz77oz6752wtz
Password is "no sound"

@kaienfr
Copy link
Contributor Author

kaienfr commented Apr 13, 2014

@RinkuFO This is only a part of update, since its completely another issue in show cunstom menu, so we separate it from our universal audio branch, only with the later one, you can play Miku. But it still not been merged yet. #5839

Miku issue is already perfectly solved there.

@Fei-Lix
Copy link

Fei-Lix commented Apr 13, 2014

@kaienfr ohhh I see, sorry I didn't noticed it because I've been excited to test it out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants