After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 398033 - [audioconvert] support more than 8 channels
[audioconvert] support more than 8 channels
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal enhancement
: 0.10.20
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 395597 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-01-18 15:09 UTC by Michael Sheldon
Modified: 2008-05-06 12:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
tentative patch; adds conversion support for an unlimited number of unpositioned channels (7.65 KB, patch)
2007-01-23 11:25 UTC, Tim-Philipp Müller
none Details | Review
add some unit tests for unpositioned channel conversion (13.70 KB, patch)
2007-01-23 11:41 UTC, Tim-Philipp Müller
none Details | Review
new patch (23.20 KB, patch)
2007-01-23 12:55 UTC, Tim-Philipp Müller
none Details | Review
Level 5 log from attempting to use 12 channels with the new patch (470.77 KB, application/gzip)
2007-01-23 19:52 UTC, Michael Sheldon
  Details
third attempt (25.18 KB, patch)
2007-01-24 11:37 UTC, Tim-Philipp Müller
none Details | Review
Third attempt patch to audiotconvert and testsplit.py script log (879.83 KB, application/x-gzip)
2007-01-25 10:44 UTC, Peteris Krisjanis
  Details
New patch (28.31 KB, patch)
2007-01-26 16:10 UTC, Tim-Philipp Müller
none Details | Review
same patch, but against current cvs (28.53 KB, patch)
2007-03-21 12:03 UTC, Tim-Philipp Müller
committed Details | Review
Gst debug log of level 5 using patch #13 + cvs gst + test script (554.45 KB, application/x-gzip)
2007-03-22 19:03 UTC, Peteris Krisjanis
  Details
Gst debug log of level 5 using patch #13 + cvs gst 25th March + test script (629.10 KB, application/x-gzip)
2007-03-24 23:53 UTC, Peteris Krisjanis
  Details

Description Michael Sheldon 2007-01-18 15:09:20 UTC
Please describe the problem:
The audioconvert element only supports a maximum of 8 channels. This causes problems with my Delta 44 sound card (as well as many other similar cards), which needs to send a 12 channel stream through audioconvert before it can be deinterleaved with the deinterleave plugin.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Michael Sheldon 2007-01-18 15:09:35 UTC
*** Bug 395597 has been marked as a duplicate of this bug. ***
Comment 2 Peteris Krisjanis 2007-01-18 20:08:03 UTC
Two post scriptums: such cards can hold up to 24 channels, AND I would like to add a remark that lot of gstreamer audio elements doesn't support more than eight channels. Question is - it is design issue to have more channels or it was left for future to improve.
Comment 3 Tim-Philipp Müller 2007-01-18 20:18:55 UTC
Don't think it's a design issue. However, the thing is that caps for more than 2 channels currently need to have a channel layout attached to them; having to specify these for lots of channels makes caps negotiation much slower; maybe we need to support default layouts for > 8 channels (with NONE positions for all channels) so this isn't so much of an issue then.
Comment 4 Tim-Philipp Müller 2007-01-23 11:25:24 UTC
Created attachment 80965 [details] [review]
tentative patch; adds conversion support for an unlimited number of unpositioned channels
Comment 5 Tim-Philipp Müller 2007-01-23 11:41:36 UTC
Created attachment 80966 [details] [review]
add some unit tests for unpositioned channel conversion

Unit test seems to pass, but haven't done a lot of testing of these patches with multichannel positioned audio yet. I'm fairly sure there are still some kinks that need to be ironed out (I've seen an assertion warning somewhere at least).
Comment 6 Tim-Philipp Müller 2007-01-23 12:55:59 UTC
Created attachment 80968 [details] [review]
new patch
Comment 7 Tim-Philipp Müller 2007-01-23 12:57:56 UTC
Comment on attachment 80968 [details] [review]
new patch

The warning that was triggered with the previous patch in the case of  positioned 8-channel audio was just a thinko and harmless - it came from a [8, 8] integer range. This patch fixes this and adds a testcase for this to the unit test as well.
Comment 8 Michael Sheldon 2007-01-23 19:52:53 UTC
Created attachment 81014 [details]
Level 5 log from attempting to use 12 channels with the new patch

I've tried getting audio from my Delta 44 (12 channels) with the new patch, but I get the following warning:

** (testsplit.py:14003): WARNING **: Failed to retrieve channel layout from caps. This usually means there is a GStreamer element that does not implement multichannel audio correctly. Please file a bug.

Followed by an internal data flow error.
Comment 9 Tim-Philipp Müller 2007-01-24 11:37:58 UTC
Created attachment 81057 [details] [review]
third attempt

Third try: the above problem was caused by two things:

 - audioconvert's fixate function did not copy the channel-positions
   over where possible/appropriate (it doesn't even make an attempt
   to fixate channel positions at all either, but that's a different
   problem and not really relevant in practice, I think).

 - audioconvert's transform_caps function did not set the
   channel-positions field on the structures of the returned caps
   either

An additional/alternative hack/solution could be to make gst_audio_get_channel_positions() automatically return a set of NONE positions for caps with more than 8 channels and no channel-positions in the caps, but then that would just cover up problems with code that will fail if there are <= 8 channels with NONE positions.
Comment 10 Peteris Krisjanis 2007-01-25 10:44:26 UTC
Created attachment 81162 [details]
Third attempt patch to audiotconvert and testsplit.py script log

I applied patch to CVS version. In result, testscript.py gave me strange results:
1) When I launched it first time, it created 12 oggs, and spawned lot of error messages something about channel. I didn't log results and didn't listened to oggs, because I tought I could reproduce it

