[Bug 785471] [API]: gst_audio_channel_mixer_new_with_matrix

[GStreamer (GNOME Bugzilla)]

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

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

We can indeed output the warning there, but what should we do then ? Not set
the matrix and proceed without one? This may be surprising to the user if he
doesn't check his logs.

Otherwise we can output the warning there and error out here, but that doesn't
make much of a difference?

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

Thing is I'd ideally set this as CONSTRUCT_ONLY, but then this won't work from
the command line, same thing for the dithering and noise shaping properties by
the way

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