audioconvert: Fix setting mix-matrix when input caps changes

When the number of input channels changes, application might have to set
a new mix-matrix. Application must set the new matrix before
audioconvert receives updated caps, otherwise negotiation would fail.
That means it should be allowed to set an invalid mix-matrix until we
receive new caps or next buffer.

This fixes a regression in GStreamer >=1.24.9 caused by:
https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7363

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/9215>
This commit is contained in:
Xavier Claessens 2025-06-12 16:32:51 -04:00 committed by GStreamer Marge Bot
parent 73d5b5b3db
commit 472d528ffe
2 changed files with 98 additions and 42 deletions

View File

@ -1343,6 +1343,7 @@ static void
gst_audio_convert_fixate_channels (GstBaseTransform * base, GstStructure * ins,
GstStructure * outs)
{
GstAudioConvert *this = GST_AUDIO_CONVERT (base);
gint in_chans, out_chans;
guint64 in_mask = 0, out_mask = 0;
gboolean has_in_mask = FALSE, has_out_mask = FALSE;
@ -1460,6 +1461,10 @@ gst_audio_convert_fixate_channels (GstBaseTransform * base, GstStructure * ins,
gst_structure_set_static_str (outs, "channel-mask", GST_TYPE_BITMASK,
intersection, NULL);
return;
} else if (this->mix_matrix_is_set) {
/* Assume the matrix matches the number of in/out channels. This will be
* validated when creating the converter. */
return;
}
/* what now?! Just ignore what we're given and use default positions */
@ -1848,22 +1853,9 @@ gst_audio_convert_prepare_output_buffer (GstBaseTransform * base,
static void
gst_audio_convert_set_mix_matrix (GstAudioConvert * this, const GValue * value)
{
gboolean mix_matrix_was_set;
GstAudioConverter *old_converter;
GValue old_mix_matrix = G_VALUE_INIT;
gboolean restore = FALSE;
g_value_init (&old_mix_matrix, GST_TYPE_ARRAY);
GST_OBJECT_LOCK (this);
mix_matrix_was_set = this->mix_matrix_is_set;
old_converter = this->convert;
if (mix_matrix_was_set) {
g_value_copy (&this->mix_matrix, &old_mix_matrix);
}
this->convert = NULL;
g_clear_pointer (&this->convert, gst_audio_converter_free);
if (!gst_value_array_get_size (value)) {
g_value_copy (value, &this->mix_matrix);
@ -1876,39 +1868,19 @@ gst_audio_convert_set_mix_matrix (GstAudioConvert * this, const GValue * value)
this->mix_matrix_is_set = TRUE;
} else {
GST_WARNING_OBJECT (this, "Empty mix matrix's first row.");
restore = TRUE;
goto done;
this->mix_matrix_is_set = FALSE;
}
}
GST_OBJECT_UNLOCK (this);
/* We need to call this here already because gst_audio_convert_transform
* might never get called otherwise if the element was set to passthrough.
*
* In any case if this succeeds we still want to reconfigure the sink to give
* upstream a chance to renegotiate channels.
*/
if (gst_audio_convert_ensure_converter (GST_BASE_TRANSFORM (this),
&this->in_info, &this->out_info)) {
gst_base_transform_reconfigure_sink (GST_BASE_TRANSFORM (this));
} else {
GST_WARNING_OBJECT (this, "Cannot build converter with this mix matrix");
restore = TRUE;
goto done;
}
done:
if (restore) {
this->mix_matrix_is_set = mix_matrix_was_set;
if (mix_matrix_was_set) {
g_value_copy (&old_mix_matrix, &this->mix_matrix);
}
this->convert = old_converter;
} else if (old_converter) {
gst_audio_converter_free (old_converter);
}
g_value_unset (&old_mix_matrix);
/* We can't create the converter here because the application could be setting
* a new mix-matrix for caps we haven't received yet (e.g. number of input
* channels changed). Assume for now we can't be passthrough and in-place,
* that will be revised once new caps or next buffer arrives. */
gst_base_transform_set_in_place (GST_BASE_TRANSFORM_CAST (this), FALSE);
gst_base_transform_set_passthrough (GST_BASE_TRANSFORM_CAST (this), FALSE);
gst_base_transform_reconfigure_sink (GST_BASE_TRANSFORM_CAST (this));
}
static void

View File

@ -2281,6 +2281,89 @@ GST_START_TEST (test_96_channels_conversion)
GST_END_TEST;
static void
update_mix_matrix (GstElement * audioconvert, gint in_channels,
gint out_channels)
{
GValue mix_matrix = G_VALUE_INIT;
g_value_init (&mix_matrix, GST_TYPE_ARRAY);
for (int i = 0; i < out_channels; i++) {
GValue row = G_VALUE_INIT;
g_value_init (&row, GST_TYPE_ARRAY);
for (int j = 0; j < in_channels; j++) {
GValue value = G_VALUE_INIT;
g_value_init (&value, G_TYPE_FLOAT);
g_value_set_float (&value, (i == j) ? 1.0 : 0.0);
gst_value_array_append_value (&row, &value);
g_value_unset (&value);
}
gst_value_array_append_value (&mix_matrix, &row);
g_value_unset (&row);
}
g_object_set_property (G_OBJECT (audioconvert), "mix-matrix", &mix_matrix);
g_value_unset (&mix_matrix);
}
GST_START_TEST (test_dynamic_mix_matrix)
{
GstCaps *incaps = gst_caps_from_string ("audio/x-raw, "
"format = (string) S16LE, "
"layout = (string) interleaved, "
"channel-mask = (bitmask) 0, "
"rate = (int) 44100, " "channels = (int) 2 ");
GstCaps *outcaps = gst_caps_from_string ("audio/x-raw,channels = (int) 2");
GstElement *audioconvert =
setup_audioconvert (outcaps, FALSE, &(GValue) G_VALUE_INIT);
fail_unless (gst_element_set_state (audioconvert,
GST_STATE_PLAYING) == GST_STATE_CHANGE_SUCCESS,
"could not set to playing");
gst_check_setup_events (mysrcpad, audioconvert, incaps, GST_FORMAT_TIME);
/* Should be passthrough initially */
GstBuffer *outbuffer = NULL;
GstBuffer *buffer = gst_buffer_new_and_alloc (1 * 2 * 2);
fail_unless_equals_int (gst_pad_push (mysrcpad, buffer), GST_FLOW_OK);
fail_unless (g_list_length (buffers) == 1);
fail_if ((outbuffer = (GstBuffer *) buffers->data) == NULL);
fail_unless (outbuffer == buffer);
buffers = g_list_remove (buffers, outbuffer);
gst_buffer_unref (outbuffer);
/* Set a 8:2 mix matrix, that's invalid with current caps, we should get an
* error if we push a buffer. This tests that setting an invalid mix matrix
* still got us out of passthrough mode. */
update_mix_matrix (audioconvert, 8, 2);
buffer = gst_buffer_new_and_alloc (1 * 2 * 2);
fail_unless_equals_int (gst_pad_push (mysrcpad, buffer), GST_FLOW_ERROR);
fail_unless (buffers == NULL, "Pushing a buffer should have failed");
/* Now update caps */
gst_caps_take (&incaps, gst_caps_from_string ("audio/x-raw, "
"format = (string) S16LE, "
"layout = (string) interleaved, "
"channel-mask = (bitmask) 0, "
"rate = (int) 44100, " "channels = (int) 8 "));
fail_unless (gst_pad_push_event (mysrcpad, gst_event_new_caps (incaps)));
/* Pushing a buffer should work */
buffer = gst_buffer_new_and_alloc (1 * 2 * 8);
fail_unless_equals_int (gst_pad_push (mysrcpad, buffer), GST_FLOW_OK);
fail_unless (g_list_length (buffers) == 1);
fail_if ((outbuffer = (GstBuffer *) buffers->data) == NULL);
fail_unless_equals_int (gst_buffer_get_size (outbuffer), 1 * 2 * 2);
buffers = g_list_remove (buffers, outbuffer);
gst_buffer_unref (outbuffer);
/* cleanup */
fail_unless (gst_element_set_state (audioconvert,
GST_STATE_NULL) == GST_STATE_CHANGE_SUCCESS, "could not set to null");
cleanup_audioconvert (audioconvert);
gst_caps_unref (incaps);
gst_caps_unref (outcaps);
}
GST_END_TEST;
static Suite *
audioconvert_suite (void)
@ -2307,6 +2390,7 @@ audioconvert_suite (void)
tcase_add_test (tc_chain, test_layout_conversion);
tcase_add_test (tc_chain, test_layout_conv_fixate_caps);
tcase_add_test (tc_chain, test_96_channels_conversion);
tcase_add_test (tc_chain, test_dynamic_mix_matrix);
return s;
}