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

From: Suhas Nandakumar <[email-masked]>
Date: Tue, 9 Oct 2018 20:42:20 -0700

Hello Magnus

 Apologies that this took a while. Please see inline.

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

> 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.
>
> Thanks for the review. I have submitted -11 version that should address
the review comments.

>
> 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.
>
>
[Suhas] Done

> 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.
>

[Suhas] Update the description to mean some examples have FEC/RTX and not
all.
The idea was to have this section with examples that cover all these cases
but
not to have all these in all the examples.

>
> 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.
>

[Suhas] . The intention was to group one AV in a lip sync
group and other can be a non-interactive stream. Updated the text to
indicate the same.

> 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.
>
>
[Suhas]. Yes, the example wanted to showcase 2 different media sources (one
encoded
 as vp8 and another as h264, can be natively encoded as well) and  each
being sent at
 2 different resolutions.

> 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.
>

[Suhas] Done, updated all the references.

>
> 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.
>

[Suhas] Updated.

>
> 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.
>

[Suhas] Agreed Magnus. On re-reading the example it confused me as well :-)
Update the example to represent SST mode SVC configuration.

>
> 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.
>

[Suhas] Good catch. Added it.

>
> I. Section 5.3.3
>
> Why isn't RTX enabled for the audio also?
>

[Suhas] No Specific reason. These are just examples to show configurations
and thus picked MVP
configurations to show the usage of SDP for scenario at hand than
describing all possible
options.

> 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.
>

[Suhas] Thanks for catching this. Matched the answer with the offer now.

>
> K. Section 5.3.4 Answer:
>
>     | a=rtpmap:101 VP8/90000                      | [RFC7741]           |
>
> Wrong media type for the payload should be RTX.
>

[Suhas] Nice find. Fixed it.

>
> L. Section 5.3.4 Answer:
>
> Missing rtcp-fb definitions to enable use of NACK which RTX depends on.
>
>
[Suhas] Agreed. Added it.

>
> 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.
>

[Suhas] Good point. Moved to just one PT. Also i think you intend to
include  repaired-rtp-stream-id for this example too ?

> 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
>

[Suhas] Dont think its needed. Removed nack and kept pli

>
> 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]
> ----------------------------------------------------------------------
>
> _______________________________________________
> rtcweb mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtcweb
>

---

From: Magnus Westerlund <[email-masked]>
Date: Mon, 15 Oct 2018 16:40:15 +0200

Hi,

I have looked at the changes in -11. I have some questions.

1. In table 29, section 5.3.2 regarding the H.264-SVC.

    | a=fmtp:100 profile-level-                   | [RFC6190] H.264     |
    | id=53001f;packetization-mode=0              | Scalable Encoding   |

Why are you using packetization-mode=0? For SVC in SST, wouldn't mode=1 
be the most reasonable choice?  Not that it will not work, but putting 
all the meta data NALUs into their own RTP packets seems unnecessary.

2. Table 30, same question as 1)

Otherwise the changes related to my review looks good.

Cheers

Magnus

On 2018-10-10 05:42, Suhas Nandakumar wrote:
> On Wed, Jul 4, 2018 at 7:57 AM Magnus Westerlund 
> <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>     Hi,
>
> Thanks for the review. I have submitted -11 version that should 
> address the review comments.
>
>
>     A. Section 5.3.1:
>
> [Suhas] Update the description to mean some examples have FEC/RTX and 
> not all.
> The idea was to have this section with examples that cover all these 
> cases but
> [Suhas]. Yes, the example wanted to showcase 2 different media sources 
> (one encoded
>  as vp8 and another as h264, can be natively encoded as well) and  
> each being sent at
>  2 different resolutions.
>
>     E. Section 5.3.1:
>
>         | a=rtcp-fb:* nack                            |
>     [RFC5104]           |
>
>         | a=rtcp-fb:* nack pli                        |
>     [RFC5104]           |
>
> [Suhas] No Specific reason. These are just examples to show 
> configurations and thus picked MVP
>     SE-164 80 Stockholm, Sweden | mailto:
>     [email-masked] <mailto:[email-masked]>
>     [email-masked] <mailto:[email-masked]>
-- 

Magnus Westerlund