[Home]

Summary:ASTERISK-11223: app_controlplayback: Remove the default skip keys * and #
Reporter:Johan Wilfer (johan)Labels:
Date Opened:2008-01-12 17:19:04.000-0600Date Closed:2008-01-30 13:00:27.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_controlplayback
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_controlplayback.c.option3.patch
( 1) app_controlplayback.c.patch
( 2) CHANGES.controlplayback.patch
( 3) UPGRADE.txt.controlplayback.patch
Description:In patch 0011753 the issue with the (badly) elected skip keys came up. My sugestion was to drop them. Jason Parker suggested that this discussion should be handled separatley.

In mail to the Asterisk dev mailinglist I raised the question if we could remove these defaults sience they seems to be strange (voip-info.org: Note that as the *** key is on the left and the *#* button is on the right of a  telephone keypad, it may make more sense to swap the default buttons: use *** for rewind and *#* for forward.)

Except for Jason, nobody replied.

Jason's idea is quoted below, for reference, I still vote to remove the defaults.

****** ADDITIONAL INFORMATION ******

Jasons mail:
-----------
I might as well throw in my $2.50, since my comment prompted this.  :)

I'm a bit torn.  On one hand, I fully agree that forcing defaults is bad and
in your case, unwelcome.

On the other hand, I don't know if it would be a good idea to change the
defaults, as that would be an unexpected change in behavior (however, it would
only go in trunk, and can thus be documented in UPGRADE.txt)

Perhaps what would make sense, is some mixture of both.  If you just do
ControlPlayback(filename), I think * and # should be ffwd and rew (I also
agree that they are backwards...).  However, if you do
ControlPlayback(filename,,,,), I think maybe it could set them to NULL.  The
only problem I can see with this method, is that it would be an unexpected
change, if you only INTENDED to change skipms (or whatever is after ffwd and rew).

Another thought I had, was that in your case, you're wanting to set every key
to be a stop key.  What if we left the default behavior, except in the case
where stopkey has a key that would normally be a default (ffwd and rew)?  For
instance: ControlPlayback(filename,123456789*0).  In this case, * would be
ONLY a stop key, and # would continue to rewind.  If you were to put # into
stopkeys, then it would no longer act as rew.


I think I like the last method the best.  Thoughts?


Comments:By: Johan Wilfer (johan) 2008-01-12 17:22:45.000-0600

I also documented the changes we made in 0011377 in CHANGES. I forgot this previously.
By: Jason Parker (jparker) 2008-01-14 15:47:08.000-0600

I still think that option 3 would be the best.
By: Johan Wilfer (johan) 2008-01-14 16:03:04.000-0600

I'm not very skilled in C, so I'm not sure how to implement it. However if I get it working I will upload a patch here implementing option 3.

Maybe the modification to CHANGES should go into trunk either way - documenting the previously added feature.
By: Johan Wilfer (johan) 2008-01-30 04:42:32.000-0600

I've implemented your option 3.
I guess there is no need to add a note to UPGRADE then, as ControlPlayback() should behave as before. Maybe you would like to mention it in CHANGES together with the feature in 0011377 (as in the patch)

