Re: [asterisk-dev] [Code Review] 4505: Asterisk 13 PJSIP sends RTP packets in wrong byte order on Intel platform when using slin codec
On March 17, 2015, 11:57 a.m., Matt Jordan wrote:
/tags/13.2.0/include/asterisk/codec.h, lines 77-80
https://reviewboard.asterisk.org/r/4505/diff/1/?file=72588#file72588line77
I don't think you can trust that the codec will know its endianness.
Looking at the resample code, I don't _think_ it actually determines the
endianness of its encoding/decoding, and instead relies on the underlying
machine to make that determination. As such, I don't think this should be a
property on the codec structure.
Frankie Chin wrote:
Matt, I actually followed the implementation in Asterisk 12.8.1 where the
AST_SMOOTHER_FLAG_BE was defined for all the SLIN codecs in main/format.c
under the format_list_init() method. Do you mean this implementation back in
12.8.1 was inappropriate? FYI, slin codec used to work fine in Asterisk
12.8.1 for our application.
Matt Jordan wrote:
Apologies; you are absolutely correct about that.
I'll need a day or two to think about whether or not this is the right
way to handle passing smoother flags in. A part of me dislikes having an
explicit BE byte order integer, but I'm also not a giant fan of storing a
bunch of smoother properties directly on the codec. There may just not be a
better place for it.
After looking at 12 some more, I think handling this in the same manner as
Asterisk 12 is probably the best way forward. That would mean adding a
'smoother_flags' option
struct ast_codec {
...
unsigned int smooth;
unsigned int smoother_flags;
}
That would change the API call from ast_format_smoothed_with_be to
ast_format_get_smoother_flags (or something similar). That way, any additional
smoother flags can be obtained without needing additional APIs added.
- Matt
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4505/#review14719
---
On March 16, 2015, 10:36 p.m., Frankie Chin wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4505/
---
(Updated March 16, 2015, 10:36 p.m.)
Review request for Asterisk Developers.
Bugs: ASTERISK-24858
https://issues.asterisk.org/jira/browse/ASTERISK-24858
Repository: Asterisk
Description
---
In Asterisk 13.2.0 when SLIN codec is used in two Asterisk servers registered
to one another via PJSIP, the RTP payload is sent in the wrong byte order.
The patch addresses the following based on the correct behavior in Asterisk
12.8.1:
1) Save ptime = 20 as the framing in the ast_rtp_codecs structure when
creating outgoing SDP packet (res_pjsip_sdp_rtp.c)
2) Do not copy the framing when copying the payload (rtp_engine.c)
3) Introduce the new smoother_be flagin the ast_codec structure. Set this
flag = 1 for all the SLIN codecs (codec_builtin.c).
4) Check for this smoother_be flag before using the smoother on the data
(res_rtp_asterisk.c)
Diffs
-
/tags/13.2.0/res/res_rtp_asterisk.c 433002
/tags/13.2.0/res/res_pjsip_sdp_rtp.c 433002
/tags/13.2.0/main/rtp_engine.c 433002
/tags/13.2.0/main/format.c 433002
/tags/13.2.0/main/codec_builtin.c 433002
/tags/13.2.0/include/asterisk/format.h 433002
/tags/13.2.0/include/asterisk/codec.h 433002
Diff: https://reviewboard.asterisk.org/r/4505/diff/
Testing
---
The patch was tested using the scenario described in ASTERISK-24858
Thanks,
Frankie Chin
--
_
-- 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] 4505: Asterisk 13 PJSIP sends RTP packets in wrong byte order on Intel platform when using slin codec
On March 17, 2015, 11:57 a.m., Matt Jordan wrote: /tags/13.2.0/include/asterisk/codec.h, lines 77-80 https://reviewboard.asterisk.org/r/4505/diff/1/?file=72588#file72588line77 I don't think you can trust that the codec will know its endianness. Looking at the resample code, I don't _think_ it actually determines the endianness of its encoding/decoding, and instead relies on the underlying machine to make that determination. As such, I don't think this should be a property on the codec structure. Frankie Chin wrote: Matt, I actually followed the implementation in Asterisk 12.8.1 where the AST_SMOOTHER_FLAG_BE was defined for all the SLIN codecs in main/format.c under the format_list_init() method. Do you mean this implementation back in 12.8.1 was inappropriate? FYI, slin codec used to work fine in Asterisk 12.8.1 for our application. Apologies; you are absolutely correct about that. I'll need a day or two to think about whether or not this is the right way to handle passing smoother flags in. A part of me dislikes having an explicit BE byte order integer, but I'm also not a giant fan of storing a bunch of smoother properties directly on the codec. There may just not be a better place for it. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/#review14719 --- On March 16, 2015, 10:36 p.m., Frankie Chin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/ --- (Updated March 16, 2015, 10:36 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24858 https://issues.asterisk.org/jira/browse/ASTERISK-24858 Repository: Asterisk Description --- In Asterisk 13.2.0 when SLIN codec is used in two Asterisk servers registered to one another via PJSIP, the RTP payload is sent in the wrong byte order. The patch addresses the following based on the correct behavior in Asterisk 12.8.1: 1) Save ptime = 20 as the framing in the ast_rtp_codecs structure when creating outgoing SDP packet (res_pjsip_sdp_rtp.c) 2) Do not copy the framing when copying the payload (rtp_engine.c) 3) Introduce the new smoother_be flagin the ast_codec structure. Set this flag = 1 for all the SLIN codecs (codec_builtin.c). 4) Check for this smoother_be flag before using the smoother on the data (res_rtp_asterisk.c) Diffs - /tags/13.2.0/res/res_rtp_asterisk.c 433002 /tags/13.2.0/res/res_pjsip_sdp_rtp.c 433002 /tags/13.2.0/main/rtp_engine.c 433002 /tags/13.2.0/main/format.c 433002 /tags/13.2.0/main/codec_builtin.c 433002 /tags/13.2.0/include/asterisk/format.h 433002 /tags/13.2.0/include/asterisk/codec.h 433002 Diff: https://reviewboard.asterisk.org/r/4505/diff/ Testing --- The patch was tested using the scenario described in ASTERISK-24858 Thanks, Frankie Chin -- _ -- 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] 4505: Asterisk 13 PJSIP sends RTP packets in wrong byte order on Intel platform when using slin codec
On March 17, 2015, 4:57 p.m., Matt Jordan wrote: /tags/13.2.0/res/res_rtp_asterisk.c, lines 3394-3406 https://reviewboard.asterisk.org/r/4505/diff/1/?file=72594#file72594line3394 I think your issue should be solved here. When you care a new smoother, you can specify whether or not it is BE or LE via the ast_smoother_set_flags call. The real issue is determining whether or not your machine is BE or LE. What distro/environment did you produce this issue on? This issue was produced using Ubuntu 14.04.1 on Intel x86 platform. In Asterisk 12.8.1 I notice that when a smoother is created, the ast_smoother_set_flags() method is used to set the format flags into the smoother. Hence it is not deciding on BE/LE based on the machine platform. On March 17, 2015, 4:57 p.m., Matt Jordan wrote: /tags/13.2.0/include/asterisk/codec.h, lines 77-80 https://reviewboard.asterisk.org/r/4505/diff/1/?file=72588#file72588line77 I don't think you can trust that the codec will know its endianness. Looking at the resample code, I don't _think_ it actually determines the endianness of its encoding/decoding, and instead relies on the underlying machine to make that determination. As such, I don't think this should be a property on the codec structure. Matt, I actually followed the implementation in Asterisk 12.8.1 where the AST_SMOOTHER_FLAG_BE was defined for all the SLIN codecs in main/format.c under the format_list_init() method. Do you mean this implementation back in 12.8.1 was inappropriate? FYI, slin codec used to work fine in Asterisk 12.8.1 for our application. - Frankie --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/#review14719 --- On March 17, 2015, 3:36 a.m., Frankie Chin wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/ --- (Updated March 17, 2015, 3:36 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24858 https://issues.asterisk.org/jira/browse/ASTERISK-24858 Repository: Asterisk Description --- In Asterisk 13.2.0 when SLIN codec is used in two Asterisk servers registered to one another via PJSIP, the RTP payload is sent in the wrong byte order. The patch addresses the following based on the correct behavior in Asterisk 12.8.1: 1) Save ptime = 20 as the framing in the ast_rtp_codecs structure when creating outgoing SDP packet (res_pjsip_sdp_rtp.c) 2) Do not copy the framing when copying the payload (rtp_engine.c) 3) Introduce the new smoother_be flagin the ast_codec structure. Set this flag = 1 for all the SLIN codecs (codec_builtin.c). 4) Check for this smoother_be flag before using the smoother on the data (res_rtp_asterisk.c) Diffs - /tags/13.2.0/res/res_rtp_asterisk.c 433002 /tags/13.2.0/res/res_pjsip_sdp_rtp.c 433002 /tags/13.2.0/main/rtp_engine.c 433002 /tags/13.2.0/main/format.c 433002 /tags/13.2.0/main/codec_builtin.c 433002 /tags/13.2.0/include/asterisk/format.h 433002 /tags/13.2.0/include/asterisk/codec.h 433002 Diff: https://reviewboard.asterisk.org/r/4505/diff/ Testing --- The patch was tested using the scenario described in ASTERISK-24858 Thanks, Frankie Chin -- _ -- 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] 4505: Asterisk 13 PJSIP sends RTP packets in wrong byte order on Intel platform when using slin codec
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/ --- (Updated March 17, 2015, 3:36 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24858 https://issues.asterisk.org/jira/browse/ASTERISK-24858 Repository: Asterisk Description (updated) --- In Asterisk 13.2.0 when SLIN codec is used in two Asterisk servers registered to one another via PJSIP, the RTP payload is sent in the wrong byte order. The patch addresses the following based on the correct behavior in Asterisk 12.8.1: 1) Save ptime = 20 as the framing in the ast_rtp_codecs structure when creating outgoing SDP packet (res_pjsip_sdp_rtp.c) 2) Do not copy the framing when copying the payload (rtp_engine.c) 3) Introduce the new smoother_be flagin the ast_codec structure. Set this flag = 1 for all the SLIN codecs (codec_builtin.c). 4) Check for this smoother_be flag before using the smoother on the data (res_rtp_asterisk.c) Diffs - /tags/13.2.0/res/res_rtp_asterisk.c 433002 /tags/13.2.0/res/res_pjsip_sdp_rtp.c 433002 /tags/13.2.0/main/rtp_engine.c 433002 /tags/13.2.0/main/format.c 433002 /tags/13.2.0/main/codec_builtin.c 433002 /tags/13.2.0/include/asterisk/format.h 433002 /tags/13.2.0/include/asterisk/codec.h 433002 Diff: https://reviewboard.asterisk.org/r/4505/diff/ Testing --- The patch was tested using the scenario described in ASTERISK-24858 Thanks, Frankie Chin -- _ -- 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] 4505: Asterisk 13 PJSIP sends RTP packets in wrong byte order on Intel platform when using slin codec
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/ --- (Updated March 17, 2015, 3:35 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-24858 https://issues.asterisk.org/jira/browse/ASTERISK-24858 Repository: Asterisk Description --- In Asterisk 13.2.0 when SLIN codec is used in two Asterisk servers registered to one another via PJSIP, the RTP payload is sent in the wrong byte order. The patch addresses the following based on the correct behavior in Asterisk 12.8.1: 1) Save ptime = 20 as the framing in the ast_rtp_codecs structure when creating outgoing SDP packet (res_pjsip_sdp_rtp.c) 2) Do not copy the framing when copying the payload (rtp_engine.c) 3) Introduce the new smoother_be flagin the ast_codec structure. Set this flag = 1 for all the SLINcodecs. 4) Check for this smoother_be flag before using the smoother on the data. Diffs - /tags/13.2.0/res/res_rtp_asterisk.c 433002 /tags/13.2.0/res/res_pjsip_sdp_rtp.c 433002 /tags/13.2.0/main/rtp_engine.c 433002 /tags/13.2.0/main/format.c 433002 /tags/13.2.0/main/codec_builtin.c 433002 /tags/13.2.0/include/asterisk/format.h 433002 /tags/13.2.0/include/asterisk/codec.h 433002 Diff: https://reviewboard.asterisk.org/r/4505/diff/ Testing (updated) --- The patch was tested using the scenario described in ASTERISK-24858 Thanks, Frankie Chin -- _ -- 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] 4505: Asterisk 13 PJSIP sends RTP packets in wrong byte order on Intel platform when using slin codec
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4505/ --- Review request for Asterisk Developers. Bugs: ASTERISK-24858 https://issues.asterisk.org/jira/browse/ASTERISK-24858 Repository: Asterisk Description --- In Asterisk 13.2.0 when SLIN codec is used in two Asterisk servers registered to one another via PJSIP, the RTP payload is sent in the wrong byte order. The patch addresses the following based on the correct behavior in Asterisk 12.8.1: 1) Save ptime = 20 as the framing in the ast_rtp_codecs structure when creating outgoing SDP packet (res_pjsip_sdp_rtp.c) 2) Do not copy the framing when copying the payload (rtp_engine.c) 3) Introduce the new smoother_be flagin the ast_codec structure. Set this flag = 1 for all the SLINcodecs. 4) Check for this smoother_be flag before using the smoother on the data. Diffs - /tags/13.2.0/res/res_rtp_asterisk.c 433002 /tags/13.2.0/res/res_pjsip_sdp_rtp.c 433002 /tags/13.2.0/main/rtp_engine.c 433002 /tags/13.2.0/main/format.c 433002 /tags/13.2.0/main/codec_builtin.c 433002 /tags/13.2.0/include/asterisk/format.h 433002 /tags/13.2.0/include/asterisk/codec.h 433002 Diff: https://reviewboard.asterisk.org/r/4505/diff/ Testing --- The patch is tested using the scenario described in ASTERISK-24858 Thanks, Frankie Chin -- _ -- 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
