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

Adds more appropriate stock music for Zandi's radio #264

Merged
merged 4 commits into from
Nov 7, 2024

Conversation

patmauro
Copy link
Contributor

@patmauro patmauro commented Nov 2, 2024

Equivalent to Commit b02d80 of content/144 ("Cleft Revamp Bundle 1") in the OpenUru Foundry, included in the Q3 2024 update to MOULa. (For submission to H'uru I am splitting this bundle into three separate PRs for increased modularity.)

Replaces the classical stock music introduced when Uru went open-source for a royalty-free track a little better suited for Zandi's radio - a blues track that feels more in keeping with both Zandi as a character as well as the "Americana" vibes of The Cleft as a setting - and modifies the attributions in all languages to compensate. As with the prior update to this audio file, the correct filters have also been applied to make sure this sounds like it is coming from a radio.

Also removes any remaining references to Build You Up, Build You Down from the credits LOC files in all languages (these references had been removed from some, but not all). (Sidenote: I only changed the relevant section in each, but I did notice the credits LOC files seem significantly different between OU/MOULa and Huru/moul-assets; in the future it may be a good idea to figure out why these diverged and try to reconcile them - I see no reason why these should be so different.)

This stock music track, from Jason Shaw at Audionautix.com, is royalty free and completely free for anyone to download and use (even for commercial purposes) as long as one provides credit - this is therefore compatible with our own license, and is perfect for this project.

The track: https://youtu.be/-tjUW-wzaZ4?si=RO41CNR7UfEfBDBr

@Deledrius
Copy link
Member

Nice music choice. That sounds like an appropriate replacement to me!

@Hoikas Hoikas added the subjective An issue or fix that is artistic or creative in nature label Nov 3, 2024
Copy link
Member

@Hoikas Hoikas left a comment

Choose a reason for hiding this comment

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

Did you verify that the sound buffer data size in the PRP is correct for this new track? Incorrect buffer lengths are known to cause problems, especially on the legacy client.

compiled/dat/GlobalEnglish.loc Outdated Show resolved Hide resolved
@Hoikas
Copy link
Member

Hoikas commented Nov 3, 2024

I did notice the credits LOC files seem significantly different between OU/MOULa and Huru/moul-assets; in the future it may be a good idea to figure out why these diverged and try to reconcile them - I see no reason why these should be so different.

We have updated the credits significantly to match the state of affairs on our end. This was done in #61.

@Hoikas Hoikas added the awaiting author This pull request or issue is blocked on a response or revision from the author. label Nov 3, 2024
@patmauro
Copy link
Contributor Author

patmauro commented Nov 3, 2024

Did you verify that the sound buffer data size in the PRP is correct for this new track? Incorrect buffer lengths are known to cause problems, especially on the legacy client.

I've tested this extremely extensively on both H'uru and Legacy clients over the last six months and I don't think that will be an issue here. In fact - from what I can tell, the PRP doesn't seem to have been updated when the original BYUBYD was swapped out for the longer classical track either - hence the playback issues that existed with this originally! This newer file is actually much closer in size to BYUBYD and if anything should be less of an issue.

We can take a deeper look at this if you want, but after looking into this extensively I'm very confident this will not be a concern in this particular case.

@patmauro
Copy link
Contributor Author

patmauro commented Nov 3, 2024

I did notice the credits LOC files seem significantly different between OU/MOULa and Huru/moul-assets; in the future it may be a good idea to figure out why these diverged and try to reconcile them - I see no reason why these should be so different.

We have updated the credits significantly to match the state of affairs on our end. This was done in #61.

That's not what alarms me - all the changes in ENG as described in #61 make sense, but what I'm really scratching my head at is how vastly different some of the international ones are - in the French one in particular, I'm seeing people removed from a longer list of credited people in the middle of the list, stuff like that. Just seems very odd. Anyway - not particularly related to this PR but seemed relevant to point out.

@patmauro
Copy link
Contributor Author

patmauro commented Nov 3, 2024

Added requested change. English language attribution now reads "'Big Blues' by Jason Shaw on Audionautix.com" which is in alignment with the creator's preferred attribution phrasing. I also simplified all international versions so they read "'Big Blues' - Jason Shaw, Audionautix.com" to avoid introducing an inaccurate translation.

As I said earlier I have tested this thoroughly on both clients and am confident the buffer length will not be an issue here - in fact, I believe this wasn't properly corrected when the file was initially changed from BYUBYD to the classical piece, which is significantly longer; as the new file is actually more similar in length to BYUBYD this should be less of an issue, if anything.

@Hoikas
Copy link
Member

Hoikas commented Nov 3, 2024

I would feel much more comfortable if the value in the PRP is correct. It seems like we're setting ourselves up for an unexpected problem down the line if we accept something that is incorrect and so easily fixable. If you're not sure what the value should be, I can make that change for you.

@patmauro
Copy link
Contributor Author

patmauro commented Nov 4, 2024

we're setting ourselves up for an unexpected problem down the line if we accept something that is incorrect and so easily fixable.

The thing is, this is already the case - Note this screenshot, which compares the sound buffer information in the original ABM release with the latest version of this same file in H'uru/moul-assets:
image

You'll note these are identical, despite the original being 02:49 with a bitrate of 705kbps, and the original replacement (the classical one) being 05:51 with a bitrate of 768kbps (this newest one, by the way, clocks in at 2:07 with a bitrate of 705kbps, so we're much closer to the original file in several respects). If we've decided we want to take this seriously now, that's great - I agree it's better to be safe than sorry. But it does seem worth noting, to be totally fair, this has already been wrong for the last 10+ years and there haven't been any issues.

