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 765684 - opusdec: Won't negotiate sampling rate anymore
opusdec: Won't negotiate sampling rate anymore
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.8.1
Other Linux
: Normal blocker
: 1.8.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-04-27 16:34 UTC by Nicolas Dufresne (ndufresne)
Modified: 2016-06-01 07:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
opusdec: remove artificial restriction on rate negotiation (1.51 KB, patch)
2016-05-02 17:29 UTC, Thiago Sousa Santos
committed Details | Review
opusdec: improve getcaps to return all possible rates (6.08 KB, patch)
2016-05-02 17:29 UTC, Thiago Sousa Santos
committed Details | Review
opusdec: intersect with the filter before returning on getcaps (2.80 KB, patch)
2016-05-02 17:29 UTC, Thiago Sousa Santos
committed Details | Review

Description Nicolas Dufresne (ndufresne) 2016-04-27 16:34:51 UTC
In 1.6, Opusdec could negotiate downstream format, specially that rate. So it would downsample to 8kHZ from it's natice 48kHz stream. As of 1.8+, it will not negotiate:

gst-launch-1.0 -v audiotestsrc ! audio/x-raw,rate=48000 ! opusenc ! opusdec ! audio/x-raw,rate=8000 ! fakesink
Comment 1 Thiago Sousa Santos 2016-05-02 02:52:49 UTC
There is something making the code trying to force too much the output to be 48000 when the input is already 48000. I couldn't find any restriction on this on the opus documentation so I guess this is a mistake.

Will continue investigating this tomorrow.
Comment 2 Sebastian Dröge (slomo) 2016-05-02 06:14:07 UTC
You can tell the Opus decoder API to produce any (supported) sample rate, independent of the input sample rate.
Comment 3 Nicolas Dufresne (ndufresne) 2016-05-02 13:39:00 UTC
Also, specially for the decoder, the resampling is extremely cheap. So we could favour the best quality, but there is no reason to enforce that.
Comment 4 Thiago Sousa Santos 2016-05-02 17:29:32 UTC
Created attachment 327173 [details] [review]
opusdec: remove artificial restriction on rate negotiation

Remove restrictions when rate is 48000, the underlying lib supports
converting any of the input to any of the output rates.
Comment 5 Thiago Sousa Santos 2016-05-02 17:29:37 UTC
Created attachment 327174 [details] [review]
opusdec: improve getcaps to return all possible rates

The library is capable of converting to different rates.

Includes tests.
Comment 6 Thiago Sousa Santos 2016-05-02 17:29:44 UTC
Created attachment 327175 [details] [review]
opusdec: intersect with the filter before returning on getcaps

So upstream gets a smaller set to decide upon as it is what it requested
with the filter
Comment 7 Sebastian Dröge (slomo) 2016-05-03 07:45:39 UTC
Comment on attachment 327173 [details] [review]
opusdec: remove artificial restriction on rate negotiation

The intention here seemed to have been to only use the "native" samplerate of Opus (48000) or the original one of the stream, and not resample to some arbitrary sample rate. But I don't see much of a reason for that here, other than accidentially downsampling something.
Comment 8 Thiago Sousa Santos 2016-05-03 10:42:31 UTC
Pushed to master:

commit 60c765174f02a16eab0480c6c9971c3edfbeeed9
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Mon May 2 14:21:55 2016 -0300

    opusdec: intersect with the filter before returning on getcaps
    
    So upstream gets a smaller set to decide upon as it is what it requested
    with the filter
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765684

commit 7a5797d3a62e47cd6cc7426dac843867445fc7d9
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Mon May 2 10:23:09 2016 -0300

    opusdec: improve getcaps to return all possible rates
    
    The library is capable of converting to different rates.
    
    Includes tests.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765684

commit 823832e293d6597454b9ddd4d06ebdae81515f60
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Mon May 2 10:21:52 2016 -0300

    opusdec: remove artificial restriction on rate negotiation
    
    Remove restrictions when rate is 48000, the underlying lib supports
    converting any of the input to any of the output rates.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765684
Comment 9 Thiago Sousa Santos 2016-05-03 10:51:05 UTC
Also in 1.8 (with a few related patches to apply cleanly):
commit 7ac32601c8a185292748b75ff2f4004cb2b8fa1b
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Mon May 2 14:21:55 2016 -0300

    opusdec: intersect with the filter before returning on getcaps
    
    So upstream gets a smaller set to decide upon as it is what it requested
    with the filter
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765684

commit 8587548767e07d679d8ad261e8d8acb9155b0a33
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Mon May 2 10:23:09 2016 -0300

    opusdec: improve getcaps to return all possible rates
    
    The library is capable of converting to different rates.
    
    Includes tests.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765684

commit 9e45251cece1072de3bc0086b6ee5ce91d67d3bd
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Mon May 2 10:21:52 2016 -0300

    opusdec: remove artificial restriction on rate negotiation
    
    Remove restrictions when rate is 48000, the underlying lib supports
    converting any of the input to any of the output rates.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765684

commit ba989ddf309ae575abd30443366778d913e1cf1d
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Sun May 1 23:19:57 2016 -0300

    opusdec: refactor getcaps repeated code into a function
    
    Easier to read and maintain

commit a089191add06e39f248a22e8f17fceb261ce59e0
Author: Thiago Santos <thiagoss@osg.samsung.com>
Date:   Mon May 2 10:36:07 2016 -0300

    tests: opus: remove apparently useless macro in tests