adhearsion / punchblock

Telephony middleware library for Ruby
adhearsion.com/punchblock
MIT License
40 stars 34 forks source link

Punchblock returns incorrect recording URI #258

Open system123 opened 7 years ago

system123 commented 7 years ago

I am initialising a recording in Adhearsion using the following code (note: I am using Asterisk 11)

def record_call record async: true, format: 'wav49' do |event| puts event.recording.uri end end

If I use the standard wav format then all recording URIs are correct, the issue comes in with the wav49 format, as this format has two possible valid extensions. It can either record to a .wav file or to a .wav49 file. Punchblock always returns the filename with a .wav49 extension however, some Asterisk instances appear to record to a .wav extension thus punchblock returns an incorrect URI for the recording.

bklang commented 7 years ago

@system123 for documentation sake, can you include Asterisk trace logs so we can see what Asterisk returns? I'd like to know whether we can rely on some specific data returned from Asterisk, or if we have to infer something (obviously the former is preferable)

bklang commented 7 years ago

Thanks for the logs (sent privately). The problem here is that Asterisk doesn't actually tell us what the filename is. When we send a request to start recording, it looks like this:

Action: monitor
ActionID: 74fa5222-d67d-43a8-a274-1d70cc98ad07
Channel: SIP/trunk-0000000d
File: /var/punchblock/record/6aadcc7b-c328-4773-9f72-9c2622debf63
Format: wav49
Mix: true

Response: Success
ActionID: 74fa5222-d67d-43a8-a274-1d70cc98ad07
Message: Started monitoring channel

Event: VarSet
Privilege: dialplan,all
Channel: SIP/trunk-0000000d
Variable: __MONITORED
Value: true
Uniqueid: 1476386347.19

Event: MonitorStart
Privilege: call,all
Channel: SIP/trunk-0000000d
Uniqueid: 1476386347.19

And when the recording completes:

Event: MonitorStop
Privilege: call,all
Channel: SIP/trunk-0000000d
Uniqueid: 1476386360.20

As you can see, we request the filename of the recording, but don't specify an extension, only a format. Asterisk gets to choose the extension based on its internal rules. We have to guess.

If we are going to fix this in Punchblock/Adhearsion 3, then we'll need to replicate the logic that Asterisk uses to pick the file name suffix.

bklang commented 7 years ago

If it helps, the place to insert the logic to infer/guess the correct file name/extension is the Asterisk record component translator

system123 commented 7 years ago

Is it not possible to force the file extension by specifying it as part of the filename passed in the Monitor data?

I will have a look to see how Asterisk determines which extension to use.

bklang commented 7 years ago

The Asterisk Monitor docs don't specify, but the source code doesn't seem to care whether a file extension is provided.

system123 commented 7 years ago

It appears that Asterisk hardcode the replacement of wav49 to WAV as the extension. This can be seen here

Thus the easiest will be for Punchblock to make the same replacement.