[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 #357578|none                        |needs-work
             status|                            |

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

Looks good in general

::: gst-libs/gst/audio/audio-channel-mixer.c
@@ +800,3 @@
  * @out_channels: number of output channels
+ * @matrix: (transfer full): channel conversion matrix,
m[@in_channels][@out_channels].
+ *   If identity matrix, passthrough applies.

Maybe allow NULL for identity?

@@ +942,3 @@
+    for (j = 0; j < mix->out_channels; j++) {
+      if ((i == j && mix->matrix[i][j] != 1.0) ||
+          (i != j && mix->matrix[i][j] != 0.0)) {

Comparing floats is not a good idea. Best if you remember from the constructor
with a flag if you're passthrough or not, based on the positions... or if we
only get a matrix, assume passthrough only if matrix == NULL and as fallback
only compare the matrix.

::: gst-libs/gst/audio/audio-channel-mixer.h
@@ +60,3 @@
+                                                                gint
in_channels,
+                                                                gint
out_channels,
+                                                                gfloat
**matrix);

const?

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