[Bug 664817] Adding opus RTP payloader/depayloader module

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=664817
  GStreamer | gst-plugins-bad | git

Vincent Penquerc'h <vincent.penquerch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #202333|none                        |needs-work
             status|                            |

--- Comment #4 from Vincent Penquerc'h <vincent.penquerch at collabora.co.uk> 2011-11-29 11:30:23 UTC ---
Review of attachment 202333:
 --> (https://bugzilla.gnome.org/review?bug=664817&attachment=202333)

::: gst/rtpopus/gstrtpopus.c
@@ +25,3 @@
+#include "gstrtpopusdepay.h"
+
+#include <gst/tag/tag.h>

Not needed.

@@ +31,3 @@
+{
+  gst_rtp_opus_pay_plugin_init (plugin);
+  gst_rtp_opus_depay_plugin_init (plugin);

Should return FALSE if any of these returns FALSE.

@@ +39,3 @@
+    GST_VERSION_MINOR,
+    "rtpopus",
+    "RTP payloader/depayloader for OPUS encoder",

Nitpick (sorry) for s/encoder/codec/.

::: gst/rtpopus/gstrtpopusdepay.c
@@ +68,3 @@
+      gst_static_pad_template_get (&gst_rtp_opus_depay_src_template));
+  gst_element_class_add_pad_template (element_class,
+      gst_static_pad_template_get (&gst_rtp_opus_depay_sink_template));

These leak (though almost every element was till recently).
Use gst_element_class_add_static_pad_template (new in core).

::: gst/rtpopus/gstrtpopusdepay.h
@@ +2,3 @@
+ * Opus Depayloader Gst Element
+ *
+ *   @author: Danilo Cesar Lemes de Paula <danilo.eu at gmail.com>

Still the old one :)

::: gst/rtpopus/gstrtpopuspay.c
@@ +68,3 @@
+      gst_static_pad_template_get (&gst_rtp_opus_pay_src_template));
+  gst_element_class_add_pad_template (element_class,
+      gst_static_pad_template_get (&gst_rtp_opus_pay_sink_template));

Leaks too, same fix as the other element.

@@ +150,3 @@
+  gst_buffer_unref (buffer);
+
+  g_warning ("SIZE: %d, PACKET: %d PAYLOAD: %d", size, packet_len,
payload_len);

GST_DEBUG_OBJECT

@@ +159,3 @@
+{
+  return gst_element_register (plugin, "rtpopuspay",
+      GST_RANK_NONE, GST_TYPE_RTP_OPUS_PAY);

This may be intended, but this has rank NONE while the depayloader has rank
PRIMARY, so I'll mention that in case there's anything going on here.

::: gst/rtpopus/gstrtpopuspay.h
@@ +2,3 @@
+ * Opus Payloader Gst Element
+ *
+ *   @author: Danilo Cesar Lemes de Paula <danilo.eu at gmail.com>

Old one.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.