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

Fixes for WavReader to explictly handle "LIST" and "cue " chunks (RA2) #15007

Merged
merged 1 commit into from Apr 11, 2018

Conversation

Projects
None yet
4 participants
@drogoganor
Copy link
Contributor

drogoganor commented Mar 30, 2018

Apologies for the noise in your queue - I closed this issue prematurely and would like to reopen it: #14986
I've found a better way to handle the junk data in the DR wavs.

When an unknown .wav data chunk type is encountered, it's handled in the current WavReader class by just reading the next int then reading that many bytes.

switch (blockType)
{
...
default:
    var unknownChunkSize = s.ReadInt32();
    s.ReadBytes(unknownChunkSize);
    break;
}

This causes a problem in my Dark Reign mod where there is junk data at the end of the wav file data (Usually with the chunk type "????" or sometimes combined with unicode chars). The chunk size value read here is huge and crashes OpenRA.

D2k uses wav files, and so does RA2, so I tested these out by playing a few matches. D2k wav files never hit the default block above. RA2 wav files hit this block for "LIST" and "cue " chunk types.

In order to handle the junk data in the DR wavs, I've altered the default statement to skip to the end of the stream, and handled the RA2 chunks explictly:

switch (blockType)
{
...
case "LIST":
case "cue ":
	var listCueChunkSize = s.ReadInt32();
	s.ReadBytes(listCueChunkSize);
	break;
default:
	s.Position = s.Length;  // Skip to end of stream
	break;
}

In addition, for one .wav file, the s.ReadByte() alignment reaches the end of the file, so we have to drop out early here:

if ((s.Position & 1) == 1)
	s.ReadByte(); // Alignment

if (s.Position == s.Length)
	break;	// Break if we aligned with end of stream

This will let me use the DR wavs without having to make a copy of WavLoader and WavReader in my mod with this fix applied.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Apr 7, 2018

@abcdefg30 can you please verify that the ra2 wavs continue to work with this PR?

@pchote
Copy link
Member

pchote left a comment

The code changes here look reasonable, but I would like to hear from @RoosterDragon and @abcdefg30 before merging.

@abcdefg30
Copy link
Member

abcdefg30 left a comment

The RA2 mod still appears to work fine (with this PR and a revert of #14043).

@@ -46,6 +46,9 @@ public static bool LoadSound(Stream s, out Func<Stream> result, out short channe
if ((s.Position & 1) == 1)
s.ReadByte(); // Alignment

if (s.Position == s.Length)
break; // Break if we aligned with end of stream

This comment has been minimized.

@abcdefg30

abcdefg30 Apr 8, 2018

Member

Please use a single space instead of a tab after the semicolon.

default:
var unknownChunkSize = s.ReadInt32();
s.ReadBytes(unknownChunkSize);
s.Position = s.Length; // Skip to end of stream

This comment has been minimized.

@abcdefg30

abcdefg30 Apr 8, 2018

Member

Two spaces instead of one after the semicolon.

@drogoganor drogoganor force-pushed the drogoganor:wavreader-fixes branch from cd83d13 to b1c6346 Apr 9, 2018

@RoosterDragon RoosterDragon merged commit 1b68595 into OpenRA:bleed Apr 11, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@RoosterDragon

This comment has been minimized.

Copy link
Member

RoosterDragon commented Apr 11, 2018

@drogoganor

This comment has been minimized.

Copy link
Contributor Author

drogoganor commented Apr 11, 2018

Thank you

@drogoganor drogoganor deleted the drogoganor:wavreader-fixes branch Oct 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.