[ previous ] [ next ] [ threads ]
 To :  Paul Chitescu <paulc@v...>, yate@v...
 From :  Gerrit Jacobsen <g_jacobsen@y...>
 Subject :  Re: [yate] Bug: codec negotiation media.cpp
 Date :  Tue, 6 Apr 2010 05:35:22 +0000 (GMT)
Paul,

"What do you think about creating an intersect between the INVITEd codecs and 
those set in routing? If the resulting set is not empty that 
set would be 
replaced in the answer."

Yes, but because there is only one codec chosen for connecting the incoming leg. Yate should take into account the order.

IMVHO there should be these steps:
1. Determine the intersect between codecs from the INVITE and those set in formats during routing, ordered by the order of the formats set in routing.
2. Remove all codecs from the intersect where there is no transcoding possible from the already connected outgoing call leg.
3a). In case the intersect is not empty, the top ordered codec from the intersect should be used for connecting the incoming leg. 
3b) If the intersect is empty then the logic should follow the same as if there was no codec set in formats, ideally picking the same codec as the outgoing call leg to minimize transcoding.

Also I would consider to hardcode "oformats" as the currently proposed 
method with setting manually in "call.execute" is hard to 
understand. I think that formats which are manually set should act as filters / intersect for incoming call legs and determine the 
order of codecs. oformats should determine offered formats in outgoing legs.

Regarding transcoding. There is one more point regarding the transcode function in regexroute. Currently transcode orders the codecs in the order as possible transcodings were found,"originating codec1, possible transcoding1 of codec1, possible transcoding2 of codec1, originating codec2, possible transcoding1 of codec2, possible 
transcoding2 of codec2"

This function would be more useful if it would order the codecs by computing costs, first the orginal input codecs where there is no transcoding necessary, then codecs with only one transcoding necessary and then double transcodings.


Thank you for your work on the formats issue.

Cheers,

Gerry







________________________________
From: Paul Chitescu 
To: yate@v...
Cc: "Provencher, Martin" 
Sent: Mon, 5 April, 2010 23:03:14
Subject: Re: [yate] Bug: codec negotiation media.cpp

Hi, Gerrit!

The code in the SDP parser (formerly in the SIP channel) did not assume a 
wrong list of formats would be set but had a trivial protection against haing 
a single (but wrong) codec hardcoded.

What do you think about creating an intersect between the INVITEd codecs and 
those set in routing? If the resulting set is not empty that set would be 
replaced in the answer.

Paul Chitescu


