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

Introduce loader for Meg V3 file format #18443

Merged
merged 1 commit into from
Dec 15, 2020

Conversation

ocdi
Copy link
Contributor

@ocdi ocdi commented Jul 26, 2020

This code will parse a Meg file used by the CnC remastered assets. I used this code to successfully extract a music file and was able to play it locally.

#18239

@ocdi
Copy link
Contributor Author

ocdi commented Jul 26, 2020

I got to the point where I was able to select this audio file from the list, however it didn't play because of unsupported audio format unfortunately. The compression type is ADPCM which I found some documentation here:

https://wiki.multimedia.cx/index.php/Microsoft_ADPCM

I see IMA_ADPCM is supported which seems simpler according to this

https://wiki.multimedia.cx/index.php/Microsoft_IMA_ADPCM

It looks like to finish this off, I'll have to add support to this audio format also.

@pchote
Copy link
Member

pchote commented Jul 27, 2020

I wouldn't rush on trying to get audio playback working in game: there are many pieces that need a lot more work before we will be able to ship that (reworking the filesystem/load code to support dynamic detection and use of the steam directory; reworking the music/notification systems to support split remaster vs classic definitions; adding the user-facing UI for selecting between the two). This PR should really focus on just the meg container format, and leave the other pieces to other PRs.

@ocdi
Copy link
Contributor Author

ocdi commented Jul 27, 2020

@pchote thanks for reviewing while this was in draft. I mainly created it as such because I wanted to indicate to anyone else that there was a WIP that could be useful. I started going down the road of actually getting the audio to work because I thought that I was very close and didn't expect the blocker of unsupported wav file format. My main aim was to actually have the MegV3Loader exercised so that I could prove the code works.

Instead I have done what I should have and created two commands that can list and extract .meg files. I actually identified a bug with the SegmentStream where Read could read past the end of the of the indicated stream. In my extractor utility I was wondering why the .wav files were massive, presumably it was reading until the end of the underlying streams.

In practice I am unsure if that will affect anything, or break anything because it was incorrectly able to read past the end.

@ocdi ocdi marked this pull request as ready for review July 27, 2020 22:25
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

I haven't had a chance to test this yet, but here are some more code style points to hopefully restart progress here.

.gitignore Outdated Show resolved Hide resolved
OpenRA.Game/Primitives/SegmentStream.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/UtilityCommands/ListMegContentsCommand.cs Outdated Show resolved Hide resolved
@pchote
Copy link
Member

pchote commented Aug 16, 2020

Confirmed that filesystem consumers can read the contents of mounted meg files without problem. I'll need to do another pass for coding style before approving, and the fixup commits will need to be squashed.

@ocdi
Copy link
Contributor Author

ocdi commented Aug 18, 2020

I rebased on bleed and squashed

Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, you might have noticed that I got a bit sidetracked by what this PR enabled 🙂

A bunch more comments to bring this more in line with OpenRA's coding conventions, and a couple of specific simplifications:

OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
OpenRA.Game/Primitives/SegmentStream.cs Outdated Show resolved Hide resolved
OpenRA.Game/Primitives/SegmentStream.cs Outdated Show resolved Hide resolved
@ocdi ocdi force-pushed the feature/meg-format branch 2 times, most recently from 4bd7371 to 0405187 Compare August 26, 2020 09:23
@ocdi
Copy link
Contributor Author

ocdi commented Aug 26, 2020

Thanks @pchote for your feedback. When I originally authored this we weren't targeting C# 7 so tuple's weren't an option; I am glad it is now! 👍 It is great to do the same stuff with less code.

Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

A few final style nits, then LGTM.

These should bring us in line with the "proper" Megafile specs revealed in the new source release: https://github.com/electronicarts/CnC_Remastered_Collection/tree/master/CnCTDRAMapEditor/Utility

OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/FileSystem/MegFile.cs Outdated Show resolved Hide resolved
@ocdi
Copy link
Contributor Author

ocdi commented Sep 12, 2020

I've updated the PR based on latest feedback and rebased on bleed.

@ocdi
Copy link
Contributor Author

ocdi commented Nov 9, 2020

didn't notice the rebase label, have done so

pchote
pchote previously approved these changes Dec 14, 2020
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

LGTM after those last two nits.

Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

Confirmed that this is still working fine on my updated remaster branch.

@abcdefg30 abcdefg30 merged commit a85da9d into OpenRA:bleed Dec 15, 2020
@abcdefg30
Copy link
Member

Changelog

@ocdi ocdi deleted the feature/meg-format branch December 15, 2020 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants