After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 736986 - qtdemux: handle AAC audio without ESDS atom
qtdemux: handle AAC audio without ESDS atom
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-19 17:21 UTC by Matej Knopp
Modified: 2014-09-22 16:10 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Patch (2.04 KB, patch)
2014-09-19 17:21 UTC, Matej Knopp 		none Details | Review
Sample file (127.92 KB, video/quicktime)
2014-09-19 17:22 UTC, Matej Knopp 		  Details
Patch (2.04 KB, patch)
2014-09-19 17:23 UTC, Matej Knopp 		none Details | Review
Patch (2.04 KB, patch)
2014-09-19 17:30 UTC, Matej Knopp 		committed Details | Review

Description Matej Knopp 2014-09-19 17:21:26 UTC 
Created attachment 286653 [details] [review]
Patch

It looks like there are files with AAC audio that are missing the ESDS atom. They play fine with QuickTime and other players. Attached patch creates codec data from mp4a atom in case ESDS is not present. 

Right now the patch assumes AAC LC, I'm not aware of any way to determine the profile properly.
Comment 1 Matej Knopp 2014-09-19 17:22:12 UTC 
Created attachment 286654 [details]
Sample file
Comment 2 Matej Knopp 2014-09-19 17:23:57 UTC 
Created attachment 286655 [details] [review]
Patch

(corrected atom name in comment)
Comment 3 Matej Knopp 2014-09-19 17:30:40 UTC 
Created attachment 286656 [details] [review]
Patch

Corrected variable declaration in block body
Comment 4 Thiago Sousa Santos 2014-09-22 15:24:45 UTC 
Review of attachment 286656 [details] [review]:

::: gst/isomp4/qtdemux.c
@@ +8700,3 @@
+          gint len = QT_UINT32 (stsd_data);
+
+          if (len > 50) {

Shouldn't it be >= ?
Comment 5 Matej Knopp 2014-09-22 15:45:30 UTC 
It probably should. Although it doesn't make much different since there are other things in the atom after time scale that we don't use. Should I submit another patch?
Comment 6 Thiago Sousa Santos 2014-09-22 16:08:37 UTC 
Did the minor update myself and pushed.

commit 8a4931726db431f19c903b22f3bdd81a9d87bf10
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Fri Sep 19 19:14:28 2014 +0200

    qtdemux: Handle mp4a without ESDS atom
    
    https://bugzilla.gnome.org/show_bug.cgi?id=736986
Comment 7 Matej Knopp 2014-09-22 16:10:59 UTC 
Thanks.