[Bug 761947] rtpbasepayload: rtpbasedepayload: Add source-info property

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=761947

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

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

--- Comment #12 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 339241:
 --> (https://bugzilla.gnome.org/review?bug=761947&attachment=339241)

::: gst-libs/gst/rtp/gstrtpbaseaudiopayload.c
@@ +892,3 @@
+  if (!gst_rtp_base_audio_payload_get_lengths (basepayload,
+          gst_rtp_base_payload_get_source_count (basepayload, buffer),
+          &min_payload_len, &max_payload_len, &align))

This probably needs similar changes in all other payloaders?

::: gst-libs/gst/rtp/gstrtpbasedepayload.h
@@ +122,3 @@
+gboolean        gst_rtp_base_depayload_is_source_info_enabled 
(GstRTPBaseDepayload * depayload);
+void            gst_rtp_base_depayload_set_source_info_enabled
(GstRTPBaseDepayload * depayload,
+                                                                gboolean
enable);

getters and setters should be named after the property: is_source_info() or
get_source_info() and set_source_info(). You probably want to rename the
property name to something more meaningful :)

::: gst-libs/gst/rtp/gstrtpbasepayload.c
@@ +1453,3 @@
+
+  if (buffer == NULL)
+    buffer = gst_rtp_buffer_new_allocate (payload_len, pad_len, csrc_count);

If we do all this, it would make sense to do also the allocation query dance
and use the provided allocator here

::: gst-libs/gst/rtp/gstrtpbasepayload.h
@@ +168,3 @@
+GstBuffer *     gst_rtp_base_payload_allocate_output_buffer (GstRTPBasePayload
* payload,
+                                                             guint
payload_len, guint8 pad_len,
+                                                             guint8
csrc_count);

I see us adding another allocation function later that has another parameter,
and then yet another one later... :) Can you think of anything else now
already?

::: gst-libs/gst/rtp/gstrtpmeta.h
@@ +29,3 @@
+typedef struct _GstRTPSourceMeta GstRTPSourceMeta;
+
+#define GST_RTP_SOURCE_META_MAX_CSRC_COUNT 15

Where does the 15 come from?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.