[Asterisk-code-review] Implement deaf mode for confbridge (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: Implement deaf mode for confbridge
......................................................................


Patch Set 3: Code-Review-1

(9 comments)

https://gerrit.asterisk.org/#/c/2933/3/apps/app_confbridge.c
File apps/app_confbridge.c:

PS3, Line 284: 				<para>If this parameter is "all", all channels will be unmuted.</para>
             : 				<para>If this parameter is "participants", all non-admin channels will be unmuted.</para>
Copy paste.  This is still referencing muting when it should be referencing deafening.


PS3, Line 300: 				<para>If this parameter is "all", all channels will be unmuted.</para>
             : 				<para>If this parameter is "participants", all non-admin channels will be unmuted.</para>
same


Line 1149: static void test_suite_feature_action_event_notify(
This should be made a macro as ast_test_suite_event_notify() is also a macro.  When TEST_FRAMEWORK is not enabled in menuselect ast_test_suite_event_notify() is defined as nothing.  Since this function only serves to parameterize a call to ast_test_suite_event_notify() it can be an empty function.


Line 1189: 	default:
Remove the default case since the switch is using an enum type.  This way the compiler can complain if you add a new enum value and don't add a specific switch case here.


Line 1871: 		user.features.deaf = 1;
The bridge core does not need to have a special deaf mode as you just need to suspend the user from the bridge and they no longer get audio from it.

This feature should be implemented like a special case of MOH.  The user becomes suspended from the bridge and hears the audio of a silence generator instead of MOH while the users audio is still fed into the bridge.

You'll need to handle the interactions of the deaf mode and MOH.  This can be structured similar to how mute mode is determined between user and system mute requests.


Line 2777: 	default:
Remove the default case and initialize verb = "".  This way the compiler can warn if a new enum value is added later.


Line 3078: static void action_confbridgelist_item(struct mansession *s, const char *id_text, struct confbridge_conference *conference, struct confbridge_user *user, int waiting)
You should add a Deafened header to this AMI event.


Line 3249: 	default:
Remove the default case and initialize verb = "".  This way the compiler can warn if a new enum value is added later.


https://gerrit.asterisk.org/#/c/2933/3/bridges/bridge_softmix.c
File bridges/bridge_softmix.c:

Line 1037: 			if (bridge_channel->features && bridge_channel->features->deaf) {
The deaf mode is already supported by just suspending the channel from the bridge.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2628bdda080fd2b1e914212de50dd26de9d3e96
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Timo Teräs <timo.teras at iki.fi>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Timo Teräs <timo.teras at iki.fi>
Gerrit-HasComments: Yes