Bug 201399 - [GStreamer] Sound is down-pitched when playing video from YLE Areena
Summary: [GStreamer] Sound is down-pitched when playing video from YLE Areena
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
Depends on:
Reported: 2019-09-02 04:51 PDT by Alberto Garcia
Modified: 2019-09-11 03:40 PDT (History)
13 users (show)

See Also:

screenshot (62.22 KB, image/png)
2019-09-03 04:28 PDT, Philippe Normand
no flags Details
WiP patch (1.85 KB, patch)
2019-09-03 04:30 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (2.51 KB, patch)
2019-09-03 08:13 PDT, Philippe Normand
calvaris: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alberto Garcia 2019-09-02 04:51:47 PDT
If I try to play a news clip from the Finnish TV the sound is completely broken (you can notice it with the voice of the host).

Here's one example: https://areena.yle.fi/1-50270885

I can reproduce the problem with WebKitGTK 2.24.4 (MiniBrowser and Epiphany). Firefox can play this just fine.
Comment 1 Adrian Perez 2019-09-02 04:56:42 PDT
I can reproduce this with recent builds of the GTK and WPE ports (r249368)
using GStreamer 1.16.0, both with MSE enabled and disabled at runtime (for
example passing the “--enable-mediasource=<boolean>” flag to Cog)

Playing around with things I have noticed that audio will play with the
correct pitch when “gst-libav” is NOT in use, but in that case usually
only <2s of audio are played, and video output remains black; seeking
will cause another chunk of <2s of audio to play with the correct pitch.
Comment 2 Philippe Normand 2019-09-03 02:57:15 PDT
The black video might be bug 201401
Comment 3 Philippe Normand 2019-09-03 03:21:21 PDT
I confirm this is a bug in gst-libav. Forcing fdkaacdec I hear Finnish news loud and clear :P
Comment 4 Philippe Normand 2019-09-03 03:22:07 PDT
Adding Alicia in CC, our AAC expert :)
Comment 5 Philippe Normand 2019-09-03 03:41:54 PDT
Alberto, can you confirm this plays in a similar borken fashion? https://ffmpeg.org/pipermail/ffmpeg-devel/2019-July/247063.html
Comment 6 Philippe Normand 2019-09-03 03:43:55 PDT
I started a patch downranking the libav AAC decoders and promoting the fdkaac decoder if it's available at runtime.
Comment 7 Alberto Garcia 2019-09-03 04:07:00 PDT
(In reply to Philippe Normand from comment #5)
> Alberto, can you confirm this plays in a similar borken fashion?
> https://ffmpeg.org/pipermail/ffmpeg-devel/2019-July/247063.html

This is broken in all players that I have tried, including Firefox and Chromium.
Comment 8 Philippe Normand 2019-09-03 04:28:56 PDT
Created attachment 377888 [details]

I have audio rendering well now (will attach a WIP patch) but no video, it blinks with a zoomed rendering of the media controls...
Comment 9 Philippe Normand 2019-09-03 04:30:23 PDT
Created attachment 377889 [details]
WiP patch

Can you try this please?
Comment 10 Adrian Perez 2019-09-03 06:42:49 PDT
(In reply to Philippe Normand from comment #9)
> Created attachment 377889 [details]
> WiP patch
> Can you try this please?

With gst-libav *and* this patch applied both the audio and video
play fine \o/

Removing gst-libav results in the same as before: some <2s of
audio play fine, no video — then again this seems to be bug 201401
as already pointed.
Comment 11 Philippe Normand 2019-09-03 06:45:51 PDT
I suspect gst-libav is used for video decoding in your case, so yeah, removing gst-libav would lead to disabled video :)
Comment 12 Alicia Boya García 2019-09-03 07:41:21 PDT
ffmpeg's aac decoder has a number of bugs that fdkaac doesn't, and this isn't the first one we find.

This patch adds fragmentation to the user base, which is bad (different decoder gets picked depending on what gstreamer packages the user has installed, which they usually are not aware of), but at least it gives users a way to fix the issue...

The real solution of course would be having ffmpeg's own aac decoder fixed. But until that happens and since this is not the first one these bugs arise, I am okay with this solution to some degree.
Comment 13 Philippe Normand 2019-09-03 08:13:06 PDT
Created attachment 377895 [details]
Comment 14 Philippe Normand 2019-09-04 02:05:33 PDT
Committed r249477: <https://trac.webkit.org/changeset/249477>
Comment 15 Alicia Boya García 2019-09-04 05:48:47 PDT
I've given a look at the file and found it's actually the same bug as in ted.com: the file uses downsampled SBR and ffmpeg corrupts the audio when handling it.

| # bits | Purpose                          | Value                        | Notes |
| ------ | -------------------------------- | ---------------------------- | ----- |
| 5      | audioObjectType                  | 5 (SBR)                      |       |
| 4      | samplingFrequencyIndex           | 3 (48000 Hz)                 |       |
| 4      | channelConfiguration             | 2 (stereo)                   |       |
| 4      | extensionSamplingFrequencyIndex  | 3 (48000 Hz)                 |       |
| 5      | nested audioObjectType           | 2 (AAC LC)                   |       |
| 1      | GA frameLengthFlag               | 0 (1024/128 lines IMDCT)     |       |
| 1      | GA dependsOnCoreCoder            | 0                            |       |
| 1      | GA extensionFlag                 | 0                            |       |
sbrPresentFlag: 1
psPresentFlag: -1

Indeed, the same workaround of doubling extensionSamplingFrequencyIndex works.

Of all the possible bugs an AAC decoder can have this one is probably not the hardest to fix.
Comment 16 Charlie Turner 2019-09-11 03:40:21 PDT
Comment on attachment 377895 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=377895&action=review

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:260
> +        GRefPtr<GstElement> aacDecoder = adoptGRef(gst_element_factory_make("fdkaacdec", nullptr));

Please be careful not to adopt floating references, this is not correct. You need to sink them instead. Fixed in 201685.