1204588 - OpenH264 calls GMPVideoDecoderCallback after DecodingComplete -- uses freed memory

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: call_stack.txt

Comment 1

[Paul Adenot (:padenot)]

Anthony, this is a problem with GMP, can you have a look ?

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Ethan, can you look at this as well and verify if it's known?

Comment 3

[Ethan Hugg [:ehugg]]

This is not the same stack I see with the OpenH264 refcount bug, but if this was created by testing WebRTC/H264, then it would be worthwhile to try with a build off the master branch to see if it's also repeatable there.

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

I'm guessing this is a WebRTC or at least general GMP issue on the basis that we're not using GMP on Linux for media playback.

Chris - who is the right person to look into this?

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

This is a bug in OpenH264.

An OpenH264 task to run the GMPVideoDecoderCallback::InputDataExhausted() callback is running after the GMPVideoDecoder::DecodingComplete() implementation has run, and the GMPVideoDecoderCallback's memory has been freed.

OpenH264's GMPVideoDecoder::DecodingComplete() implementation needs to set a flag that ensures that nothing ever tries to use the GMPVideoDecoderCallback after GMPVideoDecoder::DecodingComplete() is called.

That's what gmp-clearkey and Adobe's GMP do; we set a flag here:
https://dxr.mozilla.org/mozilla-central/source/media/gmp-clearkey/0.1/VideoDecoder.cpp#421

And we check the flag inside our GMPTask wrapper here:
https://dxr.mozilla.org/mozilla-central/source/media/gmp-clearkey/0.1/VideoDecoder.cpp#440

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

The alternative fix in Firefox would be for us to never de-allocate the thing that implements GMPVideoDecoderCallback. That's not ideal, as we'd be leaking memory then.

Comment 7

[Hank Peng]

Move to next openh264 release

Comment 8

[Al Billings [:abillings - ex-MoCo]]

Will this be fixed in 1.5? I see no progress here.

Comment 9

[Hank Peng]

(In reply to Al Billings [:abillings] from comment #8)
> Will this be fixed in 1.5? I see no progress here.

Not in the scope of 1.5. Should be released after 1.5. I should be able to provide a fix for code review soon.

Comment 10

[Hank Peng]

Code review for openh264 fix: https://rbcommons.com/s/OpenH264/r/1366/diff/1#index_header

Comment 11

[Hank Peng]

The fix was merged into the openh264 master branch with commit 955fce6 (https://github.com/cisco/openh264/commit/955fce60a1cfcbd66a778350fc4b6d8e54ea946f).