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 647748 - [aacenc] add AAC audio encoder based on vo-aacenc lib
[aacenc] add AAC audio encoder based on vo-aacenc lib
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
0.10.21
Other Linux
: Normal enhancement
: 0.10.23
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-04-14 08:29 UTC by Benjamin Gaignard
Modified: 2011-05-14 10:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
New aacenc plugin (21.70 KB, patch)
2011-04-14 08:31 UTC, Benjamin Gaignard
none Details | Review
unit test for aacenc (7.82 KB, patch)
2011-04-14 08:32 UTC, Benjamin Gaignard
none Details | Review
detect output format from downstream caps (5.81 KB, patch)
2011-04-14 11:16 UTC, Benjamin Gaignard
none Details | Review
all previous patches squashed in one (30.98 KB, patch)
2011-04-15 06:48 UTC, Benjamin Gaignard
none Details | Review
new aacenc plugin (29.65 KB, patch)
2011-04-15 09:27 UTC, Benjamin Gaignard
needs-work Details | Review
add new voaacenc plugin (35.62 KB, patch)
2011-04-19 07:22 UTC, Benjamin Gaignard
committed Details | Review

Description Benjamin Gaignard 2011-04-14 08:29:18 UTC
New AAC audio encoder based on vo-aacenc library:
http://sourceforge.net/projects/opencore-amr/files/vo-aacenc
Comment 1 Benjamin Gaignard 2011-04-14 08:31:51 UTC
Created attachment 185936 [details] [review]
New aacenc plugin

developped by Kan Hu for Linaro
Comment 2 Benjamin Gaignard 2011-04-14 08:32:50 UTC
Created attachment 185937 [details] [review]
unit test for aacenc

unit test for aacenc (similar to faac unit test)
Comment 3 Sebastian Dröge (slomo) 2011-04-14 08:35:16 UTC
Instead of having a property for the output format this can be detected from the downstream caps. This will make negotiation work better
Comment 4 Benjamin Gaignard 2011-04-14 11:16:36 UTC
Created attachment 185945 [details] [review]
detect output format from downstream caps

this patch makes aacenc detect output format (adts or raw) from downstream caps
outputformat property is now a read only property.

test examples:
gst-launch audiotestsrc num-buffers=3 ! aacenc ! "audio/mpeg, stream-format=raw" ! fakesink

gst-launch audiotestsrc num-buffers=3 ! aacenc ! "audio/mpeg, stream-format=adts" ! fakesink

unit test has modified to test that feature.
Comment 5 Sebastian Dröge (slomo) 2011-04-14 11:20:50 UTC
IMHO there's no need for this property at all and please squash together the commits
Comment 6 Benjamin Gaignard 2011-04-15 06:48:43 UTC
Created attachment 185998 [details] [review]
all previous patches squashed in one
Comment 7 Sebastian Dröge (slomo) 2011-04-15 08:31:21 UTC
there should also be only a single pad template for ADTS and raw AAC and only a single srcpad
Comment 8 Sebastian Dröge (slomo) 2011-04-15 08:31:47 UTC
Nevermind, I was looking at the unit test. Sorry
Comment 9 Benjamin Gaignard 2011-04-15 09:27:20 UTC
Created attachment 186008 [details] [review]
new aacenc plugin

after talking with slomo: completely remove outputformat property from element
Comment 10 Sebastian Dröge (slomo) 2011-04-15 13:20:46 UTC
Review of attachment 186008 [details] [review]:

* I would call it voaacenc instead of just aacenc
* Not sure if this is a problem but the Apache License is IIRC not GPL/LGPL compatible until GPL/LGPL 3

::: ext/aacenc/gstaacenc.c
@@ +30,3 @@
+ * ]|
+ * Please note that the above stream misses the header, that is needed to play
+ * the stream.

Which header? In theory it should be possible to play that file after passing it through aacparse

@@ +64,3 @@
+        "endianness = (int) BYTE_ORDER, "
+        "rate = (int) {8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000, 88200, 96000}, "
+        "channels = (int) [1, 6]")

You need a getcaps function that returns the exact channel layouts that are required for >2 channels (same goes for faac too I guess). See vorbisenc for an example

@@ +72,3 @@
+    GST_STATIC_CAPS ("audio/mpeg, " "mpegversion = (int) 4, "
+        "rate = (int) {8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000, 64000, 88200, 96000}, "
+        "channels = (int) [1, 6], " "stream-format = (string) { adts, raw } ")

