[Asterisk-code-review] ConfBridge: Make some announcements asynchronous. (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: ConfBridge: Make some announcements asynchronous.
......................................................................


Patch Set 1: Code-Review-1

(6 comments)

https://gerrit.asterisk.org/#/c/3504/1/CHANGES
File CHANGES:

Line 44: 	to wait for a leave announcement to play.
tab used instead of spaces


https://gerrit.asterisk.org/#/c/3504/1/apps/app_confbridge.c
File apps/app_confbridge.c:

Line 1738: struct async_playback_task_data {
doxygen


PS1, Line 1818: 			ast_log(LOG_WARNING, "Unable to play file '%s' to conference\n", filename);
              : 		} else {
              : 			ast_log(LOG_WARNING, "Unable to say number '%d' to conference\n", say_number);
Add conference name to msg.


Line 2143: 		async_play_sound_file(conference, join_sound);
There is the possibility for the channel joining the bridge to miss the start of the join sound because he got delayed joining the bridge.

The participant and the bridge could hear different languages for the join sound before.


PS1, Line 2275: 	/* Playing the sound asynchronously lets the sound be heard by everyone at once */
              : 	async_play_sound_helper(conference, sound_to_play, 0);
This sound could be clipped to the participant toggling the mute.  This is more likely to happen if there is more than one action to perform and this is not the last one.

This is also a case were the participant and bridge could hear different languages.


https://gerrit.asterisk.org/#/c/3504/1/apps/confbridge/conf_state_multi_marked.c
File apps/confbridge/conf_state_multi_marked.c:

PS1, Line 164: 			ast_autoservice_start(user->chan);
             : 			async_play_sound_file(user->conference,
             : 				conf_get_sound(CONF_SOUND_LEADER_HAS_LEFT, user->conference->b_profile.sounds));
             : 			ast_autoservice_stop(user->chan);
Need to remove autoservice start/stop.

For that matter we no longer need to unlock/lock the conference either since the sound is not played by this thread.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie486bb3de1646d50894489030326a423e594ab0a
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes