Subject: [rtcweb] Review of Section 5.3 of draft-ietf-rtcweb-sdp-10

From: Magnus Westerlund <[email-masked]>
Date: Wed, 4 Jul 2018 16:56:54 +0200

Hi,

I have reviewed Section 5.3 only with a focus on the simulcast and 
multi-stream aspects. For example I have not cared if the ICE details 
are correct in these examples.

A. Section 5.3.1:

BUNDLE grouping framework enables multiplexing of all the 5 streams
    (1 audio stream + 4 video streams) over a single RTP Session.

It might be good to use RFC 7656 terminology and be specific in that 
this results in 5 source RTP streams.

B. Section 5.3.1.

As 5.3 says that this will use FEC or RTX and this one doesn't maybe be 
explicit that it is not added, or rewrite 5.3.

C. section 5.3.1:

    | a=group:LS m0 m1                            | [RFC5888]           |

Is it intentional that video 2 is not included in the lip-synch group? 
May require a intention comment for this case.

D. 5.3.1:

    One video source corresponds to VP8 encoding, while the other
    corresponds to H.264 encoding.

As the m= block represents a media source, if the need is to provide one 
video camera's images (the media  source) as both VP8 and H.264 where 
each are in two different resolutions, then simulcast can handle that 
fine within a single m= block. So from my perspective this is a wrongly 
constructed example from that premise. Can you please clarify if you 
want two media source, i.e. two cameras, or two encoder formats VP8 and 
H.264, or two resolutions, or any combination of them?

I would recommend this example to be two media sources, with encoding 
simulcast. The resolution can be skipped as the later example includes 
resolution simulcasting.

E. Section 5.3.1:

    | a=rtcp-fb:* nack                            | [RFC5104]           |

    | a=rtcp-fb:* nack pli                        | [RFC5104]           |

I would note that generalized NACK as well as picture loss indication 
(PLI) is defined in RFC4585.

F. Section 5.3.2:

    This section shows an SDP Offer/Answer for a session with an audio
    and a single video source.  The video source is encoded as layered
    coding at 3 different resolutions based on [RFC5583].  The video
    m=line shows 3 streams with last stream (payload 100) dependent on
    streams with payload 96 and 97 for decoding.

Also here use of RFC 7656 terminology to talk about (source) RTP streams 
when applicable would be good.

G. Section 5.3.2:

   | a=rtpmap:96 H264/90000                      | [RFC6184]           |
    | a=fmtp:96 profile-level-id=4d0028;          | [RFC6184]H.264      |
    | packetization-mode=1;max-fr=30;max-fs=8040  | Layer 1             |
    | a=rtpmap:97 H264/90000                      | [RFC6184]           |
    | a=fmtp:97 profile-level-                    | [RFC6184] H.264     |
    | id=4d0028;packetization-mode=1; max-        | Layer 2             |
    | fr=15;max-fs=1200 |                     |
    | a=rtpmap:100 H264-SVC/90000                 | [RFC6184]           |
    | a=fmtp:100 profile-level-                   | [RFC6184]           |
    | id=4d0028;packetization-mode=1; max- |                     |
    | fr=30;max-fs=8040 |                     |
    | a=depend:100 lay m1:96,97                   | [RFC5583]Layer 3    |

I have my doubts about this configuration. First of all as it is SVC in 
Single RTP session mode (SST) I don't think it results in multiple RTP 
streams. I think the answerer will interpret this, which of these 
encodings can support. A non scalable H.264, another non-scalable H.264 
or SVC that can contain a number of layers.

Secondly a=depend is only defined for MST mode in RFC 6190.

You also have the wrong reference for the H-264-SVC a=rtpmap and a=fmtp 
line.

H. Section 5.3.3:

    | a=extmap:3 urn:ietf:params:rtp-             | [I-D.ietf-avtext-ri |
    | hdrext:sdes:rtp-stream-id                   | d]                  |

After this line you should have also this line:

    | a=extmap:4 urn:ietf:params:rtp-             | [I-D.ietf-avtext-ri |
    | hdrext:sdes:repaired-rtp-stream-id                   | 
d]                  |

This to enable the RTX streams to indicate which source RTP stream they 
are repairing. Add to both offer and answer.

I. Section 5.3.3

Why isn't RTX enabled for the audio also?

J. Section 5.3.3. Answer:

   | m=video 0 UDP/TLS/RTP/SAVPF 98 100 101 103  | BUNDLE accepted     |

Payload types 100 and 101 are undefined in this media description.

K. Section 5.3.4 Answer:

    | a=rtpmap:101 VP8/90000                      | [RFC7741]           |

Wrong media type for the payload should be RTX.

L. Section 5.3.4 Answer:

Missing rtcp-fb definitions to enable use of NACK which RTX depends on.

M. Section 5.3.5:

  | a=fmtp:101 L=5; D=10; ToP=2; repair-        | [I-D.ietf-payload-f |
    | window=200000                               | lexible-fec-scheme] |
    | a=fmtp:103 L=5; D=10; ToP=2; repair-        | [I-D.ietf-payload-f |
    | window=200000                               | lexible-fec-scheme] |

As the parameters are the same, I don't see a point with having two 
payload types. Two different RTP repair streams can use the same payload 
type. As Flex-FEC can do all the binding using the CSRC field, there are 
also no need to have the repaired-rtp-stream-id header extension for 
this one.

N. Section 5.3.5:

Does it make sense to still have NACK for a session with FEC. If it 
fails isn't PLI or FIR

Cheers

Magnus Westerlund

----------------------------------------------------------------------
Network Architecture & Protocols, Ericsson Research
----------------------------------------------------------------------
Ericsson AB                 | Phone  +46 10 7148287
Torshamnsgatan 23           | Mobile +46 73 0949079
SE-164 80 Stockholm, Sweden | mailto: [email-masked]
----------------------------------------------------------------------

---

From: Suhas Nandakumar <[email-masked]>
Date: Thu, 26 Jul 2018 11:27:43 -0700

Hello Magnus

 Thanks for the review. We will be working on your comments soon and post
an update.

cheers
Suhas

On Wed, Jul 4, 2018 at 7:57 AM Magnus Westerlund <
[email-masked]> wrote:

> _______________________________________________
> rtcweb mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtcweb
>