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 615740 - aacparse: Add conversion support from/to ADIF/ADTS/RAW/LOAS stream-formats
aacparse: Add conversion support from/to ADIF/ADTS/RAW/LOAS stream-formats
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 701588 770699 783060 (view as bug list)
Depends on:
Blocks: 615681
 
 
Reported: 2010-04-14 12:50 UTC by Danilo Freire
Modified: 2018-11-03 14:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
LATM AAC sample (115.31 KB, audio/mp4)
2010-04-14 13:07 UTC, Danilo Freire
  Details
Proposed patch (26.18 KB, patch)
2010-05-10 17:25 UTC, André Dieb Martins
needs-work Details | Review
Conversion from ADTS to raw - a first step (6.20 KB, patch)
2013-07-15 16:33 UTC, Vincent Penquerc'h
accepted-commit_now Details | Review
Fix apparent off by one in ADTS object type parsing (1.11 KB, patch)
2013-07-15 16:35 UTC, Vincent Penquerc'h
committed Details | Review
ADTS->raw conversion (8.06 KB, patch)
2013-07-25 19:20 UTC, Vincent Penquerc'h
committed Details | Review
index from AAC sample rate (3.80 KB, patch)
2013-07-25 19:21 UTC, Vincent Penquerc'h
committed Details | Review
Raw AAC -> ADTS conversion (8.21 KB, patch)
2013-08-12 11:29 UTC, Chris Bass
needs-work Details | Review
Raw AAC -> ADTS conversion (8.29 KB, patch)
2013-08-13 13:19 UTC, Chris Bass
committed Details | Review

Description Danilo Freire 2010-04-14 12:50:45 UTC
The FAAD plugin do not searches for LATM/LOAS format in the AAC stream. 

The chain function searches for the sync word (line 1061), but only for the ADTS syncword (gst_faad_sync method).

My suggestion is:

Adding a new parameter in the src caps of the plugin:

transport{adts,latm}

1) if the transport parameted is set, the plugin must looking for the correct syncword (latm or adts syncword).

2) if the transport parameter is not set, the plugin look for:
a) First the adts syncword for 17000 Bytes (thats a guess :)
b) If no ADTS syncword was found, looking for LATM syncword.
*) This approach is suggested in cases of continuous streams, such one that come from a live source, like a dvbsrc->mpegdemux for example.
Comment 1 Danilo Freire 2010-04-14 13:07:43 UTC
Created attachment 158717 [details]
LATM AAC sample
Comment 2 Danilo Freire 2010-04-14 13:09:25 UTC
A little help, the VLC player can play the attached sample.
Comment 3 Danilo Freire 2010-05-03 13:12:23 UTC
Hi,

I'm working with André Dieb in the fix of this element. When we have a stable version we let you know.
Comment 4 André Dieb Martins 2010-05-10 17:25:49 UTC
Created attachment 160744 [details] [review]
Proposed patch

This patch adds LOAS/LATM support into gstfaad.
Comment 5 Andrzej K. Haczewski 2010-07-08 21:50:13 UTC
Hi guys,

I've also been working on AAC code for GStreamer lately, and it involves LOAS. I've even posted to a mailing list a patch to extend aac typefind for LOAS streams. I would love to coordinate with you.

My task at hand is a little different: I have LOAS stream, written by commercial encoder, that I have to mux to FLV using gstreamer with video that comes from another gstreamer pipeline. The hard part is demuxing LATM in LOAS into raw AAC packets on one hand and reading and storing in "codec_data" field in caps AudioSpecificConfig that comes with each AAC stream.

The first task was to recognize LOAS stream as it comes from FIFO through simple filesrc. That's what my typefind patch is doing.
Next is to frame the LOAS packets for further processing, that's what I suppose should be done by aacparse plugin which I'm currently working on. But I see that in your patch you're doing exacly the same thing: parsing LOAS and getting LATM out of it, along with AudioSpecificConfig.

How about getting the parsing part out so both aacparse and gstfaad could use that without code duplication? Both plugins are in gst-plugins-bad module so it might be a little easier, but I'm relatively new to GStreamer development process, so I'd welcome suggestions from more experienced developers.
Comment 6 André Dieb Martins 2010-07-13 23:58:50 UTC
(In reply to comment #5)
> Hi guys,
> 
> I've also been working on AAC code for GStreamer lately, and it involves LOAS.
> I've even posted to a mailing list a patch to extend aac typefind for LOAS
> streams. I would love to coordinate with you.
> 
> My task at hand is a little different: I have LOAS stream, written by
> commercial encoder, that I have to mux to FLV using gstreamer with video that
> comes from another gstreamer pipeline. The hard part is demuxing LATM in LOAS
> into raw AAC packets on one hand and reading and storing in "codec_data" field
> in caps AudioSpecificConfig that comes with each AAC stream.
> 
> The first task was to recognize LOAS stream as it comes from FIFO through
> simple filesrc. That's what my typefind patch is doing.
> Next is to frame the LOAS packets for further processing, that's what I suppose
> should be done by aacparse plugin which I'm currently working on. But I see
> that in your patch you're doing exacly the same thing: parsing LOAS and getting
> LATM out of it, along with AudioSpecificConfig.
> 
> How about getting the parsing part out so both aacparse and gstfaad could use
> that without code duplication? Both plugins are in gst-plugins-bad module so it
> might be a little easier, but I'm relatively new to GStreamer development
> process, so I'd welcome suggestions from more experienced developers.

Sorry, I might have misunderstood: getting the LAOS/LATM parsing out of gstfaad into aacparse?

Then one would use gstfaad only for decoding what faad supports (ADTS, raw). In case of different AAC profiles, user would use aacparse+faad?

I'll gladly test any patch and help you if it is decided to go on with this.
Comment 7 Andrzej K. Haczewski 2010-07-14 09:16:10 UTC
(In reply to comment #6)
> Sorry, I might have misunderstood: getting the LAOS/LATM parsing out of gstfaad
> into aacparse?
>
> Then one would use gstfaad only for decoding what faad supports (ADTS, raw). In
> case of different AAC profiles, user would use aacparse+faad?
> 
> I'll gladly test any patch and help you if it is decided to go on with this.

Yes, more or less that's what I'd like to have in the end: parsing of different AAC streams, so that other elements could use that, not only faad decoder.

I'm working now on finalizing the aacparse patch so it gets LOAS/LATM right, but I'm not sure how it should handle multiple streams in LATM. If the output from aacparse is to be LOAS/LATM then there's no problem, but for raw AAC output that is needed by FLV should I create more src pads?

Also, if an element like flvmux requires raw AAC stream should it be able to negotiate it with aacparse? That's also a case for faad, which might want to get raw AAC data from LATM: should it negotiate or should it be a property in aacparse ("raw-output" in example)?
Comment 8 Danilo Freire 2010-07-14 12:01:16 UTC
The aacparse caps has as its output caps the following format:
stream-format: { raw, adts, adif }

ADTS and ADIF are mux/container formats such as LATM/LOAS.

Instead of make a new property, I think we should use:
1) Adding a new output format in aacparse
stream-format: { raw, adts, adif, latm }

Then Creating a separate parse for LATM/LOAS with as input format
stream-format: { latm }
and as output:
stream-format: { raw }

That way we do not need to implement something to extract raw aac from ADTS/ADIF (which most of aac decoders already make).


I think that with this approach we can handle most of cases. For example, if I implement an aac decoder that handles LATM/LOAS format, I can use it directly with aacparse (as I prefer to use my decoder LATM/LOAS parse instead of gstreamer one).

If not, I can use the loasparse with my decoder (which handles raw data).

[]s
Comment 9 Andrzej K. Haczewski 2010-07-14 15:00:53 UTC
Adding another parser element to handle conversion from LATM/LOAS to raw aac makes sense. It might actually be a demuxer (latmdemux?), since each AAC stream muxed in LATM have separate AudioSpecificConfig so it could provide pad for each stream separately, so each stream needs to expose separate codec_data (== AudioSpecificConfig) in pad caps for raw AAC stream.
Comment 10 André Dieb Martins 2010-11-24 17:34:37 UTC
(In reply to comment #9)
> Adding another parser element to handle conversion from LATM/LOAS to raw aac
> makes sense. It might actually be a demuxer (latmdemux?), since each AAC stream
> muxed in LATM have separate AudioSpecificConfig so it could provide pad for
> each stream separately, so each stream needs to expose separate codec_data (==
> AudioSpecificConfig) in pad caps for raw AAC stream.

Why can't faad expose a pad for each stream separately? I'm not sure there's a need a new element.

Do you have updates on your end?
Comment 11 Sebastian Dröge (slomo) 2011-05-20 08:02:18 UTC
I think the best would be to add conversion support between ADTS and LATM/LOAS and any other AAC formats to aacparse and let faad require ADTS input
Comment 12 Rafael Diniz 2011-05-23 16:01:36 UTC
Looking at other projects, mplayer managed to add LATM/LOAS support for faad2 also. Make faad2 support LATM/LOAS will be cleaner then add a conversion of formats in my opinion.
The stream-format for LATM/LOAS was agreed to be "loas".
Comment 13 Sebastian Dröge (slomo) 2011-05-24 07:43:23 UTC
When added to aacparse the advantage would be, that all AAC decoders could support ADIF/ADTS/LOAS/RAW without any changes and that you could transcode from one container format to another without reencoding the AAC stream.

aacparse is autoplugged by decodebin2 before faad and other AAC decoders anyway
Comment 14 Sebastian Dröge (slomo) 2011-05-24 07:43:55 UTC
FWIW, the same is done for h264. h264parse can convert between all the different stream formats and decoders only need to support at least one.
Comment 15 Tim-Philipp Müller 2011-05-24 09:43:57 UTC
The aacparse route seems preferable to me as well, since we eventually have to implement conversion there anyway, and having it in there means it immediately works with a whole bunch of other decoders as well (e.g. proprietary ones or dsp codecs and somesuch).
Comment 16 Rafael Diniz 2011-05-24 12:36:20 UTC
So the idea would be to adapt this code to aacparse?
Comment 17 Sebastian Dröge (slomo) 2011-05-24 12:52:58 UTC
Yes, and ideally aacparse would support conversion from/to ADIF/ADTS/RAW/LOAS and select the appropiate format by negotiating with downstream.
Comment 18 Wim Taymans 2013-06-04 12:41:06 UTC
*** Bug 701588 has been marked as a duplicate of this bug. ***
Comment 19 Edward Hervey 2013-06-24 16:55:46 UTC
fwiw, vlc also converts from LOAS/LATM to "raw" AAC
Comment 20 Vincent Penquerc'h 2013-07-15 16:33:40 UTC
Created attachment 249223 [details] [review]
Conversion from ADTS to raw - a first step

A patch to convert from ADTS to raw when downstream doesn't like ADTS but likes raw (such as qtmux).
It doesn't do any other conversion (because I didn't need any other).
Comment 21 Vincent Penquerc'h 2013-07-15 16:35:07 UTC
Created attachment 249224 [details] [review]
Fix apparent off by one in ADTS object type parsing

Accessory to the previous patch. This is from seeing http://wiki.multimedia.cx/index.php?title=ADTS (and confirming ffmpeg also does +1).
Comment 22 Sebastian Dröge (slomo) 2013-07-16 07:47:49 UTC
Review of attachment 249223 [details] [review]:

::: gst/audioparsers/gstaacparse.c
@@ +112,3 @@
 
+static inline gint
+gst_aac_parse_get_index_from_sample_rate (guint rate)

Maybe this should be in codec-utils too?
Comment 23 Tim-Philipp Müller 2013-07-16 09:20:44 UTC
> +static inline gint
> +gst_aac_parse_get_index_from_sample_rate (guint rate)
> 
> Maybe this should be in codec-utils too?


Hrm, higher-level functions would be better (as in: make me an aac codec data from this and that) IMHO
Comment 24 Sebastian Dröge (slomo) 2013-07-23 13:01:33 UTC
Vincent, please push
Comment 25 Vincent Penquerc'h 2013-07-25 19:19:55 UTC
Thanks. I modified the patch to move the sample rate index mapping to pbutils (I'm not sure how the other one came up to be diuplicated, the code's the same...), and added a check that the rate is in the list. I'll push that new one soon if no more comments. I've not made a higher level "make me a codec data" because the actual codec data it way more complicated, this two byte one is (thankfully) a corner case, but the spec for is is scary.
Comment 26 Vincent Penquerc'h 2013-07-25 19:20:45 UTC
Created attachment 250135 [details] [review]
ADTS->raw conversion
Comment 27 Vincent Penquerc'h 2013-07-25 19:21:14 UTC
Created attachment 250136 [details] [review]
index from AAC sample rate
Comment 28 Vincent Penquerc'h 2013-07-26 08:48:47 UTC
Pushed, consider the bug 5% fixed :)
Comment 29 Chris Bass 2013-08-12 11:29:04 UTC
Created attachment 251341 [details] [review]
Raw AAC -> ADTS conversion

