[Asterisk-code-review] res_pjsip_t38: T.38 error correction mode selection at 200 ok received (asterisk[13])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/13180 )

Change subject: res_pjsip_t38: T.38 error correction mode selection at 200 ok received
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

I'm not sure about this. Is there an RFC someplace, or a known convention as to the expectations for negotiation of the 't38faxudpec attribute?

Reasonably, it seems like if one side doesn't allow EC then EC would not be included, or processed. I'm not sure about the other two modes.

https://gerrit.asterisk.org/c/asterisk/+/13180/1/res/res_pjsip_t38.c 
File res/res_pjsip_t38.c:

https://gerrit.asterisk.org/c/asterisk/+/13180/1/res/res_pjsip_t38.c@705 
PS1, Line 705: 			if (session->t38state == T38_LOCAL_REINVITE) {
Does initiator matter? It seems like we'd want to do the same for any negotiation scenario? Meaning if Asterisk is configured to not handle error correction then we'd want to make sure support for it was disabled for incoming or outgoing? Same for the other modes?


https://gerrit.asterisk.org/c/asterisk/+/13180/1/res/res_pjsip_t38.c@709 
PS1, Line 709: 					} else if (!pj_stricmp2(&attr->value, "t38UDPRedundancy")) {
If error correction is configured for FEC why potentially allow redundancy?


https://gerrit.asterisk.org/c/asterisk/+/13180/1/res/res_pjsip_t38.c@722 
PS1, Line 722:                                 }
Spaces vs tabs issue here. Please use tabs.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13180
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I343c62253ea4c8b7ee17abbfb377a4d484a14b19
Gerrit-Change-Number: 13180
Gerrit-PatchSet: 1
Gerrit-Owner: Salah Ahmed <txrubel at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Tue, 19 Nov 2019 00:11:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191118/c9fb1530/attachment.html>