Created attachment 185936 [details] [review]
New aacenc plugin

developped by Kan Hu for Linaro

Created attachment 185937 [details] [review]
unit test for aacenc

unit test for aacenc (similar to faac unit test)

Instead of having a property for the output format this can be detected from the downstream caps. This will make negotiation work better

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.

IMHO there's no need for this property at all and please squash together the commits

Created attachment 185998 [details] [review]
all previous patches squashed in one

there should also be only a single pad template for ADTS and raw AAC and only a single srcpad

Nevermind, I was looking at the unit test. Sorry

Created attachment 186008 [details] [review]
new aacenc plugin

after talking with slomo: completely remove outputformat property from element

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

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

I'll push this with some minor changes after the release. Thanks! :)

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.