[Home]

Summary:ASTERISK-03803: [new_app] app_dictate
Reporter:Anthony Minessale (anthm)Labels:
Date Opened:2005-03-29 16:25:55.000-0600Date Closed:2008-01-15 15:33:20.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) app_dictate.c
( 1) app_dictate.tgz
( 2) dictmainmake.diff
( 3) dictmake.diff
Description:This is app_dictate Donated to the community by Sangoma Technologies

Dialplan

exten => 1000,1,Dictate()


All files are created in /var/spool/asterisk/dictate by default
you can override this in the app's argument.

example:

* call exten 1000
* dial 1234# to pick file 1234
* press 1 to switch to record mode
* dial * to unpause and begin recording
* dial * to pause/unpause recording
* optionally dial 8 to erase the whole file and start again.

When you are done recording:

* dial 1 to go back to playback mode
* dial * to toggle pause
* dial 2 to toggle fast playback
* dial 7 or 8 to seek forwards and reverse a few frames



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

Disclaimer on File
anthmct@yahoo.com
Comments:By: Anthony Minessale (anthm) 2005-03-29 16:27:09.000-0600

Everything you need is in the tgz, added .c for easy browsing.
By: Mark Spencer (markster) 2005-03-29 16:45:56.000-0600

On the legal side:

Does the copyright of this application lay with anthm or with sangoma?  I would like to have a disclaimer on file from sangoma to be on the safe side.

On the technical side:

1) ast_toggle_flag should probably go in include/asterisk/utils.h, not here.

2) The patch isn't really in the right format for the Makefile.  We need to not only add it to the apps but be sure the sounds are properly installed and that the spool directory is properly built (see voicemail stuff).

3) Obviously the documentation is very slim.  Could use some filling out.

Other than that looks great and seems like a good new feature to have.
By: Kevin P. Fleming (kpfleming) 2005-03-29 21:47:31.000-0600

I agree with markster, this is a really cool idea and I'm very glad anthm and Sangoma have built it and given it to the community!

The default base directory should be somewhere in AST_SPOOL_DIR, not hardcoded to /var/spool/asterisk. Many people move the spool directory elsewhere at run time using asterisk.conf, so including a path in the help text is probably not a good idea (unless you plan on providing run-time built help text based on AST_SPOOL_DIR <G>).

ast_toggle_flag can use an XOR operation for the desired flag, instead of an if-then construct.

Is there a reason to force the read format to SLINEAR? That will force transcoding on the way out and back in, when it may not be needed at all. I would prefer to see it just record in the channel's native format, or allow the Dictate() app to take a list of formats to record in (like app_voicemail does via voicemail.conf). Oh... I see now that skip-forward and skip-reverse are implemented directly in this app, and they are based on the number of samples in an SLINEAR frame. Hmm... Could you just add the 'speed control' function to ast_control_streamfile and then use that for playback, so you don't have to reimplement all the stuff it does?

Ick... please don't declare variables inside a switch() block... I know it's legal, but it's not pretty, and lots of people will misunderstand what is happening there.

I don't know if this is common practice in Asterisk because I haven't looked, but why are the audio files created with mode 0700? Isn't the system admin's responsibility to limit the file permissions (using umask) if they desire to do that? I know that on my system I would very much prefer to have these files group-writable by the group my 'asterisk' user belongs to, so I have Asterisk running with 'umask 002'. However, code like this will defeat the purpose of that, and it's also uncommon behavior for Unix (and Unix-like) applications.

markster: The copyright header in the file is pretty explicit that anthm is the copyright owner, so as long as Sangoma is fine with him holding copyright, there is no need for a disclaimer from Sangoma.
By: Kevin P. Fleming (kpfleming) 2005-03-29 22:03:12.000-0600

In fact, I'll take that one step further: I think it would be _very_ cool if the recording enhancements here got put into ast_record_review (or something similar), so that app_voicemail and app_record could benefit from them as well.
By: Mark Spencer (markster) 2005-03-29 22:31:45.000-0600

Since Sangoma funded the work, they could conceivably still own the copyright without a proper assignment.  This is a bit of an unusual circumstance and I would certainly feel more comfortable if they had a disclaimer on file, or in the alternative made a formal copyright assignment to anthm so his disclaimer would more clearly apply.
By: Anthony Minessale (anthm) 2005-03-30 07:06:31.000-0600

The raw audio format allows for easier audio manipulation and is in place to support future enhancements.
By: cypromis (cypromis) 2005-03-30 07:55:37.000-0600

Since this app just screames for more and more enhancement raw audio seems to be the right way to go no ?
By: Kevin P. Fleming (kpfleming) 2005-03-30 21:29:15.000-0600

So if someone wants raw audio, under my proposal they can choose it easily. For those who don't have any need for it, forcing them to transcode into raw (and back on playback) seems wasteful to me.
By: Brian West (bkw918) 2005-03-30 21:36:59.000-0600

Well you can't do anything like this unless its raw headerless audio.  Trying to rewind.. append and all that mess with somethign like WAV49 = EVIL.

/b
By: Kevin P. Fleming (kpfleming) 2005-03-30 21:48:15.000-0600

Doesn't ast_record_review already support that? ast_control_streamfile does too, for playback, and it doesn't require the use of raw audio. There certainly are formats where that doesn't work well, but many of them work just fine.
By: cypromis (cypromis) 2005-04-04 11:29:03