That being said - you're right; I'm not sure what the exact conversion on this would be. Based on what I pulled from the historical PRP that 14990478 value must correspond to the original file's 02:49, but that doesn't look like milliseconds or something to me; nor does it correspond with the total number of samples either (which appears to have been 7495239, for the record). I can make the change, but what does that value correspond to, exactly?

@dpogue
Copy link
Member

dpogue commented Nov 4, 2024

Based on what I pulled from the historical PRP that 14990478 value must correspond to the original file's 02:49, but that doesn't look like milliseconds or something to me; nor does it correspond with the total number of samples either (which appears to have been 7495239, for the record). I can make the change, but what does that value correspond to, exactly?

Looks like number of samples * 2 channels for stereo

@Hoikas
Copy link
Member

Hoikas commented Nov 4, 2024

It's the number of samples minus 1 times the block align value. The block align value is the bitrate times the number of channels right shifted by 3. This is where Korman calculates the value from an ogg file: https://github.com/H-uru/korman/blob/4a0bbf3bfdbd3a5a330b44193fb938a5b1348885/korlib/sound.cpp#L77-L103

@patmauro
Copy link
Contributor Author

patmauro commented Nov 4, 2024

Awesome - that's super helpful. Worth noting this is a mono (single channel) file, but that being said it doesn't seem like we need to change the BlockAlign value at all here. So - in that case, as the new file is 5644502 samples long, I'm guessing the correct value here should be 11289004.

I'll go ahead and edit the PRC, but I'll test this in-game first before I push it to the PR - seems like it's worth confirming this works and doesn't accidentally introduce any issues.

@Hoikas
Copy link
Member

Hoikas commented Nov 4, 2024

Yeah, it doesn't look like the block align will need to change.

@patmauro
Copy link
Contributor Author

patmauro commented Nov 4, 2024

Seems good. Buffer modification has been pushed to the PR.

@Hoikas Hoikas removed the awaiting author This pull request or issue is blocked on a response or revision from the author. label Nov 4, 2024
@Hoikas
Copy link
Member

Hoikas commented Nov 5, 2024

what I'm really scratching my head at is how vastly different some of the international ones are - in the French one in particular, I'm seeing people removed from a longer list of credited people in the middle of the list, stuff like that.

I investigated this, and it appears this was done on purpose. From what I understand, the English credits were redone between TPotS and MOUL, but the translations were incompletely updated at best due to MOUL only supporting English. The French credits were revised to be more in line with the English credits in #180.

@Hoikas
Copy link
Member

Hoikas commented Nov 7, 2024

It looks like Korman thinks the length field should be 11289002 (instead of 11289004).

@patmauro
Copy link
Contributor Author

patmauro commented Nov 7, 2024

It looks like Korman thinks the length field should be 11289002 (instead of 11289004).

hmm... that doesn't seem like it lines up with the original values in this case, which seem to be 7495239 x 2 = 14990478 (rather than 14990476) - but we can give it a shot if you're absolutely sure. (disregard this, see new comment below)

@patmauro
Copy link
Contributor Author

patmauro commented Nov 7, 2024

It looks like Korman thinks the length field should be 11289002 (instead of 11289004).

hmm... that doesn't seem like it lines up with the original values in this case, which seem to be 7495239 x 2 = 14990478 (rather than 14990476) - but we can give it a shot if you're absolutely sure.

Ah - okay! I think I solved the mystery here. The confusion stems from the fact that the original OGG file and the WAV in StreamingCache end up rendering out a little differently. I was judging this calc based on an archived copy of the old WAV, but not a copy of the old OGG. Turns out the original OGG actually shows as 7495240 samples - it seems in the conversion to WAV there is a one-sample difference. Likewise, I compared the OGG and WAV versions of the new one and sure enough, they show as 5644502 samples and 5644501 samples respectively. So that appears to account for the difference here, and you're totally right - it should be 11289002 and not 11289004. I'll go ahead and make that change right away.

@Hoikas Hoikas merged commit 82ba900 into H-uru:master Nov 7, 2024
1 check passed
@patmauro patmauro deleted the zandiRadioUpdate branch November 7, 2024 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subjective An issue or fix that is artistic or creative in nature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants