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 650695 - Patch adds LATM/LOAS support to codecmap and make distinction between ADTS and LATM/LOAS
Patch adds LATM/LOAS support to codecmap and make distinction between ADTS an...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other All
: Normal normal
: 0.10.12
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-05-20 21:18 UTC by Rafael Diniz
Modified: 2011-05-26 13:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
makes gst-ffmpeg be aware of LATM/LOAS AAC syntax (1.00 KB, patch)
2011-05-20 21:18 UTC, Rafael Diniz
needs-work Details | Review
makes gst-ffmpeg aware of LATM/LOAS AAC syntax (1.67 KB, patch)
2011-05-23 15:45 UTC, Rafael Diniz
none Details | Review
fixed latm/loas decoding and got rid of compatibilities problems in old style aac decoding (2.19 KB, patch)
2011-05-24 20:49 UTC, Rafael Diniz
committed Details | Review

Description Rafael Diniz 2011-05-20 21:18:47 UTC
Created attachment 188252 [details] [review]
makes gst-ffmpeg be aware of LATM/LOAS AAC syntax

This patch makes gst-ffmpeg be aware of the LATM/LOAS AAC format.
It's part of a work to make gstreamer play ISDB-Tb DTV transport streams, and complements the patch already applied in gst-plugins-bad: https://bugzilla.gnome.org/show_bug.cgi?id=615681
Comment 1 Sebastian Dröge (slomo) 2011-05-23 08:46:14 UTC
Review of attachment 188252 [details] [review]:

I think this breaks backwards compatibility (you can't just pass input with caps without the stream-format field anymore) but it looks more correct. There's also an inverse mapping in gstffmpegcodecmap.c (just search for CODEC_ID_AAC), please add that too :)

::: ext/ffmpeg/gstffmpegcodecmap.c
@@ +960,3 @@
     case CODEC_ID_AAC:
       caps = gst_ff_aud_caps_new (context, codec_id, "audio/mpeg",
+          "mpegversion", G_TYPE_INT, 4, "stream-format", G_TYPE_STRING, "adts", 

Doesn't this also support adif and raw AAC?

@@ +963,3 @@
+	  NULL);
+      break;
+    case CODEC_ID_AAC_LATM: // LATM/LOAS AAC syntax

Don't use C++/C99 comments
Comment 2 Rafael Diniz 2011-05-23 14:42:29 UTC
You're right.
How can I make an "or" with ( adts || adif || raw ) in ext/ffmpeg/gstffmpegcodecmap.c? Any other information I could use to do that?
Or just place CODEC_ID_AAC_LATM before CODEC_ID_AAC, and leave CODEC_ID_AAC without any stream-format?

I think I should also add in the patch this:


diff --git a/ext/ffmpeg/gstffmpegdec.c b/ext/ffmpeg/gstffmpegdec.c
index 305178c..3fba4ba 100644
--- a/ext/ffmpeg/gstffmpegdec.c
+++ b/ext/ffmpeg/gstffmpegdec.c
@@ -828,7 +828,7 @@ gst_ffmpegdec_setcaps (GstPad * pad, GstCaps * caps)
   }
 
   /* for AAC we only use av_parse if not on raw caps */
-  if (oclass->in_plugin->id == CODEC_ID_AAC) {
+  if (oclass->in_plugin->id == CODEC_ID_AAC || oclass->in_plugin->id == CODEC_ID_AAC_LATM) {
     const gchar *format = gst_structure_get_string (structure, "stream-format");
 
     if (format == NULL || strcmp (format, "raw") == 0) {
Comment 3 Sebastian Dröge (slomo) 2011-05-23 15:16:01 UTC
(In reply to comment #2)
> You're right.
> How can I make an "or" with ( adts || adif || raw ) in
> ext/ffmpeg/gstffmpegcodecmap.c? Any other information I could use to do that?
> Or just place CODEC_ID_AAC_LATM before CODEC_ID_AAC, and leave CODEC_ID_AAC
> without any stream-format?

Just add more structures to the caps, one with adts, one with adif and one with raw.

> I think I should also add in the patch this:

Makes sense
Comment 4 Rafael Diniz 2011-05-23 15:45:58 UTC
Created attachment 188390 [details] [review]
makes gst-ffmpeg aware of LATM/LOAS AAC syntax
Comment 5 Rafael Diniz 2011-05-23 15:46:40 UTC
I really don't know if this is the correct way to solve the problem. New patch attached.
Comment 6 Rafael Diniz 2011-05-24 20:49:33 UTC
Created attachment 188525 [details] [review]
fixed latm/loas decoding and got rid of compatibilities problems in old style aac decoding
Comment 7 Rafael Diniz 2011-05-24 20:54:38 UTC
People, and think the patch I posted does not cause trouble to anyone and allows the playback of any transport stream (or anything with LATM/LOAS AAC) with a custon gst-launch pipeline (auto-plugging tries to use aacparse which knows nothing about LATM/LOAS).
Comment 8 Sebastian Dröge (slomo) 2011-05-25 08:11:33 UTC
Thanks, I've changed it to include the stream-format for CODEC_ID_AAC too though

commit df40381a8362e1f1fa376bde010c02883904518e
Author: Rafael Diniz <rafael@riseup.net>
Date:   Wed May 25 10:08:06 2011 +0200

    ffmpeg: Add codec mapping for AAC LATM/LOAS
    
    Also add the stream-format fields to the CODEC_ID_AAC caps.
    
    Fixes bug #650695.
Comment 9 Rafael Diniz 2011-05-26 13:04:14 UTC
Nice to see how you deal with the stream-format slomo!
I made a small mistake, could you revert this part of the patch (after testing the code with TSs from different encoders, I realized it makes audio playback choppy, with lots of errors):


diff --git a/ext/ffmpeg/gstffmpegdec.c b/ext/ffmpeg/gstffmpegdec.c
index a377a50..5fb52fb 100644
--- a/ext/ffmpeg/gstffmpegdec.c
+++ b/ext/ffmpeg/gstffmpegdec.c
@@ -832,9 +832,8 @@ gst_ffmpegdec_setcaps (GstPad * pad, GstCaps * caps)
       || oclass->in_plugin->id == CODEC_ID_AAC_LATM) {
     const gchar *format = gst_structure_get_string (structure, "stream-format");
 
-    if (format == NULL || strcmp (format, "raw") == 0 ||
-        oclass->in_plugin->id == CODEC_ID_AAC_LATM) {
-      ffmpegdec->turnoff_parser = TRUE;
+    if (format == NULL || strcmp (format, "raw") == 0) {
+      ffmpegdec->turnoff_parser = TRUE; 
     }
   }