h264 codec can't work when setting mode 0

RESOLVED WORKSFORME

Status

()

Core
WebRTC: Audio/Video
P2
normal
Rank:
25
RESOLVED WORKSFORME
3 years ago
3 years ago

People

(Reporter: karina, Assigned: ehugg)

Tracking

33 Branch
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141030112145

Steps to reproduce:

testing on http://mozilla.github.io/webrtc-landing/pc_test.html
select "Required H.264 Video" and "Prefer H.264 Mode 0"


Actual results:

receiver can't see the video 


Expected results:

video can be seen correctly
(Reporter)

Comment 1

3 years ago
Created attachment 8521224 [details]
multi-slice-support.txt
(Reporter)

Comment 2

3 years ago
Root cause: For mode 0, the first VCL NAL is considered as the last NAL in one frame, so other NALs can’t be sent to decoder.
solution: use one bit to indicate whether the current NAL is the last NAL in one frame.
in rtp_format_h264.cc line 96
      Old code:
       if (type == kSps || type == kPps ||
        type == kSei || type == kPrefix) {
      *last_packet   = false;
    } else {
      *last_packet   = true;
    }

   Updated code: using forbiddenzerobit to judge whether current NAL is the last NAL. 
      if (type == kSps || type == kPps ||
        type == kSei || type == kPrefix) {
      *last_packet   = false;
    } else {
      if(payload_data_[0]&kFBit)
         *last_packet   = true;
      else
         *last_packet   = false;
    }
Component: Untriaged → WebRTC: Audio/Video
Product: Firefox → Core
(Reporter)

Comment 3

3 years ago
1. This is a sender side issue, the sender side doesn’t set the correct last packet flag.  One frame has several VCL NAL for Mode 0.  But the sender side sets the last packet flag in  each VCL NAL.
       2. When the receiver side receives the first VCL NAL, it thinks that the AU is complete and sends it to decoder, so the decoder can’t decode correctly.  The other VCL packets are discarded (but I don’t read which code do this,only from the trace )
  
    updated code on the sender side.
     1. In WebrtcGmpVideCodec.cpp, set last NAL flag in WebrtcGmpVideoEncoder::Encoded(),because only here can get this flag.  If it is last NAL, set forbidden bit as 1. Otherwise not set.
     2. In rtp_format_h264.cc, parser this bit to judge whether it is the last NAL and set *last_packet. And reset forbidden bit to keep the same data with encoder output.
Randell -- Can you look at this?
Assignee: nobody → rjesup
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 5

3 years ago
Currently we negotiate Mode 0, but we do not send it.  A patch to the OpenH264 wrapper to fix this is here:
https://rbcommons.com/s/OpenH264/r/584/
You can also build it quickly from Karina's repo: 
https://github.com/ruil2/openh264/tree/max_nal_size

This change respects the maxPayloadSize in the GMP wrapper.  If you try it out, Mode 1 still works but Mode 0 will not because of the issues Karina has outlined above.  We're trying to figure out what we need to Fix in Firefox so we can start properly sending Mode 0, then we will need to coordinate a release of FF with an update to the OpenH264 GMP plugin.
(Assignee)

Comment 6

3 years ago
Created attachment 8521681 [details] [diff] [review]
WebRTC Support for H264 Mode 0
(Assignee)

Updated

3 years ago
Assignee: rjesup → ethanhugg
Status: NEW → ASSIGNED
(Assignee)

Comment 7

3 years ago
Comment on attachment 8521681 [details] [diff] [review]
WebRTC Support for H264 Mode 0

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

Re-uploaded Karina's fix as a patch with couple format/style changes.

This makes Mode 0 work for loopback, but it's unclear to me that this improves our compatibility with other endpoints using Mode 0.
Ethan - this patch is totally bit-rotted due to the webrtc_40 packetization changes.  Likely there's some small bug in the mode 0 support; from my reading the code currently *should* be setting the Marker bit correctly since we're feeding the data up using the FragmentationHeader structure.  I suspect the entire (or almost) current patch is moot.

Can you work with Jon Peter Davies (who posted on dev.media) to investigate this?  Thanks.
Flags: needinfo?(ethanhugg)

Updated

3 years ago
Rank: 25
Priority: -- → P2
(Assignee)

Updated

3 years ago
Attachment #8521224 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8521681 - Attachment is obsolete: true
(Assignee)

Comment 9

3 years ago
Yes, I will take another look.
Flags: needinfo?(ethanhugg)

Updated

3 years ago
backlog: --- → webRTC+
(Assignee)

Comment 10

3 years ago
This now works since the landing of bug 1162251
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.