Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/ --- (Updated Feb. 10, 2014, 10:37 a.m.) Status -- This change has been marked as submitted. 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 407622 branches/11/apps/confbridge/include/confbridge.h 407622 branches/11/apps/confbridge/conf_state_multi_marked.c 407622 branches/11/apps/confbridge/conf_state_empty.c 407622 branches/11/apps/confbridge/conf_config_parser.c 407622 branches/11/apps/app_confbridge.c 407622 branches/11/UPGRADE.txt 407622 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/ --- (Updated Feb. 7, 2014, 8:57 a.m.) Review request for Asterisk Developers. Changes --- Address Richard's comments. 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 (updated) - branches/11/configs/confbridge.conf.sample 407622 branches/11/apps/confbridge/include/confbridge.h 407622 branches/11/apps/confbridge/conf_state_multi_marked.c 407622 branches/11/apps/confbridge/conf_state_empty.c 407622 branches/11/apps/confbridge/conf_config_parser.c 407622 branches/11/apps/app_confbridge.c 407622 branches/11/UPGRADE.txt 407622 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/#review10835 --- Ship it! This is good as is. I just have a minor concern about how spammy the deprecation warning may be. branches/11/apps/confbridge/conf_config_parser.c https://reviewboard.asterisk.org/r/3155/#comment20423 This message may be spammy. - rmudgett On Feb. 7, 2014, 8:57 a.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/ --- (Updated Feb. 7, 2014, 8:57 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 407622 branches/11/apps/confbridge/include/confbridge.h 407622 branches/11/apps/confbridge/conf_state_multi_marked.c 407622 branches/11/apps/confbridge/conf_state_empty.c 407622 branches/11/apps/confbridge/conf_config_parser.c 407622 branches/11/apps/app_confbridge.c 407622 branches/11/UPGRADE.txt 407622 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/#review10780 --- Ship it! Ship It! - Matt Jordan On Jan. 30, 2014, 4:21 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/ --- (Updated Jan. 30, 2014, 4:21 p.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 406782 branches/11/apps/confbridge/include/confbridge.h 406782 branches/11/apps/confbridge/conf_state_multi_marked.c 406782 branches/11/apps/confbridge/conf_state_empty.c 406782 branches/11/apps/confbridge/conf_config_parser.c 406782 branches/11/apps/app_confbridge.c 406782 branches/11/UPGRADE.txt 406782 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/#review10795 --- branches/11/apps/confbridge/conf_state_multi_marked.c https://reviewboard.asterisk.org/r/3155/#comment20340 This sound needs to be played using a deferred join action on the joining marked user: conf_add_post_join_action(). Otherwise, the conference bridge is locked until the sound prompt completes playing. Locking the bridge for that duration will prevent others from joining or leaving until the sound completes. - rmudgett On Jan. 30, 2014, 4:21 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/ --- (Updated Jan. 30, 2014, 4:21 p.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 406782 branches/11/apps/confbridge/include/confbridge.h 406782 branches/11/apps/confbridge/conf_state_multi_marked.c 406782 branches/11/apps/confbridge/conf_state_empty.c 406782 branches/11/apps/confbridge/conf_config_parser.c 406782 branches/11/apps/app_confbridge.c 406782 branches/11/UPGRADE.txt 406782 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
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. 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. - 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
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. 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). - Matt --- 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
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). opticron wrote: 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. While that makes sense, it is a breaking change to the configuration. Granted, the option didn't really work properly, but the documentation for that sound file is now completely wrong. What's more, there's now dead code in ConfBridge that should never be called. - there's no point playing it in conf_state_emtpy: join_marked, as that is being played to the marked user - there's no point playing it in conf_state_empty: conf_handle_first_marked_common So, if you go the route of a new sound file, and change the behaviour, you need to: 1. Add a note to the UPGRADE file noting the change and the new sound file. You should also note that it is played to all users, both waitmarked and those who have no mark. (The note in the sample config is fine) 2. The set_sound config handler should treat sound_place_into_conference as deprecated. It should warn that the sound is no longer played, and that sound_begin is now played. 3. You should remove the callbacks that explicitly played that sound file. When this goes in, make sure you also update the wiki documentation for ConfBridge (the increasingly inaptly named ConfBridge 10 page) - Matt --- 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
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/ --- (Updated Jan. 30, 2014, 4:21 p.m.) Review request for Asterisk Developers. Changes --- Address Matt's comments by adding deprecation warnings and upgrade text. 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 (updated) - branches/11/configs/confbridge.conf.sample 406782 branches/11/apps/confbridge/include/confbridge.h 406782 branches/11/apps/confbridge/conf_state_multi_marked.c 406782 branches/11/apps/confbridge/conf_state_empty.c 406782 branches/11/apps/confbridge/conf_config_parser.c 406782 branches/11/apps/app_confbridge.c 406782 branches/11/UPGRADE.txt 406782 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- 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. Changes --- Addressed Mark's and Richard's comments. 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 (updated) - 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 (updated) --- 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/#review10713 --- branches/11/apps/confbridge/conf_state_multi_marked.c https://reviewboard.asterisk.org/r/3155/#comment20214 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. - Matt Jordan 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/#review10708 --- If you haven't already, run the ConfBridge tests in the Asterisk Test Suite and verify that all tests pass with your changes. I suspect the marked users test may fail, as it checks for certain sound file prompts being played. You may also want to update the appropriate tests to verify that the correct sound file is played back with your changes. - Matt Jordan On Jan. 24, 2014, 1:05 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/ --- (Updated Jan. 24, 2014, 1:05 p.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 406292 branches/11/apps/confbridge/include/confbridge.h 406292 branches/11/apps/confbridge/conf_state_multi_marked.c 406292 branches/11/apps/confbridge/conf_config_parser.c 406292 branches/11/apps/app_confbridge.c 406292 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. Thanks, opticron -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/#review10690 --- branches/11/apps/confbridge/conf_state_multi_marked.c https://reviewboard.asterisk.org/r/3155/#comment20183 The sound can be played back in a couple of situations where it may not be desired. 1) Conference starts empty. The first user that enters is marked. The second user enters (may be marked, waitmarked, or have no affiliation with marks). When the second user enters, the conference will have CONF_SOUND_BEGIN played to them. In this case, since no one was waiting on the marked user, it seems weird to do this. You can get around this by adding the check that was previously here, which is to check if cbu is a marked user. 2) A bunch of non-waitmarked users are in the conference chatting away, and a marked user enters. CONF_SOUND_BEGIN will play when the marked user enters. You could get around this by not playing the sound if there were no waitmarked users in the conference when the marked user entered. - Mark Michelson On Jan. 24, 2014, 7:05 p.m., opticron wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3155/ --- (Updated Jan. 24, 2014, 7:05 p.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 406292 branches/11/apps/confbridge/include/confbridge.h 406292 branches/11/apps/confbridge/conf_state_multi_marked.c 406292 branches/11/apps/confbridge/conf_config_parser.c 406292 branches/11/apps/app_confbridge.c 406292 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. Thanks, opticron -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
