[Asterisk-code-review] bridge softmix: Forward TEXT frames (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/8762 )

Change subject: bridge_softmix:  Forward TEXT frames
......................................................................


Patch Set 3: Code-Review-1

(7 comments)

https://gerrit.asterisk.org/#/c/8762/3/funcs/func_frame_trace.c
File funcs/func_frame_trace.c:

https://gerrit.asterisk.org/#/c/8762/3/funcs/func_frame_trace.c@377
PS3, Line 377: 	 case AST_FRAME_RTCP:
The indention is off by one.


https://gerrit.asterisk.org/#/c/8762/2/main/channel.c
File main/channel.c:

https://gerrit.asterisk.org/#/c/8762/2/main/channel.c@4888
PS2, Line 4888: 
> > For channel drivers that only support T.140 or the old send_text
We did not allow anything other than text/plain before and the old methods do not have a way to say what kind of text we are passing.  This is why we should enforce only text/plain data to the legacy methods.


https://gerrit.asterisk.org/#/c/8762/2/main/channel.c@4897
PS2, Line 4897: 		f.src = "DIALPLAN";
> > Probably should not send empty text messages over T.140.
This is a behavior change as we discarded empty text messages before.


https://gerrit.asterisk.org/#/c/8762/2/main/channel.c@4932
PS2, Line 4932: {
> Replace text with:
Did you miss this one?  Or are you assuming that nobody will call this with a NULL pointer?


https://gerrit.asterisk.org/#/c/8762/3/main/message.c
File main/message.c:

https://gerrit.asterisk.org/#/c/8762/3/main/message.c@1390
PS3, Line 1390: 	for(i=0; i < count; i++) {
for (i = 0;...


https://gerrit.asterisk.org/#/c/8762/3/main/message.c@1411
PS3, Line 1411: 	for(i=0; i < count; i++) {
for (i = 0;..


https://gerrit.asterisk.org/#/c/8762/2/res/res_pjsip_messaging.c
File res/res_pjsip_messaging.c:

https://gerrit.asterisk.org/#/c/8762/2/res/res_pjsip_messaging.c@64
PS2, Line 64: static enum pjsip_status_code check_content_type(const pjsip_rx_data *rdata)
> > This allows any MESSAGE content type even for out-of-dialog
True but I don't see any other necessary changes to support other types of content type for out-of-dialog messages.

The main/message.c code and out-of-dialog API would need updating to allow the user to get and set the content type.  chan_sip, chan_pjsip, and res_xmpp would need updating to allow this as they currently assume the content type is text/plain.



-- 
To view, visit https://gerrit.asterisk.org/8762
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: Idacf5900bfd5f22ab8cd235aa56dfad090d18489
Gerrit-Change-Number: 8762
Gerrit-PatchSet: 3
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Mon, 16 Apr 2018 17:34:34 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180416/3f08d649/attachment.html>