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.

Sign up for GitHub

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

Jump to bottom

Follow-up: Verify properties of audio signals #985

Merged
merged 5 commits into from
Jul 28, 2016
Merged

Follow-up: Verify properties of audio signals #985

merged 5 commits into from
Jul 28, 2016

Conversation

uklotzde
Copy link
Contributor

Verify various properties before reading from audio sources:

  • to detect and report malformed audio files early
  • to avoid crashes when trying to decode inconsistent audio data

@uklotzde
Limit valid range for number of channels in an audio signal
bb1503b
@uklotzde
Rename member functions of audio signals
d25eeb1
@uklotzde
Log invalid properties of audio signals as warnings
3f7906d 
The warnings in the log should help to identify corrupt or invalid
files.

Example:
Warning [CachingReaderWorker 1]: Invalid sampling rate [Hz]: 1000 is out of range [ 8000 , 192000 ]
@uklotzde
Verify various properties before reading from audio sources
6c2d18f
@daschuer
Copy link
Member

Thank you very much, for this one again.
It "looks" good, however there is an issue with a taglib testing file.

Debug [Main]: BaseTrackPlayerImpl::slotLoadTrack
Debug [Main]: WOverview(0x3afd1b60) WOverview::slotLoadingTrack true false
Debug [CachingReaderWorker 1]: SoundSourceProvider "Nero FAAD2" created a SoundSource for file "file:///home/daniel/Downloads/taglib-1.10/tests/data/ilst-is-last.m4a" of type "m4a"
ReadAtom: "/home/daniel/Downloads/taglib-1.10/tests/data/ilst-is-last.m4a": invalid atom size, extends outside parent atom - skipping to end of "" "free" 49658 vs 32768
Debug [CachingReaderWorker 1]: Opened AudioSource for file "file:///home/daniel/Downloads/taglib-1.10/tests/data/ilst-is-last.m4a" with provider "Nero FAAD2"

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffb7fff700 (LWP 3768)]
0xffffffffffffffe8 in ?? ()
(gdb) bt
#0 0xffffffffffffffe8 in ?? ()
#1 0x0000000000b1265e in SoundSourceProxy::openAudioSource (
this=this@entry=0x7fffb7ffec20, audioSrcCfg=...)
at src/sources/soundsourceproxy.cpp:586
#2 0x000000000067c355 in openAudioSourceForReading (audioSrcCfg=...,
pTrack=...) at src/engine/cachingreaderworker.cpp:102
#3 CachingReaderWorker::loadTrack (this=this@entry=0x1eb2260, pTrack=...)
at src/engine/cachingreaderworker.cpp:134
#4 0x000000000067d606 in CachingReaderWorker::run (this=0x1eb2260)
at src/engine/cachingreaderworker.cpp:85
#5 0x00007ffff4ec232f in QThreadPrivate::start (arg=0x1eb2260)
at thread/qthread_unix.cpp:349
#6 0x00007ffff2c8f184 in start_thread (arg=0x7fffb7fff700)
at pthread_create.c:312
#7 0x00007ffff0cf137d in clone ()
at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Zipped file:
ilst-is-last..zip

@uklotzde
Copy link
Contributor Author

Thanks again for your feedback, Daniel!

Maybe you need to re-compile the M4A plugin? One of the base classes has
been modified. The corrupt example file is detected and rejected here:

Debug [AnalyzerQueue 1]: SoundSourceProvider "Nero FAAD2" created a
SoundSource for file "file:///home/uk/Music/Corrupt
Tracks/list-is-last/ilst-is-last.m4a" of type "m4a"
Debug [Main]: WSpinny::slotCoverFound WSpinny(0x26757ce0, name =
"SpinnySingleton1") "CoverInfo(NONE,GUESSED,,0,/home/uk/Music/Corrupt
Tracks/list-is-last/ilst-is-last.m4a)" QSize(0, 0)
ReadAtom: "/home/uk/Music/Corrupt Tracks/list-is-last/ilst-is-last.m4a":
invalid atom size, extends outside parent atom - skipping to end of ""
"free" 49658 vs 32768
Debug [AnalyzerQueue 1]: Opened AudioSource for file
"file:///home/uk/Music/Corrupt Tracks/list-is-last/ilst-is-last.m4a" with
provider "Nero FAAD2"
Debug [AnalyzerQueue 1]: AnalysisDAO fetched 0 analyses, 0 bytes for track 1
in 0 ms
Debug [AnalyzerQueue 1]: Vampanalyzer BlockSize: 1024
Debug [AnalyzerQueue 1]: Vampanalyzer StepSize: 512
ReadBytes: read failed: errno: 0 (src/mp4file_io.cpp,94)
Warning [CachingReaderWorker 1]: Failed to read MP4 input data for sample
block 1 ( min = 1 , max = 2833 )
Warning [CachingReaderWorker 1]: Failed to read chunk samples: actual = 0 ,
expected = 8192
Debug [AnalyzerQueue 1]: Beat calculation started with plugin
"qm-tempotracker:0"
Debug [AnalyzerQueue 1]: Vampanalyzer BlockSize: 32768
Debug [AnalyzerQueue 1]: Vampanalyzer StepSize: 32768
Debug [AnalyzerQueue 1]: Key calculation started with plugin "qm-keydetector:2"
ReadBytes: read failed: errno: 2 (src/mp4file_io.cpp,94)
Warning [AnalyzerQueue 1]: Failed to read MP4 input data for sample block 1
( min = 1 , max = 2833 )
Warning [AnalyzerQueue 1]: Failed to read sample data from file:
"/home/uk/Music/Corrupt Tracks/list-is-last/ilst-is-last.m4a" @ 0
Debug [AnalyzerQueue 1]: Waveform generation for track 1 done 0 s
Debug [AnalyzerQueue 1]: Beat Calculation complete
Debug [AnalyzerQueue 1]: Could not detect beat positions from Vamp.
Debug [AnalyzerQueue 1]: Key Detection complete
Warning [AnalyzerQueue 1]: AnalyzerKey: Key sequence and list of times do
not match.

