Open Bug 1282012 Opened 8 years ago Updated 2 years ago

Seek to nearest keyframe when resuming videos with no audio

Categories

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

defect

Tracking

()

People

(Reporter: u480271, Unassigned)

References

(Depends on 2 open bugs, Blocks 2 open bugs)

Details

When a video is suspended in background tab we always use an accurate seek when resuming. For videos that don't have any audio to keep in sync with just seek to the nearest keyframe instead.

There are 3 possibilities for "no audio":
1. Element is muted,
2. Video file has no audio track, and
3. Video file has audio track of silence
Blocks: 1276556
If the nearest key frame is after the previous playing position, user will not see those frames between the nearest key frame and the previous playing position. It should be better to seek to the previous key frame or use accurate seek if that previous key frame is too far from the previous playing position.
Mass change P2 -> P3
Priority: P2 → P3
Assignee: nobody → kaku
(In reply to Dan Glastonbury :kamidphish from comment #0)
> When a video is suspended in background tab we always use an accurate seek
> when resuming. For videos that don't have any audio to keep in sync with
> just seek to the nearest keyframe instead.
> 
> There are 3 possibilities for "no audio":
> 1. Element is muted,

Since the script or the user can unmute the element at any time, we still need an accurate seek (wrt both audio and video) to keep AV sync, right?
(In reply to Dan Glastonbury :kamidphish from comment #0)
> 3. Video file has audio track of silence

I think this is hard to implement because you won't know if it is silence or not until examining all the bytes of the audio track.
(In reply to JW Wang [:jwwang] from comment #3)
> (In reply to Dan Glastonbury :kamidphish from comment #0)
> > When a video is suspended in background tab we always use an accurate seek
> > when resuming. For videos that don't have any audio to keep in sync with
> > just seek to the nearest keyframe instead.
> > 
> > There are 3 possibilities for "no audio":
> > 1. Element is muted,
> 
> Since the script or the user can unmute the element at any time, we still
> need an accurate seek (wrt both audio and video) to keep AV sync, right?
Is it possible to invoke a FastSeek to both audio/video and keeps the sought result be AV-synced?
(In reply to Tzuhao Kuo [:kaku] from comment #5)
> Is it possible to invoke a FastSeek to both audio/video and keeps the sought
> result be AV-synced?

Our full seek (including accurate and fast seek) always guarantees AV sync. In fact, our VideoSink should guarantee AV sync no matter it is a full seek or partial seek (video-only seek). But I will suggest to give the later a try to make sure it is true.
Agree to JW that muted element case is somewhat tricky and examining all samples in the audio track is not realistic. So, I think the second case, element without audio track, might be the only case for putting efforts.

About the FastSeek, although it is very efficient, I am afraid that it might lead to weird behavior for some special files. For example, if the video file's key frame space is large then it might seek to somewhere that is not reasonable. In particular, if the file has only one key frame, then the element will be sought to the same position each time it is resumed.

@Dan, WDYT? Have you ever discussed the special files?
Flags: needinfo?(dglastonbury)
(In reply to JW Wang [:jwwang] from comment #4)
> (In reply to Dan Glastonbury :kamidphish from comment #0)
> > 3. Video file has audio track of silence
> 
> I think this is hard to implement because you won't know if it is silence or
> not until examining all the bytes of the audio track.

I think we do what alwu needs to do for detecting silent video. If there is no audio for some amount for time (a few seconds?) at the beginning of playback we consider the video silent. As soon as we detect audio data then we disable video suspend.

Or will this cause A/V sync issues?  We should develop a test case!
(In reply to Tzuhao Kuo [:kaku] from comment #7)
> About the FastSeek, although it is very efficient, I am afraid that it might
> lead to weird behavior for some special files. For example, if the video
> file's key frame space is large then it might seek to somewhere that is not
> reasonable. In particular, if the file has only one key frame, then the
> element will be sought to the same position each time it is resumed.
> 
> @Dan, WDYT? Have you ever discussed the special files?

Kaku, we've discussed the large key frame spacing issue. This is why :gerald has added telemetry to collect information about key frame spacing. We'll use that to guide development of a heuristic and disable video suspend for media that have key frames too far apart, or only one key frame at the very beginning.
Flags: needinfo?(dglastonbury)
(In reply to Dan Glastonbury :kamidphish from comment #9)
> Kaku, we've discussed the large key frame spacing issue. This is why :gerald
> has added telemetry to collect information about key frame spacing. We'll
> use that to guide development of a heuristic and disable video suspend for
> media that have key frames too far apart, or only one key frame at the very
> beginning.

Note that the inter-keyframe probe only gathers durations between successive decoded keyframes (see bug 1289668), meaning that if a video only has one keyframe it will not show up in the telemetry.
It may be possible to add a special case when we only see one keyframe, and record a '0' in this case, it would show up in the telemetry and could give us some sense of how prevalent they are. Interested?
(In reply to Gerald Squelart [:gerald] from comment #10)
> Note that the inter-keyframe probe only gathers durations between successive
> decoded keyframes (see bug 1289668), meaning that if a video only has one
> keyframe it will not show up in the telemetry.
> It may be possible to add a special case when we only see one keyframe, and
> record a '0' in this case, it would show up in the telemetry and could give
> us some sense of how prevalent they are. Interested?
Yes, I am interested!
Depends on: 1294384
Per discussion offline, I will first implement this feature for "video file has no audio track" cases, which is the most safe one and will be shipped in phase 1. 

Will then split this bug into sub-bugs for each case.
Depends on: 1294656
Depends on: 1294657
Depends on: 1294658
(In reply to Tzuhao Kuo [:kaku] from comment #11)
> (In reply to Gerald Squelart [:gerald] from comment #10)
> > It may be possible to add a special case when we only see one keyframe[...] Interested?
> Yes, I am interested!

-> bug 1295831
(In reply to Tzuhao Kuo [:kaku] from comment #14)
> The data is beyond my imagination......,
> 
> https://telemetry.mozilla.org/new-pipeline/dist.html#!
> cumulative=0&end_date=2016-08-21&keys=All!AV%252C240%253Ch%253C%253D480!
> AV%252C576%253Ch%253C%253D720!
> V%252C240%253Ch%253C%253D480&max_channel_version=nightly%252F51&measure=VIDEO
> _INTER_KEYFRAME_MAX_MS&min_channel_version=null&product=Firefox&sanitize=1&so
> rt_keys=submissions&start_date=2016-08-
> 21&table=0&trim=1&use_submission_date=0
> 
> Over 50% videos has only one single (observable) key frame (from the data
> collected at 2016/8/21, one single day).

And we're certain we have no bugs in the reporting?
Flags: needinfo?(gsquelart)
(In reply to Dan Glastonbury :kamidphish from comment #15)
> And we're certain we have no bugs in the reporting?
I also reviewed again, http://searchfox.org/mozilla-central/source/dom/html/HTMLMediaElement.cpp#3232, and I think it's right. 

The only point is that what we collected are videos having only one key frame during a long enough playing period (larger than the media.suspend-bkgnd-video.delay-ms value which is 10 seconds in default). What if someone change the default to a small value? In that case, the data is not meaningful, I think.
Thank you Kaku.

Now this bug (about seeking) is probably not the best place to discuss this -- Sorry for the distraction started in comment 10!
I've opened bug 1297971 so we can move the discussion there.
Flags: needinfo?(gsquelart)
No longer blocks: 1276556
Depends on: 1358057

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Assignee: kakukogou → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.