gst/rtp/gstrtpamrdepay.c: Added more meaningfull warnings when something goes wrong.

Original commit message from CVS:
* gst/rtp/gstrtpamrdepay.c: (gst_rtp_amr_depay_chain):
Added more meaningfull warnings when something goes wrong.
Clear F bit on outgoing AMR packets.

* gst/rtp/gstrtpamrpay.c: (gst_rtp_amr_pay_class_init),
(gst_rtp_amr_pay_handle_buffer):
Added debugging category
Support payloading of multiple AMR frames.

* gst/rtp/gstrtpmp4vpay.c: (gst_rtp_mp4v_pay_depay_data):
Added some debugging.

3 files changed, 153 insertions, 31 deletions

diff --git a/gst/rtp/gstrtpamrdepay.c b/gst/rtp/gstrtpamrdepay.c
index 2314c368..28ec3e39 100644
--- a/gst/rtp/gstrtpamrdepay.c
+++ b/gst/rtp/gstrtpamrdepay.c
@@ -272,11 +272,14 @@ gst_rtp_amr_depay_chain (GstPad * pad, GstBuffer * buf)
   if (!rtpamrdepay->negotiated)
     goto not_negotiated;
 
-  if (!gst_rtp_buffer_validate (buf))
+  if (!gst_rtp_buffer_validate (buf)) {
+    GST_ELEMENT_WARNING (rtpamrdepay, STREAM, DECODE,
+        (NULL), ("AMR RTP packet did not validate"));
     goto bad_packet;
+  }
 
   /* when we get here, 1 channel, 8000 Hz, octet aligned, no CRC, 
-   * no robust sorting, no interleaving data is to be depayd */
+   * no robust sorting, no interleaving data is to be depayloaded */
   {
     gint payload_len;
     guint8 *payload, *p, *dp;
@@ -289,8 +292,11 @@ gst_rtp_amr_depay_chain (GstPad * pad, GstBuffer * buf)
     payload_len = gst_rtp_buffer_get_payload_len (buf);
 
     /* need at least 2 bytes for the header */
-    if (payload_len < 2)
+    if (payload_len < 2) {
+      GST_ELEMENT_WARNING (rtpamrdepay, STREAM, DECODE,
+          (NULL), ("AMR RTP payload too small (%d)", payload_len));
       goto bad_packet;
+    }
 
     payload = gst_rtp_buffer_get_payload (buf);
 
@@ -308,6 +314,8 @@ gst_rtp_amr_depay_chain (GstPad * pad, GstBuffer * buf)
     payload_len -= 1;
     payload += 1;
 
+    GST_DEBUG_OBJECT (rtpamrdepay, "payload len %d", payload_len);
+
     if (rtpamrdepay->interleaving) {
       ILL = (payload[0] & 0xf0) >> 4;
       ILP = (payload[0] & 0x0f);
@@ -315,8 +323,11 @@ gst_rtp_amr_depay_chain (GstPad * pad, GstBuffer * buf)
       payload_len -= 1;
       payload += 1;
 
-      if (ILP > ILL)
+      if (ILP > ILL) {
+        GST_ELEMENT_WARNING (rtpamrdepay, STREAM, DECODE,
+            (NULL), ("AMR RTP wrong interleaving"));
         goto bad_packet;
+      }
     }
 
     /* 
@@ -338,8 +349,12 @@ gst_rtp_amr_depay_chain (GstPad * pad, GstBuffer * buf)
       FT = (payload[i] & 0x78) >> 3;
 
       fr_size = frame_size[FT];
-      if (fr_size == -1)
+      GST_DEBUG_OBJECT (rtpamrdepay, "frame size %d", fr_size);
+      if (fr_size == -1) {
+        GST_ELEMENT_WARNING (rtpamrdepay, STREAM, DECODE,
+            (NULL), ("AMR RTP frame size == -1"));
         goto bad_packet;
+      }
 
       if (fr_size > 0) {
         amr_len += fr_size;
@@ -352,23 +367,33 @@ gst_rtp_amr_depay_chain (GstPad * pad, GstBuffer * buf)
     }
 
     /* this is impossible */
-    if (num_packets == payload_len)
+    if (num_packets == payload_len) {
+      GST_ELEMENT_WARNING (rtpamrdepay, STREAM, DECODE,
+          (NULL), ("AMR RTP num_packets == payload_len"));
       goto bad_packet;
+    }
 
     if (rtpamrdepay->crc) {
       /* data len + CRC len + header bytes should be smaller than payload_len */
-      if (num_packets + num_nonempty_packets + amr_len > payload_len)
+      if (num_packets + num_nonempty_packets + amr_len > payload_len) {
+        GST_ELEMENT_WARNING (rtpamrdepay, STREAM, DECODE,
+            (NULL), ("AMR RTP wrong length 1"));
         goto bad_packet;
+      }
     } else {
       /* data len + header bytes should be smaller than payload_len */
-      if (num_packets + amr_len > payload_len)
+      if (num_packets + amr_len > payload_len) {
+        GST_ELEMENT_WARNING (rtpamrdepay, STREAM, DECODE,
+            (NULL), ("AMR RTP wrong length 2"));
         goto bad_packet;
+      }
     }
 
     timestamp = gst_rtp_buffer_get_timestamp (buf);
 
     outbuf = gst_buffer_new_and_alloc (payload_len);
-    GST_BUFFER_TIMESTAMP (outbuf) = timestamp * GST_SECOND / rtpamrdepay->rate;
+    GST_BUFFER_TIMESTAMP (outbuf) =
+        gst_util_uint64_scale_int (timestamp, GST_SECOND, rtpamrdepay->rate);
 
     /* point to destination */
     p = GST_BUFFER_DATA (outbuf);
@@ -384,8 +409,8 @@ gst_rtp_amr_depay_chain (GstPad * pad, GstBuffer * buf)
 
       fr_size = frame_size[(payload[i] & 0x78) >> 3];
       if (fr_size > 0) {
-        /* copy FT */
-        *p++ = payload[i];
+        /* copy FT, clear F bit */
+        *p++ = payload[i] & 0x7f;
         /* copy data packet, FIXME, calc CRC here. */
         memcpy (p, dp, fr_size);
 
@@ -404,18 +429,18 @@ gst_rtp_amr_depay_chain (GstPad * pad, GstBuffer * buf)
 
   return ret;
 
+  /* ERRORS */
 not_negotiated:
   {
     GST_ELEMENT_ERROR (rtpamrdepay, STREAM, NOT_IMPLEMENTED,
-        ("not negotiated"), (NULL));
+        (NULL), ("not negotiated"));
     gst_buffer_unref (buf);
     return GST_FLOW_NOT_NEGOTIATED;
   }
 bad_packet:
   {
-    GST_ELEMENT_WARNING (rtpamrdepay, STREAM, DECODE,
-        ("amr packet did not validate"), (NULL));
     gst_buffer_unref (buf);
+    /* no fatal error */
     return GST_FLOW_OK;
   }
 }
