[Bug 740236] New interleave2 based on GstAggregator and create GstAudioAggregator from audiomixer

[GStreamer (GNOME Bugzilla)]

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

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

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

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

Needs to be resynced against latest audiomixer, but generally looks good.
Please push afterwards :)

::: gst/audiomixer/gstaudioaggregator.c
@@ +699,3 @@
+  GST_OBJECT_LOCK (bpad);
+  if (bpad->segment.format == GST_FORMAT_TIME ||
+      bpad->segment.format == GST_FORMAT_DEFAULT)

I don't think we should work at all in non-TIME/DEFAULT segment formats

@@ +711,3 @@
+ */
+static gboolean
+gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg,

Please check this complete function against the current audiomixer code again
:)

@@ +731,3 @@
+
+  if (!GST_BUFFER_PTS_IS_VALID (inbuf) ||
+      aggpad->segment.format != GST_FORMAT_TIME) {

We should require properly timestamps input buffers

@@ +880,3 @@
+
+static void
+gst_audio_aggregator_mix_buffer (GstAudioAggregator * aagg,

And also this function should be compared against the latest audiomixer code

::: gst/audiomixer/gstaudioaggregator.h
@@ +161,3 @@
+void
+gst_audio_aggregator_set_sink_caps (GstAudioAggregator * aagg,
+    GstAudioAggregatorPad * pad, GstCaps * caps);

Why should subclasses be able to set caps on the sinkpad?

::: gst/audiomixer/gstaudiomixer.c
@@ +393,2 @@
+  if (ret)
+    gst_audio_aggregator_set_sink_caps (aagg, GST_AUDIO_AGGREGATOR_PAD (pad),

Why? Shouldn't that just be the default behaviour for setcaps()?

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