Skip to content

Commit

Permalink
Use AGI native RECORD FILE command instead of Exec Record
Browse files Browse the repository at this point in the history
  • Loading branch information
@bklang
bklang committed Mar 30, 2010
1 parent 86a7259 commit 0444c69
Showing 1 changed file with 15 additions and 2 deletions.
17 changes: 15 additions & 2 deletions lib/adhearsion/voip/asterisk/commands.rb
View file
Expand Up @@ -174,10 +174,23 @@ def play(*arguments)
def record(*args)
options = args.last.kind_of?(Hash) ? args.pop : {}
filename = args.shift || "/tmp/recording_%d.gsm"
if (!options.has_key?(:format))
format = filename.slice!(/\.[^\.]+$/)
if (format.nil?)
ahn_log.agi.warn "Format not specified and not detected. Defaulting to \"gsm\""
format = gsm
end
format.sub!(/^\./, "")
end
silence = options.delete(:silence) || 0
maxduration = options.delete(:maxduration) || 0
maxduration = options.delete(:maxduration) || -1
escapedigits = options.delete(:escapedigits) || "#"

execute("Record", filename, silence, maxduration)
if (silence > 0)
response("RECORD FILE", filename, format, escapedigits, maxduration,0, "BEEP", "s=#{silence}")
else
response("RECORD FILE", filename, format, escapedigits, maxduration, 0, "BEEP")
end

# If the user hangs up before the recording is entered, -1 is returned and RECORDED_FILE
# will not contain the name of the file, even though it IS in fact recorded.
Expand Down

4 comments on commit 0444c69

@eric
Copy link
Contributor

@eric eric commented on 0444c69 Sep 15, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to use RECORD FILE instead of the Record app?

@bklang
Copy link
Member Author

@bklang bklang commented on 0444c69 Sep 15, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the biggest problem is that the Record app's arguments and behavior changes between versions of Asterisk. In 1.6 and later the Record app (by default) deletes the file if the user hangs up without terminating the recording. There's also the bug in some versions of Asterisk that if the caller hangs up before terminating the recording and a "%d" replacement is in use, the RECORDED_FILE channel variable does not get set and the filename cannot be returned. These problems are mitigated by moving that logic into Adhearsion.

One side benefit to this approach is that we can also specify the audio prompt to be played immediately before recording begins, although we are not exposing that functionality today.

In general my preference is to use AGI primitives instead of applications where possible as there is more stability across versions.

@eric
Copy link
Contributor

@eric eric commented on 0444c69 Sep 15, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the implementation of the options parsing in 1.4's Record app, it looks like passing the k argument by default shouldn't break existing behavior.

It may require a little bit of extra work to be able to fetch the RECORDED_FILE after the Hangup has been raised, but it shouldn't be that hard (I wonder if just catching the exception and doing the variable call would work).

@bklang
Copy link
Member Author

@bklang bklang commented on 0444c69 Sep 15, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem isn't the Hangup exception, it's that in some versions of Asterisk (not exactly sure which versions, or even if it has been fixed yet) the RECORDED_FILE channel var isn't accessible after hangup. It either isn't set (maybe) or can't be retrieved from a dead channel (probably).

You might be able to work around that by using AMI to listen for the event that is fired when the channel var is set, but that breaks if you have AMI disabled or if you are receiving AGI commands from multiple Asterisk servers.

My opinion is that if we can't get a reasonable solution to the %d problem, we should deprecate its usage entirely prior to releasing Adhearsion 1.0.

Please sign in to comment.