GNOME Bugzilla – Bug 317038
use default channel layout if none is specified in multichannel caps
Last modified: 2006-02-16 11:48:43 UTC
For example, [gst-head] [thomas@ana plugins]$ gst-launch -v -m sinesrc ! audioconvert ! audio/x-raw-int,channels=3,width=8,depth=8 ! fakesink silent=TRUE PAUSE pipeline ... /pipeline0/sinesrc0.src: caps = audio/x-raw-int, endianness=(int)1234, signed=(boolean)true, width=(int)16, depth=(int)16, rate=(int)44100, channels=(int)1 ** (process:1651): CRITICAL **: gst_audio_get_channel_positions: assertion `pos_val_arr != NULL' failed
for channels > 2 you need to specify a channel-positions property in the caps. We might want to use a default layout if the property is not there instead of this error.
Created attachment 52770 [details] [review] patch to make gst_audio_get_channel_positions() return default positions if no positions are set (for up to 7 channels) Attached patch makes gst_audio_get_channel_positions() return default positions if no positions are set (for up to 7 channels, don't know the default layout for 8 channels). Not entirely sure if this is the right thing to do though. If it makes things in plugins "just work" for more than 2 channels, then chances are that things won't work as expected in the end because the plugin didn't have multichannel supported implemented properly. I think Ronald (?) wanted to remove the default layout mapping even for mono and stereo in 0.9.x, at least there's a comment in the code implying that. Cheers -Tim
Still fails with a critical warning here. I don't think we should have to specify positions for mono and stereo data tho -- lots of pain for no gain.
Looking at this again, there are two questions: (a) Is this really a bug or just inconvenience that's worth paying to uncover potential bugs more easily? I don't think audioconvert is actually buggy here. So far the rule has been that if we have more than 2 channels, the producer or recipient must specify a channel layout in the caps. The warning from audioconvert simply notifies us of the fact that there are elements involved which don't adhere to that requirement, in this case capsfilter/fakesink. If we apply something like that patch above, a default layout will be used if none is provided explicitly. The question is if we really want that, because then we don't get warnings any longer when there are decoders or audiosinks or encoders involved that don't specify a channel layout. Users might just find that things 'sound weird', which isn't very helpful when trying to locate the problem later. (b) if we do want to use default layouts like in the patch - what should the default layouts be? The layouts in the patch don't exactly match the ones used by alsasink it seems (I can't remember the source of the above layouts though). Do we just want to use the layouts used by alsa?
What about an ugly warning if no layout is specified and use a default one (to sortof make something work until we fix the bad element)?
Created attachment 59466 [details] [review] new patch New patch. Prints warning in case of caps with more than 2 channels but without channel layout, and returns some default layout. So now: % gst-launch-0.10 audiotestsrc ! audioconvert ! audio/x-raw-int,channels=2 ! fakesink works without warning, % gst-launch-0.10 audiotestsrc ! audioconvert ! audio/x-raw-int,channels=3 ! fakesink works, but prints a few warnings, and % gst-launch-0.10 audiotestsrc ! audioconvert ! 'audio/x-raw-int,channels=3,channel-positions=(GstAudioChannelPosition)<GST_AUDIO_CHANNEL_POSITION_FRONT_LEFT,GST_AUDIO_CHANNEL_POSITION_FRONT_RIGHT,GST_AUDIO_CHANNEL_POSITION_FRONT_CENTER>' ! fakesink works without warnings.
Actually, I don't really see a good reason not to commit this, so ... 2006-02-16 Tim-Philipp Müller <tim at centricular dot net> * gst-libs/gst/audio/multichannel.c: (gst_audio_get_channel_positions): When we have more than 2 channels, but no channel layout is specified in the caps, return some default channel layout to the caller and warn about about a possibly buggy element (could be buggy filtercaps as well of course) (#317038).