Closed Bug 960599 Opened 7 years ago Closed 6 years ago

Changing playbackRate on a video element causes the image to freeze

Categories

(Core :: Audio/Video, defect)

26 Branch
x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox26 --- wontfix
firefox27 + wontfix
firefox28 + wontfix
firefox29 + wontfix
firefox30 --- fixed
firefox-esr24 --- unaffected

People

(Reporter: stefan.buller, Unassigned)

References

Details

(Keywords: regression, Whiteboard: [bugday-20140120])

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140116030250

Steps to reproduce:

On Fedora 19 running KDE, in both the installed version 26.0 and nightlies up to 2014-01-16:

I loaded an mp4 file in a new tab, and then on the console:
> var video = document.getElementsByTagName("video")[0];
> video.playbackRate = 1.5;

The issue is also noticeable on websites like Youtube that use HTML5 video.


Actual results:

Playback sped up as expected, but the image froze after playing a few frames. Seeking causes a few more frames to play at the new position, but once again the image freezes. Throughout, the audio plays as expected.
(WebM)

Image froze with 2014-01-20-03-02-02-mozilla-central-firefox-29.0a1.en-US.linux-x86_64 (haven't checked audio).

WFM with 27.0 Beta ≈7 with my profile (many extensions, including NoScript).
Component: Untriaged → Video/Audio
Product: Firefox → Core
Whiteboard: [bugday-20140120]
NEW based on comment 1
Status: UNCONFIRMED → NEW
Ever confirmed: true
(WFM on 2014-01-20 - linux 64)
It might happen on the second try, with a different rate.

Reproduced with 2013-05-20-03-12-30-mozilla-central-firefox-24.0a1.en-US.linux-x86_64.
Confirmed in 29.0a1(2014-01-21), ubuntu 13.04 x64 on youtube/html5 with
var video = document.getElementsByTagName("video")[0];
video.playbackRate = 2.5;

The image froze, the sound didn't.
I can repro, but only on youtube videos.
Regression winodw(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/a4e9c9c9dbf9
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130912030201
Bad:
http://hg.mozilla.org/mozilla-central/rev/b9029b1de410
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130913030201
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a4e9c9c9dbf9&tochange=b9029b1de410

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/e9ee43da0c16
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130911162107
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/0192ffffc633
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0 ID:20130911162531
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e9ee43da0c16&tochange=0192ffffc633

Regressed by:
0192ffffc633	Edwin Flores — Bug 886181 - Pref on gstreamer backend r=cpearce
Blocks: 886181
Version: 29 Branch → 26 Branch
Edwin, can you have a look at this?
Flags: needinfo?(edwin)
Keywords: qawanted
Attached patch 960599.patchSplinter Review
The problem here was that GStreamer was trying to sync the video stream with a clock running in realtime. When playbackRate > 1, we get ahead of the GStreamer clock and it throttles video decode to keep us from getting too far ahead of the clock.

This patch simply disables GStreamer's clock sync.
Attachment #8363931 - Flags: review?(cpearce)
Flags: needinfo?(edwin)
Comment on attachment 8363931 [details] [diff] [review]
960599.patch

Review of attachment 8363931 [details] [diff] [review]:
-----------------------------------------------------------------

This makes sense to me, but I want Alessandro to f+ before you land, in case this will have unexpected consequences.
Attachment #8363931 - Flags: review?(cpearce) → review+
Comment on attachment 8363931 [details] [diff] [review]
960599.patch

Apparently we're doing


  mVideoSink = gst_parse_bin_from_description("capsfilter name=filter ! "
      "appsink name=videosink sync=true max-buffers=1 "
      "caps=video/x-raw-yuv,format=(fourcc)I420"
      , TRUE, nullptr);

notice sync=true.

In the 1.0 patch I already changed it to sync=false. Edwin, have you tried to flip sync to false? It should be enough, and shouldn't require tinkering with the pipeline clock directly.
(In reply to Alessandro Decina from comment #11)
> In the 1.0 patch I already changed it to sync=false. Edwin, have you tried
> to flip sync to false? It should be enough, and shouldn't require tinkering
> with the pipeline clock directly.

Yes, I tried that, but gstreamer ends up trying to sync the audio and video streams with each other, which can conflict with gecko's queueing/syncing. This way audio and video are kept separate.
(In reply to Edwin Flores [:eflores] [:edwin] from comment #12)
> (In reply to Alessandro Decina from comment #11)
> > In the 1.0 patch I already changed it to sync=false. Edwin, have you tried
> > to flip sync to false? It should be enough, and shouldn't require tinkering
> > with the pipeline clock directly.
> 
> Yes, I tried that, but gstreamer ends up trying to sync the audio and video
> streams with each other, which can conflict with gecko's queueing/syncing.
> This way audio and video are kept separate.

Weird, do you know where it's trying to do that? Because synchronization in gst should happen only in sinks, and only in sinks having sync=true
Fwiw, I can't reproduce this with the 1.0 patch from https://bugzilla.mozilla.org/show_bug.cgi?id=806917 applied (which sets sync=false)
(In reply to Alessandro Decina from comment #14)
> Fwiw, I can't reproduce this with the 1.0 patch from
> https://bugzilla.mozilla.org/show_bug.cgi?id=806917 applied (which sets
> sync=false)

This bug happens with vsync true and async false. Bug 884651 happens when vsync = async.
I think the main issue with https://bugzilla.mozilla.org/show_bug.cgi?id=884651 was that we were not setting max-buffers=1 (as we do now), so gst never stopped decoding, even if MediaDecoderStateMachine was paused (because playback was paused). Now with the 1.0 patch, sync=false and max-buffers=1 I can't repro that bug either.
(In reply to Alessandro Decina from comment #16)
> I think the main issue with
> https://bugzilla.mozilla.org/show_bug.cgi?id=884651 was that we were not
> setting max-buffers=1 (as we do now), so gst never stopped decoding, even if
> MediaDecoderStateMachine was paused (because playback was paused). Now with
> the 1.0 patch, sync=false and max-buffers=1 I can't repro that bug either.

Sorry that's wrong. The problem there was that we were setting sync=true in both sinks, and because firefox was not consuming fast enough (eg on paused playback), gst QoS was kicking in, causing all the frames to be decoded and discarded. Anyway, I am confident that setting sync=false in both sinks won't cause that issue again.
Removing qawanted since this request was already answered.
Keywords: qawanted
I am pretty sure this was fixed as part of landing https://bugzilla.mozilla.org/show_bug.cgi?id=806917. I can't repro, can someone double check?
I can confirm this is fixed.

Progression window
Bad:
https://hg.mozilla.org/integration/mozilla-inbound/rev/af35bd7bd9a8
Mozilla/5.0 (X11; Linux i686; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140211061742
Fixed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4a99ebc2f515
Mozilla/5.0 (X11; Linux i686; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140211062342
Fixed pushlog
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=af35bd7bd9a8&tochange=4a99ebc2f515

Yes, Bug 806917 seemed to fix this problem.
Marking affected on 30 so this can be updated when landed to central - also please nominate for branch uplift with a risk evaluation.
Bug 806917 won't be uplifted (a 82kb patch) cf comment 119. So, this bug won't be fixed for 28 or 29.
Tagging this bug as wontfix then.
No longer reproducible.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.