Closed Bug 1262276 Opened 7 years ago Closed 3 months ago

Seamless video looping

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
108 Branch
Tracking Status
firefox107 --- wontfix
firefox108 --- fixed

People

(Reporter: limeclipse, Assigned: alwu)

References

(Blocks 3 open bugs, Regressed 1 open bug)

Details

Attachments

(20 files)

470.22 KB, video/webm
Details
234 bytes, text/html
Details
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160315153207

Steps to reproduce:

Encoded a VP9 video (no audio source) in a WebM container and used the <video> tag with loop & autoplay attributes to play it in Firefox.


Actual results:

While the video itself is a seamless loop, the playback in Firefox is not. There is a noticable skip from the end to the beginning of the video. This happens with all kinds of configurations: CBR, ABR, BVR, 25 const FPS, 30 const FPS, and even with the OGG/Theora combination. This does NOT happen with MPeG4/MP4 configuration.


Expected results:

It should be a seamless loop. Other browsers do not have this issue.
Component: Untriaged → Audio/Video: Playback
Product: Firefox → Core
Attached file 1262276.html
I don't see a noticable skip between end and beginning on my side.
(In reply to Loic from comment #2)
> I don't see a noticable skip between end and beginning on my side.

That could be related to machine performance. I think the issue is that we don't support seamless looping.
Mass change P2 -> P3
Priority: P2 → P3
Assignee: nobody → alwu
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Webm video loop → Seamless video looping
Depends on: 1498440

People are using videos a lot more often for things that used to be GIFs (finally!), but this bug makes things painful on Firefox. It would be great if we could raise the priority here. I see looping jank on Linux, Android, and Windows.

Because our resource is limited. Before we implement this, I would like to add some telemetry probes to see how many video are used in that, comparing with non-looping video playback.

Depends on: 1687544

Got reminded of this issue due to an e-mail. Here to report that the issue is no longer present (fixed). It must have happened quite some time ago, but my current version is 103.0.2.

Type: defect → enhancement
OS: Unspecified → All
Hardware: Unspecified → All
Version: 45 Branch → Trunk

FYI we're tracking the Meet issue here - Bug 1789881 - Google Meet background video experiences black frame or stutter at video loop point

Blocks: 1789881
See Also: → 1794000
Depends on: 1794496
Severity: normal → S3

Depends on D159125

Depends on D159127

If the video looping is not seamless, when playback reaches to the end,
MDSM would trigger a seek in order to get the new frame from the start
position. That would notify the media element that the status of the next
frame is not available now due to seeking (NEXT_FRAME_UNAVAILABLE_SEEKING)
and causes the media element dispatching waiting event.

Above situaion shouldn't happen when we're in the seamless looping. The
added test covers that situation.

That ready state change also causes the google meet issue [1], because
the spec only allows capturing an image from a media element if it's
ready state is at least in HAVE_CURRENT_DATA.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1789881#c3

Depends on D159217

Attachment #9298122 - Attachment description: WIP: Bug 1262276 - part1 : generalize the request data function in order to support requesting video data as well. → Bug 1262276 - part1 : generalize the request data function in order to support requesting video data as well.
Attachment #9298123 - Attachment description: WIP: Bug 1262276 - part2 : support seamless video looping. → Bug 1262276 - part2 : support seamless video looping.
Attachment #9298124 - Attachment description: WIP: Bug 1262276 - part3 : allow the format reader to request video data during audio seeking. → Bug 1262276 - part3 : allow the format reader to request video data during audio seeking.
Attachment #9298125 - Attachment description: WIP: Bug 1262276 - part4 : add more debug logs. → Bug 1262276 - part4 : add more debug logs.
Attachment #9298291 - Attachment description: WIP: Bug 1262276 - part5 : only seek one track at a time otherwise the previous seek target would be overwritten. → Bug 1262276 - part5 : only seek one track at a time otherwise the previous seek target would be overwritten.
Attachment #9298292 - Attachment description: WIP: Bug 1262276 - part6 : add a test to ensure that media element always has current frame during seamless looping. → Bug 1262276 - part6 : add a test to ensure that media element always has current frame during seamless looping.

This behavior is optional [1] and currently this test only passes on
Edge [2].

The reason of becoming timeout is because seamless looping now won't
change the media element's ready state. When looping is seamless,
element's ready state will change below HAVE_CURRENT_DATA (like what I
describe in D159218), and when we have enough data (HAVE_FUTURE_DATA or
HAVE_ENOUGH_DATA) the element would dispatch playing which causes the
condition check failure [3].

As now we're using seamless looping, now playing event will be resent
which causes the timeout.

[1] https://html.spec.whatwg.org/multipage/media.html#ready-states:eligible-for-autoplay-2
[2] https://wpt.fyi/results/html/semantics/embedded-content/media-elements/ready-states/autoplay-hidden.optional.html?label=experimental&view=subtest
[3] https://searchfox.org/mozilla-central/rev/76ccfc801e6b736c844cde3fddeab7a748fc8515/testing/web-platform/tests/html/semantics/embedded-content/media-elements/ready-states/autoplay-hidden.optional.html#27

