[Asterisk-code-review] bridge softmix / res rtp asterisk: Fix packet loss and reneg... (asterisk[master])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/6032 )

Change subject: bridge_softmix / res_rtp_asterisk: Fix packet loss and renegotiation issues.
......................................................................


Patch Set 3: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/6032/2/apps/confbridge/conf_config_parser.c
File apps/confbridge/conf_config_parser.c:

https://gerrit.asterisk.org/#/c/6032/2/apps/confbridge/conf_config_parser.c@453
PS2, Line 453:                                 <configOption name="video_update_discard" default="2000">
             :                                         <synopsis>Sets the amount of time in milliseconds after sending a video update to discard subsequent video updates</synopsis>
             :                                         <description><para>
             :                                                 Sets the amount of time in milliseconds after sending a video update request
             :                                                 that subsequent video updates should be discarded. This means that if we
             :                                                 send a video update we will discard any other video update requests until
             :                                                 after the configured amount of time has elapsed. This prevents flooding of
             :                                                 video update requests from clients.
             :                                         </para></description>
             :                                 </configOption>
spaces instead of tabs


https://gerrit.asterisk.org/#/c/6032/2/configs/samples/confbridge.conf.sample
File configs/samples/confbridge.conf.sample:

https://gerrit.asterisk.org/#/c/6032/2/configs/samples/confbridge.conf.sample@221
PS2, Line 221: ;video_update_discard=2000 ; Amount of time (in milliseconds) to discard video update requests after sending a video
             :                            ; update request. Default is 2000.
Can you explain a little more about "video update requests"?


https://gerrit.asterisk.org/#/c/6032/2/main/rtp_engine.c
File main/rtp_engine.c:

https://gerrit.asterisk.org/#/c/6032/2/main/rtp_engine.c@3416
PS2, Line 3416: }
Did whitespace get added after the '}'?


https://gerrit.asterisk.org/#/c/6032/3/res/res_rtp_asterisk.c
File res/res_rtp_asterisk.c:

https://gerrit.asterisk.org/#/c/6032/3/res/res_rtp_asterisk.c@3983
PS3, Line 3983: difference && 
Let's see if richard marks this as an unnecessary comparison. :)


https://gerrit.asterisk.org/#/c/6032/3/res/res_rtp_asterisk.c@3990
PS3, Line 3990: !difference || difference > 0
Why not just "difference >= 0" ?



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd91362f0e4990b6129638e712bc8adf0899fd45
Gerrit-Change-Number: 6032
Gerrit-PatchSet: 3
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Comment-Date: Wed, 19 Jul 2017 13:17:43 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170719/39e71e94/attachment.html>