Re: [Wish] 2nd WG Last Call for draft-ietf-wish-whip: PATCH and trickle ICE

[Sergio Garcia Murillo <[e-mail]>]

On Fri, Jan 12, 2024 at 3:10 PM Christer Holmberg <
[e-mail]> wrote:

> Hi,
>
> >>      Section 4.1.1. of WHIP-11 says:
> >>
> >>          “When used for trickle ICE, the body of this PATCH message will
> >>          contain the new gathered set of ICE candidates and when used
> >>          for ICE restarts, it will also contain the new ICE ufrag/pwd
> pair as
> >>         defined in [RFC8838] Section 5.4.”
> >>
> >>      Q1: My reading of the text above is that ICE ufrag/pwd is only
> >>          included for ICE restarts. However, they are included in the
> trickle ICE
> >>          example in Section 4.1.2.
> >
> > This is not mandatory text, so just a description of the important
> fields.
>
> Not sure what you mean by "not mandatory text", as it describes what the
> body of the PATCH message will contain.
>
> Having said that, I did realize that for ICE restart it says *new* ICE
> uftag/pwd, so that is correct.
>
> BTW, there is no Section 5.4 in RFC 8838.
>

right, there are a couple other mismatched references, I think they are
fine now:

https://github.com/wish-wg/webrtc-http-ingest-protocol/commit/f5acd16b172fdc0294719b3fcc3a87502eb96610


> > Probably we should explicitly state in 4.1.2. Trickle ICE that the
> creation of
> > the HTTP PATCH BODY and processing by the WHIP session MUST follow
> > section 4.4. Delivering Candidates in INFO Requests of RFC 8840.
>
> That can be done.
>
> But, then it is perhaps enough to only say the following in Section 4.1.1.
> :
>
>    "The WHIP client MAY perform trickle ICE or ICE restarts by sending an
>    HTTP PATCH request as per [RFC5789] to the WHIP session URL, with a
>    body containing a SDP fragment with media type "application/trickle-
>    ice-sdpfrag" as specified in [RFC8840] carrying ICE information.
>
> Sections 4.1.2 and 4.1.3 would then provide the details on what ICE
> candidates and credentials are included.



Fine for me.
https://github.com/wish-wg/webrtc-http-ingest-protocol/commit/1ac7f07aaa26afdb2847871bf868284874ddcb44



>
> >>      Q2: My reading of the text above is that for trickle ICE the PATCH
> >>          message body will not contain the previous candidates, only
> the new
> >>          gathered set.
> >>
> >>          However, Section 4.4. of RFC 8838, which defines the SDP
> trickle-ice-
> >>          sdpfrag content type, and the usage with SIP INFO, says:
> >>
> >>      “Since the agent is not fully aware of the state of the ICE
> >>          Negotiation Session at its peer, it MUST include all currently
> >>         known and used local candidates in every INFO request.”
> >>
> >>      Since WHIP does not (AFAIU) apply to the MUST above, I think it
> >>         would be useful with some justification in the draft. For
> example, is there
> >>         something HTTP PATCH provides, but SIP INFO does not, that
> justifies not
> >>         adopting the MUST?
> >>
> >>      (Related to Q1, RFC 8838 also mandates sending of ICE pwd/ufrag for
> >>          trickle ICE)
> >
> > I was not aware of the in-order delivery requirement of RFC 8838. In
> order to
> > fulfill that requirement, I don't think that sending all the candidates
> is
> > required,  but only the ones that are "in-flight".
> >
> > That is, two concurrent patch requests can be received out of order by
> the
> > media server. So the WHIP client needs to keep an ordered list of
> candidates
> > sent on the PATCH request, and remove them from the list when the 204
> > response is received. If a new candidate is gathered, we just append it
> to the
> > list, and send a new request with the full list of pending candidates.
>
> If we refer to RFC 8840 (as you suggested earlier) I think that will
> clarify it.
>
>
Changed in here:
https://github.com/wish-wg/webrtc-http-ingest-protocol/commit/70d3a8fdfbc13873aa760bb542428bea1bba9437

Best regards
Sergio