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 789810 - qtdemux: Add support for AC4
qtdemux: Add support for AC4
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.x
Other Windows
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-11-02 10:28 UTC by Savinderjit Kaur
Modified: 2018-11-03 15:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for generating proper caps for AC4 content (555 bytes, patch)
2017-11-02 10:28 UTC, Savinderjit Kaur
needs-work Details | Review
Updated patch with GST_MAKE_FOURCC (577 bytes, patch)
2017-11-03 06:48 UTC, Savinderjit Kaur
none Details | Review
Updated the AC4 patch to git format (774 bytes, patch)
2017-11-09 09:30 UTC, Savinderjit Kaur
reviewed Details | Review

Description Savinderjit Kaur 2017-11-02 10:28:35 UTC
Created attachment 362804 [details] [review]
Patch for generating proper caps for AC4 content

The caps received from qtdemux for AC-4 content are audio/x-gst-fourcc-ac_4.
The attached patch has been created for generating proper caps (audio/x-ac4). Please let me know if this is valid for submission.
Comment 1 Sebastian Dröge (slomo) 2017-11-02 15:33:19 UTC
Review of attachment 362804 [details] [review]:

::: gst1-plugins-good-1.12.0.orig/gst/isomp4/qtdemux.c
@@ +14287,3 @@
       break;
     }
+    case 0X342d6361:

Please use GST_MAKE_FOURCC() like everywhere else here
Comment 2 Savinderjit Kaur 2017-11-03 06:48:54 UTC
Created attachment 362869 [details] [review]
Updated patch with GST_MAKE_FOURCC
Comment 3 Savinderjit Kaur 2017-11-03 06:50:25 UTC
Hi Sebastian,
I have updated the patch. Please review it & let me know if there are any additional comments.
Comment 4 Savinderjit Kaur 2017-11-09 09:30:22 UTC
Created attachment 363281 [details] [review]
Updated the AC4 patch to git format
Comment 5 Nicolas Dufresne (ndufresne) 2017-12-06 20:57:25 UTC
That patch looks correct, though this also looks incomplete if I compare to the code required to support AC3. Is there a decoder that exist somewhere that could be used in GStreamer ? Would you be able to share a sample ? Have you tested muxing this format ?
Comment 6 Jun Xie 2017-12-07 01:50:47 UTC
any open sourced AC4 codec? 
AFAIK, AC4 is quite different from traditional audio codec
Comment 7 Nicolas Dufresne (ndufresne) 2017-12-07 02:01:20 UTC
Out of curiosity, how far did you get with just this patch ? Did you have a proprietary plugin of yours to test with ? Or did you just wanted to map it to some sensible caps ?
Comment 8 Savinderjit Kaur 2017-12-07 17:30:29 UTC
In my implementation this patch was used just for sending proper caps. Rest of the processing was done in a proprietary element & a hardware decoder was used for consumption of the data.
Comment 9 Nicolas Dufresne (ndufresne) 2017-12-08 00:59:40 UTC
Ok, so maybe that is sufficient for demuxing. Did you attempt to encode and mux or transmux ? I see that for AC3 there is lot more code, that's why I'm asking all these questions. I'm trying to see if I need to create a new issue for AC4 muxing for someone to pick it up later.
Comment 10 Savinderjit Kaur 2017-12-08 01:21:43 UTC
No I didn't try encoding, muxing or transmuxing. qtdemux was able to demux the AC-4 audio from video. A little processing was needed for AC-4 header in proprietary element & then hardware decoder was able to understand this AC-4 data & audio was heard properly.
Comment 11 Nicolas Dufresne (ndufresne) 2017-12-08 01:51:00 UTC
Ok, that little processing is rather important. It is parsing from QTFF/ISOMP4 bits there ? If so, this patch is not ideal.
Comment 12 Savinderjit Kaur 2017-12-08 02:14:11 UTC
Ac-4 header bits were added for every data chunk received from qtdemux in proprietary element. That was requirement for our hardware decoder requirement.
Comment 13 Nicolas Dufresne (ndufresne) 2017-12-08 03:04:45 UTC
Yeah, but that information is probably located in some QTFF boxes / atoms that are not parsed yet. I also wonder if AC-4 inside ISOMP4 is documented somewhere ... (my copy of the spec is old).
Comment 14 Savinderjit Kaur 2017-12-12 01:50:29 UTC
I checked the MP4 files I had in hex editor, there were no atoms having any information regarding headers. 
A spec is available here http://www.etsi.org/deliver/etsi_ts/103100_103199/103190/01.01.01_60/ts_103190v010101p.pdf
Comment 15 Jun Xie 2017-12-12 01:55:00 UTC
(In reply to Savinderjit Kaur from comment #12)
> Ac-4 header bits were added for every data chunk received from qtdemux in
> proprietary element. That was requirement for our hardware decoder
> requirement.

it is also possible that ac-4 header bits has been already contained in es stream.
Comment 16 Jun Xie 2017-12-12 01:56:30 UTC
(In reply to Savinderjit Kaur from comment #14)
> I checked the MP4 files I had in hex editor, there were no atoms having any
> information regarding headers. 
> A spec is available here
> http://www.etsi.org/deliver/etsi_ts/103100_103199/103190/01.01.01_60/
> ts_103190v010101p.pdf


Annex E (normative): 
AC-4 Bitstream Storage in the ISO Base Media File Format 

this give some hints
Comment 17 Savinderjit Kaur 2017-12-12 02:03:07 UTC
(In reply to Jun Xie from comment #16)
> (In reply to Savinderjit Kaur from comment #14)
> > I checked the MP4 files I had in hex editor, there were no atoms having any
> > information regarding headers. 
> > A spec is available here
> > http://www.etsi.org/deliver/etsi_ts/103100_103199/103190/01.01.01_60/
> > ts_103190v010101p.pdf
> 
> 
> Annex E (normative): 
> AC-4 Bitstream Storage in the ISO Base Media File Format 
> 
> this give some hints

Yes, but I wasn't able to relate it to the content I had. Maybe someone can take a look at this if qtdemux has any way of handling this.
Comment 18 Nicolas Dufresne (ndufresne) 2017-12-12 03:56:43 UTC
Thanks for linking the spec. It not clear what the ac4 specific box is needed for, it's stated that it's not needed for decoding, there is ac4_toc for that. As it's embedded into the Sample box, qtdemux should be stripping it off completely.

The down side is that we cannot easily mix or transmux, as the data is lost, but decoders likely succeed. Is there any samples that would demonstrate a valid stream ?
Comment 19 Savinderjit Kaur 2017-12-13 01:50:59 UTC
(In reply to Nicolas Dufresne (stormer) from comment #18)
> Thanks for linking the spec. It not clear what the ac4 specific box is
> needed for, it's stated that it's not needed for decoding, there is ac4_toc
> for that. As it's embedded into the Sample box, qtdemux should be stripping
> it off completely.
> 
> The down side is that we cannot easily mix or transmux, as the data is lost,
> but decoders likely succeed. Is there any samples that would demonstrate a
> valid stream ?

I'm afraid I can't share the streams I have. Some of the test content is available at the dolby site using a valid login.
Comment 20 GStreamer system administrator 2018-11-03 15:23:24 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/413.