Depends on D159218

This test ensures that we can always capture correct video frame via
canvas.

The black frame issue was discoverd in bug 1789881, where the canvas
capture is racing with the media element state change [1], which results
in capturing a black frame.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1789881#c3

Depends on D159333

As we need some time to ensure that they can work well, so these patches will be targeting on Fx108.

Priority: P3 → P2
Blocks: 1795738
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/70bc7e69d03a
part1 : generalize the request data function in order to support requesting video data as well. r=padenot
https://hg.mozilla.org/integration/autoland/rev/7d876e0ef72a
part2 : support seamless video looping. r=padenot
https://hg.mozilla.org/integration/autoland/rev/72a9ff64ef1b
part3 : allow the format reader to request video data during audio seeking. r=padenot
https://hg.mozilla.org/integration/autoland/rev/4724dd3b433f
part4 : add more debug logs. r=padenot
https://hg.mozilla.org/integration/autoland/rev/143255ae75ed
part5 : only seek one track at a time otherwise the previous seek target would be overwritten. r=padenot
https://hg.mozilla.org/integration/autoland/rev/c937297d3874
part6 : add a test to ensure that media element always has current frame during seamless looping. r=padenot
https://hg.mozilla.org/integration/autoland/rev/92e2db60f44a
part7 : change the expectation of 'autoplay-hidden.optional.html.ini' from `Failed` to `Timeout`. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0e8e7d877d08
part8 : add a canvas test to ensure that video is seamless looping. r=padenot
Flags: needinfo?(alwu)

We don't want to trigger skip-to-next-keyframe after reaching video
EOS. Because now we've reset the demuxer to 0, and are going to request
data from start.

If playback hasn't looped back, the media time would still be too large,
which makes the reader think the playback is way behind and performs
nonecessary skipping which causes stutter.

Eg. Media is 10s long, reaching EOS at 8s, requesting data at 9s.

Assume media's keyframe interval is 3s, which means keyframes will
appear on 0s, 3s, 6s and 9s.

If we uses current media time as a threshold, the reader sees the next
key frame is 3s but the threashold is 9s, which usually happens when
the decoding is too slow.

But that is not the case for us, we should by pass the
skip-to-next-keyframe logic until the media loops back.

Depends on D160159

For pretty short media, (eg. gizmo-short.mp4), after reaching audio EOS,
the next audio sampe would be put on waiting because we can't adjust its
timestamp yet.

If so, we should stop prerolling otherwise the playback would never stop
because we won't ask for more audio data when there is always a data on
waiting.

Depends on D160160

This patch includes different test cases which caused problems before
during the development, and they are all some basic behavior tests.

Depends on D160163

When leaving looping state to another state, media data stored in the
media queue are already adjusted. If the new state requests a new data
but doesn't adjust its timestamp, then the data in the media queue will
be out of order.

If that happens on video data, it would causes a/v unsync and the video
frame would be discarded because it doesn't catch up with the clock
time, which might have grown a lot via looping multiple times.

The example transitions from the looping state which would encounter
this situation are buffering state (decoding too slow), decoding state
(cancel looping) and video only seek (bkg video resume).

In the premise of letting the clock time keep growing, we would need to
put the offset to somewhere independent to states. Therefore, we choose
to let the media queue do the task of the timestamp adjustment.

So even if we leave the looping state, the new coming data would be
adjusted their timestamp correctly and match the clock time. If we enter
the looping state again, we can also smoothly keep adding more offset to
all future data.

Depends on D160163

Attachment #9300043 - Attachment description: WIP: Bug 1262276 - part12 : add more tests for seamless looping. → WIP: Bug 1262276 - part13 : add more tests for seamless looping.
Attachment #9300032 - Attachment description: WIP: Bug 1262276 - part9 : adjust looping timestamp until knowing the longest track duration. → Bug 1262276 - part9 : adjust looping timestamp until knowing the longest track duration.
Attachment #9300033 - Attachment description: WIP: Bug 1262276 - part10 : bypass skip to the next frame until media loops back. → Bug 1262276 - part10 : bypass skip to the next frame until media loops back.
Attachment #9300042 - Attachment description: WIP: Bug 1262276 - part11 : stop prerolling if the track data is waiting for the timestamp adjustment. → Bug 1262276 - part11 : stop prerolling if the track data is waiting for the timestamp adjustment.
Attachment #9300668 - Attachment description: WIP: Bug 1262276 - part12 : store looping offset in the media queue in order to keep timestamp consistantly increasing across different states. → Bug 1262276 - part12 : store looping offset in the media queue in order to keep timestamp consistantly increasing across different states.
Attachment #9300043 - Attachment description: WIP: Bug 1262276 - part13 : add more tests for seamless looping. → Bug 1262276 - part13 : add more tests for seamless looping.

