1368619 - MFR may returns multiple frames with the same timestamp on Android

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

from bug 1345898 comment 23

"I'm a bit cautious with P3, as, as I understand it, on Android calling Flush() will invalidate all decoded frames previously returned even when they haven't been painted yet.
Mind you, if this patch could cause problem, then things are currently broken anyway.

The issue at hand, is when we encounter a gap in the data. We then drain the decoder to make sure you return all frames up to the gap.

We then seek the decoder to the last position decoded, just so that we can resume the decoding once more data is appended to the sourcebuffer.
If Flush() was to invalidate all frames, then things would be broken, as the first thing occurring when seeking is to call Flush(), it's likely not all the frames returned during the previous drain have been painted yet....

Now the peculiar situation occurs should, when performing this internal seek, we detect a change of resolution (this can happen when the previous data is overwritten with new ones).
Previously, nothing would be done, we would drain the decoder once again and get new data .
The consequences to this as that we get those frames twice in the decoded queue (the frames from the first resolution followed by the new ones).

P3 as such, Flush the decoder and any pending decoded frames (not yet returned to the MDSM) so that we don't get duplicated frames.

If calling Flush() does invalidate all returned frames so far, then doing P3 isn't any worse than what is being done.. But it's something we will want to fix anyway."

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1368619: Flush decoder on Android.

In the situation described in bug 1345898, on android where the decoder is recycled rather than shut down, we would have potentially returned multiple frames with the same timestamp.
It is necessary to flush the decoder.

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

Comment 2

[John Lin [:jhlin][:jolin]]

Comment on attachment 8872530 [details]
Bug 1368619: Flush decoder on Android.

https://reviewboard.mozilla.org/r/144074/#review148196

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/92b3f353c684
Flush decoder on Android. r=jolin

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/92b3f353c684