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 597824 - Add rtpg723depay plugin
Add rtpg723depay plugin
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-10-08 15:31 UTC by tiagokatcipis
Modified: 2010-09-23 16:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtpg723depay patch (10.55 KB, patch)
2009-10-08 21:00 UTC, tiagokatcipis
needs-work Details | Review

Description tiagokatcipis 2009-10-08 15:31:49 UTC
See: https://bugzilla.gnome.org/show_bug.cgi?id=597715

Here goes the git format-patch and the source code. I ran autogen and make and
the plugin i added compiled successfully.
Comment 1 tiagokatcipis 2009-10-08 15:43:27 UTC
im not being able to upload the patch, it keeps telling me that the file does
not exist or that i dont have permission to read it....but it does exists and i
can read it. So im pasting here the patch content:

From 4eb67c08db92522857fe1a4a1fe80eaadf1e81b0 Mon Sep 17 00:00:00 2001
From: Tiago Katcipis <katcipis@katcipis-laptop.(none)>
Date: Thu, 8 Oct 2009 12:24:57 -0300
Subject: [PATCH] adding rtpg723depay plugin

---
 configure.ac                       |    2 +
 gst/rtpg723depay/Makefile.am       |   11 ++
 gst/rtpg723depay/gstrtpg723depay.c |  232 ++++++++++++++++++++++++++++++++++++
 gst/rtpg723depay/gstrtpg723depay.h |   62 ++++++++++
 4 files changed, 307 insertions(+), 0 deletions(-)
 create mode 100644 gst/rtpg723depay/Makefile.am
 create mode 100644 gst/rtpg723depay/gstrtpg723depay.c
 create mode 100644 gst/rtpg723depay/gstrtpg723depay.h

diff --git a/configure.ac b/configure.ac
index 7c32666..0db9e37 100644
--- a/configure.ac
+++ b/configure.ac
@@ -285,6 +285,7 @@ AG_GST_CHECK_PLUGIN(pnm)
 AG_GST_CHECK_PLUGIN(qtmux)
 AG_GST_CHECK_PLUGIN(rawparse)
 AG_GST_CHECK_PLUGIN(real)
+AG_GST_CHECK_PLUGIN(rtpg723depay)
 AG_GST_CHECK_PLUGIN(rtpmux)
 AG_GST_CHECK_PLUGIN(scaletempo)
 AG_GST_CHECK_PLUGIN(sdp)
@@ -1767,6 +1768,7 @@ gst/pnm/Makefile
 gst/qtmux/Makefile
 gst/rawparse/Makefile
 gst/real/Makefile
+gst/rtpg723depay/Makefile
 gst/rtpmux/Makefile
 gst/scaletempo/Makefile
 gst/sdp/Makefile