diff --git a/gst/rtp/gstrtpamrpay.c b/gst/rtp/gstrtpamrpay.c
index 8dfed712..7dacce5a 100644
--- a/gst/rtp/gstrtpamrpay.c
+++ b/gst/rtp/gstrtpamrpay.c
@@ -22,6 +22,9 @@
 
 #include "gstrtpamrpay.h"
 
+GST_DEBUG_CATEGORY (rtpamrpay_debug);
+#define GST_CAT_DEFAULT (rtpamrpay_debug)
+
 /* references:
  *
  * RFC 3267 - Real-Time Transport Protocol (RTP) Payload Format and File 
@@ -128,6 +131,10 @@ gst_rtp_amr_pay_class_init (GstRtpAMRPayClass * klass)
 
   gstbasertppayload_class->set_caps = gst_rtp_amr_pay_setcaps;
   gstbasertppayload_class->handle_buffer = gst_rtp_amr_pay_handle_buffer;
+
+  GST_DEBUG_CATEGORY_INIT (rtpamrpay_debug, "rtpamrpay", 0,
+      "AMR RTP Depayloader");
+
 }
 
 static void
@@ -156,6 +163,12 @@ gst_rtp_amr_pay_setcaps (GstBaseRTPPayload * basepayload, GstCaps * caps)
   return TRUE;
 }
 
+/* -1 is invalid */
+static gint frame_size[16] = {
+  12, 13, 15, 17, 19, 20, 26, 31,
+  5, -1, -1, -1, -1, -1, -1, 0
+};
+
 static GstFlowReturn
 gst_rtp_amr_pay_handle_buffer (GstBaseRTPPayload * basepayload,
     GstBuffer * buffer)
