gst/audioconvert/audioconvert.c: Add docs to the integer pack functions and implement...

Original commit message from CVS:
* gst/audioconvert/audioconvert.c:
Add docs to the integer pack functions and implement proper
rounding. Before we had rounding towards negative infinity, i.e.
always the smaller number was taken. Now we use natural rounding,
i.e. rounding to the nearest integer and to the one with the largest
absolute value for X.5. The old rounding introduced some minor
distortions. Fixes #420079
* tests/check/elements/audioconvert.c: (GST_START_TEST):
Fix one unit test that assumed the old rounding and added unit tests
for checking signed/unsigned int16 <-> signed/unsigned int16 with
depth 8, one for signed int16 <-> unsigned int16 and one for the new
rounding from signed int32 to signed/unsigned int16.

diff --git a/ChangeLog b/ChangeLog
index 722b4eec00f0e5bbc4c7fbd60dd1f245de093d98..6a05e08d0dadd73b90852b2c98990693e4f49b83 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2007-03-27  Sebastian Dröge  <slomo@circular-chaos.org>
+
+       * gst/audioconvert/audioconvert.c:
+       Add docs to the integer pack functions and implement proper
+       rounding. Before we had rounding towards negative infinity, i.e.
+       always the smaller number was taken. Now we use natural rounding,
+       i.e. rounding to the nearest integer and to the one with the largest
+       absolute value for X.5. The old rounding introduced some minor
+       distortions. Fixes #420079
+       * tests/check/elements/audioconvert.c: (GST_START_TEST):
+       Fix one unit test that assumed the old rounding and added unit tests
+       for checking signed/unsigned int16 <-> signed/unsigned int16 with
+       depth 8, one for signed int16 <-> unsigned int16 and one for the new
+       rounding from signed int32 to signed/unsigned int16.
+
 2007-03-27  Michael Smith  <msmith@fluendo.com>
 
        * gst/audioconvert/gstaudioconvert.c: (strip_width_64),

diff --git a/gst/audioconvert/audioconvert.c b/gst/audioconvert/audioconvert.c
index cfdfda9bb0a26dc3a30495abb973afa3244cac02..cadb82773e5cd72014c45d013ce0eb611714567c 100644 (file)
--- a/gst/audioconvert/audioconvert.c
+++ b/gst/audioconvert/audioconvert.c
@@ -121,17 +121,56 @@ MAKE_UNPACK_FUNC (s32_be, 4, 0, READ32_FROM_BE);
 #define MAKE_PACK_FUNC_NAME(name)                                       \
 audio_convert_pack_##name
 
