Summary:	ASTERISK-24798: Documentation - Clarify That Format Is Set By File Name Extension In MixMonitor
Reporter:	xrobau (xrobau)	Labels:
Date Opened:	2015-02-16 13:23:53.000-0600	Date Closed:	2020-02-20 09:26:04.000-0600
Priority:	Minor	Regression?
Status:	Closed/Complete	Components:	Applications/app_mixmonitor Documentation
Versions:	13.1.1	Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	I'm not sure how far back this one goes, but running mixmon with these params: {code} MixMonitor("2015/02/17/internal-300-303-20150217-051706-1424114226.982.wav49,ai(LOCAL_MIXMON_ID),/tmp/post.sh") {code} Results in the file being SAVED as internal-300-303-20150217-051706-1424114226.982.WAV Note the different suffix.  Mixmon itself seems unaware of this, however, as $\{MIXMONITOR_FILENAME\} reports it correctly: {code} Set("SIP/303-00000070", "__REC_FILENAME=/var/spool/asterisk/monitor/2015/02/17/internal-300-303-20150217-051706-1424114226.982.wav49") in new stack {code} {code} [root@localhost callrecording]# ls -al /var/spool/asterisk/monitor/2015/02/17/internal-300-303-20150217-051706-1424114226.982.* -rw-rw---- 1 asterisk asterisk 3765 Feb 17 05:17 /var/spool/asterisk/monitor/2015/02/17/internal-300-303-20150217-051706-1424114226.982.WAV [root@localhost callrecording]# {code}
Comments:	By: Michael L. Young (elguero) 2015-02-16 20:46:34.321-0600 Your comments lead one to believe that it never behaved this way.  When did it ever act differently?  I am curious about that. As far as I know, that has always been the behavior.  The extension of the file name sets the format that the audio is to be recorded in.  In this case, you are telling Asterisk to record in MS GSM 6.10 format which uses the extension ".WAV".  The resulting file that is outputted is correct.  So, Asterisk would be working properly unless you can show us where the behavior has changed. By: xrobau (xrobau) 2015-02-16 21:58:50.443-0600 Well, that's not what the documentation says. {code}[Description] Records the audio on the current channel to the specified file. {code} ...and... {code}$\{MIXMONITOR_FILENAME\}: Will contain the filename used to record.{code} Both from 'core show application MixMonitor'. It explicitly DOESN'T say 'Asterisk will guess the format based on the extension you provided, and then *change the filename* to something different, possibly the first valid file extension of that format.' Or maybe I'm reading it wrong 8-) Even so, if that WAS the way it was documented, MIXMONITOR_FILENAME should have the correct filename, which it doesn't. By: Michael L. Young (elguero) 2015-02-17 07:39:44.361-0600 Okay, thanks for clarifying where the source of confusion is. Yes, the documentation could be improved upon.  As I mentioned above, this has been the behavior of MixMonitor for as long as I can remember. By: xrobau (xrobau) 2015-02-17 14:22:08.419-0600 I think that mixmon changing the filename is a terrible idea.  Just because 'that's the way it's always been' doesn't make it right. It just means it's a bug that hasn't been resolved in a long time. By: xrobau (xrobau) 2015-02-17 14:49:57.574-0600 Edit: Note, I have changed my mind. I am now thinking that 'b' is the correct problem, not a. Also, just to clarify, no matter which way you look at it, there's a bug here. a: MixMon shouldn't change the filename.  If that's the bug, then my original point stands. or b: The documentation is wrong. If thats true, then MIXMONITOR_FILENAME should have the correct filename in it, which it doesn't. I'm firmly in the camp of 'If I gave MixMonitor a valid filename, it should use that valid filename'.  After a quick discussion on IRC with @mjordan about 'what should happen if you asked MixMon to record a file with an invalid file type' the consensus there was 'MixMon should fail to record, and MIXMONITOR_FILENAME should be blank'. By: Michael L. Young (elguero) 2015-02-17 14:54:24.485-0600 Who is to say that this was not the intended behavior?  I am not trying to say that the current functionality is right but you can't just start saying "bug, bug" because it doesn't match with what you would like to have happen when all along this is the way it was intended to function by whoever programmed it, it was subsequently reviewed and then accepted into the code base. Let me clarify something.  MixMonitor is not changing the file name.  The extension being used for the final file outputted is being set by the file write functions within Asterisk.  All MixMonitor is doing is passing the requested format by using the extension that the user is specifying to these functions that are outside of MixMonitor. The documentation that you quoted says that `MIXMONITOR_FILENAME` will _contain_ the filename used.  I guess that whoever did this programming figured that the person using this application would know what format they used and just need to extract the file name (the part before the extension).  Who knows. I don't know of any parts of Asterisk that will save a wav49 file with the extension ".wav49".  It only knows that if someone says "I want to record a wav49 or play a wav49, the file being referred to must have an extension of .WAV". The only thing I am wondering and would have to look into, is, if there is a way to know the file name that Asterisk is writing to instead of setting MIXMONITOR_FILENAME to the name that the user passed in. Please don't take my comments the wrong way.  I am just trying to help clarify why things are the way they are right now and any suggestions on what should be happening can be discussed on the development list so that a patch can be contributed to improve upon the current code unless you would like to make a code contribution. By: Michael L. Young (elguero) 2015-02-17 14:57:22.653-0600 Our comments crossed paths. Here is the full log from IRC so that the context is all there: {quote} 15:22 < X-Rob> Hmm 15:22 < X-Rob> OK, I'd like to appeal this -- https://issues.asterisk.org/jira/browse/ASTERISK-24798 15:22 < X-Rob> I don't think that MixMonitor should be changing the filename given to it. 15:22 < X-Rob> Just because 'it's always done it' doesn't make it right. 15:23 <@mjordan> I don't disagree with that 15:23 <@mjordan> there may be some issues though with Asterisk figuring out what format you meant 15:23 <@mjordan> and if you don't have that format loaded, it will probably have to fail out 15:23 < X-Rob> Nope, it's saving in the correct format 15:23 < X-Rob> it's just changing the filename 15:23 <@mjordan> right, but that's because it is using the extension that it knows for the format. Let's say that's a bad idea (which I don't disagree with) 15:24 <@mjordan> however, let's say you don't have res_format_wav49 loaded 15:24 <@mjordan> er 15:24 <@mjordan> format_wav49 15:24 <@mjordan> whatever its name is :-P 15:24 <@mjordan> what should Asterisk do? 15:24 < X-Rob> but wav49 -is- valid for that format 15:24 <@mjordan> X-Rob: Pretend you don't have the actual module loaded in Asterisk 15:24 <@mjordan> I'm thinking about beyond your specific case :-) 15:24 < X-Rob> Then mixmon should crash, or, possibly save in slin or something? 15:25 <@mjordan> I'd say that we would have to bail out of recording 15:25 < X-Rob> I think bailing would be better. 15:25 <@mjordan> if we trust the user to hand us something, and what they handed us is invalid, we have to fail 15:25 < X-Rob> Then if you checked MIXMONITOR_FILENAME it would be blank {quote} By: xrobau (xrobau) 2015-02-17 15:06:21.303-0600 After further discussions on IRC, I'm in your camp now.   So, this is a documentation issue, but, with a minor secondary bug that MIXMONITOR_FILENAME should be correct, rather than just handing back what it was given to start with. By: Friendly Automation (friendly-automation) 2020-02-20 09:26:05.165-0600 Change 13808 merged by George Joseph: app_mixmonitor: Set MIXMONITOR_FILENAME to correct value when wav49 is used [https://gerrit.asterisk.org/c/asterisk/+/13808|https://gerrit.asterisk.org/c/asterisk/+/13808] By: Friendly Automation (friendly-automation) 2020-02-20 09:26:30.471-0600 Change 13799 merged by George Joseph: app_mixmonitor: Set MIXMONITOR_FILENAME to correct value when wav49 is used [https://gerrit.asterisk.org/c/asterisk/+/13799|https://gerrit.asterisk.org/c/asterisk/+/13799] By: Friendly Automation (friendly-automation) 2020-02-20 10:51:53.448-0600 Change 13800 merged by George Joseph: app_mixmonitor: Set MIXMONITOR_FILENAME to correct value when wav49 is used [https://gerrit.asterisk.org/c/asterisk/+/13800|https://gerrit.asterisk.org/c/asterisk/+/13800] By: Friendly Automation (friendly-automation) 2020-02-20 10:52:12.302-0600 Change 13798 merged by George Joseph: app_mixmonitor: Set MIXMONITOR_FILENAME to correct value when wav49 is used [https://gerrit.asterisk.org/c/asterisk/+/13798|https://gerrit.asterisk.org/c/asterisk/+/13798]