984576 - Lag observed when seeking to a new position while playing video files

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[bhargavg1]

Observe that it takes time for the playback to start when seeking to a new position while doing video playback

Comment 1

[Jason Smith [:jsmith]]

What device? What build ID? What Android OS version?

Comment 2

[bhargavg1]

Build IDs which I tested on

GAIA:66fb457e892af3463bb4fbd1461c7e67865c416e
GECKO:0bacffad36f65a27929ca85164792750c695c3cb

Android OS: Kit-Kat

Device: 8x26 

I guess we can see that it takes time to start playing again, quite easily seen on most of the builds

Comment 3

[Preeti Raghunath(:Preeti)]

What is the acceptable time?

Comment 4

[Preeti Raghunath(:Preeti)]

CJ.

Please review

Comment 5

[bhargavg1]

(In reply to Preeti Raghunath(:Preeti) from comment #3)
> What is the acceptable time?

Dont think we have a bench mark for the timing, except for that to be smooth.

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Currently Gecko does accurate seek. That is, if you seek to 1.23242 seconds into a video, we'll seek to the preceding keyframe, and then decode the media forward until we reach exactly 1.23242 in the video and audio streams. We seek to the closest audio sample. The delay that you're seeing may be the time we're spending decoding forward from the preceding keyframe to the seek target.

I'm working on implementing HTMLMediaElement.fastSeek() in bug 778077.
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#dom-media-fastseek

When using fastSeek() we will skip the "decode forward to the seek target" step. We should probably use this in the video controls instead of precise seek, since it's pretty much impossible to give precise inputs via touch controls anyway. This should reduce the delay when seeking.

Comment 7

[Timothy B. Terriberry (:derf)]

(In reply to Chris Pearce (:cpearce) from comment #6)
> step. We should probably use this in the video controls instead of precise
> seek, since it's pretty much impossible to give precise inputs via touch
> controls anyway. This should reduce the delay when seeking.

There are other players that do this, and it is fine until some joker makes a video file that goes minutes between keyframes. Then it is very frustrating.

Comment 8

[Preeti Raghunath(:Preeti)]

Bhargav,

Can we please have a video of the same? Lets get a visual on KK device and JB device to understand the lag here?

What was the time it took in 1.3?

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Timothy B. Terriberry (:derf) from comment #7)
> (In reply to Chris Pearce (:cpearce) from comment #6)
> > step. We should probably use this in the video controls instead of precise
> > seek, since it's pretty much impossible to give precise inputs via touch
> > controls anyway. This should reduce the delay when seeking.
> 
> There are other players that do this, and it is fine until some joker makes
> a video file that goes minutes between keyframes. Then it is very
> frustrating.

Just tell gmaxwell to stop doing that.  ;)

To a certain degree, we've already lost when playing videos with large keyframe distances, as if we're doing exact seek on low end hardware, the seek will take ages, and that's frustrating too.

I'm not suggesting we use fastSeek in video controls on desktop, only mobile, where the scrubber bar is only a few times wider than the finger pressing it.

Comment 10

[u459114]

Hi Bruce,
Is this issue relative to your work?

Comment 11

[Bruce Sun [:brsun]]

(In reply to C.J. Ku[:CJKu] from comment #10)
> Hi Bruce,
> Is this issue relative to your work?

Hi CJ,
Not so relevant. Changing the codec from sync to async doesn't help on this issue. Users still need to wait necessary video data to be downloaded before they can see the video frame.

Comment 12

[Preeti Raghunath(:Preeti)]

Please provide benchmark numbers for 1.3. It will be easy to compare against.

Comment 13

[Preeti Raghunath(:Preeti)]

CJ

Is this reproducible on the QRD?

Comment 14

[u459114]

Sorry, have no QRD in team. 

Preeti, I think Eric Chou's team is more suitable to give you an answer on this bug, since they are working on B2G media playback.

NI Eric here to let him get notice on this bug.

Comment 15

[bhavana bajaj [:bajaj]]

bhargav, can you please renominate this once you have the additional information requested. At this time we cannot make a blocking decision here with the info at hand ?

Comment 16

[Sotaro Ikeda [:sotaro]]

IIRC, Bug 973408 is recent big change that could affect to seek.

Comment 17

[Inder]

:bajaj -- can you please not remove the 1.4? nom when we are actively working on the bug. Bhargav is working on providing this info and we also have daily sync-up to track it. So, no need to remove the nom here. Thanks.

Comment 18

[Inder]

Adding back the ni on Bhargav.

Comment 19

[Bruce Sun [:brsun]]

Remove echou's ni since he is aware of this issue already.

bhargavg1:
May I know what use case you are referring to?
 - 1. Approximate seek for speed: In some scenarios, users just want to see the video frame as quick as possible after seeking, and probably they don't care very much about whether the displayed video is the same video frame at the seek position or not. For example, users just want to skip some period of video content that they are not interested in. In this case, probably the nearest individual frame would be a good choice to be played from.
 - 2. Frame-accurate seek: In some scenarios, users want to see the exact video frame at the seek position. For example, users might want to step the video frame-by-frame. Since some referenced video frames are needed to be decoded in advance before the exact frame can be decoded, users might experience some lead time before the exact frame can be displayed.

Please kindly share what scenario you are testing and what kind of lag is acceptable.

Comment 20

[bhargavg1]

(In reply to Preeti Raghunath(:Preeti) from comment #8)
> Bhargav,
> 
> Can we please have a video of the same? Lets get a visual on KK device and
> JB device to understand the lag here?
> 
> What was the time it took in 1.3?

Shared clip at, https://drive.google.com/file/d/0B0zTAnPOpx-xV0VwdGtBRjk3VG8/edit?usp=sharing

Comment 21

[Jason Smith [:jsmith]]

(In reply to bhargavg1 from comment #20)
> (In reply to Preeti Raghunath(:Preeti) from comment #8)
> > Bhargav,
> > 
> > Can we please have a video of the same? Lets get a visual on KK device and
> > JB device to understand the lag here?
> > 
> > What was the time it took in 1.3?
> 
> Shared clip at,
> https://drive.google.com/file/d/0B0zTAnPOpx-xV0VwdGtBRjk3VG8/edit?usp=sharing

Can you attach or provide to the video you used to reproduce this bug?

Comment 22

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #21)
> (In reply to bhargavg1 from comment #20)
> > (In reply to Preeti Raghunath(:Preeti) from comment #8)
> > > Bhargav,
> > > 
> > > Can we please have a video of the same? Lets get a visual on KK device and
> > > JB device to understand the lag here?
> > > 
> > > What was the time it took in 1.3?
> > 
> > Shared clip at,
> > https://drive.google.com/file/d/0B0zTAnPOpx-xV0VwdGtBRjk3VG8/edit?usp=sharing
> 
> Can you attach or provide to the video you used to reproduce this bug?

provide a link*

Comment 23

[Chris Pearce [:cpearce (Not reading bugmail)]]

Once Bug 778077 has landed (should be up for review in a the next few working days), we can test changing to using that in the video app and default controls, and should that make average seek times faster, though less precise.

Comment 24

[bhargavg1]

(In reply to Bruce Sun [:brsun] from comment #19)
> Remove echou's ni since he is aware of this issue already.
> 
> bhargavg1:
> May I know what use case you are referring to?
>  - 1. Approximate seek for speed: In some scenarios, users just want to see
> the video frame as quick as possible after seeking, and probably they don't
> care very much about whether the displayed video is the same video frame at
> the seek position or not. For example, users just want to skip some period
> of video content that they are not interested in. In this case, probably the
> nearest individual frame would be a good choice to be played from.
>  - 2. Frame-accurate seek: In some scenarios, users want to see the exact
> video frame at the seek position. For example, users might want to step the
> video frame-by-frame. Since some referenced video frames are needed to be
> decoded in advance before the exact frame can be decoded, users might
> experience some lead time before the exact frame can be displayed.
> 
> Please kindly share what scenario you are testing and what kind of lag is
> acceptable.

This is more from UX perspective, I see moving to the nearest position should be fine

Comment 25

[Preeti Raghunath(:Preeti)]

CJ

please review the video and check if we need anything else from QC.

Comment 26

[Eric Chou [:ericchou] [:echou]]

Hi Preeti,

(In reply to Preeti Raghunath(:Preeti) from comment #25)
> CJ
> 
> please review the video and check if we need anything else from QC.

Please see comment 14. CJ's team takes media recording bugs, not media playback bugs. Please ni? me directly next time you have device-specific media playback issues. Thank you.

Per comment 23 & 24, I think Chris' fastSeek API can fulfill the reporter's requirement. I suggest that we should keep this bug open, once bug 778077 has landed, we could use this bug to track the implementation of Video app, which can improve its seeking time by using fastSeek(). (Not exactly sure if this is the best answer, ni? Bruce again to double confirm)

Comment 27

[Bruce Sun [:brsun]]

From comment 24, it seems this bug is referring to the similar problem that fastseek api is going to solve. Probably the necessary modification could be done in gecko without modifying gonk, but I am not very sure.

cpearce: do you have any plan on gonk regarding to fastweek api implementation?

Comment 28

[Preeti Raghunath(:Preeti)]

Eric,

Do we need anything from QC? We would like to understand if we can have a patch 3/27?

Comment 29

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Bruce Sun [:brsun] from comment #27)
> From comment 24, it seems this bug is referring to the similar problem that
> fastseek api is going to solve. 

Yes.

> Probably the necessary modification could be
> done in gecko without modifying gonk, but I am not very sure.

Yes, we should only need to modify gecko.


> cpearce: do you have any plan on gonk regarding to fastweek api
> implementation?

I will implement this in gecko for Gecko 31/B2G 1.5 in bug 778077, and then we can consider backporting if desired.

Comment 30

[Kai-Chih Hu [:khu]]

Is there any particular reason that we should consider to have this bug as a 1.4 blocker?

Comment 31

[Eric Chou [:ericchou] [:echou]]

(In reply to Preeti Raghunath(:Preeti) from comment #28)
> Eric,
> 
> Do we need anything from QC? 

Please see comment 26. All we need to do with this bug are (1) fix bug 778077, which Chris has been already dealing with, and (2) let Video app use fastSeek() API.

> We would like to understand if we can have a patch 3/27?

No. AFAIK the best solution should be waiting for bug 778077 landing on 1.5, then we can decide if we're going to backport to 1.4. Moreover, I don't even think this should be a 1.4 blocker since we don't even have a benchmark to define how 'fast' the seek should be from UX perspective.

Comment 32

[Jason Smith [:jsmith]]

Marvin - Can you make a product call on what release this should block, if any? This would require implementing a new feature (see the dependency here), so if we take this into 1.4, then we're going to add a feature after FC.

Comment 33

[Preeti Raghunath(:Preeti)]

(In reply to Eric Chou [:ericchou] [:echou] from comment #31)
> (In reply to Preeti Raghunath(:Preeti) from comment #28)
> > Eric,
> > 
> > Do we need anything from QC? 
> 
> Please see comment 26. All we need to do with this bug are (1) fix bug
> 778077, which Chris has been already dealing with, and (2) let Video app use
> fastSeek() API.
> 
> > We would like to understand if we can have a patch 3/27?
> 
> No. AFAIK the best solution should be waiting for bug 778077 landing on 1.5,
> then we can decide if we're going to backport to 1.4. Moreover, I don't even
> think this should be a 1.4 blocker since we don't even have a benchmark to
> define how 'fast' the seek should be from UX perspective.

Thanks Eric. Will provide the same feedback to QC.

Comment 34

[Inder]

Thanks for feedback, I have communicated it to test teams and will wait for their feedback on the same.

Comment 35

[Marvin Khoo [:Marvin_Khoo]]

Hi Jason, this improvement is planned in 1.5, consider 1.4 is under stabilization, adding feature work or pull in may bring concerns / work from various team. this should not be blocker, improvement will be made in 1.5, backporting to 1.4 is an option.

Comment 36

[Inder]

As per test team's feedback it's not a good UX to see pauses in 30fps playback. I am moving it to CS blocking to see if we can backport this fix and at least fix this by CS timeframe.

Comment 37

[Jason Smith [:jsmith]]

(In reply to Marvin Khoo [:Marvin_Khoo] from comment #35)
> Hi Jason, this improvement is planned in 1.5, consider 1.4 is under
> stabilization, adding feature work or pull in may bring concerns / work from
> various team. this should not be blocker, improvement will be made in 1.5,
> backporting to 1.4 is an option.

I don't think it makes sense to consider backporting a feature - that poses regression risk to the product, which is going to risk making the product stability worse. We can certainly get a risk assessment here though if need be.

(In reply to Inder from comment #36)
> As per test team's feedback it's not a good UX to see pauses in 30fps
> playback. I am moving it to CS blocking to see if we can backport this fix
> and at least fix this by CS timeframe.

Something that isn't great UX doesn't necessarily make a blocker. In order to block here, we really need a clarification of why this is absolutely needed for 1.4 specifically. What partner use cases does this block for 1.4? Are there partner apps requiring this? Product is currently against blocking on this, so I suggest following up with the product team here to clarify what the 1.4 specific need is here.

Comment 38

[Inder]

Jason - I kept it in CS blocking because it was suggested that backporting is an option and so we could track the work. If we are not planning to do it at all for v1.4 then I guess we can remove the blocker. The use case is simply to not see pauses during seek in 30fps playback. The request was from test team's perspective where we try to make sure the UX is similar to other OSes on the same chipset.

Comment 39

[Jason Smith [:jsmith]]

From an IRC discussion with Chris Pearce:

There is some regression risk to taking the required dependencies here. It's changing the logic for determining a playback position after a seek. Chris wants to bake this first for a few weeks before we consider uplifting it. The implementation does make the product nicer overall. The risk is that playback position could be wrong after seeking, which is an unharmful thing to UX if there does end up being a bug in there. Fail case is - user seeks to time T, Video App reports ending up at time X, user can still play and playback still ends when end of media is reached. We normally don't uplift features, so we need a strong case on why this is required on 1.4.

Comment 40

[Preeti Raghunath(:Preeti)]

The ask is for 1.5. Will be tracked to block 1.5 QC FC

Comment 41

[Jason Smith [:jsmith]]

Chris - With the dependent bug fixed, is this bug fixed as well?

Comment 42

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Jason Smith [:jsmith] from comment #41)
> Chris - With the dependent bug fixed, is this bug fixed as well?

For <video> in the Browser App yes. For video in the Video App no, bug 996398 needs to land to improve that. I haven't had a chance to address the review comments on that, someone else is welcome to take it off me if they wish.

Comment 43

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

ni? to Ivan as he knows who to take look into.
nominate 2.1? as CAF requests for 2.1.

Comment 44

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

(In reply to Wesley Huang [:wesley_huang] from comment #43)
> ni? to Ivan as he knows who to take look into.
> nominate 2.1? as CAF requests for 2.1.
Sorry, request is on 2.0

Comment 45

[Ivan Tsay (:ITsay)]

This one is more like a meta bug for lag issue. Need to finish the dependent bug 996398. Already find the engineer to work on bug 996398 and evaluate it to see if we can get it done in 2.0 FL time frame. Will update in the bug 996398.

Comment 46

[bhavana bajaj [:bajaj]]

Clearing the blocking flag as this does not technically block ship the release and rather setting the feature-b2g:2.0 as this is targeted by 2.0 FL.

Comment 47

[Jean Gong :jgong]

Ivan, please assign a dev owner for if this is still targeted for v2.0 FL.  Also, if this is a meta bug to track Lag issue, please change the summary title to include [Meta].

Comment 48

[Ivan Tsay (:ITsay)]

Hi Hema,

Mind if you can take this bug and help to monitor the feature on video app to use new fask seek API (). As far as I understand, once the bug 996398 is landed, this one is resolved as well.

Comment 49

[Hema Koka [:hema]]

(In reply to Ivan Tsay (:ITsay) from comment #48)
> Hi Hema,
> 
> Mind if you can take this bug and help to monitor the feature on video app
> to use new fask seek API (). As far as I understand, once the bug 996398 is
> landed, this one is resolved as well.

Russ has agreed to address the review comments for bug 996398 and try to wrap it up before FL since John is out. Once that is done, we need to ensure that it indeed fixes the lag (If it does not, Eric's team is probably the best to take this up). 

I will assign this to Russ for now, but leave it under Audio/Video component so it is under Eric's radar. 



Thanks
Hema

Comment 50

[Ivan Tsay (:ITsay)]

(In reply to Hema Koka [:hema] from comment #49)
> (In reply to Ivan Tsay (:ITsay) from comment #48)
> > Hi Hema,
> > 
> > Mind if you can take this bug and help to monitor the feature on video app
> > to use new fask seek API (). As far as I understand, once the bug 996398 is
> > landed, this one is resolved as well.
> 
> Russ has agreed to address the review comments for bug 996398 and try to
> wrap it up before FL since John is out. Once that is done, we need to ensure
> that it indeed fixes the lag (If it does not, Eric's team is probably the
> best to take this up). 
> 
> I will assign this to Russ for now, but leave it under Audio/Video component
> so it is under Eric's radar. 

Thanks and agree. Please ni? me if the lag is not resolved after fixing the dependent bug 996398.

Comment 51

[Russ Nicoletti [:russn]]

From my testing with the 996398 change, there is no lag when using the slider to seek within the video.