+/*
+ * These functions convert the signed 32 bit integers to the
+ * target format. For this to work the following steps are done:
+ *
+ * 1) If the output format is smaller than 32 bit we add 0.5LSB of
+ *    the target format (i.e. 1<<(scale-1)) to get proper rounding.
+ *    Shifting will result in rounding towards negative infinity (for
+ *    signed values) or zero (for unsigned values). As we might overflow
+ *    an overflow check is performed.
+ *    Additionally, if our target format is signed and the value is smaller
+ *    than zero we decrease it by one to round -X.5 downwards.
+ *    This leads to the following rounding:
+ *    -1.2 => -1    1.2 => 1
+ *    -1.5 => -2    1.5 => 2
+ *    -1.7 => -2    1.7 => 2
+ * 2) If the output format is unsigned we will XOR the sign bit. This
+ *    will do the same as if we add 1<<31.
+ * 3) Afterwards we shift to the target depth. It's necessary to left-shift
+ *    on signed values here to get arithmetical shifting.
+ * 4) This is then written into our target array by the corresponding write
+ *    function for the target width.
+ */
+
 #define MAKE_PACK_FUNC(name, stride, sign, WRITE_FUNC)                  \
 static void                                                             \
 MAKE_PACK_FUNC_NAME (name) (gint32 *src, gpointer dst,                  \
         gint scale, gint count)                                         \
 {                                                                       \
   guint8 *p = (guint8 *)dst;                                            \
-  guint32 tmp;                                                          \
-  for (;count; count--) {                                               \
-    tmp = (*src++ ^ (sign)) >> scale;                                   \
-    WRITE_FUNC (p, tmp);                                                \
-    p+=stride;                                                          \
+  gint32 tmp;                                                           \
+  if (scale > 0) {                                                      \
+    guint32 bias = 1 << (scale - 1);                                    \
+    for (;count; count--) {                                             \
+      tmp = *src++;                                                     \
+      if (tmp > 0 && G_MAXINT32 - tmp < bias)                           \
+        tmp = G_MAXINT32;                                               \
+      else                                                              \
+        tmp += bias;                                                    \
+      if (sign == 0 && tmp < 0)                                         \
+        tmp--;                                                          \
+      tmp = ((tmp) ^ (sign)) >> scale;                                  \
+      WRITE_FUNC (p, tmp);                                              \
+      p+=stride;                                                        \
+    }                                                                   \
+  } else {                                                              \
+    for (;count; count--) {                                             \
+      tmp = (*src++ ^ (sign));                                          \
+      WRITE_FUNC (p, tmp);                                              \
+      p+=stride;                                                        \
+    }                                                                   \
   }                                                                     \
 }
 

diff --git a/tests/check/elements/audioconvert.c b/tests/check/elements/audioconvert.c
index 3517b825b96bbab3b11e4a6196fb9243e8b158d5..65ee7e00e3009b88ab38de7d572e96dc11bc3393 100644 (file)
--- a/tests/check/elements/audioconvert.c
+++ b/tests/check/elements/audioconvert.c
@@ -430,8 +430,8 @@ GST_START_TEST (test_int_conversion)
   }
   /* 16 -> 8 signed */
   {
-    gint16 in[] = { 0, 255, 256, 257 };
-    gint8 out[] = { 0, 0, 1, 1 };
+    gint16 in[] = { 0, 127, 128, 256, 256 + 127, 256 + 128 };
+    gint8 out[] = { 0, 0, 1, 1, 1, 2 };
 
     RUN_CONVERSION ("16 bit to 8 signed",
         in, get_int_caps (1, "BYTE_ORDER", 16, 16, TRUE),
@@ -471,6 +471,92 @@ GST_START_TEST (test_int_conversion)
             24, TRUE), in, get_int_caps (1, "BYTE_ORDER", 8, 8, TRUE)
         );
   }
