H264::DecodeNALUnit doesn't properly handle emulation prevention bytes

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla39
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
The H264 SPS parser expect data to have been decoded and that the emulation prevention bytes been removed.

However, the H264::DecodeNALUnit doesn't properly handle those that leads to invalid data being decoded.

In particular, with some videos the aspect ratios are incorrectly read.
(Assignee)

Updated

4 years ago
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Comment on attachment 8580553 [details] [diff] [review]
Properly skip the NAL's emulation prevention byte

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

Legend!
Attachment #8580553 - Flags: review?(cpearce) → review+
(Assignee)

Comment 3

4 years ago
I can't right now think on why it would have been a problem, but I'm now not confident with that optimisation. So follow exactly the algorithm as per ITU H.264 7.3.1
Attachment #8580567 - Flags: review?(cpearce)
(Assignee)

Comment 4

4 years ago
sorry for that
Attachment #8580576 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8580567 - Attachment is obsolete: true
Attachment #8580567 - Flags: review?(cpearce)
Attachment #8580576 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/45d65bb9e0ef
https://hg.mozilla.org/mozilla-central/rev/63b94f124c88
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8580553 [details] [diff] [review]
Properly skip the NAL's emulation prevention byte

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: EME videos will appear at incorrect apsect ratio
[Describe test coverage new/current, TreeHerder]: Local testing.
[Risks and why]: Pretty low, we've tested this pretty thoroughly locally over the weekend.
[String/UUID change made/needed]: None.
Attachment #8580553 - Flags: approval-mozilla-aurora?
Comment on attachment 8580576 [details] [diff] [review]
Part2. Follow exactly ITU H.264 7.3.1

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: EME videos will appear at incorrect apsect ratio
[Describe test coverage new/current, TreeHerder]: Local testing.
[Risks and why]: Pretty low, we've tested this pretty thoroughly locally over the weekend.
[String/UUID change made/needed]: None.
Attachment #8580576 - Flags: approval-mozilla-aurora?
Attachment #8580553 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8580576 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(cpearce)
You need to log in before you can comment on or make changes to this bug.