1474684 - Stack overflows when decoding av1

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Description

[Dan Minor [:dminor]]

The stack trace can be seen here: https://bugzilla.mozilla.org/show_bug.cgi?id=1445683#c50

This did not occur after I rebased on top of central, but there's a chance that we're close to running out of stack space when decoding and that the crash may start happening again.

Comment 1

[Dan Minor [:dminor]]

Setting this as a low priority until we see how frequently this problem occurs once the aom update lands.

Comment 2

[(Away)]

If you can get me a minidump of this happening on Windows, I'd really like to take a peek in case I can find a way around this that doesn't increase stack size.

Comment 3

[Jean-Yves Avenard [:jya]]

dmajor: "What info is needed in the needinfo?

Now that bug 1474684 is filed let's move stack size discussion there.
"

That if the solution of using different stack sizes depending on thread type would be enough for now. That is bump the stack size to 512kB for the media threads, all the others stay as 256kB.

For now that's a default a 8 threads impacted only.

Comment 4

[(Away)]

If I don't have any better ideas after looking at a minidump, then adjusting only the media threads sounds fine.

Comment 5

[Dan Minor [:dminor]]

Checked this morning, and a local windows build is fine using my test file. If something shows up after I land the aom update on nightly, I'll attach it here.

Comment 6

[Dan Minor [:dminor]]

The plan is to increase the stack size, but only for "media" threads:

11:22 < jya> dminor: is this a task you can take on ? maybe add an argument to the threadpool constructor telling it it's a media threadpool, which would then use a bigger 
             stack
11:22 < jya> would be interesting to see if we can reduce the stack size for all the others
11:22 < jya> I expect that this code will be able to go away once we ship the remote data decoder that mjf is working on

I'll have a look at this.

Comment 7

[Dan Minor [:dminor]]

Attachment: Bug 1474684 - Use larger stack for media decoder threads; r=jya

We've hit stack overflows while decoding, in particular for av1. This increases
the thread size for the platform decoder threads, while leaving the others at
their default values.

Comment 8

[Phabricator Automation]

Comment on attachment 8993056 [details]
Bug 1474684 - Use larger stack for media decoder threads; r=jya

Jean-Yves Avenard [:jya] has approved the revision.

https://phabricator.services.mozilla.com/D2226

Comment 9

[Phabricator Automation]

Comment on attachment 8993056 [details]
Bug 1474684 - Use larger stack for media decoder threads; r=jya

Nathan Froyd [:froydnj] has approved the revision.

https://phabricator.services.mozilla.com/D2226

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fac4d7589e5d
Use larger stack for media decoder threads; r=jya,froydnj

Comment 11

[Dan Minor [:dminor]]

Attachment: Bug 1474684 - Remove unnecessary already_addRefed from return; r=jya

Comment 12

[Phabricator Automation]

Comment on attachment 8993653 [details]
Bug 1474684 - Remove unnecessary already_addRefed from return; r=jya

Jean-Yves Avenard [:jya] has approved the revision.

https://phabricator.services.mozilla.com/D2264

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/fac4d7589e5d

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/367993fd2ba3
Remove unnecessary already_addRefed from return; r=jya

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/367993fd2ba3