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

[Kevin Harwell]

Kevin Harwell has posted comments on this change.

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


Patch Set 2: Code-Review-1

(3 comments)

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

Line 282: 			<parameter name="Channel" required="true" />
Looks like in the handling of this command (also the undeafen and CLI commands) there is nothing limiting the user to specifying "all" or "participants", or a channel prefix.

If there is a reason to limit those options for the [un]defeafen stuff (any reason not to?) then code should be put in place to do so. If not then I'd suggest adding more to the various descriptions (AMI/CLI commands) so the user is aware of those options.


PS2, Line 2097: 	/* Deafen or undeafen yourself */
              : 	user->features.deaf = !user->features.deaf;
              : 	ast_test_suite_event_notify("CONF_DEAF",
              : 		"Message: participant %s %s\r\n"
              : 		"Conference: %s\r\n"
              : 		"Channel: %s",
              : 		ast_channel_name(chan),
              : 		user->features.deaf ? "deafened" : "undeafened",
              : 		user->b_profile.name,
              : 		ast_channel_name(chan));
              : 	if (user->features.deaf) {
              : 		send_deafen_event(user, conference);
              : 	} else {
              : 		send_undeafen_event(user, conference);
              : 	}
Should just be able to call 'generic_feature_action_helper_user' here instead.


https://gerrit.asterisk.org/#/c/2933/2/main/bridge_channel.c
File main/bridge_channel.c:

PS2, Line 975: 	if (fr->frametype == AST_FRAME_VOICE &&
             : 	    (bridge_channel->features && bridge_channel->features->deaf)) {
             : 		short buf[fr->samples];
             : 		struct ast_frame sframe = {
             : 			.frametype = AST_FRAME_VOICE,
             : 			.subclass.format = ast_format_slin,
             : 			.data.ptr = buf,
             : 			.samples = fr->samples,
             : 			.datalen = sizeof(buf),
             : 		};
             : 		memset(buf, 0, sizeof(buf));
             : 		dup = ast_frdup(&sframe);
             : 	} else {
             : 		dup = ast_frdup(fr);
             : 	}
Can a silence generator on the channel not be used instead?


-- 
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: 2
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-HasComments: Yes