A patch to convert from raw AAC to ADTS.
Comment 30 Sebastian Dröge (slomo) 2013-08-13 09:01:00 UTC
Review of attachment 251341 [details] [review]:

Looks almost good, just some nitpicks :)

::: gst/audioparsers/gstaacparse.c
@@ +967,3 @@
+  srccaps = gst_pad_get_current_caps (GST_BASE_PARSE_SRC_PAD (aacparse));
+  srcstruct = gst_caps_get_structure (srccaps, 0);
+  profile = gst_structure_get_value (srcstruct, "profile");

gst_structure_get_string(), and if it's not successful or NULL return

@@ +1079,3 @@
+  gsize frame_size;
+  guint8 id, profile, channel_configuration, sampling_frequency_index;
+  const gsize adts_headers_length = 7UL;        /* Total byte-length of fixed

Maybe make this a #define
Comment 31 Chris Bass 2013-08-13 13:19:52 UTC
Created attachment 251486 [details] [review]
Raw AAC -> ADTS conversion

Thanks. Patch modified as per your comments.
Comment 32 Sebastian Dröge (slomo) 2013-08-13 13:59:24 UTC
commit b40bf67526189db009bf768c0541aefeb0d697ca
Author: Chris Bass <floobleflam@gmail.com>
Date:   Tue Aug 13 14:09:20 2013 +0100

    aacparse: allow conversion from raw AAC to ADTS
    
    This patch will prepend ADTS headers to raw AAC audio frames, allowing
    upstream elements to link to decoders that only support AAC in ADTS format.
    
    Note that no error correction bits are added to ADTS frames in this code.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=615740
Comment 33 Javier Jardón (IRC: jjardon) 2013-10-17 19:04:53 UTC
Hi! So what is remaining here? 

LOAS/LATM to raw?

Seems that bug #678078 would be a duplicate of this then?
Comment 34 Sebastian Dröge (slomo) 2013-10-17 19:56:14 UTC
Conversion from/to LOAS/LATM and ADIF is missing. Only raw<->ADTS is implemented right now

bug #678078 is related but IMHO not 100% the same, independent of aacparse decodebin should not fail like that in any case.
Comment 35 Sebastian Dröge (slomo) 2016-09-01 11:01:36 UTC
*** Bug 770699 has been marked as a duplicate of this bug. ***
Comment 36 Tim-Philipp Müller 2017-05-24 21:16:50 UTC
*** Bug 783060 has been marked as a duplicate of this bug. ***
Comment 37 GStreamer system administrator 2018-11-03 14:41:32 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/25.