2) BUT in second launch it didn't reproduce it, but it went to create two splits from PCM instead, creating two oggs. Both of them contains constant click noise, nothing more;

This log is from second launch.
Comment 11 Michael Sheldon 2007-01-25 10:56:17 UTC
I think that's more a problem with the test script than anything, I've edited the test script slighty and have it producing oggs without any errors (although the oggs are only a very short length I think this is due to the stream not being allowed to finish properly, just ctrl-c'd). I'll try and integrate deinterleave into Jokosher over the next few days so we can see how it works in a proper pipeline. But it looks to me as though the audioconvert problem is fixed now, thanks a lot :).
Comment 12 Tim-Philipp Müller 2007-01-26 16:10:55 UTC
Created attachment 81273 [details] [review]
New patch

Previous patch still has a problem: I just ran into this assertion with the thehillshaveeyes trailer:

** CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed

caps has 2 channels and a channel-position array with 8 channel positions. Looks like somewhere (in transform_caps?) we manage to produce caps with a [1; 8] channel range AND channel-positions and the fixate function later fixates the channel number on that later. The bug might have existed before but was conveniently covered up by the fact that the fixate function stripped channel positions in certain circumstances anyway, I think.

The updated patch removes channel-positions fields from the new output structure in transform_caps() if setting a channel range on the copied structure. This fixes problems if the input structure had 8 channels input.

Also added unit tests for this problem.
Comment 13 Tim-Philipp Müller 2007-03-21 12:03:14 UTC
Created attachment 85035 [details] [review]
same patch, but against current cvs
Comment 14 Peteris Krisjanis 2007-03-22 18:59:50 UTC
Last patch (#13) + Gst CVS from 22nd of March + test script gave me this:

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src0 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src1 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src2 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src3 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src4 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src5 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src6 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src7 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src8 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src9 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed
/bin0/deinterleave0.src10 (gst.Pad)

** (test.py:9667): CRITICAL **: gst_audio_get_channel_positions: assertion `gst_value_array_get_size (pos_val_arr) == channels' failed


GST_DEBUG=*:5 is coming.
Comment 15 Peteris Krisjanis 2007-03-22 19:03:16 UTC
Created attachment 85127 [details]
Gst debug log of level 5 using patch #13 + cvs gst + test script
Comment 16 Peteris Krisjanis 2007-03-24 23:47:17 UTC
Last patch #13 + Gst CVS from 25nd of March  + test script gave me no errors spawned as previously mentioned, and deinterleave gave all 12 channels, bet there was no output produced at all - just zero size 0.ogg, and no other ogg files for corresponding channels.

See attachment for gst level 5 log.
Comment 17 Peteris Krisjanis 2007-03-24 23:53:28 UTC
Created attachment 85238 [details]
Gst debug log of level 5 using patch #13 + cvs gst 25th March + test script
Comment 18 Sebastian Dröge (slomo) 2007-12-12 12:26:48 UTC
Any news on this?
Comment 19 Tim-Philipp Müller 2007-12-12 12:51:52 UTC
> Any news on this?

Yes, I've ported to patch to cvs head, but the unit tests fail in ways which I didn't expect now and I haven't had time to look into why that is exactly.


Comment 20 Sebastian Dröge (slomo) 2008-05-06 12:11:24 UTC
2008-05-06  Sebastian Dröge  <slomo@circular-chaos.org>

        Based on a patch by: Tim-Philipp Müller  <tim.muller at collabora co uk>

        * gst/audioconvert/audioconvert.c: (audio_convert_prepare_context):
        * gst/audioconvert/audioconvert.h:
        * gst/audioconvert/gstaudioconvert.c:
        (gst_audio_convert_parse_caps),
        (structure_has_fixed_channel_positions),
        (gst_audio_convert_transform_caps):
        * gst/audioconvert/gstchannelmix.c: (gst_channel_mix_fill_matrix):
        Add support for more than 8 channels and NONE channel layouts. For
        more than 8 channels no channel conversion is supported yet, only
        format conversions are supported. Fixes bug #398033.

        * tests/check/elements/audioconvert.c: (verify_convert),
        (GST_START_TEST), (audioconvert_suite):
        Add some unit tests by Tim for checking the NONE channel layouts
        and more than 8 channels and add some more unit tests for channel
        conversions.