Seeking in WebMs with clusters that start without keyframes fails

RESOLVED FIXED in Firefox 46

Status

()

defect
P1
normal
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: bryce, Assigned: jya)

Tracking

(Blocks 1 bug, {regression})

46 Branch
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46+ fixed, firefox47+ fixed, firefox48 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Seeking in WebM files that have clusters that do not that with keyframes may fail. Such files should not be created as per the WebM guidelines:

> Muxers should treat all guidelines marked SHOULD in this section as MUST.
> Key frames SHOULD be placed at the beginning of clusters.

However, such files exist in the wild: http://downloads.webmproject.org/media/video/webmproject.org/big_buck_bunny_trailer_480p_logo.webm

(note also this file has duplicate seek heads)

Firefox 45 handles seeking in such files, however starting with Firefox 46 seeking in these files can fail.

Looking at this with Mozregression it appears the problem is introduced around the following:

app_name: firefox
build_date: 2016-01-06 12:52:18.447000
build_file: C:\Users\Bryce Van Dyk\.mozilla\mozregression\persist\84db9e4cf212--mozilla-inbound--firefox-46.0a1.en-US.win64.zip
build_type: inbound
build_url: https://queue.taskcluster.net/v1/task/GXXvjIySRT6xqMwXoGN2SA/runs/0/artifacts/public%2Fbuild%2Ffirefox-46.0a1.en-US.win64.zip
changeset: 84db9e4cf21291b85f40963867467d9650e72453
pushlog_url: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=84db9e4cf21291b85f40963867467d9650e72453&tochange=48fcdba5ff2b83906d62f5d5ecea39913a391bf0
repo_name: mozilla-inbound
repo_url: https://hg.mozilla.org/integration/mozilla-inbound
task_id: GXXvjIySRT6xqMwXoGN2SA
(Reporter)

Updated

3 years ago
Flags: needinfo?(jyavenard)
Keywords: regression
(Assignee)

Comment 1

3 years ago
what do you mean by "can't seek" ?

The change linked to is only changing the decoder, it has nothing to do with the webm demuxer (which is what handles seeking)

Bug 1214462 also only introduces the ffvp9 decoder, it doesn't activate it yet, and it is as such unused.
Flags: needinfo?(jyavenard)
(Assignee)

Updated

3 years ago
Assignee: nobody → jyavenard
(Assignee)

Updated

3 years ago
Priority: -- → P1
(Assignee)

Comment 2

3 years ago
after flushing, ffmpeg must be fed a keyframe first. it will return an error otherwise.

webmdemuxer ideally should seek to a keyframe.
(Assignee)

Comment 3

3 years ago
Same after a reset or the first frame ever returned by the demuxer.

Review commit: https://reviewboard.mozilla.org/r/44767/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44767/
Attachment #8738910 - Flags: review?(kinetik)
(Assignee)

Comment 4

3 years ago
[Tracking Requested - why for this release]: regression due to use of ffvpx. It exposes a bug to our webm demuxer
Comment on attachment 8738910 [details]
MozReview Request: Bug 1262727: [webm] Ensure first frame returned after seek is a keyframe. r?kinetik

https://reviewboard.mozilla.org/r/44767/#review41901
Attachment #8738910 - Flags: review?(kinetik) → review+

Comment 7

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bfc6effa82c9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Assignee)

Comment 8

3 years ago
Comment on attachment 8738910 [details]
MozReview Request: Bug 1262727: [webm] Ensure first frame returned after seek is a keyframe. r?kinetik

Approval Request Comment
[Feature/regressing bug #]:1262727
[User impact if declined]:errors when seeking in some webm, regression over previous version
[Describe test coverage new/current, TreeHerder]: in central, mocha test, manual testing
[Risks and why]: low, we ensure the data returned is valid
[String/UUID change made/needed]: none
Attachment #8738910 - Flags: approval-mozilla-beta?
Attachment #8738910 - Flags: approval-mozilla-aurora?
Comment on attachment 8738910 [details]
MozReview Request: Bug 1262727: [webm] Ensure first frame returned after seek is a keyframe. r?kinetik

Good to have seek working in videos,  has existing test coverage. 
Please uplift to aurora and beta. This would land for beta 11.
Attachment #8738910 - Flags: approval-mozilla-beta?
Attachment #8738910 - Flags: approval-mozilla-beta+
Attachment #8738910 - Flags: approval-mozilla-aurora?
Attachment #8738910 - Flags: approval-mozilla-aurora+
jya is there new test coverage you could add to cover this regression? Would that be useful?
Flags: needinfo?(jyavenard)
(Assignee)

Comment 11

3 years ago
We don't. I'll add one..
Flags: needinfo?(jyavenard)
(Assignee)

Updated

3 years ago
Blocks: 1264121
You need to log in before you can comment on or make changes to this bug.