[Bug 785471] [API]: gst_audio_channel_mixer_new_with_matrix

[GStreamer (GNOME Bugzilla)]

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

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

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

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

Looks good generally, just some minor things left

::: gst/audioconvert/gstaudioconvert.c
@@ +350,3 @@
+          other_channels = gst_value_array_get_size (first_row);
+        } else {
+          GST_WARNING_OBJECT (this, "Empty mix matrix's first row");

Maybe we can catch the "invalid matrix" cases in set_property() already?

@@ +952,3 @@
+
+      g_value_init (&this->mix_matrix, GST_TYPE_ARRAY);
+      g_value_copy (value, &this->mix_matrix);

It seems like there is no way to actually unset the matrix after you set it
once. You could provide an empty array I guess (i.e. a value just initialized
to GST_TYPE_ARRAY), but this would still succeed the G_IS_VALUE() checks. You
should probably special-case the empty array as "unset".

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