search

complexlogic / rsgain

A simple, but powerful ReplayGain 2.0 tagging utility
Other
289 stars 21 forks source link

opus files with more than 2 channels don't work and are broken when processed #118

Closed diomhaireachd closed 5 months ago

diomhaireachd commented 5 months ago

I've tested 3-6 channels opus files and they all break when processed. command: rsgain custom -s i 30secsilence6channel.opus all opus modes break the file.

output:

[✔] Scanning '30secsilence6channel.opus'
[✘] Could not open input: End of file

binary diff between original and broken

-00000010: 5401 0000 0000 6e10 cf6a 011b 4f70 7573  T.....n..j..Opus
+00000010: 5401 0000 0000 957a b365 011b 4f70 7573  T......z.e..Opus

I eventually figured out that the error is caused by the zeroing of the header output (scan.cpp line 214) and that its specifically something to do with the crc as commenting out line 750 in tag.cpp stops the damage occurring.

The attached files are as named the 30seconds of silence file with 6 channels made in Audacity and the file after being broken. testfiles.zip

complexlogic commented 5 months ago

I can confirm this on my end. I'll investigate a solution.

complexlogic commented 5 months ago

I have discovered the root cause of this bug. The current code does not take into account the Opus "channel mapping table". Per RFC 7845:

All fields in the ID headers are REQUIRED, except for 'channel mapping table', which MUST be omitted when the channel mapping family is 0, but is REQUIRED otherwise.

So, the channel mapping table is not present in files that are of "channel mapping family 0", i.e. mono or sterero files. But it is present in files with 3 or more audio channels. Failing to take the existence of the channel mapping table into account results in the size of the Opus header packet being incorrect, which, in turn, leads to an incorrect CRC32 calculation. Since the CRC32 is invalid, most players will reject the file.

I did not test the program with multichannel Opus files, which is why the bug has not been discovered until now. I'm going to push a fix soon. The new code will verify the length of the Opus header packet in a more robust manner, so the function will work for mono, stereo, and multichannel files alike.

The good news is that any files damaged by this bug can be repaired simply by scanning them again with the fix incorporated. So, there should not be any permanent data loss due to this bug.

diomhaireachd commented 5 months ago

Excellent, tested and it does everything you said. Especially nice that you can fix the damage. Thanks for this and for the work you put into rsgain.