1358057 - The animations are not displaying the correct final frame

Status: RESOLVED FIXED

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

Description

[roxleitan]

Attachment: 2017-04-20_11h44_30.mp4

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170419030223

[Affected versions]:
Nightly 55.0a1

[Affected platforms]:
All platforms: Windows 10, Mac OS X 10.10

[Steps to reproduce]:
1.Launch Nightly 55.0a1 with a new profile
2.Open http://imgur.com/a/iKvu3 in 3 tabs
3.Open a new tab
4.Open a new window and navigate to about:memory
5.Click "Minimize memory usage" button
6.Wait 30 seconds 
7.Navigate through the tabs opened at step 2

[Expected result]:
The animations should stop and display the correct final frame

[Actual result]:
The animations are not displaying the correct final frame( please see the attachment)

Comment 1

[Timothy Nikkel (:tnikkel)]

Are you actually getting animated images for this? I'm getting html5 video. When I right click on it I get a video context menu (save video as etc), not an image context menu.

Comment 2

[roxleitan]

Yes, you are right, this is a html5 video. I uploaded a local gif in imgur and was saved as html5.
I cannot reproduce this issue using a local gif.

But this seems to be Audio/Video:Playback bug.
Regression range:

Last good revision: ea104eeb14cc54da9a06c3766da63f73117723a0
First bad revision: bbfb1c7bcc50f7b335d34c65fc54fb3af9584596
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ea104eeb14cc54da9a06c3766da63f73117723a0&tochange=bbfb1c7bcc50f7b335d34c65fc54fb3af9584596

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Can you go to about:config and change "media.suspend-bkgnd-video.enabled" to false and then try to repro the issue again? Thanks!

Comment 4

[roxleitan]

Hi,

I cannot reproduce the issue with "media.suspend-bkgnd-video.enabled" set to false.

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

Then kaku is the one you need.

Comment 6

[Tzuhao Kuo [:kaku]]

Will check.

Comment 7

[Tzuhao Kuo [:kaku]]

Okay, we don't need to open three tabs and minimize memory usage to reproduce this bug. Just open http://imgur.com/a/iKvu3 in one tab and send it to background to make its video decoder suspended, wait until the end of the video, and refocus the tab.

So, the problem is that a suspend video element becomes visible after the playback finished and we try to resume the element video decoder and trigger a seek operation which seeks to the end of the video. However, the video in http://imgur.com/a/iKvu3 has no audio track, and we have an optimization to this scenario that we invoke fast-seek to get better performance and fast-seek might not seek to the exact time we aim to.

Fixing this is easy. What we need to do is never invoke fast-seek while resuming to the end of the video. A patch in following.

Comment 8

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1358057 P1 - don't use fastseek while resuming video decoder to the last frame;

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

Comment 9

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1358057 P2 - add a mochitest;

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

Comment 10

[Tzuhao Kuo [:kaku]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=70973a9715e406e78495eb296ac5085bb1eb9e3b
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5198ecf5924942ee81597fcc9df30aa01db24985

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8861313 [details]
Bug 1358057 P2 - add a mochitest;

https://reviewboard.mozilla.org/r/133290/#review136088

::: dom/media/test/test_background_video_resume_after_end_show_last_frame.html:33
(Diff revision 1)
> +  document.body.appendChild(c2);
> +
> +  let gfx1 = c1.getContext('2d');
> +  let gfx2 = c2.getContext('2d');
> +  if (!gfx1 || !gfx2) {
> +    throw Error("Unable to obtain context '2d' from canvas");

Just call ok(false, ...) and return so we can continue other tests in case of errors.

::: dom/media/test/test_background_video_resume_after_end_show_last_frame.html:76
(Diff revision 1)
> +    let v = appendVideoToDoc(test.name, token);
> +    let vReference = appendVideoToDoc(test.name, token);
> +    manager.started(token);
> +
> +    let count = 0;
> +    v.addEventListener("ended", function() {

This test is irrelevant to bug 1358057.

::: dom/media/test/test_background_video_resume_after_end_show_last_frame.html:81
(Diff revision 1)
> +    v.addEventListener("ended", function() {
> +      is(++count, 1, `${token} should get only one 'ended' event.`);
> +    });
> +
> +    /*
> +     * This test checks that, after a video element had finished its playback,

This comment is irrelevant to bug 1358057.

::: dom/media/test/test_background_video_resume_after_end_show_last_frame.html:85
(Diff revision 1)
> +    /*
> +     * This test checks that, after a video element had finished its playback,
> +     * resuming video decoder doesn't dispatch 2nd ended event.
> +     * This issue was found in bug 1349097.
> +     */
> +     waitUntilPlaying(vReference)

waitUntilPlaying(vReference) and waitUntilPlaying(v) can be called in parallel.

::: dom/media/test/test_background_video_resume_after_end_show_last_frame.html:88
(Diff revision 1)
> +     * This issue was found in bug 1349097.
> +     */
> +     waitUntilPlaying(vReference)
> +      .then(() => waitUntilPlaying(v))
> +      .then(() => testVideoSuspendsWhenHidden(v))
> +      .then(() => waitUntilEnded(vReference))

waitUntilEnded(vReference) and waitUntilEnded(v) can be called in parallel to make the test run faster.

Comment 12

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8861312 [details]
Bug 1358057 P1 - don't use fastseek while resuming video decoder to the last frame;

https://reviewboard.mozilla.org/r/133288/#review136100

::: dom/media/MediaDecoderStateMachine.cpp:2044
(Diff revision 1)
>    bool hw = Reader()->VideoIsHardwareAccelerated();
>  
>    // Start video-only seek to the current time.
>    SeekJob seekJob;
>  
> -  const SeekTarget::Type type = mMaster->HasAudio()
> +  // Only use fastseek if the media has no audio and not seeking to the end.

Might be worth adding a comment to explain when to use fastSeek and when not.

E.g. "Don't use fastSeek since we always want to present the last frame to the user when seeking to the end."

and "fastSeek is used for video-only media since it is fast and we don't need to worry about A/V sync."

::: dom/media/MediaDecoderStateMachine.cpp:2045
(Diff revision 1)
>  
>    // Start video-only seek to the current time.
>    SeekJob seekJob;
>  
> -  const SeekTarget::Type type = mMaster->HasAudio()
> +  // Only use fastseek if the media has no audio and not seeking to the end.
> +  auto type = mMaster->HasAudio() || aTarget == this->mMaster->Duration()

Omit |this->|.

Comment 13

[Tzuhao Kuo [:kaku]]

Comment on attachment 8861312 [details]
Bug 1358057 P1 - don't use fastseek while resuming video decoder to the last frame;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133288/diff/1-2/

Comment 14

[Tzuhao Kuo [:kaku]]

Comment on attachment 8861313 [details]
Bug 1358057 P2 - add a mochitest;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133290/diff/1-2/

Comment 15

[Tzuhao Kuo [:kaku]]

Try again:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=38717ad5d2ea9237b95f4151168c52d12b039ba0
https://treeherder.mozilla.org/#/jobs?repo=try&revision=87a48c9ce54a0d786fd97a5c553f207fd09b3d5d

Comment 16

[Tzuhao Kuo [:kaku]]

Some test cases failed on the try but should not caused by these patches.
Thanks for the review!

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f19d82b20158
P1 - don't use fastseek while resuming video decoder to the last frame; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/29da30de06bc
P2 - add a mochitest; r=jwwang

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f19d82b20158
https://hg.mozilla.org/mozilla-central/rev/29da30de06bc