Closed Bug 1145517 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Audio/Video, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files, 1 obsolete file)

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: 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+
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)
sorry for that
Attachment #8580576 - Flags: review?(cpearce)
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
Closed: 6 years ago
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.