voamrwbenc: Fix truncation of audio data at end-of-stream when audio data
authorDevin Anderson <danderson@microsoft.com>
Thu, 15 Sep 2022 00:52:14 +0000 (00:52 +0000)
committerDevin Anderson <danderson@microsoft.com>
Fri, 16 Sep 2022 00:14:58 +0000 (00:14 +0000)
doesn't align on 20 millisecond frame size.

The AMR-WB codec imposes a fixed 20 millisecond frame size.  In its current
form, the `voamrwbenc` plugin deals with this limitation by discarding any
audio at the end of the stream that falls short of 20 milliseconds.  This patch
keeps the audio data, and appends silence to the end to preserve frame size
alignment.

The patch also adds tests to check for the updated behavior.  I noticed that
tests weren't being built, so I changed the build to allow for building the
tests when the `tests` and `voamrwbenc` options are set.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3027>

subprojects/gst-plugins-bad/ext/voamrwbenc/gstvoamrwbenc.c
subprojects/gst-plugins-bad/tests/check/elements/voamrwbenc.c
subprojects/gst-plugins-bad/tests/check/meson.build

index f48ac09..28ac85c 100644 (file)
@@ -40,6 +40,8 @@
 
 #include "gstvoamrwbenc.h"
 
+#include <string.h>
+
 #define MR660  0
 #define MR885  1
 #define MR1265 2
@@ -289,18 +291,22 @@ gst_voamrwbenc_handle_frame (GstAudioEncoder * benc, GstBuffer * buffer)
 
   gst_buffer_map (buffer, &map, GST_MAP_READ);
 
-  if (G_UNLIKELY (map.size < buffer_size)) {
-    GST_DEBUG_OBJECT (amrwbenc, "discarding trailing data %d", (gint) map.size);
-    gst_buffer_unmap (buffer, &map);
-    ret = gst_audio_encoder_finish_frame (benc, NULL, -1);
-    goto done;
-  }
-
   out = gst_buffer_new_and_alloc (buffer_size);
   gst_buffer_map (out, &omap, GST_MAP_WRITE);
+
   /* encode */
-  outsize = E_IF_encode (amrwbenc->handle, amrwbenc->bandmode,
-      (const short *) map.data, (unsigned char *) omap.data, 0);
+  if (G_UNLIKELY (map.size < buffer_size)) {
+    short input_buffer[L_FRAME16k] = { 0, };
+
+    GST_DEBUG_OBJECT (amrwbenc, "add silence to packet of size %d",
+        (gint) map.size);
+    memcpy ((void *) input_buffer, map.data, map.size);
+    outsize = E_IF_encode (amrwbenc->handle, amrwbenc->bandmode,
+        (const short *) input_buffer, (unsigned char *) omap.data, 0);
+  } else {
+    outsize = E_IF_encode (amrwbenc->handle, amrwbenc->bandmode,
+        (const short *) map.data, (unsigned char *) omap.data, 0);
+  }
 
   GST_LOG_OBJECT (amrwbenc, "encoded to %d bytes", outsize);
   gst_buffer_unmap (out, &omap);
index 1a8bf4e..e487a74 100644 (file)
@@ -20,8 +20,7 @@
  * Boston, MA 02110-1301, USA.
  */
 
-#include <unistd.h>
-
+#include <gst/audio/audio-format.h>
 #include <gst/check/gstcheck.h>
 
 /* For ease of programming we use globals to keep refs for our floating
  * get_peer, and then remove references in every test function */
 static GstPad *mysrcpad, *mysinkpad;
 
-#if G_BYTE_ORDER == G_BIG_ENDIAN
-#define AFORMAT "S16BE"
-#else
-#define AFORMAT "S16LE"
-#endif
-
 #define AUDIO_CAPS_STRING "audio/x-raw, " \
-                           "format = (string) " AFORMAT ", "\
-                           "layout = (string) interleaved, " \
-                           "rate = (int) 16000, " \
-                           "channels = (int) 1 "
+                          "format = (string) " GST_AUDIO_NE (S16) ", " \
+                          "layout = (string) interleaved, " \
+                          "rate = (int) 16000, " \
+                          "channels = (int) 1 "
 
 
 #define AMRWB_CAPS_STRING "audio/AMR-WB"
@@ -88,13 +81,14 @@ cleanup_voamrwbenc (GstElement * voamrwbenc)
 }
 
 static void
-do_test (void)
+do_test (gsize sample_count)
 {
   GstElement *voamrwbenc;
   GstBuffer *inbuffer, *outbuffer;
   GstCaps *caps;
   gint i, num_buffers;
-  const gint nbuffers = 10;
+  const gsize buffer_size = sample_count * 2;
+  const gsize nbuffers = (sample_count + 319) / 320;
 
   voamrwbenc = setup_voamrwbenc ();
   fail_unless (gst_element_set_state (voamrwbenc,
@@ -102,9 +96,9 @@ do_test (void)
       "could not set to playing");
 
   /* corresponds to audio buffer mentioned in the caps */
-  inbuffer = gst_buffer_new_and_alloc (320 * nbuffers * 2);
+  inbuffer = gst_buffer_new_allocate (NULL, buffer_size, NULL);
   /* makes valgrind's memcheck happier */
-  gst_buffer_memset (inbuffer, 0, 0, 1024 * nbuffers * 2 * 2);
+  gst_buffer_memset (inbuffer, 0, 0, buffer_size);
   caps = gst_caps_from_string (AUDIO_CAPS_STRING);
 
   gst_check_setup_events (mysrcpad, voamrwbenc, caps, GST_FORMAT_TIME);
@@ -156,13 +150,26 @@ do_test (void)
   buffers = NULL;
 }
 
-GST_START_TEST (test_enc)
+GST_START_TEST (test_enc_aligned)
 {
-  do_test ();
+  do_test (3200);
 }
 
 GST_END_TEST;
 
+GST_START_TEST (test_enc_minus_one)
+{
+  do_test (3199);
+}
+
+GST_END_TEST;
+
+GST_START_TEST (test_enc_plus_one)
+{
+  do_test (3201);
+}
+
+GST_END_TEST;
 
 static Suite *
 voamrwbenc_suite (void)
@@ -171,7 +178,9 @@ voamrwbenc_suite (void)
   TCase *tc_chain = tcase_create ("general");
 
   suite_add_tcase (s, tc_chain);
-  tcase_add_test (tc_chain, test_enc);
+  tcase_add_test (tc_chain, test_enc_aligned);
+  tcase_add_test (tc_chain, test_enc_minus_one);
+  tcase_add_test (tc_chain, test_enc_plus_one);
 
   return s;
 }
index b817b45..a85d70e 100644 (file)
@@ -69,6 +69,7 @@ base_tests = [
   [['elements/switchbin.c'], get_option('switchbin').disabled()],
   [['elements/videoframe-audiolevel.c'], get_option('videoframe_audiolevel').disabled()],
   [['elements/viewfinderbin.c']],
+  [['elements/voamrwbenc.c'], not voamrwbenc_dep.found(), [voamrwbenc_dep]],
   [['elements/vp9parse.c'], false, [gstcodecparsers_dep]],
   [['elements/av1parse.c'], false, [gstcodecparsers_dep]],
   [['elements/wasapi.c'], host_machine.system() != 'windows', ],