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 761514 - riff: remove limits for sample rate and channel count
riff: remove limits for sample rate and channel count
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal enhancement
: 1.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-02-03 18:20 UTC by Carlos Rafael Giani
Modified: 2016-08-02 12:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
riff patch for 384 kHz sample ratess (762 bytes, patch)
2016-02-03 18:20 UTC, Carlos Rafael Giani
none Details | Review
sample rate & channel count limit removal patch (2.40 KB, patch)
2016-02-21 20:01 UTC, Carlos Rafael Giani
none Details | Review
Removed channel and sample rate restrictions (9.27 KB, patch)
2016-08-02 10:02 UTC, Carlos Rafael Giani
none Details | Review
Removed channel and sample rate restrictions (9.26 KB, patch)
2016-08-02 10:04 UTC, Carlos Rafael Giani
committed Details | Review

Description Carlos Rafael Giani 2016-02-03 18:20:35 UTC
Created attachment 320386 [details] [review]
riff patch for 384 kHz sample ratess

This allows for playing WAV files with more than 192 kHz sample rate. Some master recordings use this. Also, DoP (DSD-over-PCM) WAV files can have such high rates.
Comment 1 Nicolas Dufresne (ndufresne) 2016-02-03 18:30:19 UTC
Review of attachment 320386 [details] [review]:

Still seems arbitrary, is it specified, or is there a number of bits that could fixate the maximum ? (Looks good otherwise)
Comment 2 Carlos Rafael Giani 2016-02-03 19:03:38 UTC
I didn't find anything about this, so in theory, we shouldn't have a defined maximum at all. 192 kHz was probably chosen back then because this is a limit for most consumer-grade hardware.

Given the fact that WAV files are limited to 4 GB sizes, it sounds dubious to have a 2 GHz sample rate for example. However, this is a vague safety check at best, so if we want to get rid of arbitrary values, I'd get rid of the maximum in general.
Comment 3 Nicolas Dufresne (ndufresne) 2016-02-03 19:32:10 UTC
The orginal commit message just say "Support higher max audio rates for some formats (WAV, Vorbis, LPCM). b7f763b2

I think we should updated those 2 together at least.
Comment 4 Nicolas Dufresne (ndufresne) 2016-02-03 19:32:44 UTC
Review of attachment 320386 [details] [review]:

So let's update all 3, so at least it follows the original change.
Comment 5 Carlos Rafael Giani 2016-02-21 20:01:53 UTC
Created attachment 321790 [details] [review]
sample rate & channel count limit removal patch

Improved patch that effectively removes the limits instead of just arbitrarily raising them. Also lifts the limit for Vorbis and ADPCM (Microsoft only though for now due to unknown requirements for the other ADPCM schemes).
Comment 6 Sebastian Dröge (slomo) 2016-02-22 08:04:43 UTC
This limit is for demuxing, why is there a limit at all? If the file says 1000000 kHz sample rate, then we should probably just believe that and check if we have a decoder that handles it?
Comment 7 Sebastian Dröge (slomo) 2016-02-22 08:05:02 UTC
And I mean that in general for all the limits there, not just these specifically. Any opinions?
Comment 8 Carlos Rafael Giani 2016-02-22 08:19:59 UTC
With WAV, anything is possible, really. Just look at where WAV can be found these days. It is used way beyond its originally intended use cases, just like AVI was before MP4 and MKV became the commonly used container formats for video. Therefore, it is very difficult to come up with a reasonable value for a maximum sample rate (or channel count), at least for PCM. So yes, if there's a PCM WAV file with 1000000 kHz, then we should accept it.

However, other formats inside WAV may have an actual reason for a limit. This is why I didn't lift it from all of them - I do not know the other formats well enough for that. I guess the various ADPCM schemes could all have their sample rate limit lifted, but I just am not sure if this makes sense with them.
Comment 9 Sebastian Dröge (slomo) 2016-02-22 08:25:12 UTC
My point is that if there are limitations, then the further downstream elements should already know about them
Comment 10 Carlos Rafael Giani 2016-02-22 08:34:14 UTC
Right. But you can actually make this argument for any demuxer or parser. Limitations do make sense sometimes. That said, perhaps in the WAV case, almost all limitations should be gone, since there are virtually no specs for valid sample rate ranges etc. and only those which are clearly defined by the format should stay (for example, AMR NB/WB will never use anything other than 8 / 16 kHz).
Comment 11 Carlos Rafael Giani 2016-05-30 13:29:13 UTC
Alright, let's say we remove sample rate limitations. What about sample format and channel count? Should they be removed as well? (The code in riff-media.c can generate caps for templates and fixed caps for caps events - I am talking about the former.) Also, riff-media.c contains lines like this one: "/* FIXME: this is pretty useless - we need fixed caps */" but this is completely untrue if I want *template* caps? Are these FIXMEs wrong and/or pointless?
Comment 12 Carlos Rafael Giani 2016-08-02 10:02:54 UTC
Created attachment 332512 [details] [review]
Removed channel and sample rate restrictions

I have come to agree with the idea of getting rid of the channel count and sample rate restrictions. This patch does exactly that.
Comment 13 Carlos Rafael Giani 2016-08-02 10:04:06 UTC
Created attachment 332513 [details] [review]
Removed channel and sample rate restrictions

Typo
Comment 14 Sebastian Dröge (slomo) 2016-08-02 12:26:17 UTC
commit 9adaeb0a7118b36dcf2a4ce6d240df8d447641dd
Author: Carlos Rafael Giani <dv@pseudoterminal.org>
Date:   Tue Aug 2 12:03:18 2016 +0200

    riff: Remove sample rate and channel count boundaries in caps
    
    WAV is too generic to impose more-or-less arbitrary boundaries on the
    sample rate and channel count caps. For example, there are 384 kHz WAV
    files. Another example: it is in theory possible that somebody puts DSD
    data into a WAV file, which will then have a sample rate of ~2.8 MHz.
    
    For this reason, get rid of the rate and channel caps unless they are
    fixed values. Downstream anyway usually knows the limitations better.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761514