diff --git a/gst/rtpg723depay/Makefile.am b/gst/rtpg723depay/Makefile.am
new file mode 100644
index 0000000..28ff700
--- /dev/null
+++ b/gst/rtpg723depay/Makefile.am
@@ -0,0 +1,11 @@
+
+plugin_LTLIBRARIES = libgstrtpg723depay.la
+
+libgstrtpg723depay_la_SOURCES = gstrtpg723depay.c 
+libgstrtpg723depay_la_CFLAGS = $(GST_CFLAGS) $(GST_PLUGINS_BASE_CFLAGS)
+libgstrtpg723depay_la_LIBADD = $(GST_LIBS) $(GST_BASE_LIBS) $(GST_PLUGINS_BASE_LIBS)
+libgstrtpg723depay_la_LDFLAGS = $(GST_PLUGIN_LDFLAGS)
+libgstrtpg723depay_la_LIBTOOLFLAGS = --tag=disable-static
+
+noinst_HEADERS = \
+	gstrtpg723depay.h
diff --git a/gst/rtpg723depay/gstrtpg723depay.c b/gst/rtpg723depay/gstrtpg723depay.c
new file mode 100644
index 0000000..fd09859
--- /dev/null
+++ b/gst/rtpg723depay/gstrtpg723depay.c
@@ -0,0 +1,232 @@
+/* GStreamer
+ *  @author: Tiago Katcipis <tiago.katcipis@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+#ifdef HAVE_CONFIG_H
+#  include "config.h"
+#endif
+
+#include <gst/rtp/gstrtpbuffer.h>
+#include <stdlib.h>
+#include <string.h>
+#include "gstrtpg723depay.h"
+
+/* references:
+ * RFC 3551 (4.5.3)
+ */
+
+#define GST_G723_RTP_DEPAY_FRAME_SIZE 24
+#define GST_G723_RTP_DEPAY_SID_SIZE 4
+
+/* elementfactory information */
+static const GstElementDetails gst_rtp_g723depay_details =
+GST_ELEMENT_DETAILS ("RTP G.723 depayloader",
+    "Codec/Depayloader/Network",
+    "Extracts G.723 audio from RTP packets (RFC 3551)",
+    "Tiago Katcipis <tiagokatcipis@gmail.com>");
+
+enum
+{
+  /* FILL ME */
+  LAST_SIGNAL
+};
+
+enum
+{
+  ARG_0
+};
+
+/* input is an RTP packet
+ *
+ */
+static GstStaticPadTemplate gst_rtp_g723_depay_sink_template =
+    GST_STATIC_PAD_TEMPLATE ("sink",
+    GST_PAD_SINK,
+    GST_PAD_ALWAYS,
+    GST_STATIC_CAPS ("application/x-rtp, "
+        "media = (string) \"audio\", "
+        "payload = (int) " GST_RTP_PAYLOAD_DYNAMIC_STRING ", "
+        "clock-rate = (int) 8000, "
+        "encoding-name = (string) \"G723\"; "
+        "application/x-rtp, "
+        "media = (string) \"audio\", "
+        "payload = (int) " GST_RTP_PAYLOAD_G723_STRING ", "
+        "clock-rate = (int) 8000")
+    );
+
+static GstStaticPadTemplate gst_rtp_g723_depay_src_template =
+GST_STATIC_PAD_TEMPLATE ("src",
+    GST_PAD_SRC,
+    GST_PAD_ALWAYS,
+    GST_STATIC_CAPS ("audio/G723, " "channels = (int) 1," "rate = (int) 8000")
+    );
+
+static gboolean gst_rtp_g723_depay_setcaps (GstBaseRTPDepayload * depayload,
+    GstCaps * caps);
+static GstBuffer *gst_rtp_g723_depay_process (GstBaseRTPDepayload * depayload,
+    GstBuffer * buf);
+
+GST_BOILERPLATE (GstRtpG723Depay, gst_rtp_g723_depay, GstBaseRTPDepayload,
+    GST_TYPE_BASE_RTP_DEPAYLOAD);
+
+static void
+gst_rtp_g723_depay_base_init (gpointer klass)
+{
+  GstElementClass *element_class = GST_ELEMENT_CLASS (klass);
+
+  gst_element_class_add_pad_template (element_class,
+      gst_static_pad_template_get (&gst_rtp_g723_depay_src_template));
+  gst_element_class_add_pad_template (element_class,
+      gst_static_pad_template_get (&gst_rtp_g723_depay_sink_template));
+
+  gst_element_class_set_details (element_class, &gst_rtp_g723depay_details);
+}
+
+static void
+gst_rtp_g723_depay_class_init (GstRtpG723DepayClass * klass)
+{
+  GstBaseRTPDepayloadClass *gstbasertpdepayload_class;
+
+  gstbasertpdepayload_class = (GstBaseRTPDepayloadClass *) klass;
+
+  parent_class = g_type_class_peek_parent (klass);
+
+  gstbasertpdepayload_class->process = gst_rtp_g723_depay_process;
+  gstbasertpdepayload_class->set_caps = gst_rtp_g723_depay_setcaps;
+}
+
+static void
+gst_rtp_g723_depay_init (GstRtpG723Depay * rtpg723depay,
+    GstRtpG723DepayClass * klass)
+{
+  GstBaseRTPDepayload *depayload;
+
+  depayload = GST_BASE_RTP_DEPAYLOAD (rtpg723depay);
+
+  gst_pad_use_fixed_caps (GST_BASE_RTP_DEPAYLOAD_SRCPAD (depayload));
+}
+
+static gboolean
+gst_rtp_g723_depay_setcaps (GstBaseRTPDepayload * depayload, GstCaps * caps)
+{
+  GstStructure *structure;
+  GstCaps *srccaps;
+  GstRtpG723Depay *rtpg723depay;
+  const gchar *params;
+  gint clock_rate, channels;
+  gboolean ret;
+
+  rtpg723depay = GST_RTP_G723_DEPAY (depayload);
+
+  structure = gst_caps_get_structure (caps, 0);
+
+  if (!(params = gst_structure_get_string (structure, "encoding-params")))
+    channels = 1;
+  else {
+    channels = atoi (params);
+  }
+
+  if (!gst_structure_get_int (structure, "clock-rate", &clock_rate))
+    clock_rate = 8000;
+
+  if (channels != 1)
+    goto wrong_channels;
+
+  if (clock_rate != 8000)
+    goto wrong_clock_rate;
+
+  depayload->clock_rate = clock_rate;
+
+  srccaps = gst_caps_new_simple ("audio/G723",
+      "channels", G_TYPE_INT, channels, "rate", G_TYPE_INT, clock_rate, NULL);
+  ret = gst_pad_set_caps (GST_BASE_RTP_DEPAYLOAD_SRCPAD (depayload), srccaps);
+  gst_caps_unref (srccaps);
+
+  return ret;
+
+  /* ERRORS */
+wrong_channels:
+  {
+    GST_DEBUG_OBJECT (rtpg723depay, "expected 1 channel, got %d",
+        rtpg723depay->channels);
+    return FALSE;
+  }
+wrong_clock_rate:
+  {
+    GST_DEBUG_OBJECT (rtpg723depay, "expected 8000 clock-rate, got %d",
+        clock_rate);
+    return FALSE;
+  }
+}
+
+
+static GstBuffer *
+gst_rtp_g723_depay_process (GstBaseRTPDepayload * depayload, GstBuffer * buf)
+{
+  GstRtpG723Depay *rtpg723depay;
+  GstBuffer *outbuf = NULL;
+  gint payload_len;
+  gboolean marker;
+
+  rtpg723depay = GST_RTP_G723_DEPAY (depayload);
+
+  payload_len = gst_rtp_buffer_get_payload_len (buf);
+
+  /* At least 4 bytes (SID from G723) */
+  if (payload_len < GST_G723_RTP_DEPAY_SID_SIZE) {
+    GST_ELEMENT_WARNING (rtpg723depay, STREAM, DECODE,
+        (NULL), ("G723 RTP payload too small (%d)", payload_len));
+    goto bad_packet;
+  }
+
+  GST_DEBUG_OBJECT (rtpg723depay, "payload len %d", payload_len);
+
+  if ((payload_len % GST_G723_RTP_DEPAY_FRAME_SIZE) == GST_G723_RTP_DEPAY_SID_SIZE) {
+    GST_DEBUG_OBJECT (rtpg723depay, "G723 payload contains SID frame");
+  }
+
+  outbuf = gst_rtp_buffer_get_payload_buffer (buf);
+  marker = gst_rtp_buffer_get_marker (buf);
+
+  if (marker) {
+    /* marker bit starts talkspurt */
+    GST_BUFFER_FLAG_SET (outbuf, GST_BUFFER_FLAG_DISCONT);
+  }
+
+  GST_DEBUG ("gst_rtp_g723_depay_chain: pushing buffer of size %d", GST_BUFFER_SIZE (outbuf));
+  return outbuf;
+
+  /* ERRORS */
+bad_packet:
+  {
+    /* no fatal error */
+    return NULL;
+  }
+}
+
+/*Plugin init functions*/
+static gboolean
+plugin_init (GstPlugin * plugin)
+{
+  return gst_element_register (plugin, "rtpg723depay", GST_RANK_NONE, gst_rtp_g723_depay_get_type());
+}
+
+GST_PLUGIN_DEFINE (GST_VERSION_MAJOR,
+    GST_VERSION_MINOR,
+    "rtpg723depay",
+    "Extracts G.723 audio from RTP packets",
+    plugin_init, VERSION, "LGPL", GST_PACKAGE_NAME, GST_PACKAGE_ORIGIN);
diff --git a/gst/rtpg723depay/gstrtpg723depay.h b/gst/rtpg723depay/gstrtpg723depay.h
new file mode 100644
index 0000000..9cd4ee4
--- /dev/null
+++ b/gst/rtpg723depay/gstrtpg723depay.h
@@ -0,0 +1,62 @@
+/* GStreamer
+ * Author Tiago Katcipis <tiagokatcipis@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+#ifndef __GST_RTP_G723_DEPAY_H__
+#define __GST_RTP_G723_DEPAY_H__
+
+#include <gst/gst.h>
+#include <gst/rtp/gstbasertpdepayload.h>
+
+G_BEGIN_DECLS
+
+#define GST_TYPE_RTP_G723_DEPAY \
+  (gst_rtp_g723_depay_get_type())
+  
+#define GST_RTP_G723_DEPAY(obj) \
+  (G_TYPE_CHECK_INSTANCE_CAST((obj),GST_TYPE_RTP_G723_DEPAY,GstRtpG723Depay))
+  
+#define GST_RTP_G723_DEPAY_CLASS(klass) \
+  (G_TYPE_CHECK_CLASS_CAST((klass),GST_TYPE_RTP_G723_DEPAY,GstRtpG723DepayClass))
+  
+#define GST_IS_RTP_G723_DEPAY(obj) \
+  (G_TYPE_CHECK_INSTANCE_TYPE((obj),GST_TYPE_RTP_G723_DEPAY))
+  
+#define GST_IS_RTP_G723_DEPAY_CLASS(klass) \
+  (G_TYPE_CHECK_CLASS_TYPE((klass),GST_TYPE_RTP_G723_DEPAY))
+
+typedef struct _GstRtpG723Depay GstRtpG723Depay;
+typedef struct _GstRtpG723DepayClass GstRtpG723DepayClass;
+
+struct _GstRtpG723Depay
+{
+  GstBaseRTPDepayload depayload;
+
+  gint     channels;
+};
+
+struct _GstRtpG723DepayClass
+{
+  GstBaseRTPDepayloadClass parent_class;
+};
+
+GType gst_rtp_g723_depay_get_type(void);
+
+G_END_DECLS
+
+#endif /* __GST_RTP_G723_DEPAY_H__ */
-- 
1.6.0.4
Comment 2 tiagokatcipis 2009-10-08 21:00:13 UTC
Created attachment 145077 [details] [review]
rtpg723depay patch
Comment 3 tiagokatcipis 2009-10-08 21:01:43 UTC
(In reply to comment #2)
> Created an attachment (id=145077) [details]
> rtpg723depay patch

Now at home the patches are going :-)
Comment 4 tiagokatcipis 2009-12-16 15:11:55 UTC
Have my patch been accepted? if don't...what i have to do so it can be accepted?
Comment 5 Olivier Crête 2009-12-31 20:34:13 UTC
Review of attachment 145077 [details] [review]:

I was just reading RFC 3551 (section 4.5.3), and you should do that too. I noticed that G.723 does not have 2 different types of frames, but 3. And that they can be intermixed in any way you want. So you have to look at the first 2 bits of the frame to know the size. Also, this codec is single channel, so 

Also, it shouldn't be its own plugin, but part of the good -> gst/rtp plugin

This also applies to the G723 payloader in the tree. Which is therefore entirely wrong and I feel that we should just remove it unless its re-written before the next -good release.
Comment 6 Tim-Philipp Müller 2010-01-01 15:05:27 UTC
Moving to -good since that's where we usually add new rtp payloaders/depayloaders as well.


> This also applies to the G723 payloader in the tree. Which is therefore
> entirely wrong and I feel that we should just remove it unless its re-written
> before the next -good release.

Could you please file a separate blocker bug about that then? (or clone this one)
Comment 7 Wim Taymans 2010-09-23 10:49:45 UTC
what exactly is wrong with the existing g723 pay/depayloader?
Comment 8 Tim-Philipp Müller 2010-09-23 16:36:22 UTC
Olivier: is this still an issue after:

commit d6d06630e8e1d9f60794791766597c9c12c337e2
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Tue Jan 5 18:33:25 2010 +0100

    rtpg723pay: rewrite payloader
    
    Handle all 3 packet sizes according to RFC 3551.
    Totally untested, we don't have a G723 encoder.
    
    Fixes #605882

?
Comment 9 Wim Taymans 2010-09-23 16:42:33 UTC
this bug is obsolete. The payloader was commited some time ago, then it was rewritten and then a depayloader was written. All of this is in -good. 

If there are problems with those elements, please open a new bug.