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 317038 - use default channel layout if none is specified in multichannel caps
use default channel layout if none is specified in multichannel caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.4
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-09-23 14:19 UTC by Thomas Vander Stichele
Modified: 2006-02-16 11:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to make gst_audio_get_channel_positions() return default positions if no positions are set (for up to 7 channels) (2.95 KB, patch)
2005-09-28 10:56 UTC, Tim-Philipp Müller
none Details | Review
new patch (4.12 KB, patch)
2006-02-16 10:42 UTC, Tim-Philipp Müller
committed Details | Review

Description Thomas Vander Stichele 2005-09-23 14:19:36 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
Comment 1 Wim Taymans 2005-09-24 09:24:17 UTC
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.
Comment 2 Tim-Philipp Müller 2005-09-28 10:56:25 UTC
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
Comment 3 Andy Wingo 2005-11-14 17:50:20 UTC
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.
Comment 4 Tim-Philipp Müller 2006-02-14 17:52:18 UTC
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?

Comment 5 Wim Taymans 2006-02-14 18:22:30 UTC
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)?
Comment 6 Tim-Philipp Müller 2006-02-16 10:42:09 UTC
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.
Comment 7 Tim-Philipp Müller 2006-02-16 11:48:43 UTC
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).