FFmpeg PDM uses too many threads

RESOLVED FIXED in Firefox 46

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

Trunk
mozilla46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

4 years ago
FFmpeg PDM currently use as many cores (virtual or otherwise) is available *per* decoder.

On your typical quad-core machines, that's 8 threads per video.

We should reduces the number of threads to a more reasonable amount, similar to what libvpx is doing and do so according to the resolution.

Currently this appear to be the primary suspect for bug 1236167.

We can end up in the layout/reftest/webm-video with over 2000 threads in use. This leads to various crashes in the gfx component due to OOM.
Assignee

Comment 1

4 years ago
A common initialization procedure is too restrictive, and we end up setting up audio configuration for video decoding and vice-versa
Attachment #8704545 - Flags: review?(gsquelart)
Assignee

Comment 2

4 years ago
We use the same logic as what is used in libvpx to ensure that we won't get a regression due to excessive memory/thread use when replacing libvpx with ffmpeg.
Attachment #8704546 - Flags: review?(gsquelart)
Comment on attachment 8704545 [details] [diff] [review]
[ffmpeg] P1. Have specialized AVCodecContext initialization.

Review of attachment 8704545 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with tiny nit (if you agree).

::: dom/media/platforms/ffmpeg/FFmpegAudioDecoder.cpp
@@ +47,5 @@
> +  uint32_t major, minor;
> +  FFmpegRuntimeLinker::GetVersion(major, minor);
> +  // LibAV 0.8 produces rubbish float interleaved samples, request 16 bits audio.
> +  mCodecContext->request_sample_fmt = major == 53 ?
> +    AV_SAMPLE_FMT_S16 : AV_SAMPLE_FMT_FLT;

Nit: For slightly better readability, I'd suggest moving the test down in the ?: statement, and adding parens around the test, i.e.:
  mCodecContext->request_sample_fmt =
    (major == 53) ? AV_SAMPLE_FMT_S16 : AV_SAMPLE_FMT_FLT;
Attachment #8704545 - Flags: review?(gsquelart) → review+
Attachment #8704546 - Flags: review?(gsquelart) → review+
Assignee

Comment 4

4 years ago
It was a copy/paste of the original code. But sure.

Comment 6

4 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e079e281ae09
https://hg.mozilla.org/mozilla-central/rev/84db9e4cf212
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.