[asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target

[opticron]

> On Jan. 29, 2014, 9:34 a.m., Matt Jordan wrote:
> > branches/11/apps/confbridge/conf_state_multi_marked.c, line 203
> > <https://reviewboard.asterisk.org/r/3155/diff/3/?file=53245#file53245line203>
> >
> >     I don't think this is the right sound file. Whey aren't we playing CONF_SOUND_PLACE_IN_CONF to all of the users?
> >     
> >     The point of this is that the person being notified that they are being placed into the conference is the marked user, and not all other users who might be waiting. Why would we need a new sound prompt for this when we have one that says "You are now being placed into the conference?"
> >     
> >     Note that the documentation for this sound already says when it should be played:
> >     
> >     ;sound_place_into_conference ; The sound played when someone is placed into the conference
> >                                  ; after waiting for a marked user.
> >     
> >     
> >     Also: this code change should take into account the EMPTY state. Currently, that plays the prompt to the marked user as well, which is wrong.
> 
> opticron wrote:
>     The CONF_SOUND_PLACE_IN_CONF sound doesn't make sense to conference users who are already in the conference, are not waitmarked, and can already talk to each other so another available prompt was chosen.
> 
> Matt Jordan wrote:
>     Who all are you playing this to?
>     
>     The bug here is that the sound is being played to marked users, which doesn't make any sense.
>     
>     It should be played for waitmarked users, and waitmarked users only. Playing anything else to any other user doesn't make much sense, as those users would join for any other type of user (normal or marked).

This sound is now being played to the entire conference after waitmarked users have joined since the way confbridge is setup does not allow for sounds to be played to a subset of users in a conference in an asynchronous manner while simultaneously maintaining mixing for the remainder of the conference.


- opticron


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3155/#review10713
-----------------------------------------------------------


On Jan. 29, 2014, 8:26 a.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3155/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2014, 8:26 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: PQ-1396
>     https://issues.asterisk.org/jira/browse/PQ-1396
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Currently, when the first marked user enters the conference that contains waitmarked users, a prompt is played indicating that the user is being placed into the conference. Unfortunately, this prompt is played to the marked user and not the waitmarked users which is not very helpful.
> 
> This patch changes that behavior to play a prompt stating "The conference will now begin" to the entire conference after adding and unmuting the waitmarked users since the design of confbridge is not conducive to playing a prompt to a subset of users in a conference in an asynchronous manner.
> 
> 
> Diffs
> -----
> 
>   branches/11/configs/confbridge.conf.sample 406774 
>   branches/11/apps/confbridge/include/confbridge.h 406774 
>   branches/11/apps/confbridge/conf_state_multi_marked.c 406774 
>   branches/11/apps/confbridge/conf_config_parser.c 406774 
>   branches/11/apps/app_confbridge.c 406774 
> 
> Diff: https://reviewboard.asterisk.org/r/3155/diff/
> 
> 
> Testing
> -------
> 
> Verified that the prompt is heard by users already in the conference when a marked user enters and that ConfBridge tests pass with modified event expectations.
> 
> 
> Thanks,
> 
> opticron
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140130/cf748445/attachment.html>