Closed Bug 1145517 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla39
Tracking Flags:
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Properly skip the NAL's emulation prevention byte
1007 bytes, patch
cpearce
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Part2. Follow exactly ITU H.264 7.3.1
2.41 KB, patch
cpearce
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review
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.
Doh!
Attachment #8580553 - Flags: review?(cpearce)
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: 11 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.

Attachment

General

Creator:
Created:
Updated:
Size: