1036049 - Add H.264 STAP-A reception support

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

We must support STAP-A reception for mode-1 H.264 packetization, which is easy.  We do not have to support sending STAP-A, though it would be nice for SPS/PPS.

The original patches from bug 985254 did not include STAP-A support.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Support H.264 STAP-A depacketization in webrtc

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8452610 [details] [diff] [review]
Support H.264 STAP-A depacketization in webrtc

Also, could you try it with some of your existing H.264 equipment?  Thanks

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

2.0? because RFC 6184 mandates that for H.264 mode 1 packetization, you MUST support receiving STAP-A aggregate packets - and most mode-1 equipment will send them.

Comment 4

[Ethan Hugg [:ehugg]]

Comment on attachment 8452610 [details] [diff] [review]
Support H.264 STAP-A depacketization in webrtc

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

This looks good to me but I am unable at this time to connect it to try it out.

Comment 5

[Jason Smith [:jsmith]]

(In reply to Randell Jesup [:jesup] from comment #3)
> 2.0? because RFC 6184 mandates that for H.264 mode 1 packetization, you MUST
> support receiving STAP-A aggregate packets - and most mode-1 equipment will
> send them.

Triage is having some difficulty understanding this. Can you explain this in terms of end-user impact if what would happen if we did not fix this?

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

So if we connect (via gateway) to any standard H.264 equipment, such as Cisco gear (which is a lot of why they're providing OpenH264 gratis), they will use RFC 6184 mode-1 packetization (which is all we support currently).  The problem is that they will send both FU-A (fragmented) packets, and STAP-A (aggregate packets), and without this patch we don't understand STAP-A (we currently only send FU-A).  STAP-A is mandated if you do mode 1.

In addition, if we fix our code to generate STAP-A in the future, we will then be incompatible with older revs of Firefox (i.e. a FF33 or 34 calls a B2G 2.0 phone (based on 32) - it would fail.)

I plan to land this on inbound today; I was waiting until I could double-check functionality (with a temporary patch to force sending some STAP-A packets) since we don't have a working gateway to other H.264 stuff yet.

Comment 7

[Jason Smith [:jsmith]]

Needed for compat, so blocking.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Support H.264 STAP-A depacketization in webrtc

Now includes test code; works.  Fixed the header data, which needed to be set up like single-nals before forwarding the extracted packet onwards, and ntohs() the length words.

Comment 9

[Ethan Hugg [:ehugg]]

Comment on attachment 8454228 [details] [diff] [review]
Support H.264 STAP-A depacketization in webrtc

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

This looks OK and appears to work for me on OSX with TEST_STAP_A turned on.

::: media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc
@@ +292,5 @@
> +      h264_header->nalu_header        = payload[0];
> +      switch (*payload & RtpFormatH264::kH264NAL_TypeMask) {
> +        case RtpFormatH264::kH264NALU_SPS:
> +          // TODO(jesup): Evil hack.  see below
> +          rtp_header->header.timestamp = timestamp - 20;

The evil hack below has SPS and PPS both -10, what am I missing that would make this one -20 instead?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

> > +      h264_header->nalu_header        = payload[0];
> > +      switch (*payload & RtpFormatH264::kH264NAL_TypeMask) {
> > +        case RtpFormatH264::kH264NALU_SPS:
> > +          // TODO(jesup): Evil hack.  see below
> > +          rtp_header->header.timestamp = timestamp - 20;
> 
> The evil hack below has SPS and PPS both -10, what am I missing that would
> make this one -20 instead?

Actually, the lower one gets -10 and -20 due to the fall-throughs

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2bdf8a5472ce

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2bdf8a5472ce

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d4d12cf51c49