Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

[rmudgett]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4000/
---

(Updated Sept. 19, 2014, 12:08 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 423561


Bugs: AFS-162
https://issues.asterisk.org/jira/browse/AFS-162


Repository: Asterisk


Description
---

Outgoing PJSIP calls can result in non-negotiated formats listed in the 
channel's native formats if video formats are listed in the endpoint's 
configuration.  The resulting call could then use a non-negotiated format 
resulting in one way audio.

* Simplified the update of session->req_caps in set_caps().  Why do something 
in five steps when only one is needed?


Diffs
-

  /branches/13/res/res_pjsip_sdp_rtp.c 423446 
  /branches/13/channels/chan_pjsip.c 423446 

Diff: https://reviewboard.asterisk.org/r/4000/diff/


Testing
---

Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
Called from D40 with g722 among other formats enabled to a Polycom that 
negotiates ulaw.
Before the patch, Asterisk would send g722 frames to the Polycom.  The 
resulting call had one way audio because the Polycom does not understand g722.
After the patch, Asterisk sends ulaw frames to the Polycom.


Thanks,

rmudgett

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

[Mark Michelson]

> On Sept. 18, 2014, 9:35 p.m., Mark Michelson wrote:
> > /branches/13/res/res_pjsip_sdp_rtp.c, line 259
> > 
> >
> > Since joint only has formats of type media_type, would specifying 
> > media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here?
> 
> rmudgett wrote:
> It's a case of six of one and half a dozen of another.  It won't make any 
> difference in this case since all formats will be appended anyway.  It's a 
> little more efficient to use the constant since the function has to test if 
> it isn't UNKNOWN and then test to see if it is the specified type.
> 
> I'll leave it as is unless there is a stronger argument for changing it.

Okay, that's good enough for me.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4000/#review13351
---


On Sept. 18, 2014, 6:26 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4000/
> ---
> 
> (Updated Sept. 18, 2014, 6:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-162
> https://issues.asterisk.org/jira/browse/AFS-162
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Outgoing PJSIP calls can result in non-negotiated formats listed in the 
> channel's native formats if video formats are listed in the endpoint's 
> configuration.  The resulting call could then use a non-negotiated format 
> resulting in one way audio.
> 
> * Simplified the update of session->req_caps in set_caps().  Why do something 
> in five steps when only one is needed?
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 423446 
>   /branches/13/channels/chan_pjsip.c 423446 
> 
> Diff: https://reviewboard.asterisk.org/r/4000/diff/
> 
> 
> Testing
> ---
> 
> Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
> Called from D40 with g722 among other formats enabled to a Polycom that 
> negotiates ulaw.
> Before the patch, Asterisk would send g722 frames to the Polycom.  The 
> resulting call had one way audio because the Polycom does not understand g722.
> After the patch, Asterisk sends ulaw frames to the Polycom.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4000/#review13356
---

Ship it!


Ship It!

- Mark Michelson


On Sept. 18, 2014, 6:26 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4000/
> ---
> 
> (Updated Sept. 18, 2014, 6:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-162
> https://issues.asterisk.org/jira/browse/AFS-162
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Outgoing PJSIP calls can result in non-negotiated formats listed in the 
> channel's native formats if video formats are listed in the endpoint's 
> configuration.  The resulting call could then use a non-negotiated format 
> resulting in one way audio.
> 
> * Simplified the update of session->req_caps in set_caps().  Why do something 
> in five steps when only one is needed?
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 423446 
>   /branches/13/channels/chan_pjsip.c 423446 
> 
> Diff: https://reviewboard.asterisk.org/r/4000/diff/
> 
> 
> Testing
> ---
> 
> Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
> Called from D40 with g722 among other formats enabled to a Polycom that 
> negotiates ulaw.
> Before the patch, Asterisk would send g722 frames to the Polycom.  The 
> resulting call had one way audio because the Polycom does not understand g722.
> After the patch, Asterisk sends ulaw frames to the Polycom.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

[rmudgett]

> On Sept. 18, 2014, 4:35 p.m., Mark Michelson wrote:
> > /branches/13/res/res_pjsip_sdp_rtp.c, line 259
> > 
> >
> > Since joint only has formats of type media_type, would specifying 
> > media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here?

It's a case of six of one and half a dozen of another.  It won't make any 
difference in this case since all formats will be appended anyway.  It's a 
little more efficient to use the constant since the function has to test if it 
isn't UNKNOWN and then test to see if it is the specified type.

I'll leave it as is unless there is a stronger argument for changing it.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4000/#review13351
---


On Sept. 18, 2014, 1:26 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4000/
> ---
> 
> (Updated Sept. 18, 2014, 1:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-162
> https://issues.asterisk.org/jira/browse/AFS-162
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Outgoing PJSIP calls can result in non-negotiated formats listed in the 
> channel's native formats if video formats are listed in the endpoint's 
> configuration.  The resulting call could then use a non-negotiated format 
> resulting in one way audio.
> 
> * Simplified the update of session->req_caps in set_caps().  Why do something 
> in five steps when only one is needed?
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 423446 
>   /branches/13/channels/chan_pjsip.c 423446 
> 
> Diff: https://reviewboard.asterisk.org/r/4000/diff/
> 
> 
> Testing
> ---
> 
> Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
> Called from D40 with g722 among other formats enabled to a Polycom that 
> negotiates ulaw.
> Before the patch, Asterisk would send g722 frames to the Polycom.  The 
> resulting call had one way audio because the Polycom does not understand g722.
> After the patch, Asterisk sends ulaw frames to the Polycom.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

[Mark Michelson]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4000/#review13351
---



/branches/13/res/res_pjsip_sdp_rtp.c


Since joint only has formats of type media_type, would specifying 
media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here?


- Mark Michelson


On Sept. 18, 2014, 6:26 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4000/
> ---
> 
> (Updated Sept. 18, 2014, 6:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-162
> https://issues.asterisk.org/jira/browse/AFS-162
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Outgoing PJSIP calls can result in non-negotiated formats listed in the 
> channel's native formats if video formats are listed in the endpoint's 
> configuration.  The resulting call could then use a non-negotiated format 
> resulting in one way audio.
> 
> * Simplified the update of session->req_caps in set_caps().  Why do something 
> in five steps when only one is needed?
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 423446 
>   /branches/13/channels/chan_pjsip.c 423446 
> 
> Diff: https://reviewboard.asterisk.org/r/4000/diff/
> 
> 
> Testing
> ---
> 
> Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
> Called from D40 with g722 among other formats enabled to a Polycom that 
> negotiates ulaw.
> Before the patch, Asterisk would send g722 frames to the Polycom.  The 
> resulting call had one way audio because the Polycom does not understand g722.
> After the patch, Asterisk sends ulaw frames to the Polycom.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

[rmudgett]

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4000/
---

Review request for Asterisk Developers.


Bugs: AFS-162
https://issues.asterisk.org/jira/browse/AFS-162


Repository: Asterisk


Description
---

Outgoing PJSIP calls can result in non-negotiated formats listed in the 
channel's native formats if video formats are listed in the endpoint's 
configuration.  The resulting call could then use a non-negotiated format 
resulting in one way audio.

* Simplified the update of session->req_caps in set_caps().  Why do something 
in five steps when only one is needed?


Diffs
-

  /branches/13/res/res_pjsip_sdp_rtp.c 423446 
  /branches/13/channels/chan_pjsip.c 423446 

Diff: https://reviewboard.asterisk.org/r/4000/diff/


Testing
---

Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
Called from D40 with g722 among other formats enabled to a Polycom that 
negotiates ulaw.
Before the patch, Asterisk would send g722 frames to the Polycom.  The 
resulting call had one way audio because the Polycom does not understand g722.
After the patch, Asterisk sends ulaw frames to the Polycom.


Thanks,

rmudgett

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev