1823953 - Use libvorbis though ffmpeg

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Update ffmpeg to revision 37cde570bc2. r?alwu,#media-playback-reviewers

A regular update to current tip prior to adding a new decoder.

Comment 2

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Import libvorbis decoder integration into ffvpx. r?alwu,#media-playback-reviewers

Depends on D173456

Comment 3

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Create libgkcodecs, move libvorbis and libogg to it. r?#firefox-build-system-reviewers

We need to use libvorbis both from libavcodec and libxul during a transition
period while re-architecturing things, so this moves libvorbis and libogg (a
dependency) to a separate shared lib, linked when needed.

Depends on D173457

Comment 4

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Tell FFmpegAudioDecoder that it can decode vorbis now. r?alwu,#media-playback-reviewers

We'll control this via a pref in a subsequent patch. The ffvpx decoder module
always takes precedence to other decoders (such as the Agnostic Decoder Module
in this case) so we don't have anything else to do here.

Depends on D173458

Comment 5

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Loop over avcodec_receive_frame when decoding using FFmpegAudioDecoder. r?alwu,#media-playback-reviewers

It seems to work without, but the officiel ffmpeg command line utility and the
example for their internal library all do this.

This also splits the big DoDecode function into three sub-function for clarity,
as it was a bit hard with the #if statements.

Depends on D173459

Comment 6

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Control whether to decode vorbis using ffmpeg using a pref. r?alwu,#media-playback-reviewers

Depends on D173460

Comment 7

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Teach FFmpegAudioDecoder to now decode PCM. r?alwu,#media-playback-reviewers

It's needed for some codecs but not all of them, and the decoder that wraps
libvorbis is one of them. This is necessary when a codec capabilities flag
include: AV_CODEC_CAP_CHANNEL_CONF. All channels are decoded as they are
present in the file, and the sample rate is also necessary when this flag is
present.

https://searchfox.org/mozilla-central/rev/8433b62e54fd30663e82f090c4d31554531a2e66/dom/media/platforms/ffmpeg/ffmpeg57/include/libavcodec/avcodec.h#911-914

Depends on D173461

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Update ffvpx's README_MOZILLA to mention that it can use libvorbis now. r?alwu

Depends on D173461

Comment 10

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Add vorbis, opus, ogg headers to the system wrapper list, and fix all users of those headers to consistently include those headers with <>. r?glandium

Comment 11

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - bug603918.webm now plays, like on other browser. r?alwu

Depends on D181526

Comment 12

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Format test_mediaDecoding.html. r?karlt

Depends on D181378

Comment 13

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Regenerate expected data for test_mediaDecoding.html. r?karlt

When inverting the phase of the new results, and summing with the previous
expected result, the resulting audio has an RMS level of -83.6091dB. This is
because ffmpeg converts to int16_t after decoding, when using libvorbis.

Depends on D181529

Comment 14

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Adjust utility process tests: the expected decoders are now ffmpeg. r?gerard-majax

Depends on D182189

Comment 15

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Move libtheroa to libgkcodecs. r?alwu

Comment 16

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Clean up FFmpegAudioDecoder loops and provide the actual bytestream. r?alwu

This simplifies the code. ffmpeg docs say that sending should always work, so
we're asserting in this case.

Depends on D182576

Comment 17

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Fix switch statement. r?alwu

Depends on D182790

Comment 18

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Add a logging statement when draining when using FFmpegDataDecoder. r?alwu

Depends on D182791

Comment 19

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Handle a quirk of the libvorbis API in FFmpegAudioDecoder. r?alwu

It's possible to submit a packet and to have no audio returned, only when
decoding vorbis:

https://searchfox.org/mozilla-central/source/dom/media/platforms/agnostic/VorbisDecoder.cpp#184

This causes a mismatch between trimmers and packets, and cause the wrong packet
to be trimmed. Instead, when decoding vorbis, we insert a phony packet when we
encounter this situation, so that we can keep the rest of the system intact and
uniform.

Depends on D182792

Comment 20

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Properly free extradata in FFmpegAudioDecoder if avcodec_open2 fails. r?alwu

Depends on D182794

Comment 21

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Add a suppression for vorbis_synthesis_init. r?decoder,mccr8

Comment 22

[Paul Adenot (:padenot)]

Attachment: Bug 1823953 - Add logging to diagnose intermittent. r?alwu

Depends on D183279

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05dc13775fd6
Update ffmpeg to revision 37cde570bc2. r=alwu,media-playback-reviewers DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/1000c4a6a92a
Create libgkcodecs, move libvorbis and libogg to it. r=firefox-build-system-reviewers,geckoview-reviewers,glandium,owlish DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/b9ca30a0a066
Import libvorbis decoder integration into ffvpx. r=alwu,media-playback-reviewers DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/93042f1becec
Tell FFmpegAudioDecoder that it can decode vorbis now. r=alwu,media-playback-reviewers DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/f7f4a7585ceb
Loop over avcodec_receive_frame when decoding using FFmpegAudioDecoder. r=alwu,media-playback-reviewers DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/4b1dc01525af
Control whether to decode vorbis using ffmpeg using a pref. r=alwu,media-playback-reviewers DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/3113ad2e0987
Update ffvpx's README_MOZILLA to mention that it can use libvorbis now. r=alwu DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/7827183776e1
Teach FFmpegAudioDecoder to now decode PCM. r=alwu,media-playback-reviewers DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/8a47f6882e61
Add vorbis, opus, ogg headers to the system wrapper list, and fix all users of those headers to consistently include those headers with <>. r=glandium,media-playback-reviewers,webrtc-reviewers,mjf,karlt DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/3ea1f43e4024
bug603918.webm now plays, like on other browser. r=alwu DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/a4eb210620ff
Format test_mediaDecoding.html. r=karlt DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/f08ee5de9370
Regenerate expected data for test_mediaDecoding.html. r=karlt DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/a25abd05302c
Adjust utility process tests: the expected decoders are now ffmpeg. r=gerard-majax DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/828792b886bd
Move libtheroa to libgkcodecs. r=alwu DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/8929288d5aec
Clean up FFmpegAudioDecoder loops and provide the actual bytestream. r=alwu DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/c0249c90bc31
Fix switch statement. r=alwu DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/c11b58ac0709
Add a logging statement when draining when using FFmpegDataDecoder. r=alwu DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/650e35803834
Handle a quirk of the libvorbis API in FFmpegAudioDecoder. r=alwu DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/4b841e8dd033
Properly free extradata in FFmpegAudioDecoder if avcodec_open2 fails. r=alwu DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/85785505b6d6
Add a suppression for vorbis_synthesis_init. r=decoder DONTBUILD
https://hg.mozilla.org/integration/autoland/rev/61356e1447e3
1826382, 1828912, 1833654, 1837160, 1839389, 1839391, 1840399, 1840402, 1840869, 603918: apply code formatting via Lando

Comment 24

[Cristian Tuns]

Backed out for causing build bustages

• Backout link
• Push with failures
• Failure Log

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/471e9f82be05
Update ffmpeg to revision 37cde570bc2. r=alwu,media-playback-reviewers
https://hg.mozilla.org/integration/autoland/rev/48505a6dd45f
Create libgkcodecs, move libvorbis and libogg to it. r=firefox-build-system-reviewers,geckoview-reviewers,glandium,owlish
https://hg.mozilla.org/integration/autoland/rev/255a8d31fa30
Import libvorbis decoder integration into ffvpx. r=alwu,media-playback-reviewers
https://hg.mozilla.org/integration/autoland/rev/b141c6be7a35
Tell FFmpegAudioDecoder that it can decode vorbis now. r=alwu,media-playback-reviewers
https://hg.mozilla.org/integration/autoland/rev/cd7210964527
Loop over avcodec_receive_frame when decoding using FFmpegAudioDecoder. r=alwu,media-playback-reviewers
https://hg.mozilla.org/integration/autoland/rev/6ef95f6cfc70
Control whether to decode vorbis using ffmpeg using a pref. r=alwu,media-playback-reviewers
https://hg.mozilla.org/integration/autoland/rev/b5de80ded096
Update ffvpx's README_MOZILLA to mention that it can use libvorbis now. r=alwu
https://hg.mozilla.org/integration/autoland/rev/16cf423c977a
Teach FFmpegAudioDecoder to now decode PCM. r=alwu,media-playback-reviewers
https://hg.mozilla.org/integration/autoland/rev/21b00fe97569
Add vorbis, opus, ogg headers to the system wrapper list, and fix all users of those headers to consistently include those headers with <>. r=glandium,media-playback-reviewers,webrtc-reviewers,mjf,karlt
https://hg.mozilla.org/integration/autoland/rev/a66d57a2c942
bug603918.webm now plays, like on other browser. r=alwu
https://hg.mozilla.org/integration/autoland/rev/5afe59a72ef9
Format test_mediaDecoding.html. r=karlt
https://hg.mozilla.org/integration/autoland/rev/b63dba46cf98
Regenerate expected data for test_mediaDecoding.html. r=karlt
https://hg.mozilla.org/integration/autoland/rev/caa4ca652e45
Adjust utility process tests: the expected decoders are now ffmpeg. r=gerard-majax
https://hg.mozilla.org/integration/autoland/rev/e2eff775781c
Move libtheroa to libgkcodecs. r=alwu
https://hg.mozilla.org/integration/autoland/rev/420290afa063
Clean up FFmpegAudioDecoder loops and provide the actual bytestream. r=alwu
https://hg.mozilla.org/integration/autoland/rev/38f6e4c814d2
Fix switch statement. r=alwu
https://hg.mozilla.org/integration/autoland/rev/c74ebfe0f673
Add a logging statement when draining when using FFmpegDataDecoder. r=alwu
https://hg.mozilla.org/integration/autoland/rev/3bef9bfef223
Handle a quirk of the libvorbis API in FFmpegAudioDecoder. r=alwu
https://hg.mozilla.org/integration/autoland/rev/7804dfbaadea
Properly free extradata in FFmpegAudioDecoder if avcodec_open2 fails. r=alwu
https://hg.mozilla.org/integration/autoland/rev/edecf3c12704
Add a suppression for vorbis_synthesis_init. r=decoder
https://hg.mozilla.org/integration/autoland/rev/062a5e572906
1826382, 1828912, 1833654, 1837160, 1839389, 1839391, 1840399, 1840402, 1840869, 603918: apply code formatting via Lando

Comment 26

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/471e9f82be05
https://hg.mozilla.org/mozilla-central/rev/48505a6dd45f
https://hg.mozilla.org/mozilla-central/rev/255a8d31fa30
https://hg.mozilla.org/mozilla-central/rev/b141c6be7a35
https://hg.mozilla.org/mozilla-central/rev/cd7210964527
https://hg.mozilla.org/mozilla-central/rev/6ef95f6cfc70
https://hg.mozilla.org/mozilla-central/rev/b5de80ded096
https://hg.mozilla.org/mozilla-central/rev/16cf423c977a
https://hg.mozilla.org/mozilla-central/rev/21b00fe97569
https://hg.mozilla.org/mozilla-central/rev/a66d57a2c942
https://hg.mozilla.org/mozilla-central/rev/5afe59a72ef9
https://hg.mozilla.org/mozilla-central/rev/b63dba46cf98
https://hg.mozilla.org/mozilla-central/rev/caa4ca652e45
https://hg.mozilla.org/mozilla-central/rev/e2eff775781c
https://hg.mozilla.org/mozilla-central/rev/420290afa063
https://hg.mozilla.org/mozilla-central/rev/38f6e4c814d2
https://hg.mozilla.org/mozilla-central/rev/c74ebfe0f673
https://hg.mozilla.org/mozilla-central/rev/3bef9bfef223
https://hg.mozilla.org/mozilla-central/rev/7804dfbaadea
https://hg.mozilla.org/mozilla-central/rev/edecf3c12704
https://hg.mozilla.org/mozilla-central/rev/062a5e572906

Comment 27

[Intermittent Failures Robot]

2 failures in 3076 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• toolchains: 2

Platform and build breakdown:

• android-4-1-armv7: 1
  • debug: 1
• android-5-0-armv7: 1
  • debug: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1823953&startday=2023-08-14&endday=2023-08-20&tree=all