[Bug 785471] [API]: gst_audio_channel_mixer_new_with_matrix

[GStreamer (GNOME Bugzilla)]

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

--- Comment #15 from Mathieu Duponchelle <mduponchelle1 at gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #10)
> Review of attachment 357578 [details] [review]:
> 
> 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?

Done

> 
> @@ +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.

I have updated the patch to compare values to 0.0f and 1.0f explicitly, this
will work for:

* The matrices we generate in audio_channel_mixer
* The matrices that may be passed through the command line

This may not work for matrices generated through calculations, it is the
responsibility of the user to explicitly assign 0.0 or 1.0 to values within an
epsilon distance of their choosing if they want passthrough to potentially
happen.


This is documented in the parameter comment.

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

Well that's transfer full, so I don't think we want that ?

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