@@ -164,30 +177,63 @@ gst_rtp_amr_pay_handle_buffer (GstBaseRTPPayload * basepayload,
   GstFlowReturn ret;
   guint size, payload_len;
   GstBuffer *outbuf;
-  guint8 *payload, *data;
+  guint8 *payload, *data, *payload_amr;
   GstClockTime timestamp;
+  guint packet_len, mtu;
+  gint i, num_packets, num_nonempty_packets;
+  gint amr_len;
 
   rtpamrpay = GST_RTP_AMR_PAY (basepayload);
+  mtu = GST_BASE_RTP_PAYLOAD_MTU (rtpamrpay);
 
   size = GST_BUFFER_SIZE (buffer);
+  data = GST_BUFFER_DATA (buffer);
   timestamp = GST_BUFFER_TIMESTAMP (buffer);
 
-  /* FIXME, only one AMR frame per RTP packet for now, 
+  /* FIXME, only 
    * octet aligned, no interleaving, single channel, no CRC,
    * no robust-sorting. */
 
+  GST_DEBUG_OBJECT (basepayload, "got %d bytes", size);
+
+  /* first count number of packets and total amr frame size */
+  amr_len = num_packets = num_nonempty_packets = 0;
+  for (i = 0; i < size; i++) {
+    guint8 FT;
+    gint fr_size;
+
+    FT = (data[i] & 0x78) >> 3;
+
+    fr_size = frame_size[FT];
+    GST_DEBUG_OBJECT (basepayload, "frame size %d", fr_size);
+    /* FIXME, we don't handle this yet.. */
+    if (fr_size <= 0)
+      goto wrong_size;
+
+    amr_len += fr_size;
+    num_nonempty_packets++;
+    num_packets++;
+    i += fr_size;
+  }
+  if (amr_len > size)
+    goto incomplete_frame;
+
   /* we need one extra byte for the CMR, the ToC is in the input
    * data */
   payload_len = size + 1;
 
+  /* get packet len to check against MTU */
+  packet_len = gst_rtp_buffer_calc_packet_len (payload_len, 0, 0);
+  if (packet_len > mtu)
+    goto too_big;
+
+  /* now alloc output buffer */
   outbuf = gst_rtp_buffer_new_allocate (payload_len, 0, 0);
-  /* FIXME, assert for now */
-  g_assert (GST_BUFFER_SIZE (outbuf) < GST_BASE_RTP_PAYLOAD_MTU (rtpamrpay));
 
   /* copy timestamp */
   GST_BUFFER_TIMESTAMP (outbuf) = timestamp;
 
-  /* get payload */
+  /* get payload, this is now writable */
   payload = gst_rtp_buffer_get_payload (outbuf);
 
   /*   0 1 2 3 4 5 6 7 
@@ -197,24 +243,70 @@ gst_rtp_amr_pay_handle_buffer (GstBaseRTPPayload * basepayload,
    */
   payload[0] = 0xF0;            /* CMR, no specific mode requested */
 
-  data = GST_BUFFER_DATA (buffer);
-
-  /* copy data in payload */
-  memcpy (&payload[1], data, size);
-
-  /*   0 1 2 3 4 5 6 7
-   *  +-+-+-+-+-+-+-+-+
-   *  |F|  FT   |Q|P|P|
-   *  +-+-+-+-+-+-+-+-+
-   */
-  /* clear F flag */
-  payload[1] = payload[1] & 0x7f;
+  /* this is where we copy the AMR data, after num_packets FTs and the
+   * CMR. */
+  payload_amr = payload + num_packets + 1;
+
+  /* copy data in payload, first we copy all the FTs then all
+   * the AMR data. The last FT has to have the F flag cleared. */
+  for (i = 1; i <= num_packets; i++) {
+    guint8 FT;
+    gint fr_size;
+
+    /*   0 1 2 3 4 5 6 7
+     *  +-+-+-+-+-+-+-+-+
+     *  |F|  FT   |Q|P|P| more FT...
+     *  +-+-+-+-+-+-+-+-+
+     */
+    FT = (*data & 0x78) >> 3;
+
+    fr_size = frame_size[FT];
+
+    if (i == num_packets)
+      /* last packet, clear F flag */
+      payload[i] = *data & 0x7f;
+    else
+      /* set F flag */
+      payload[i] = *data | 0x80;
+
+    memcpy (payload_amr, &data[1], fr_size);
+
+    /* all sizes are > 0 since we checked for that above */
+    data += fr_size + 1;
+    payload_amr += fr_size;
+  }
 
   gst_buffer_unref (buffer);
 
   ret = gst_basertppayload_push (basepayload, outbuf);
 
   return ret;
+
+  /* ERRORS */
+wrong_size:
+  {
+    GST_ELEMENT_ERROR (basepayload, STREAM, FORMAT,
+        (NULL), ("received AMR frame with size <= 0"));
+    gst_buffer_unref (buffer);
+
+    return GST_FLOW_ERROR;
+  }
+incomplete_frame:
+  {
+    GST_ELEMENT_ERROR (basepayload, STREAM, FORMAT,
+        (NULL), ("received incomplete AMR frames"));
+    gst_buffer_unref (buffer);
+
+    return GST_FLOW_ERROR;
+  }
+too_big:
+  {
+    GST_ELEMENT_ERROR (basepayload, STREAM, FORMAT,
+        (NULL), ("received too many AMR frames for MTU"));
+    gst_buffer_unref (buffer);
+
+    return GST_FLOW_ERROR;
+  }
 }
 
 gboolean