+
+  /* 16 bit signed <-> unsigned */
+  {
+    gint16 in[] = { 0, 128, -128 };
+    guint16 out[] = { 32768, 32896, 32640 };
+    RUN_CONVERSION ("16 signed to 16 unsigned",
+        in, get_int_caps (1, "BYTE_ORDER", 16, 16, TRUE),
+        out, get_int_caps (1, "BYTE_ORDER", 16, 16, FALSE)
+        );
+    RUN_CONVERSION ("16 unsigned to 16 signed",
+        out, get_int_caps (1, "BYTE_ORDER", 16, 16, FALSE),
+        in, get_int_caps (1, "BYTE_ORDER", 16, 16, TRUE)
+        );
+  }
+
+  /* 16 bit signed <-> 8 in 16 bit signed */
+  {
+    gint16 in[] = { 0, 64 << 8, -64 << 8 };
+    gint16 out[] = { 0, 64, -64 };
+    RUN_CONVERSION ("16 signed to 8 in 16 signed",
+        in, get_int_caps (1, "BYTE_ORDER", 16, 16, TRUE),
+        out, get_int_caps (1, "BYTE_ORDER", 16, 8, TRUE)
+        );
+    RUN_CONVERSION ("8 in 16 signed to 16 signed",
+        out, get_int_caps (1, "BYTE_ORDER", 16, 8, TRUE),
+        in, get_int_caps (1, "BYTE_ORDER", 16, 16, TRUE)
+        );
+  }
+
+  /* 16 bit unsigned <-> 8 in 16 bit unsigned */
+  {
+    guint16 in[] = { 1 << 15, (1 << 15) - (64 << 8), (1 << 15) + (64 << 8) };
+    guint16 out[] = { 1 << 7, (1 << 7) - 64, (1 << 7) + 64 };
+    RUN_CONVERSION ("16 unsigned to 8 in 16 unsigned",
+        in, get_int_caps (1, "BYTE_ORDER", 16, 16, FALSE),
+        out, get_int_caps (1, "BYTE_ORDER", 16, 8, FALSE)
+        );
+    RUN_CONVERSION ("8 in 16 unsigned to 16 unsigned",
+        out, get_int_caps (1, "BYTE_ORDER", 16, 8, FALSE),
+        in, get_int_caps (1, "BYTE_ORDER", 16, 16, FALSE)
+        );
+  }
+
+  /* 32 bit signed -> 16 bit signed for rounding check */
+  {
+    gint32 in[] = { 0, G_MININT32, G_MAXINT32,
+      (32 << 16), (32 << 16) + (1 << 15), (32 << 16) - (1 << 15),
+      (32 << 16) + (2 << 15), (32 << 16) - (2 << 15),
+      (-32 << 16) + (1 << 15), (-32 << 16) - (1 << 15),
+      (-32 << 16) + (2 << 15), (-32 << 16) - (2 << 15),
+      (-32 << 16)
+    };
+    gint16 out[] = { 0, G_MININT16, G_MAXINT16,
+      32, 33, 32,
+      33, 31,
+      -32, -33,
+      -31, -33,
+      -32
+    };
+    RUN_CONVERSION ("32 signed to 16 signed for rounding",
+        in, get_int_caps (1, "BYTE_ORDER", 32, 32, TRUE),
+        out, get_int_caps (1, "BYTE_ORDER", 16, 16, TRUE)
+        );
+  }
+
+  /* 32 bit signed -> 16 bit unsigned for rounding check */
+  {
+    gint32 in[] = { 0, G_MININT32, G_MAXINT32,
+      (32 << 16), (32 << 16) + (1 << 15), (32 << 16) - (1 << 15),
+      (32 << 16) + (2 << 15), (32 << 16) - (2 << 15),
+      (-32 << 16) + (1 << 15), (-32 << 16) - (1 << 15),
+      (-32 << 16) + (2 << 15), (-32 << 16) - (2 << 15),
+      (-32 << 16)
+    };
+    guint16 out[] = { (1 << 15), 0, G_MAXUINT16,
+      (1 << 15) + 32, (1 << 15) + 33, (1 << 15) + 32,
+      (1 << 15) + 33, (1 << 15) + 31,
+      (1 << 15) - 31, (1 << 15) - 32,
+      (1 << 15) - 31, (1 << 15) - 33,
+      (1 << 15) - 32
+    };
+    RUN_CONVERSION ("32 signed to 16 unsigned for rounding",
+        in, get_int_caps (1, "BYTE_ORDER", 32, 32, TRUE),
+        out, get_int_caps (1, "BYTE_ORDER", 16, 16, FALSE)
+        );
+  }
 }
 
 GST_END_TEST;
@@ -518,7 +604,7 @@ GST_START_TEST (test_float_conversion)
     gdouble out[] = { 0.0,
       4.6566128752457969e-10 * (gdouble) (-32768L << 16),       /* ~ -1.0 */
       4.6566128752457969e-10 * (gdouble) (16384L << 16),        /* ~ 0.5 */
-      4.6566128752457969e-10 * (gdouble) (-16384L << 16), /* ~ -0.5 */
+      4.6566128752457969e-10 * (gdouble) (-16384L << 16),       /* ~ -0.5 */
     };
 
     RUN_CONVERSION ("16 signed to 64 float",
@@ -530,7 +616,7 @@ GST_START_TEST (test_float_conversion)
     gdouble out[] = { 0.0,
       4.6566128752457969e-10 * (gdouble) (-1L << 31),   /* ~ -1.0 */
       4.6566128752457969e-10 * (gdouble) (1L << 30),    /* ~ 0.5 */
-      4.6566128752457969e-10 * (gdouble) (-1L << 30), /* ~ -0.5 */
+      4.6566128752457969e-10 * (gdouble) (-1L << 30),   /* ~ -0.5 */
     };
 
     RUN_CONVERSION ("32 signed to 64 float",