Closed
Bug 1045468
Opened 10 years ago
Closed 10 years ago
firefox doesn't support SVC NAL(NAL type 14)
Categories
(Core :: WebRTC: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: ruil2, Assigned: ruil2)
Details
Attachments
(1 file, 1 obsolete file)
3.82 KB,
patch
|
ehugg
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; WOW64; Trident/5.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; InfoPath.3; .NET4.0C; .NET4.0E) Steps to reproduce: interact with the product which creates SVC NAL Actual results: firefox can't decode the bitstream from the product with SVC NAL. no video display Expected results: firefox can dislay the video correclty.
Summary: Don't support SVC NAL(NAL type 14) → firefox doesn't support SVC NAL(NAL type 14)
Comment 2•10 years ago
|
||
Comment on attachment 8463821 [details] [diff] [review] add SVC NAL(NAL type 14)support Review of attachment 8463821 [details] [diff] [review]: ----------------------------------------------------------------- Setting Randell as reviewer. ::: media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc @@ +300,5 @@ > rtp_header->header.timestamp = timestamp - 10; > rtp_header->frameType = kVideoFrameKey; > break; > + case RtpFormatH264::kh264NALU_PREFIX: > + rtp_header->header.timestamp = timestamp - 10; I believe the evil hack requires this timestamp changing to be different than the one above. That is essentially what you have on the change below because of the fallthrough. You get -30, -20, -10 there. I'll defer to Randell on this and set him as reviewer.
Attachment #8463821 -
Flags: review?(rjesup)
Comment 3•10 years ago
|
||
Comment on attachment 8463821 [details] [diff] [review] add SVC NAL(NAL type 14)support Review of attachment 8463821 [details] [diff] [review]: ----------------------------------------------------------------- The same value is ok if (and only if) the PREFIX NAL will not be immediately adjacent to a PPS. If you have SPS PPS PREFIX, it will fail. (Or PREFIX PPS SPS, but that would be unexpected - I've always see SPS before PPS). If it will never follow a PPS, -10 is fine. Otherwise choose a different value - and to avoid confusing it, the timestamps in that case must go in the "right" direction, so try -5.
Attachment #8463821 -
Flags: review?(rjesup) → review+
Comment 4•10 years ago
|
||
Karina, please respond to Randell's comments above. Let us know if you want to update this patch or go with what you have here and then I can push this for you tomorrow.
Updated•10 years ago
|
Assignee: nobody → ruil2
Comment 5•10 years ago
|
||
If you simply make it -5, it will handle all cases except PREFIX SPS or PREFIX PPS. That'd be good by me, as I'm guessing you wouldn't see those sequences.
Hi Ethan: I had updated the patch, what is the next step for this bug? thanks a lot!
Flags: needinfo?(ethanhugg)
Comment 8•10 years ago
|
||
Comment on attachment 8464396 [details] [diff] [review] svc_nal_support_update.patch Bringing forward r+ from jesup
Attachment #8464396 -
Flags: review+
Flags: needinfo?(ethanhugg)
Updated•10 years ago
|
Attachment #8463821 -
Attachment is obsolete: true
Comment 9•10 years ago
|
||
Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/641d9afb5bd2
Comment 10•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/641d9afb5bd2
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Updated•10 years ago
|
Component: Untriaged → WebRTC: Audio/Video
Product: Firefox → Core
Target Milestone: Firefox 34 → ---
Updated•10 years ago
|
Target Milestone: --- → mozilla34
Comment 11•10 years ago
|
||
Comment on attachment 8464396 [details] [diff] [review] svc_nal_support_update.patch Approval Request Comment [Feature/regressing bug #]: N/A [User impact if declined]: PREFIX H.264 NALs won't be processed and ignored properly; forces rebase of Mode 0 support. [Describe test coverage new/current, TBPL]: On m-c for several weeks; testing requires Cisco equipment that generates SVC-based streams. Tested by Cisco [Risks and why]: Very low; trivial patch to add some entries to case statements (basically). [String/UUID change made/needed]: none
Attachment #8464396 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
status-firefox33:
--- → affected
status-firefox34:
--- → fixed
Updated•10 years ago
|
Attachment #8464396 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in
before you can comment on or make changes to this bug.
Description
•