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

Handle incorrect channel mask in WAV files #180

Merged
merged 3 commits into from Feb 2, 2023
Merged

Handle incorrect channel mask in WAV files #180

merged 3 commits into from Feb 2, 2023

Conversation

jesnor
Copy link
Contributor

@jesnor jesnor commented Jan 27, 2023

This PR contains a work around for some WAV files that have an incorrect channel mask value. For example VLC can still play them.

@pdeljanov
Copy link
Owner

Hi @jesnor,

I've considered this workaround before since I've seen a single file with this issue. I didn't implement it because it didn't seem common and also because the channel mapping will be very wrong if > 2 channels.

Do you think we should limit this workaround if only n_channels is 1 or 2 (if 0, the workaround shouldn't be applied)? The mapping wouldn't be wrong in that case. Also, we'll definitely need a warning log if the workaround is applied.

Do you also happen to know if the WAV files were produced by a particular app or encoder?

Thanks!

@jesnor
Copy link
Contributor Author

jesnor commented Jan 29, 2023

The file that failed for me had 9 channels, created with Audacity I think. Interestingly a similar file with 8 channels had correct channel mask.

Since VLC can play the file, I think the patch is reasonable. It's better to be able to decode something rather than nothing, although the channel mask might be weird.

@pdeljanov
Copy link
Owner

Found this documentation from Microsoft. Looks like 0 just means the channels are not assigned a "position", or are ambiguous and the decoder can device.

We don't have a way to express un-positioned channels with Symphonia's current API, so your solution is valid for all channels then. Only other alternative would be to assume certain channel layouts for each number (i.e., 2.1 for 3, etc.).

Just going to leave a couple small comments and then I'll merge the change once fixed.

let mut channel_mask = reader.read_u32()?;

// Graceful handling of "empty" channel mask value. Use the first n_channels channels.
if channel_mask == 0 {
Copy link
Owner

Choose a reason for hiding this comment

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

If n_channels is also 0 then this should be executed because of the next comment.

symphonia-format-wav/src/chunks.rs Outdated Show resolved Hide resolved
@pdeljanov pdeljanov merged commit ad4ba8f into pdeljanov:master Feb 2, 2023
@pdeljanov
Copy link
Owner

Thanks! This is great!

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.

None yet

2 participants