GNOME Bugzilla – Bug 597824
Add rtpg723depay plugin
Last modified: 2010-09-23 16:42:33 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.
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
Created attachment 145077 [details] [review] rtpg723depay patch
(In reply to comment #2) > Created an attachment (id=145077) [details] > rtpg723depay patch Now at home the patches are going :-)
Have my patch been accepted? if don't...what i have to do so it can be accepted?
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.
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)
what exactly is wrong with the existing g723 pay/depayloader?
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 ?
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.