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

FAAD 2.8.8 crashes with some MP4 files (e.g. Youtube) where 2.7 did not #13

Closed
lordmulder opened this issue Jun 2, 2018 · 5 comments
Closed

Comments

@lordmulder
Copy link
Contributor

Hello.

It was brought to my attention that latest FAAD v2.8.8. (built latest sources straight from Git master) crashes with certain MP4 files, e.g. MP4 files downloaded from YouTube.

FAAD v2.7 works flawlessly on these exact files!

I tracked down the problem to a "division by zero" error at this line:

image

Sample file (zipped) to reproduce the issue is here:
https://files.fm/u/bf4ugc6k

Regards,
MuldeR

@lordmulder
Copy link
Contributor Author

lordmulder commented Jun 3, 2018

Update:

Problem is caused by the following line returning NULL and memset'ing the frameInfo to all zero:

sample_buffer = NeAACDecDecode(hDecoder, &frameInfo, mp4config.bitbuf.data, mp4config.bitbuf.size);

We get division by zero, because frameInfo.channels is zero, but frameInfo.samples is zero too.

But I guess the "real" problem here is that the returned sample_buffer ends up being NULL.

The first couple of calls to NeAACDecDecode() do not return NULL though!

I tried to insert a continue statement in case that sample_buffer is NULL, but it results in infinite loop.

(So, once NULL was returned, all subsequent calls return NULL as well)

@fabiangreffrath
Copy link
Collaborator

I guess the decoder should simply abort if there is an AAC frame that it cannot parse.

I tried to insert a continue statement in case that sample_buffer is NULL, but it results in infinite loop.

What if you add break there?

@lordmulder
Copy link
Contributor Author

lordmulder commented Jun 4, 2018

If I add a break, then it will stop decoding very quickly and leave an (almost) empty WAV file.

My guess: The new MP4 parsing code introduced in FAAD v2.8 does not support MP4 files containing a video stream – or, more generally, MP4 files with more than a single stream – yet.

If we have more than one stream, the "MDAT" atom of the MP4 file will contain chunks of the various streams, in an interleaved fashion. For this reason, it is necessary to parse the chunk offsets and lengths from the tables in the "MOOV" atom, and then jump to the chunks belonging to the desired stream.

Simply assuming that the "MDAT" atom contains a sequence of AAC frames (all belonging to the same stream) is not going work for files with more than one stream! ...and thus for most MP4 files out there.

I hope this can be fixed, because otherwise we will be stuck with FAAD 2.7 😟

@lordmulder
Copy link
Contributor Author

Just wondering, any chance for a fix? ❓

@lordmulder
Copy link
Contributor Author

Bump ✨

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