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

Chunk with odd-sized length before the data chunk causes the data chunk to never be read #101

Open
mskurnik opened this issue Apr 20, 2018 · 8 comments

Comments

@mskurnik
Copy link
Contributor

mskurnik commented Apr 20, 2018

All forks of TagLib# do not handle reading a chunk when the position value is odd. This happens especially if bext (Broadcast Wave Format) chunks are present.

Having a chunk with an odd size before the data chunk forces TagLib# to never read the data chunk. Therefore the duration is never calculated as well as some other members are not populated. In the case of the audio file that I am trying to read, it has the following chunks:

  • bext
  • fmt
  • ID3
  • iXML
  • axml
  • data

My axml chunk's size is an odd value of 495 bytes. In the TagLib.Riff.Read method, When the while loop goes to read the next chunk (which happens to be the data chunk), it reads the fourcc chunk header name as \0DAT. TagLib# only (and correctly) accepts data but it is never found in the switch as there needs to be a one byte offset.

This can easily be fixed with a single line in the TagLib.Riff class. In the Read method where the do/while loop is, the code below should be added.

// Check if the current position is an odd number and increment it so it is even
// This is done when the previous chunk size was an odd number.
// If this is not done, the chunk being read after the odd chunk will not be read.
if (position > 12 && (position & 1) != 0) { position++; }

I am assuming this can also happen in other formats. The above code should likely be added at least to all other RIFF based formats such as AIFF and AVI in the same location.

Note: @jamsoft in his fork, which he mentions in issue #74, addresses the Broadcast Wave Format but does not take into consideration odd sized chunks (nor does he handle null XML elements but that is a different matter). Otherwise his fork is great as far as I can tell and also cleans up a lot of code style issues with this master repo.

@mskurnik
Copy link
Contributor Author

It looks like this is also related to #76 (WAV File Chunk Handling ?PMX). I am sorry I did not find this earlier before making this new issue otherwise I would have posted to it.

@mskurnik
Copy link
Contributor Author

I was able to discover how to fix this with some code I found during a research phase on my project before decided to use TagLib. I found this over a year ago on CodeProject (this code is almost 13 years old). In the RiffParser.ReadElement(...) method it has a conditional check to provided a padded size to offset the word position.

The RiffParser code actually implements a few additional checks that TagLib# is missing.

@jamsoft
Copy link

jamsoft commented Apr 21, 2018

Yes, when I picked up and created my fork the WAV implementation is thoroughly broken.

The odd chunk size issue has never been implemented meaning that reading and writing WAV files doesn't work reliably. I have been working on this but time has been limited.

There is a similar issue inside the process of writing LIST chunks as well.

@mskurnik
Copy link
Contributor Author

mskurnik commented May 7, 2018

Yeah. The checking of the chunk position (odd/even) in the OP should work for both. I know for a fact that it fixes the reading of odd-sized chunks but since I am not doing any modifications to chunks I don't know about writing.

@jamsoft
Copy link

jamsoft commented Jun 22, 2018

So has anything been done with this issue?

@mskurnik
Copy link
Contributor Author

As far as I can tell, nothing has unless it was fixed in another branch.

@mskurnik
Copy link
Contributor Author

@decriptor, I just did a PR that address this "oddly" fun little bug. #115

@mskurnik
Copy link
Contributor Author

@jamsoft can you provide a piece of audio that will trip up TagLib# in its current state? I am trying to find a way to do a unit test for PR #115 but all of my audio have copyrights on them.

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

No branches or pull requests

2 participants