1167544 - OpenH264 appears to send STAP-A aggregation packets in mode 0

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P1)

Description

[jon.peter.davies]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/42.0.2311.152 Safari/537.36

Steps to reproduce:

WebRTC call with a desktop (non-browser) endpoint on Windows, using JsSIP and Kamailio SIP proxy. The endpoint offers this in its SDP:

m=video <port> RTP/SAVPF 97
a=fmtp:97 profile-level-id=42801F

Following discussions with Randell Jesup (at mozilla.dev.media), I managed to alter the SDP so that Firefox accepts the SDP by changing the profile-level-id to 42e01F (Constrained Baseline?). 


Actual results:

The call succeeds, and we get 2-way video. Firefox is decoding the H264 video sent by the endpoint, but the endpoint is not able to decode the video sent by Firefox. 

The call negotiated packetisation-mode 0 (which is what the endpoint expects) since this was not specified in the SDP offer/answer exchange and is the default. 

On analysis on the remote endpoint by my colleague, the NALU header received is 0x78 indicating STAP-A. My colleague asserts that this NALU value should not have been sent.  


Expected results:

If the negotiation was for mode 0, then Firefox should not send a STAP-A (reference discussion https://groups.google.com/forum/#!topic/mozilla.dev.media/z3HrVEfvlsU).

Comment 1

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

possibly a regression in 38 due to the webrtc_40 update

Likely we need to add the mode to the codecSpecific structure and use that in the h264 packetizer, and key off that to skip both FuA and STAP-A processing.  (FuA shouldn't get invoked as the max encoder NAL size should be < 1 RTP packet)

Comment 2

[jon.peter.davies]

When do you this might be resolved?

Comment 3

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

This will be resolved in 42, and may uplift to 41 (likely the fix is simple)

Comment 5

[Timothy Steeves]

I'm having the same issue. P slices, etc are packetized as Single NAL units as expected, but I'm seeing SPS/PPS, and the first I slice sent as STAP-A.

May be related to webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:80-86

Comment 6

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

(In reply to Timothy Steeves from comment #5)
> I'm having the same issue. P slices, etc are packetized as Single NAL units
> as expected, but I'm seeing SPS/PPS, and the first I slice sent as STAP-A.
> 
> May be related to webrtc/modules/rtp_rtcp/source/rtp_format_h264.cc:80-86

Yes.  The issue is some work needs to be done to expose the negotiated mode to the level of code that packetizes.

Comment 7

[Maire Reavy [:mreavy]]

Bumping the rank up because we'd love to land this in Fx50 if we can.

Comment 8

[Dan Minor [:dminor]]

Attachment: Bug 1167544 - OpenH264 should not send STAP-A aggregation packets in mode 0;

Review commit: https://reviewboard.mozilla.org/r/58254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58254/

Comment 9

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

Comment on attachment 8760810 [details]
Bug 1167544 - OpenH264 should not send STAP-A aggregation packets in mode 0;

https://reviewboard.mozilla.org/r/58254/#review55376

Thanks!

::: media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp:646
(Diff revision 1)
> -      mCallback->Encoded(unit, nullptr, &fragmentation);
> +      GMPCodecSpecificInfo info;
> +      memcpy(&info, aCodecSpecificInfo.Elements(), sizeof(GMPCodecSpecificInfo));
> +      webrtc::CodecSpecificInfo codecSpecificInfo;
> +      codecSpecificInfo.codecType = webrtc::kVideoCodecH264;
> +      // TODO: Currently the OpenH264 codec does not preserve any codec
> +      //       specific info passed into it, so even if we were to add
> +      //       packetization mode to the codec specific info, the value
> +      //       passed in would not be returned to us. If this changes in the
> +      //       future, it would be nice to get rid of mPacketizationMode.
> +      codecSpecificInfo.codecSpecific.H264.packetizationMode = mPacketizationMode;
> +      codecSpecificInfo.codecSpecific.H264.simulcastIdx = info.mCodecSpecific.mH264.mSimulcastIdx;
> +      mCallback->Encoded(unit, &codecSpecificInfo, &fragmentation);

this data is basically static, is it not?  Can we just generate it once in mCodecSpecificInfo?  Perhaps assert the simulcast index doesn't change if we're paranoid

::: media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_format_h264.h:25
(Diff revision 1)
> -  RtpPacketizerH264(FrameType frame_type, size_t max_payload_len);
> +  RtpPacketizerH264(FrameType frame_type,
> +                    size_t max_payload_len,
> +                    bool use_stap_a);

We should pass the packetization mode in (or rename the boolean as use_mode_1) - since we also want to disallow FU-A packets in mode 0, and if the encoder generates too-large payloads just make an oversize packet and hope (or just fail)

::: media/webrtc/trunk/webrtc/video_engine/vie_encoder.cc:636
(Diff revision 1)
>    }
>  #endif
>    // XXX fix VP9 (bug 1138629)
>  
> +  // See Bug 1167544, we need to pass in the packetization mode to avoid
> +  // sending STA-A packets in mode 0. Although VideoCodingModule has all

STAP-A

Comment 10

[Dan Minor [:dminor]]

Comment on attachment 8760810 [details]
Bug 1167544 - OpenH264 should not send STAP-A aggregation packets in mode 0;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58254/diff/1-2/

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3374f4fc0705
OpenH264 should not send STAP-A aggregation packets in mode 0; r=jesup

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3374f4fc0705