< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index  

Re: [reSIProcate] Multiple provisional with the same SDP: bug or feature?


Hi Robert,

I've committed this fix to SVN main line - thanks!

Scott

On Fri, Sep 17, 2010 at 5:12 AM, Robert Szokovacs <rszokovacs@xxxxxxxxxxxxxxx> wrote:
On 2010 September 16, Thursday 21:03:13 Scott Godin wrote:
> Ah I see the problem in the code if you call provisional with the earlyFlag
> as true first then false.  I agree we need to fix it.

Proposed fix attached

br

Szo


> On Thu, Sep 16, 2010 at 2:59 PM, Scott Godin <sgodin@xxxxxxxxxxxxxxx> wrote:
> > Are you saying that the following doesn't work?
> >
> > sis->provisional(183, true /* earlyFlag */);
> > sis->provisional(180, false /* earlyFlag */);
> >
> > Scott
> >
> > On Thu, Sep 16, 2010 at 2:53 PM, SZOKOVACS Robert <
> >
> > rszokovacs@xxxxxxxxxxxxxxx> wrote:
> >> On 2010. September 16., Scott Godin wrote:
> >> > I don't quite get it.  If you are calling the provisional API twice
> >> > and you don't want SDP on either message, then you need to pass the
> >> > flag as false for both calls.
> >>
> >> I want the SDP in the first one and I don't want it in the second one.
> >>
> >> Szo
> >>
> >> > Scott
> >> >
> >> > On Thu, Sep 16, 2010 at 2:47 PM, SZOKOVACS Robert <
> >> >
> >> > rszokovacs@xxxxxxxxxxxxxxx> wrote:
> >> > > On 2010. September 16., Scott Godin wrote:
> >> > > > What do you mean by "the second message"?
> >> > >
> >> > > First, there is a 183 with SDP then a 180 without. The problem is
> >> > > even if I send the 180 with earlyFlag=false, the SDP from the 183
> >> > > will be in the message.
> >> > >
> >> > > br
> >> > >
> >> > > Szo
> >> > >
> >> > > > Scott
> >> > > >
> >> > > > On Thu, Sep 16, 2010 at 10:13 AM, Robert Szokovacs <
> >> > > >
> >> > > > rszokovacs@xxxxxxxxxxxxxxx> wrote:
> >> > > > > Hi again,
> >> > > > >
> >> > > > > I used the earlyFlag, and resiprocate still included the SDP in
> >>
> >> the
> >>
> >> > > > > second message. It was because the m1xx SipMessage is never
> >> > > > > reset, only updated in ServerInviteSession::sendProvisional().
> >> > > > > I think
> >>
> >> the
> >>
> >> > > > > solution would be to call
> >> > > > > releaseContents() in case of false earlyFlag. Should I create a
> >> > > > > patch?
> >> > > > >
> >> > > > > br
> >> > > > >
> >> > > > > Szo
> >> > > > >
> >> > > > > On 2010 September 16, Thursday 10:22:05 Robert Szokovacs wrote:
> >> > > > > > On 2010 September 15, Wednesday 18:43:18 Scott Godin wrote:
> >> > > > > > > There is a parameter to the provisional method, called
> >> > > > > > > earlyFlag, that specifies if you would like the SDP to be
> >> > > > > > > provided in the 18x response
> >> > > > >
> >> > > > > or
> >> > > > >
> >> > > > > > > not (if you happened to provide the SDP to resip before
> >>
> >> calling
> >>
> >> > > > > > > this).
> >> > > > > >
> >> > > > > > My bad, I was sidetracked by the comments in the header and
> >> > > > > > failed to realize there's already a solution :(
> >> > > > > >
> >> > > > > > Thank you for the answer!
> >> > > > > >
> >> > > > > > br
> >> > > > > >
> >> > > > > > Szo
> >> > > > > >
> >> > > > > > > If you are not using PRACK then any SDP send in a 18x
> >> > > > > > > response must
> >> > > > >
> >> > > > > also
> >> > > > >
> >> > > > > > > be repeated in the 200 response, and it cannot be changed -
> >> > > > > > > this is an RFC requirement.
> >> > > > > > >
> >> > > > > > > Scott
> >> > > > > > >
> >> > > > > > > On Wed, Sep 15, 2010 at 10:31 AM, Robert Szokovacs <
> >> > > > > > >
> >> > > > > > > rszokovacs@xxxxxxxxxxxxxxx> wrote:
> >> > > > > > > > Hi,
> >> > > > > > > >
> >> > > > > > > > We have  a B2BUA based on resiprocate and we noticed that
> >> > > > > > > > if the "c" leg (the
> >> > > > > > > > one we call out to) sends a 183 with SDP and then a 180
> >> > > > > > > > without, we pass the
> >> > > > > > > > "a" leg (the one originated the call) the 180 with the SDP
> >> > > > > > > > from the
> >> > > > >
> >> > > > > 183
> >> > > > >
> >> > > > > > > > included. This is not the desired behaviour, since there
> >> > > > > > > > are some 3rd party devices that doesn't seem to tolerate
> >> > > > > > > > this. This happens because we call ServerInviteSession's
> >> > > > > > > > provide[Answer|Offer]() before calling provisional() for
> >> > > > > > > > the 183, and then calling provisional() for
> >> > > > > > > > the 180, and have no way of telling resiprocate not to
> >> > > > > > > > include the
> >> > > > >
> >> > > > > SDP.
> >> > > > >
> >> > > > > > > > The problem is that according to the comments in
> >> > > > > > > > ServerInviteSession.hxx it is not
> >> > > > > > > > necessary:
> >> > > > > > > > /** Called to set the offer that will be used in the next
> >> > > > > > > > message
> >> > > > >
> >> > > > > that
> >> > > > >
> >> > > > > > > >          sends an offer. [...] */
> >> > > > > > > >
> >> > > > > > > >      virtual void provideOffer(const SdpContents& offer);
> >> > > > > > > >
> >> > > > > > > > (same with provideAnswer)
> >> > > > > > > >
> >> > > > > > > > We interpreted it as "the next and only the next" message
> >> > > > > > > > will
> >> > > > >
> >> > > > > include
> >> > > > >
> >> > > > > > > > the SDP, but the code doesn't do that way. Is that the
> >> > > > > > > > correct behaviour? If it's
> >> > > > > > > > a bug, which way should I look for solution? Should we
> >> > > > > > > > empty the mCurrentLocalSdp & mProposedLocalSdp after
> >> > > > > > > > sending it? Should we transition to
> >> > > > > > > > a state that doesn't call setSdp() in sendProvisional()
> >> > > > > > > > (which
> >> > > > >
> >> > > > > state?)
> >> > > > >
> >> > > > > > > > Any help is appreciated!
> >> > > > > > > >
> >> > > > > > > > TIA
> >> > > > > > > >
> >> > > > > > > > br
> >> > > > > > > >
> >> > > > > > > > Szo
> >> > > > > > > > _______________________________________________
> >> > > > > > > > resiprocate-devel mailing list
> >> > > > > > > > resiprocate-devel@xxxxxxxxxxxxxxx
> >>
> >> https://list.resiprocate.org/mailman/listinfo/resiprocate-dev
> >>
> >> > > > > > > > el