Crash (including fix) in CoreAudioLayouts::getCoreAudioLayoutChannels() when loading .mp4 or mov without audio

MacOSX and iOS
1

JUCE v5.2.1, macOS 10.13.3

reproduction (code):

File file("no-audio.mov");
AudioFormatManager audioFormatManager;
audioFormatManager.registerBasicFormats();
ScopedPointer<AudioFormatReader> reader = audioFormatManager.createReaderFor(file);

reproduction (data):

no-audio.mov (9.2 KB)

File needs to be renamed from .crash to .mov - I hope the file survives the upload. Otherwise just create a screen capture with no audio device selected via quicktime.

manifestation: Exception: EXC_BAD_ACCESS in CoreAudioLayouts::getCoreAudioLayoutChannels():

// layout.mNumberChannelDescriptions has some weird-ass value
for (int j = 0; channels.size() < static_cast<int> (layout.mNumberChannelDescriptions); ++j)
    channels.addIfNotAlreadyThere (static_cast<AudioChannelSet::ChannelType> (AudioChannelSet::discreteChannel0 + j));

cause: the CoreAudioReader constructor tries to get the channel layout here. with this particular file the return value of AudioFileGetPropertyInfo was noErr, but the out parameter sizeOfLayout is set to 0 (note: it’s set to 0 by AudioFileGetPropertyInfo, it’s not the default value!). so apparently a core audio file with a layout of zero size is a thing.

fix: change the lines following the call to AudioFileGetPropertyInfo() to:

status = AudioFileGetPropertyInfo (audioFileID, kAudioFilePropertyChannelLayout, &sizeOfLayout, &isWritable);
jassert(sizeOfLayout == sizeof(AudioChannelLayout) || sizeOfLayout == 0);

if (status == noErr && sizeOfLayout == sizeof(AudioChannelLayout))
{
    caLayout.malloc (1, static_cast<size_t> (sizeOfLayout));
    // …

@fabian : git-blame (or git-praise, depending on your view :wink: ) showed, that you wrote this code, so I’m pinging you here. Patch attached (832 Bytes)

@jules : can we enable .patch and .diff for uploading to the forum ?

2

Hi @pixlpacker, thanks for the details report. I’ll take care of this.

3

OK this is on develop now with commit 46a596. Btw, checking for sizeOfLayout == sizeof(AudioChannelLayout) is not correct here as AudioChannelLayout can have a variable length depending on the number of channels. I’ve now checked if it has the minimum necessary length.

4

Brilliant, thanks fabian! And kudos to you for double checking.