Disclaimer from Sangoma is since Friday on file in digium.
By: jmls (jmls) 2005-04-05 01:17:34

Would it be possible to add an option to send the recording to an email recipient ?
By: opsys (opsys) 2005-04-05 01:39:09

It would great to be able to pass options like filename (relative to AST_SPOOL_DIR)and final file format.  This could then be used to record IVR, Auto Attendent, Greetings, Etc.  It would provide much more control than the simple Record App does today.
By: Anthony Minessale (anthm) 2005-04-05 09:35:22

jmls:
If you want more advanced recirding call Monitor on the channel before
you start spying and then use that facility to send email.

opsys:
The files are numeric names that you choose yourself.

Say you want to make changes to live ivr audio like you suggested

call Dictate(/var/lib/asterisk/sounds)

then name the files numeric names from within the app e.g. 1234
then reference them in your dialplan

Playback(1234)

The format is sln which is the most optimal audio format to encode anyway
(WAY more efficient than gsm we mostly use which ends up being transcoded to sln anyway on it's way to whatever format your channel is in)
By: Mark Spencer (markster) 2005-04-05 14:04:29

I can confirm we did receive the disclaimer from Sangoma for tony's changes, so pending the completion of the technical discussion this is ready to merge.
By: sangoma-michaelb (sangoma-michaelb) 2005-04-19 14:48:41

So what is blocking that from making it into CVS ?
By: Anthony Minessale (anthm) 2005-04-21 15:14:35

spool dir now derived from makefile
By: Kevin P. Fleming (kpfleming) 2005-04-22 08:28:11

That change to use the Makefile ASTSPOOLDIR definition is still not the right thing to do; the spool directory can be set at run time via asterisk.conf, and all modules should use the runtime value. For an example, see app_voicemail.c.

There are enough potential improvements to this code that at this point I think I'd rather just clean it up myself and commit it; anyone have any opposition to that?
By: opsys (opsys) 2005-04-22 08:40:01

Having it NOT take the spooldir directive from the asterisk.conf file was the only thing I had a hangup about.  I have used it and it "works for me"
By: Anthony Minessale (anthm) 2005-04-22 10:01:58

after some detective work, I figured out how to get the spooldir from the header file I never heard of in the nostandard place so now it should be fine.  I dont want many changes to happen to this code unless it's necessary because there are improvements planned in the very near future.
By: Mark Spencer (markster) 2005-04-26 23:31:45

okay so what does that mean, does that mean we shouldn't commit it, pending your updates?  Does this mean you have fixes for some of the issues that were mentioned by kevin above?
By: Anthony Minessale (anthm) 2005-04-27 08:43:16

oh sorry, the code is already changed to find spooldir right so it is ok to commit was what i meant.
By: Kevin P. Fleming (kpfleming) 2005-04-28 11:57:02

Well, I think the bigger concern was your statement that you 'didn't want many changes to happen to this code'. If it gets committed, and someone decides to improve it (besides you), then those changes will probably get accepted, even though you are working on enhancements as well. If you'd rather not have to deal with that, then we can just suspend this bug until the enhanced version is ready.
By: Michael Jerris (mikej) 2005-04-29 11:06:42

I think Tony is more than capable of commenting on any bug that modifies or adds to app_dictate to add functionality and suggest a possible better way to implement.  Can we just get this in allready?
By: Kevin P. Fleming (kpfleming) 2005-04-29 11:31:31

OK, so to commit this, we use all the files from the tarball _except_ for app_dictate.c, correct?
By: Anthony Minessale (anthm) 2005-04-29 11:38:27

yes
By: Michael Jerris (mikej) 2005-05-02 09:36:40

Is there anything keeping this from going in?
By: Brian West (bkw918) 2005-05-03 22:36:52

Anyone?
By: Kevin P. Fleming (kpfleming) 2005-05-03 22:54:56

The only think keeping this from going in is Mark's time at the moment... I'm not comfortable adding it without his OK, and he's been swamped the last few days (and is off line right now).

There are no technical issues stopping the merge at this point that I am aware of.
By: Mark Spencer (markster) 2005-05-04 14:47:47

It has my OK to merge.
By: Kevin P. Fleming (kpfleming) 2005-05-04 15:27:01

Committed to CVS HEAD, thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:33:20.000-0600

Repository: asterisk
Revision: 5579

U   trunk/Makefile
U   trunk/apps/Makefile
A   trunk/apps/app_dictate.c
A   trunk/sounds/dictate/
A   trunk/sounds/dictate/both_help.gsm
A   trunk/sounds/dictate/enter_filename.gsm
A   trunk/sounds/dictate/forhelp.gsm
A   trunk/sounds/dictate/pause.gsm
A   trunk/sounds/dictate/paused.gsm
A   trunk/sounds/dictate/play_help.gsm
A   trunk/sounds/dictate/playback.gsm
A   trunk/sounds/dictate/playback_mode.gsm
A   trunk/sounds/dictate/record.gsm
A   trunk/sounds/dictate/record_help.gsm
A   trunk/sounds/dictate/record_mode.gsm
A   trunk/sounds/dictate/truncating_audio.gsm
U   trunk/sounds.txt

------------------------------------------------------------------------
r5579 | kpfleming | 2008-01-15 15:33:19 -0600 (Tue, 15 Jan 2008) | 2 lines

add app_dictate (bug ASTERISK-3803)

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

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