I constructed this dialplan to test it:
context catchall {
       _X. => {
               Answer();
               Wait(1);
               ControlPlayback(tt-monkeys,3000);
               ControlPlayback(tt-monkeys);
               ControlPlayback(tt-monkeys,,,,1234567890*#);
               ControlPlayback(tt-monkeys,,,,1234567890,*,#);
               ControlPlayback(tt-monkeys,,,,1234567890,,#);
               ControlPlayback(tt-monkeys,,,,1234567890,,*);
               ControlPlayback(tt-monkeys,3000,,,*,,1);
               ControlPlayback(tt-monkeys,3000,,,,#,1);
Hangup();
}

And the output (together with some debug statements that isn't included in the path:
   -- Executing [0303350971@catchall:1] Answer("SIP/0303350971-08229408", "") in new stack
   -- Executing [0303350971@catchall:2] Wait("SIP/0303350971-08229408", "1") in new stack
   -- Executing [0303350971@catchall:3] ControlPlayback("SIP/0303350971-08229408", "tt-monkeys,3000") in new stack
[Jan 30 11:25:22] WARNING[13022]: app_controlplayback.c:133 controlplayback_exec: args.fwd = #
[Jan 30 11:25:22] WARNING[13022]: app_controlplayback.c:143 controlplayback_exec: args.rew = *
   -- <SIP/0303350971-08229408> Playing 'tt-monkeys.gsm' (language 'en')
   -- Executing [0303350971@catchall:4] ControlPlayback("SIP/0303350971-08229408", "tt-monkeys") in new stack
[Jan 30 11:25:38] WARNING[13022]: app_controlplayback.c:133 controlplayback_exec: args.fwd = #
[Jan 30 11:25:38] WARNING[13022]: app_controlplayback.c:143 controlplayback_exec: args.rew = *
   -- <SIP/0303350971-08229408> Playing 'tt-monkeys.gsm' (language 'en')
   -- Executing [0303350971@catchall:5] ControlPlayback("SIP/0303350971-08229408", "tt-monkeys,,,,1234567890*#") in new stack
[Jan 30 11:25:55] WARNING[13022]: app_controlplayback.c:136 controlplayback_exec: args.fwd = NULL
[Jan 30 11:25:55] WARNING[13022]: app_controlplayback.c:146 controlplayback_exec: args.rew = NULL
   -- <SIP/0303350971-08229408> Playing 'tt-monkeys.gsm' (language 'en')
   -- Executing [0303350971@catchall:6] ControlPlayback("SIP/0303350971-08229408", "tt-monkeys,,,,1234567890,*,#") in new stack
[Jan 30 11:26:11] WARNING[13022]: app_controlplayback.c:136 controlplayback_exec: args.fwd = NULL
[Jan 30 11:26:11] WARNING[13022]: app_controlplayback.c:146 controlplayback_exec: args.rew = NULL
   -- <SIP/0303350971-08229408> Playing 'tt-monkeys.gsm' (language 'en')
   -- Executing [0303350971@catchall:7] ControlPlayback("SIP/0303350971-08229408", "tt-monkeys,,,,1234567890,,#") in new stack
[Jan 30 11:26:27] WARNING[13022]: app_controlplayback.c:136 controlplayback_exec: args.fwd = NULL
[Jan 30 11:26:27] WARNING[13022]: app_controlplayback.c:143 controlplayback_exec: args.rew = *
   -- <SIP/0303350971-08229408> Playing 'tt-monkeys.gsm' (language 'en')
   -- Executing [0303350971@catchall:8] ControlPlayback("SIP/0303350971-08229408", "tt-monkeys,,,,1234567890,,*") in new stack
[Jan 30 11:26:43] WARNING[13022]: app_controlplayback.c:133 controlplayback_exec: args.fwd = #
[Jan 30 11:26:43] WARNING[13022]: app_controlplayback.c:146 controlplayback_exec: args.rew = NULL
   -- <SIP/0303350971-08229408> Playing 'tt-monkeys.gsm' (language 'en')
   -- Executing [0303350971@catchall:9] ControlPlayback("SIP/0303350971-08229408", "tt-monkeys,3000,,,*,,1") in new stack
[Jan 30 11:26:59] WARNING[13022]: app_controlplayback.c:133 controlplayback_exec: args.fwd = #
[Jan 30 11:26:59] WARNING[13022]: app_controlplayback.c:146 controlplayback_exec: args.rew = NULL
   -- <SIP/0303350971-08229408> Playing 'tt-monkeys.gsm' (language 'en')
   -- Executing [0303350971@catchall:10] ControlPlayback("SIP/0303350971-08229408", "tt-monkeys,3000,,,,#,1") in new stack
[Jan 30 11:27:15] WARNING[13022]: app_controlplayback.c:136 controlplayback_exec: args.fwd = NULL
[Jan 30 11:27:15] WARNING[13022]: app_controlplayback.c:143 controlplayback_exec: args.rew = *
   -- <SIP/0303350971-08229408> Playing 'tt-monkeys.gsm' (language 'en')
   -- Executing [0303350971@catchall:11] Hangup("SIP/0303350971-08229408", "") in new stack
 == Spawn extension (catchall, 0303350971, 11) exited non-zero on 'SIP/0303350971-08229408'
By: Digium Subversion (svnbot) 2008-01-30 13:00:26.000-0600

Repository: asterisk
Revision: 101296

U   trunk/apps/app_controlplayback.c

------------------------------------------------------------------------
r101296 | qwell | 2008-01-30 13:00:24 -0600 (Wed, 30 Jan 2008) | 10 lines

Allow disabling the default ffwd/rewind keys in the ControlPlayback application.
This is done in a backward compat way.
If the "default" key for ffwd/rew is used for another option (such as stop), the "default" is removed.

(closes issue ASTERISK-11223)
Reported by: johan
Patches:
     app_controlplayback.c.option3.patch uploaded by johan (license 334)
Tested by: johan, qwell

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=101296