Closed Bug 1262727 Opened 6 years ago Closed 6 years ago

Seeking in WebMs with clusters that start without keyframes fails

Categories

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

46 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox46 + fixed
firefox47 + fixed
firefox48 --- fixed

People

(Reporter: bryce, Assigned: jya)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file)

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
Flags: needinfo?(jyavenard)
Keywords: regression
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: nobody → jyavenard
Priority: -- → P1
after flushing, ffmpeg must be fed a keyframe first. it will return an error otherwise.

webmdemuxer ideally should seek to a keyframe.
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)
[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+
https://hg.mozilla.org/mozilla-central/rev/bfc6effa82c9
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
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)
We don't. I'll add one..
Flags: needinfo?(jyavenard)
Blocks: 1264121
You need to log in before you can comment on or make changes to this bug.