On Monday 05 April 2010 09:38:32 pm Gerrit Jacobsen wrote:
> Martin,
> 
> I dug further into this and it appears that the behaviour is as following. 
> 
> If in routing stage more than one formats is set then yate assumes that the 
formats are valid for the incoming leg, no matter whether the incoming leg 
announced the capability or not.
> 
> If only one, but wrong, format is set then yate ignores that and connects 
the incoming leg with the capability that was announced by the incoming side.
> 
> The same effect happens when formats are set seperately for the outgoing leg 
in the call.execute message and for the incoming leg in routing.
> 
> So when setting formats manually in regexroute or register one has to make 
dead sure that the parties really support them. Which is indeed a problem 
since one cannot predict for some third parties which formats they will 
announce on each call.
> 
> IMVHO yate should rather treat formats list as preferred codecs and not 
assume that they are compulsory codecs.
> 
> In any case I consider it a bug that yate tries to connect calls in a codec 
that was not announced by the other side.
> 
> Cheers
> 
> Gerry
> 
> ________________________________
> From: "Provencher, Martin" 
> To: Gerrit Jacobsen 
> Sent: Mon, 5 April, 2010 15:28:37
> Subject: Re: [yate] Bug: codec negotiation media.cpp
> 
>  Hi Gerrit,
>       I'm very interested in your bug report. If you have any answer
> coming from Yate team that does not go throught the list, could you
> foward me the information.
> 
> Thanks
> 
> 
> Martin
> Provencher 
> Software Developer 
> 
> TelcoBridges Inc. 
> 91 De La Barre, Boucherville, QC, Canada, J4B 2X6 
> Phone: +1 450 655 8993 ext 161 
> Fax: +1 450 655 9511 
> Web: www.telcobridges.com 
> BEST
> TECHNOLOGY> BRIGHT IDEAS > CLEAR
> FUTURE 
> P Before
> printing, think of the environment / Avant d'imprimer, pensez à
> l'environnement 
> Le 2010-04-02 13:38, Gerrit Jacobsen a écrit : 
> Hello yate team,
> >
> >>there seems to be a bug/logic flaw in the SDP negotiation.
> >
> >>Situation: Two SIP entities, SIP1 and SIP2 and yate
> >>SIP1 to SIP2 call, via yate, proxying RTP
> >
> >>1. SIP1 INVITES to yate and announces alaw in SDP
> >>2. In route stage the formats parameter is set by regexroute or other
> >routing module to "g729,alaw"
> >>e.g.
> >>^\(.*\)$=sip/sip:@2...; xsip_auth_bye=false; formats=g729,alaw
> >>3. SIP2 sends 200ok and accepts g729 codec in sdp
> >
> >>and now comes the problem:
> >>4. Yate sends 200ok with "g729" to SIP1
> >This should not
> >happen as SIP1 announced only alaw as capability.
> >
> >>5. SIP1 disconnects the call because it doesnt support G729.
> >
> >>The problem does not happen when at point 2.
> >>the formats parameter is set to g729 only (instead of g729 and alaw). 
> >>Yate log then shows the line "Debug(DebugInfo,"Not changing to '%s'
> >from '%s' [%p]", formats,m_formats.c_str(),this);"
> >>Yate then sends correctly at point 3. 200ok with alaw in SDP
> >
> >
> >>The problem seems to be in media.cpp when the bug happens (two codecs
> >in formats set) the SDPMedia::update function concludes that only G729
> >should be used and changes the formats to G729.
> >
> >
> >>bool SDPMedia::update(const char* formats, int rport, int lport)
> >>{
> >>    DDebug(DebugAll,"SDPMedia::update('%s',%d,%d)
> >[%p]",formats,rport,lport,this);
> >>    bool chg = false;
> >>    String tmp(formats);
> >>    if (m_formats != tmp) {
> >>        if ((tmp.find(',') < 0) && m_formats &&
> >m_formats.find(tmp) < 0)
> >>            Debug(DebugInfo,"Not changing to '%s' from '%s' [%p]",
> >>                formats,m_formats.c_str(),this);
> >>        else
> >{                                                                                                
> >chg =
> >true;                                                                                        
> >m_formats = tmp;
> >>            int q = m_formats.find(',');
> >>            m_format = m_formats.substr(0,q);
> >>        }
> >>    }
> >
> >>Thanks for looking into this.
> >
> >>Cheers 
> >
> >>Gerry



      


Paul,

"What do you think about creating an intersect between the INVITEd codecs and
those set in routing? If the resulting set is not empty that set would be
replaced in the answer."

Yes, but because there is only one codec chosen for connecting the incoming leg. Yate should take into account the order.

IMVHO there should be these steps:
1. Determine the intersect between codecs from the INVITE and those set in formats during routing, ordered by the order of the formats set in routing.
2. Remove all codecs from the intersect where there is no transcoding possible from the already connected outgoing call leg.
3a). In case the intersect is not empty, the top ordered codec from the intersect should be used for connecting the incoming leg.
3b) If the intersect is empty then the logic should follow the same as if there was no codec set in formats, ideally picking the same codec as the outgoing call leg to minimize transcoding.

Also I would consider to hardcode "oformats" as the currently proposed method with setting manually in "call.execute" is hard to understand. I think that formats which are manually set should act as filters / intersect for incoming call legs and determine the order of codecs. oformats should determine offered formats in outgoing legs.

Regarding transcoding. There is one more point regarding the transcode function in regexroute. Currently transcode orders the codecs in the order as possible transcodings were found,"originating codec1, possible transcoding1 of codec1, possible transcoding2 of codec1, originating codec2, possible transcoding1 of codec2, possible transcoding2 of codec2"

This function would be more useful if it would order the codecs by computing costs, first the orginal input codecs where there is no transcoding necessary, then codecs with only one transcoding necessary and then double transcodings.


Thank you for your work on the formats issue.

Cheers,

Gerry





From: Paul Chitescu <paulc@v...>
To: yate@v...
Cc: "Provencher, Martin" <mprovencher@t...>
Sent: Mon, 5 April, 2010 23:03:14
Subject: Re: [yate] Bug: codec negotiation media.cpp

Hi, Gerrit!

The code in the SDP parser (formerly in the SIP channel) did not assume a
wrong list of formats would be set but had a trivial protection against haing
a single (but wrong) codec hardcoded.

What do you think about creating an intersect between the INVITEd codecs and
those set in routing? If the resulting set is not empty that set would be
replaced in the answer.

Paul Chitescu


On Monday 05 April 2010 09:38:32 pm Gerrit Jacobsen wrote:
> Martin,
>
> I dug further into this and it appears that the behaviour is as following.
>
> If in routing stage more than one formats is set then yate assumes that the
formats are valid for the incoming leg, no matter whether the incoming leg
announced the capability or not.
>
> If only one, but wrong, format is set then yate ignores that and connects
the incoming leg with the capability that was announced by the incoming side.
>
> The same effect happens when formats are set seperately for the outgoing leg
in the call.execute message and for the incoming leg in routing.
>
> So when setting formats manually in regexroute or register one has to make
dead sure that the parties really support them. Which is indeed a problem
since one cannot predict for some third parties which formats they will
announce on each call.
>
> IMVHO yate should rather treat formats list as preferred codecs and not
assume that they are compulsory codecs.
>
> In any case I consider it a bug that yate tries to connect calls in a codec
that was not announced by the other side.
>
> Cheers
>
> Gerry
>
> ________________________________
> From: "Provencher, Martin" <mprovencher@t...>
> To: Gerrit Jacobsen <g_jacobsen@y...>
> Sent: Mon, 5 April, 2010 15:28:37
> Subject: Re: [yate] Bug: codec negotiation media.cpp
>
>  Hi Gerrit,
>      I'm very interested in your bug report. If you have any answer
> coming from Yate team that does not go throught the list, could you
> foward me the information.
>
> Thanks
>
>
> Martin
> Provencher
> Software Developer
>
> TelcoBridges Inc.
> 91 De La Barre, Boucherville, QC, Canada, J4B 2X6
> Phone: +1 450 655 8993 ext 161
> Fax: +1 450 655 9511
> Web: www.telcobridges.com
> BEST
> TECHNOLOGY> BRIGHT IDEAS > CLEAR
> FUTURE
> P Before
> printing, think of the environment / Avant d'imprimer, pensez à
> l'environnement
> Le 2010-04-02 13:38, Gerrit Jacobsen a écrit :
> Hello yate team,
> >
> >>there seems to be a bug/logic flaw in the SDP negotiation.
> >
> >>Situation: Two SIP entities, SIP1 and SIP2 and yate
> >>SIP1 to SIP2 call, via yate, proxying RTP
> >
> >>1. SIP1 INVITES to yate and announces alaw in SDP
> >>2. In route stage the formats parameter is set by regexroute or other
> >routing module to "g729,alaw"
> >>e.g.
> >>^\(.*\)$=sip/sip:@2...; xsip_auth_bye=false; formats=g729,alaw
> >>3. SIP2 sends 200ok and accepts g729 codec in sdp
> >
> >>and now comes the problem:
> >>4. Yate sends 200ok with "g729" to SIP1
> >This should not
> >happen as SIP1 announced only alaw as capability.
> >
> >>5. SIP1 disconnects the call because it doesnt support G729.
> >
> >>The problem does not happen when at point 2.
> >>the formats parameter is set to g729 only (instead of g729 and alaw).
> >>Yate log then shows the line "Debug(DebugInfo,"Not changing to '%s'
> >from '%s' [%p]", formats,m_formats.c_str(),this);"
> >>Yate then sends correctly at point 3. 200ok with alaw in SDP
> >
> >
> >>The problem seems to be in media.cpp when the bug happens (two codecs
> >in formats set) the SDPMedia::update function concludes that only G729
> >should be used and changes the formats to G729.
> >
> >
> >>bool SDPMedia::update(const char* formats, int rport, int lport)
> >>{
> >>    DDebug(DebugAll,"SDPMedia::update('%s',%d,%d)
> >[%p]",formats,rport,lport,this);
> >>    bool chg = false;
> >>    String tmp(formats);
> >>    if (m_formats != tmp) {
> >>        if ((tmp.find(',') < 0) && m_formats &&
> >m_formats.find(tmp) < 0)
> >>            Debug(DebugInfo,"Not changing to '%s' from '%s' [%p]",
> >>                formats,m_formats.c_str(),this);
> >>        else
> >{                                                                                               
> >chg =
> >true;                                                                                       
> >m_formats = tmp;
> >>            int q = m_formats.find(',');
> >>            m_format = m_formats.substr(0,q);
> >>        }
> >>    }
> >
> >>Thanks for looking into this.
> >
> >>Cheers
> >
> >>Gerry