GNOME Bugzilla – Bug 737127
interleave: interleaving does not respect the channel positions default order
Last modified: 2014-10-02 07:22:18 UTC
Created attachment 286823 [details] examples to show how interleave ignores the channel positions default ordering Hi, this report results from some investigation I did with regards to bug 735673 (and maybe bug 708988 is connected too). First some background. With the two following pipelines I was getting exactly the same file (despite the swapped channel-mask properties): gst-launch-1.0 interleave name=i ! audioconvert ! wavenc ! filesink location=audio-interleave-test_1.wav audiotestsrc wave=0 num-buffers=100 ! audioconvert ! "audio/x-raw,channels=1,channel-mask=(bitmask)0x1" ! queue ! i.sink_0 audiotestsrc wave=2 num-buffers=100 ! audioconvert ! "audio/x-raw,channels=1,channel-mask=(bitmask)0x2" ! queue ! i.sink_1 gst-launch-1.0 interleave name=i ! audioconvert ! wavenc ! filesink location=audio-interleave-test_2.wav audiotestsrc wave=0 num-buffers=100 ! audioconvert ! "audio/x-raw,channels=1,channel-mask=(bitmask)0x2" ! queue ! i.sink_0 audiotestsrc wave=2 num-buffers=100 ! audioconvert ! "audio/x-raw,channels=1,channel-mask=(bitmask)0x1" ! queue ! i.sink_1 I would have expected the channels to be swapped in the second file. The same behaviour can be obtained by setting the channel-positions property in code, with a different order; see the attached archive with two examples with switched channel positions. This is the diff between the two example programs: --- interleave_wav_left-right.c 2014-09-22 13:30:10.878415269 +0200 +++ interleave_wav_right-left.c 2014-09-22 13:31:04.774844535 +0200 @@ -73,7 +73,7 @@ make_n_channel_wav (const gint channels, filesink = gst_element_factory_make ("filesink", NULL); fail_unless (filesink != NULL); - g_object_set (G_OBJECT (filesink), "location", "left-right.wav", NULL); + g_object_set (G_OBJECT (filesink), "location", "right-left.wav", NULL); gst_bin_add (GST_BIN (pipeline), filesink); fail_unless (gst_element_link (wavenc, filesink)); @@ -112,10 +112,10 @@ GST_START_TEST (test_encode_stereo) arr = g_value_array_new (2); g_value_init (&val, GST_TYPE_AUDIO_CHANNEL_POSITION); - g_value_set_enum (&val, GST_AUDIO_CHANNEL_POSITION_FRONT_LEFT); + g_value_set_enum (&val, GST_AUDIO_CHANNEL_POSITION_FRONT_RIGHT); g_value_array_append (arr, &val); g_value_reset (&val); - g_value_set_enum (&val, GST_AUDIO_CHANNEL_POSITION_FRONT_RIGHT); + g_value_set_enum (&val, GST_AUDIO_CHANNEL_POSITION_FRONT_LEFT); g_value_array_append (arr, &val); g_value_unset (&val); They also produce the exact same file. AFAIU a "channel position" in this context is meant to indicate the loudspeaker position in the "physical" sense, it's not about the positions of the channel in the wav stream, however this still affects the order of channels in the stream: according to http://msdn.microsoft.com/en-us/library/windows/hardware/dn653308%28v=vs.85%29.aspx the "Default Channel Ordering" together with dwChannelMask are enough to represent a complete mapping between channel position and loudspeaker position. Right now, the interleave element is able to retrieve the info about channel positions correctly (even when passing channel-mask via caps in the source pads), but then it ignores it when actually interleaving the samples. So I think the interleave element should interleave the samples reordering the channels to follow the Default Ordering. To be more explicit, in the example above, if sink_0 is RIGHT and sink_1 is LEFT, the interleave_N() functions should put samples from sink_1 _before_ samples from sink_0 in order to re-enforce the "Default Ordering" from the specification above. Right now, the interleaving is done following the sink order, not the "channel position order". Please let me know if I was not able to elaborate well enough. I'll try to some up with a proof of concept patch if you agree with the reasoning above.
Created attachment 286870 [details] [review] interleave samples following the Default Channel Ordering In order to have a full mapping between channel positions in the audio stream and loudspeaker positions, the channel-mask alone is not enough: the channels must be interleaved following some Default Channel Ordering as mentioned in the WAVEFORMATEXTENSIBLE[1] specification. As a Default Channel Ordering we can use the one implied by GstAudioChannelPosition which follows the ordering defined in SMPTE 2036-2-2008[2]. NOTE that the relative order in the Top Layer is not exactly the same as the one from the WAVEFORMATEXTENSIBLE[1] specification, is it safe to assume that users using so may channels are already aware of such discrepancies. [1] http://msdn.microsoft.com/en-us/library/windows/hardware/dn653308%28v=vs.85%29.aspx [2] http://www.itu.int/dms_pub/itu-r/opb/rep/R-REP-BS.2159-2-2011-PDF-E.pdf I am attaching a proof of the concept of what I mean. In the final patch I can move the ordering map creation in a dedicated function, and maybe use qsort instead of the naive bubblesort used for now.
Review of attachment 286870 [details] [review]: Looks good to me except for some little detail :) ::: gst/interleave/interleave.c @@ +288,3 @@ + default_ordering_map[j] = tmp; + } + } You could use something like g_qsort_with_data() here, but not really important other than that we don't have our own sorting here then ::: gst/interleave/interleave.h @@ +61,3 @@ gboolean channel_positions_from_input; + gint *default_channels_ordering_map; You can just store a 64 element array here, no need for dynamic allocation
Created attachment 287357 [details] [review] interleave: interleave samples following the Default Channel Ordering Attached a v2 with these changes: - using g_qsort_with_data() to sort the channel map NOTE, the sorting is done using the foreign keys from pos[]. - using a static array for the channel map I was not sure about the second one, but not having a strong opinion, I just complied to your comments. BTW, maybe this could have also be done with the audio channels functions from gst-plugins-base: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst-libs/gst/audio/audio-channels.h In fact, a similar channel map is built also in /riff/riff-media.c: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst-libs/gst/riff/riff-media.c#n1056 However I don't know if I have the motivation to still work on this. Thanks, Antonio
Thanks for the patch :) commit 7ae7f657fae542f06ba4c22d65a2084f5a4a3a5d Author: Antonio Ospite <ao2@ao2.it> Date: Tue Sep 23 10:48:09 2014 +0200 interleave: interleave samples following the Default Channel Ordering In order to have a full mapping between channel positions in the audio stream and loudspeaker positions, the channel-mask alone is not enough: the channels must be interleaved following some Default Channel Ordering as mentioned in the WAVEFORMATEXTENSIBLE[1] specification. As a Default Channel Ordering use the one implied by GstAudioChannelPosition which follows the ordering defined in SMPTE 2036-2-2008[2]. NOTE that the relative order in the Top Layer is not exactly the same as the one from the WAVEFORMATEXTENSIBLE[1] specification; let's hope users using so may channels are already aware of such discrepancies. [1] http://msdn.microsoft.com/en-us/library/windows/hardware/dn653308%28v=vs.85%29.aspx [2] http://www.itu.int/dms_pub/itu-r/opb/rep/R-REP-BS.2159-2-2011-PDF-E.pdf Fixes: https://bugzilla.gnome.org/show_bug.cgi?id=737127