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 733444 - wavenc: does not support more than 2 channel
wavenc: does not support more than 2 channel
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.3.91
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-07-20 09:53 UTC by Peter G. Baum
Modified: 2014-09-15 08:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to wavenc to support more than 2 channels (1.45 KB, patch)
2014-07-20 09:53 UTC, Peter G. Baum
needs-work Details | Review
wavenc: use WAVE_FORMAT_EXTENSIBLE for more than 2 channels (8.85 KB, patch)
2014-09-08 12:12 UTC, Peter G. Baum
needs-work Details | Review
wavenc: use WAVE_FORMAT_EXTENSIBLE for more than 2 channels (12.16 KB, patch)
2014-09-13 16:56 UTC, Peter G. Baum
committed Details | Review

Description Peter G. Baum 2014-07-20 09:53:55 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.
Comment 1 Stefan Sauer (gstreamer, gtkdoc dev) 2014-07-23 19:45:31 UTC
Can you still mux a stereo stream that has channel-mask set?
Comment 2 Peter G. Baum 2014-07-24 19:02:08 UTC
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
Comment 3 Peter G. Baum 2014-08-28 15:32:18 UTC
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.
Comment 4 Peter G. Baum 2014-08-28 15:36:27 UTC
See also bug #733444
Comment 5 Peter G. Baum 2014-08-28 15:37:15 UTC
Sorry, I meant bug #733405
Comment 6 Nicolas Dufresne (ndufresne) 2014-08-28 16:15:23 UTC
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 7 Sebastian Dröge (slomo) 2014-08-29 09:04:18 UTC
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.
Comment 8 Peter G. Baum 2014-08-29 14:37:53 UTC
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).
Comment 9 Sebastian Dröge (slomo) 2014-09-01 06:55:58 UTC
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 ;)
Comment 10 Peter G. Baum 2014-09-07 14:29:24 UTC
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?
Comment 11 Sebastian Dröge (slomo) 2014-09-07 14:36:03 UTC
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.
Comment 12 Peter G. Baum 2014-09-07 15:00:16 UTC
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.
Comment 13 Sebastian Dröge (slomo) 2014-09-07 17:32:47 UTC
(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.
Comment 14 Peter G. Baum 2014-09-08 12:12:28 UTC
Created attachment 285642 [details] [review]
wavenc: use WAVE_FORMAT_EXTENSIBLE for more than 2 channels
Comment 15 Sebastian Dröge (slomo) 2014-09-12 11:50:06 UTC
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).
Comment 16 Peter G. Baum 2014-09-13 16:56:54 UTC
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.
Comment 17 Sebastian Dröge (slomo) 2014-09-15 08:20:13 UTC
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