1301059 - When suspending decode, the BlankDecoder shouldn't be wrapped in a H264Converter

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

Issue presented in bug 1300296 comment 21.

when the video goes into the background, the current decoder is destroyed and switched for a BlankDecoder. Decoding continue where the previous decoder left off.

The H264Converter now ensures that the first frame seen is always a keyframe. But that means that when the video is suspended, all frames will be dropped until a new keyframes is seen.

It's probably harmless, but 1- it's unnecessary cycles wasted at monitoring the h264 streams we don't need and 2- it means some events could be missed while the video is in the background.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1301059: Do not use H264Converter when video decode is suspended.

Review commit: https://reviewboard.mozilla.org/r/77224/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/77224/

Comment 2

[Tzuhao Kuo [:kaku]]

Comment on attachment 8788902 [details]
Bug 1301059: Do not use H264Converter when video decode is suspended.

https://reviewboard.mozilla.org/r/77224/#review75746

By this way, the extra data is not passed into the constructor of BlankMediaDataDecoder anymore, right?
I think we should also make the BlankMediaDataDecoder::mMaxRefFrames to be 16 for H264 cases.
http://searchfox.org/mozilla-central/rev/8eb4fd2c7be150b0aa1b05c0f3707e82dc8f2dc8/dom/media/platforms/agnostic/BlankDecoderModule.cpp#37

Comment 3

[Jean-Yves Avenard [:jya]]

Comment on attachment 8788902 [details]
Bug 1301059: Do not use H264Converter when video decode is suspended.

https://reviewboard.mozilla.org/r/77224/#review75746

good point.

Then in case we don't have a SPS, yes, using 16 instead is a safe approach....
feel free to take on this bug if you want to...

Comment 4

[Tzuhao Kuo [:kaku]]

Okay, I will then upload a new patch for it; will also do it for bug 1301061.

Comment 5

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1301059: Do not use H264Converter when video decode is suspended.

Review commit: https://reviewboard.mozilla.org/r/78052/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/78052/

Comment 6

[Tzuhao Kuo [:kaku]]

Comment on attachment 8788902 [details]
Bug 1301059: Do not use H264Converter when video decode is suspended.

https://reviewboard.mozilla.org/r/77224/#review76516

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8790099 [details]
Bug 1301059: Do not use H264Converter when video decode is suspended.

https://reviewboard.mozilla.org/r/78052/#review76870

Comment 8

[Tzuhao Kuo [:kaku]]

Thanks for the review, try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f9eea166a46a

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/293b5a36dc0e
Do not use H264Converter when video decode is suspended. r=jya

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/293b5a36dc0e