Does this library support output as mpeg2 stream too?

@@ +197,3 @@
+  aacenc->rate = AAC_ENC_DEFAULT_RATE;
+  aacenc->channels = AAC_ENC_DEFAULT_CHANNELS;
+  aacenc->output_format = AAC_ENC_DEFAULT_OUTPUTFORMAT;

Why? You'll always get the rate and channels and format from the caps, just keep them at 0 here and check in the chain function if you got rate/channel (and return NOT_NEGOTIATED otherwise, but you're doing that part already)

@@ +238,3 @@
+        aacenc->output_format = 0;
+      } else {
+        GST_DEBUG_OBJECT (aacenc, "unknown stream-format: %s", str);

Set an output format here if none is given

@@ +277,3 @@
+    gst_caps_unref (src_caps);
+    ret = aacenc_core_set_parameter (aacenc);
+  }

I guess if src_caps == NULL you want to return FALSE here?

@@ +303,3 @@
+
+  /* take latest timestamp, FIXME timestamp is the one of the
+   * first buffer in the adapter. */

Use gst_adapter_prev_timestamp() here and interpolate between these timestamps

@@ +322,3 @@
+
+    /* max size */
+    out = gst_buffer_new_and_alloc (aacenc->inbuf_size);

Use gst_pad_alloc_buffer()

@@ +329,3 @@
+    if (aacenc->ts != -1) {
+      aacenc->ts += aacenc->duration;
+    }

You will accumulate rounding errors here

@@ +337,3 @@
+    gst_buffer_set_caps (out, GST_PAD_CAPS (aacenc->srcpad));
+
+    data = gst_adapter_take (aacenc->adapter, aacenc->inbuf_size);

You could use gst_adapter_peek() here, this will prevent one additional malloc() per frame and will often prevent memcpy()

@@ +397,3 @@
+  switch (transition) {
+    case GST_STATE_CHANGE_READY_TO_NULL:
+      aacenc_core_uninit (aacenc);

clear the adapter here

::: ext/aacenc/gstaacenc.h
@@ +23,3 @@
+#include <gst/gst.h>
+#include "vo-aacenc/voAAC.h"
+#include "vo-aacenc/cmnMemory.h"

Use <> here, "" is usually used for including headers relative to the current directory
Comment 11 Benjamin Gaignard 2011-04-19 07:22:22 UTC
Created attachment 186252 [details] [review]
add new voaacenc plugin 

fix the remarks done by slomo: add channel layout, correctly use adapter, rework timestamping, use gst_pad_alloc_buffer, fix comments in header. unit test updated.
The library only support audio mpeg4 and not mpeg2
Comment 12 Sebastian Dröge (slomo) 2011-04-19 07:50:59 UTC
I'll push this with some minor changes after the release. Thanks! :)
Comment 13 Sebastian Dröge (slomo) 2011-05-14 10:16:17 UTC
commit 2c14a8fbcd5873495b61c4d69926fdc2bb3284b4
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Apr 19 10:32:50 2011 +0200

    aacenc: Integrate into the documentation


commit 31a65287a2a01b89809ced7f9e5a8aee057a9ed3
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Apr 19 09:52:23 2011 +0200

    voaaenc: Fix CFLAGS/LIBS of the unit test

commit dc7f93c05b168814d20380599a4cd7770533ebaf
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Apr 19 09:49:08 2011 +0200

    voaacenc: Only generate sinkcaps once and in a threadsafe way

commit adb3ac92375c62afec8afb64209ac47497be509a
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Apr 19 09:42:22 2011 +0200

    voaacenc: Add NULL terminator to gst_structure_new()

commit 41bb35f38cbcb74c63dd9d845a1a2bfabc8c77e3
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Apr 19 09:40:48 2011 +0200

    voaacenc: Some minor cleanup


commit 55d8c526401f590321b6a2fa5c26298e05eaac2b
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Tue Apr 19 09:34:03 2011 +0200

    voaacenc: Fix CFLAGS and LIBS

commit 988516ca638c2be8aac866c278ecb1b05e46cf36
Author: benjamin gaignard <benjamin.gaignard@linaro.org>
Date:   Mon Apr 18 17:19:00 2011 +0200

    voaacenc: Add new plugin for audio AAC encoder based on vo-aacenc lib
    
    Add plugin and unit test.
    
    Fixes bug #647748.