On 27.07.2016 21:10, Daniel Schürmann wrote:

Thank you very much, for this one again.
It "looks" good, however there is an issue with a taglib testing file.

Debug [Main]: BaseTrackPlayerImpl::slotLoadTrack
Debug [Main]: WOverview(0x3afd1b60) WOverview::slotLoadingTrack true false
Debug [CachingReaderWorker 1]: SoundSourceProvider "Nero FAAD2" created a
SoundSource for file
"file:///home/daniel/Downloads/taglib-1.10/tests/data/ilst-is-last.m4a" of
type "m4a"
ReadAtom:
"/home/daniel/Downloads/taglib-1.10/tests/data/ilst-is-last.m4a": invalid
atom size, extends outside parent atom - skipping to end of "" "free"
49658 vs 32768
Debug [CachingReaderWorker 1]: Opened AudioSource for file
"file:///home/daniel/Downloads/taglib-1.10/tests/data/ilst-is-last.m4a"
with provider "Nero FAAD2"

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffb7fff700 (LWP 3768)]
0xffffffffffffffe8 in ?? ()
(gdb) bt
#0 0xffffffffffffffe8 in ?? ()
#1 #1 0x0000000000b1265e in
SoundSourceProxy::openAudioSource (
this=this@entry=0x7fffb7ffec20, audioSrcCfg=...)
at src/sources/soundsourceproxy.cpp:586
#2 #2 0x000000000067c355 in
openAudioSourceForReading (audioSrcCfg=...,
pTrack=...) at src/engine/cachingreaderworker.cpp:102
#3 #3
CachingReaderWorker::loadTrack (this=this@entry=0x1eb2260, pTrack=...)
at src/engine/cachingreaderworker.cpp:134
#4 #4 0x000000000067d606 in
CachingReaderWorker::run (this=0x1eb2260)
at src/engine/cachingreaderworker.cpp:85
#5 #5 0x00007ffff4ec232f in
QThreadPrivate::start (arg=0x1eb2260)
at thread/qthread_unix.cpp:349
#6 #6 0x00007ffff2c8f184 in
start_thread (arg=0x7fffb7fff700)
at pthread_create.c:312
#7 #7 0x00007ffff0cf137d in clone ()
at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

Zipped file:
ilst-is-last..zip
https://github.com/mixxxdj/mixxx/files/386933/ilst-is-last.zip

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#985 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/ABZ-alvNqAvmTioJEekIubWIwZwcTU3yks5qZ606gaJpZM4JWch1.

@daschuer
Copy link
Member

Ah yes, after using the right plug-In the crash is gone. I should have been aware of this issue since I trapped into it earlier ;-)
So we need to increase the plug-In version?

@uklotzde
Copy link
Contributor Author

I would not increase the plugin version for internal refactorings during
development.

Once more this shows how the current plugin system is broken by revealing
too many Mixxx internals. As already mentioned by RJ it should be removed,
until a more robust design and technology is in place.

On 27.07.2016 22:37, Daniel Schürmann wrote:

Ah yes, after using the right plug-In the crash is gone. I should have
been aware of this issue since I trapped into it earlier ;-)
So we need to increase the plug-In version?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#985 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/ABZ-ak1aDYgL9bBDWDe1i0Uxq8n8RMfqks5qZ8FtgaJpZM4JWch1.

@daschuer
Copy link
Member

Since we have a significant number of alpha testers using master, we cannot merge this without a new version number.

What could be a better technology? A base class change in the Plug-In interface will always leads to incompatibility. Is there a way to check the API programmatic before using it and crashing?

@uklotzde
Copy link
Contributor Author

I've bumped the version number.

For the future we have to think about redefinining the API in terms of pure
interfaces and plain data types. Maybe a solution based on lightweight RPC
technologies like protobuf or gRPC that would allow aus to implement reading
of audio data and reading/writing of metadata in separate worker processes
that are isolated from Mixxx.

On 28.07.2016 00:29, Daniel Schürmann wrote:

Since we have a significant number of alpha testers using master, we
cannot merge this without a new version number.

What could be a better technology? A base class change in the Plug-In
interface will always leads to incompatibility. Is there a way to check
the API programmatic before using it and crashing?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#985 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/ABZ-ascL2BpH5j-DyahnncxQtqyLxvIqks5qZ9vNgaJpZM4JWch1.

@daschuer
Copy link
Member

Thanks! LGTM

@daschuer daschuer merged commit a597fd7 into mixxxdj:master Jul 28, 2016
@uklotzde uklotzde deleted the ValidateAudioSignals branch August 16, 2016 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@uklotzde @daschuer