diff --git a/gst/rtp/gstrtpmp4vpay.c b/gst/rtp/gstrtpmp4vpay.c
index 91ef88d0..ce291a3c 100644
--- a/gst/rtp/gstrtpmp4vpay.c
+++ b/gst/rtp/gstrtpmp4vpay.c
@@ -275,6 +275,7 @@ gst_rtp_mp4v_pay_depay_data (GstRtpMP4VPay * enc, guint8 * data, guint size,
     return FALSE;
 
   code = GST_READ_UINT32_BE (data);
+  GST_DEBUG_OBJECT (enc, "start code 0x%08x", code);
 
   switch (code) {
     case VOS_STARTCODE:
@@ -287,6 +288,8 @@ gst_rtp_mp4v_pay_depay_data (GstRtpMP4VPay * enc, guint8 * data, guint size,
       /* profile_and_level_indication */
       profile = data[4];
 
+      GST_DEBUG_OBJECT (enc, "VOS profile 0x%08x", profile);
+
       if (profile != enc->profile) {
         newprofile = TRUE;
         enc->profile = profile;
@@ -322,10 +325,12 @@ gst_rtp_mp4v_pay_depay_data (GstRtpMP4VPay * enc, guint8 * data, guint size,
       break;
     }
     case VOP_STARTCODE:
+      GST_DEBUG_OBJECT (enc, "VOP");
       /* VOP startcode, we don't have to flush the packet */
       result = FALSE;
       break;
     default:
+      GST_DEBUG_OBJECT (enc, "other startcode");
       /* all other startcodes need a flush */
       result = TRUE;
       break;