When requesting data from demuxer, it should always use normalized data,
which is within [0, original-track-duration]. So we need to adjust seek
target time in order not to use clock time which has grown over the
duration due to multiple times of looping (that time is larger than
duration).

Depends on D160164

Using mozPresentedFrames is incorrect, because that represents how
many frames the video sink sends to the compositor, and that can include
the same frame multiple time. [1]

Due to the seamless looping, now we could have more frames in the video
queue, so mozPresentedFrames could be much larger than the actual
frames already be rendered. Therefore, we should use mozPaintedFrames
instead, which is closer to the situation we wanted to measure (submit
native layer to screen)

This patch also increases the playback rate and the threshold in the
test in order to reduce test running time.

[1] https://searchfox.org/mozilla-central/rev/fe5c9c39a879b07d5b629257f63d825c3c8cd0ed/dom/media/mediasink/VideoSink.cpp#515

Depends on D160699

Per offline discussion with Kesley, we decided to disable seamless
looping for these tests first, and then spend time to figure the gfx
side crash due to not be able to manage texture's lifetime correctly.

Depends on D160700

This doesn't work for seamless looping because the video end time can
be larger than the media time. We would need to revisit these mechaniam
later to see if we can have a better way to skip to next frame.

Depends on D160701

Blocks: 1799205
Attachment #9300032 - Attachment description: Bug 1262276 - part9 : adjust looping timestamp until knowing the longest track duration. → Bug 1262276 - part9 : don't adjust looping timestamp until the longest track duration is known.
Attachment #9300894 - Attachment description: Bug 1262276 - part15 : fix test failure for test_video_low_power_telemetry.html. → Bug 1262276 - part15 : disable seamless looping for test_video_low_power_telemetry.html in order to fix the test failure.
Attachment #9300895 - Attachment description: Bug 1262276 - part16 : disable seamless looping for generated/test_conformance__textures__misc__texture-corner-case-videos.html. → Bug 1262276 - part16 : disable seamless looping for webgl-conf test.
Blocks: 1799213
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1df4774b2c53
part1 : generalize the request data function in order to support requesting video data as well. r=padenot
https://hg.mozilla.org/integration/autoland/rev/3e647187e78b
part2 : support seamless video looping. r=padenot
https://hg.mozilla.org/integration/autoland/rev/f6c98cf1d45f
part3 : allow the format reader to request video data during audio seeking. r=padenot
https://hg.mozilla.org/integration/autoland/rev/6672431765dc
part4 : add more debug logs. r=padenot
https://hg.mozilla.org/integration/autoland/rev/1dd4093cd374
part5 : only seek one track at a time otherwise the previous seek target would be overwritten. r=padenot
https://hg.mozilla.org/integration/autoland/rev/55e490e2de4c
part6 : add a test to ensure that media element always has current frame during seamless looping. r=padenot
https://hg.mozilla.org/integration/autoland/rev/1dac017b6089
part7 : change the expectation of 'autoplay-hidden.optional.html.ini' from `Failed` to `Timeout`. r=padenot
https://hg.mozilla.org/integration/autoland/rev/e807704e1c9b
part8 : add a canvas test to ensure that video is seamless looping. r=padenot
https://hg.mozilla.org/integration/autoland/rev/d122b10f03ad
part9 : don't adjust looping timestamp until the longest track duration is known. r=padenot
https://hg.mozilla.org/integration/autoland/rev/abd21243cfda
part10 : bypass skip to the next frame until media loops back. r=padenot
https://hg.mozilla.org/integration/autoland/rev/44e50217fdc9
part11 : stop prerolling if the track data is waiting for the timestamp adjustment. r=padenot
https://hg.mozilla.org/integration/autoland/rev/e43f99440e29
part12 : store looping offset in the media queue in order to keep timestamp consistantly increasing across different states. r=padenot
https://hg.mozilla.org/integration/autoland/rev/12aaeb776da1
part13 : add more tests for seamless looping. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9e9f7deff834
part14 : video-only seek should use original time as a seek target. r=padenot
https://hg.mozilla.org/integration/autoland/rev/b1a5972d0cee
part15 : disable seamless looping for test_video_low_power_telemetry.html in order to fix the test failure. r=padenot
https://hg.mozilla.org/integration/autoland/rev/ee73611a2993
part16 : disable seamless looping for webgl-conf test. r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/f436cf1a01cc
part17 : disable late time check for seamless looping. r=padenot
https://hg.mozilla.org/integration/autoland/rev/1d0656c6185d
part18 : add a pref to control video seamless looping. r=padenot
Regressions: 1799335
Regressions: 1799464
Regressions: 1799476
Regressions: 1799695
Regressions: 1800233
Regressions: 1800201
Regressions: 1800254
Depends on: 1801692
Regressions: 1801692
Regressions: 1801610
You need to log in before you can comment on or make changes to this bug.