GNOME Bugzilla – Bug 733444
wavenc: does not support more than 2 channel
Last modified: 2014-09-15 08:20:33 UTC
Created attachment 281230 [details] [review] Patch to wavenc to support more than 2 channels WAV could contain up to 65535 channels, but Microsoft recommends to use WAVFORMATEX for more than 2 channels since the channel mapping is otherwise undefined (http://msdn.microsoft.com/en-us/library/windows/hardware/dn653308%28v=vs.85%29.aspx). Currently wavenc aborts if it gets more than 2 channels. As long as we don't support WAVFORMATEx wavenc should accept more than 2 channels and just ignore the mapping.
Can you still mux a stereo stream that has channel-mask set?
All these examples still work: gst-launch-1.0 audiotestsrc ! audio/x-raw, channels=2, channel-mask=0 ! wavenc ! filesink location=q.wav gst-launch-1.0 audiotestsrc ! audio/x-raw, channels=2, channel-mask=3 ! wavenc ! filesink location=q.wav gst-launch-1.0 audiotestsrc ! audio/x-raw, channels=2 ! wavenc ! filesink location=q.wav
This patch is important for the following use cases: 1) File to file processing: since playback is irrelevant, the channel mapping is not important. 2) input of many channels, which are in a second step (another plug-in) rendered to the existing loudspeaker set-up. This is what happens for example in MPEG-H, the new standard for 3D audio. 3) In the professional market, you really want to play let's say 22 channels, but then you have the hardware and the know-how to route each channel to the required audio output.
See also bug #733444
Sorry, I meant bug #733405
Review of attachment 281230 [details] [review]: This mean that layout information will be ignored / lost when creating the wav file. We need to add a GST_FIXME trace when that happen.
Comment on attachment 281230 [details] [review] Patch to wavenc to support more than 2 channels Instead of this, please write a WAVFORMATEX header. Otherwise we produce multi-channel wav files that won't work properly in most software.
Without that patch we don't produce any output file at all, which seems slightly worse :-) But if we want to be correct, you're right. I'll look into WAVFORMATEX as soon as I find time and the rf64 wavenc patch is accepted (otherwise I'll get merge conflicts I don't want to handle).
I would argue that creating broken files without any notification about that is worse, those files will then probably be circulated and then people are confused why these files are broken and blame it on GStreamer ;)
To be honest in my professional life I get currently a lot more questions why gstreamer does not support these allegedly broken files (it's just the channel mapping which is missing). We are dealing with a lot of these multi channel wav files without anybody caring that WAVFORMATEX should be used. Out of curiosity, could you tell me which programs fail with this kind of files?
So maybe we should just add a property to wavenc to let it produce broken files and have this disabled by default? Basically what you would do if this is the default is that you will silently drop information (the channel layout), which is rather unexpected and should not happen without warning the user. The user would then be confused why his 5.1 WAV file he just created has all the channels mixed up when he plays it again and report a bug here.
I would recommend to add the reading of these files (bug #733405) as soon as possible, because it is fully backward compatible. For writing, I think it does not make sense to add an additional property, which has to be supported in the future. Until now I didn't see a program, which couldn't handle WAVFORMATEX, even though many of them just ignored the additional information. I'll implement writing of WAVFORMATEX as soon as the patches for bug #735627 are accepted because they touch the same source code.
(In reply to comment #12) > I would recommend to add the reading of these files (bug #733405) as soon as > possible, because it is fully backward compatible. Sure, that one seems ok but I had no time to merge it yet. > For writing, I think it does not make sense to add an additional property, > which has to be supported in the future. > Until now I didn't see a program, which couldn't handle WAVFORMATEX, even > though many of them just ignored the additional information. So you would suggest to just wait and then implement WAVFORMATEX support? Sounds like a better solution to me, especially considering that adding WAVFORMATEX support is relatively simple.
Created attachment 285642 [details] [review] wavenc: use WAVE_FORMAT_EXTENSIBLE for more than 2 channels
Review of attachment 285642 [details] [review]: ::: gst/wavenc/gstwavenc.c @@ +172,3 @@ +use_format_ext (GstWavEnc * wavenc) +{ + return wavenc->channels > 2 || wavenc->width > 16; Why for width > 16 too? @@ +247,3 @@ + /* TODO: how to get the number of samples for compressed data? */ + GST_WRITE_UINT32_LE (header + 8, + wavenc->audio_length / (wavenc->width / 8) / wavenc->channels); Duration query in TIME format, and then with the sample rate @@ +378,3 @@ wavenc->channels = chans; wavenc->rate = rate; + wavenc->channel_mask = mask; You need to convert the GStreamer channel mask to the WAV channel mask, and potentially have to reorder channels (there's convenience API for the latter in libgstaudio).
Created attachment 286139 [details] [review] wavenc: use WAVE_FORMAT_EXTENSIBLE for more than 2 channels This should set the mask correctly. But I haven't tested this since I don't have an audio file where the mask is set.
commit f8f61237f80013b89c6675c1a5f2ab1d1d6ff4f6 Author: Peter G. Baum <peter@dr-baum.net> Date: Mon Sep 8 14:06:00 2014 +0200 wavenc: use WAVE_FORMAT_EXTENSIBLE for more than 2 channels https://bugzilla.gnome.org/show_bug.cgi?id=733444