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 489010 - Please change default channel order for WAVE_EXT-less .wav files to follow ALSA's behaviour
Please change default channel order for WAVE_EXT-less .wav files to follow AL...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.15
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-10-22 13:44 UTC by Lennart Poettering
Modified: 2007-10-28 11:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
multichannel-wav.diff (9.51 KB, patch)
2007-10-23 14:25 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Lennart Poettering 2007-10-22 13:44:26 UTC
In gst-libs/gst/riff/riff-media.c:781 the fallback channel order for .WAV files without WAVE_FORMAT_EXTENSIBLE is defined. It defaults to something inspired by WAVEX. All .WAV files I could find on the Internet which lack WAVE_FORMAT_EXTENSIBLE follow a different order however, which happens to be the native one ALSA and AC3 uses. Specifically and most notable chan-id.wav from the SDL surround test files uses the ALSA/AC3 order. The SDL surround test files are arguably the most used test cases for surround sound:

ftp://ling.lll.hawaii.edu/pub/greg/Surround-SDL-testfiles.tgz

I would thus suggest that GST follows the ALSA/AC3 order, not because some formal standard for .WAV would demand so, but just because this seems to be the order which is most often used for .WAV without WAVE_FORMAT_EXTENSIBLE.

It would be great if this gst-launch pipeline for chan_id.wav would yield proper results out-of-the-box, which it currently does not:

gst-launch filesrc location=chan_id.wav ! wavparse ! pulsesink
Comment 1 Sebastian Dröge (slomo) 2007-10-22 17:25:14 UTC
I'll take a look at this tomorrow, would be nice if you could try the resulting patch then :)
Comment 2 Sebastian Dröge (slomo) 2007-10-23 14:25:51 UTC
Created attachment 97733 [details] [review]
multichannel-wav.diff

Is this patch working fine for you? Apart from your bug it also adds support for more numbers of channels with no channel mask set.
Comment 3 Michael Smith 2007-10-25 14:02:01 UTC
Some discussion on irc led to us deciding that the patch is probably sensible, but we want someone with _actual_ surround hardware to test it.

We also noted that this is NOT the ac3 order (ac3 order is different from the alsa order, and also different from what the existing code does), so we'll fix up the comments to not refer to AC3.
Comment 4 Lennart Poettering 2007-10-26 22:46:22 UTC
OK, I did some testing now. Works perfectly.

However, before this is being committed, please remove the debug output.

Thanks!
Comment 5 Sebastian Dröge (slomo) 2007-10-28 07:54:47 UTC
Which debug output? 
Comment 6 Lennart Poettering 2007-10-28 11:40:48 UTC
woops, sorry. May bad. It was the surround patch I recently commited to gst-pulse which was the cause of that debug output. Your patch is fine.

Sorry for the confusion.

Please go a ahead and commit!

Thanks!
Comment 7 Sebastian Dröge (slomo) 2007-10-28 11:46:31 UTC
Great, thanks for testing :)

2007-10-28  Sebastian Dröge  <slomo@circular-chaos.org>

        (gst_riff_wavext_add_channel_layout),
        (gst_riff_wave_add_default_channel_layout),
        (gst_riff_wavext_get_default_channel_mask),
        (gst_riff_create_audio_caps):
        Use the ALSA channel layout as default for wav files without channel
        layout information. This fixes playback of chan-id.wav on 5.1 systems
        for example. Also refactor the channel layout setting a bit and add
        more default channel orders. Fixes #489010.