app_record: Add RECORD_TIME output variable.

[InterLinked1]

This adds the RECORD_TIME variable to Record(),
which is set to the recording duration before
the application returns.

Resolves: #548

[InterLinked1]

cherry-pick-to: none

[jcolp]

A user note should be added to the commit message about the addition of this dialplan variable.

[seanbright]

This could clobber an existing variable. Maybe it should be master only?

[jcolp]

This could clobber an existing variable. Maybe it should be master only?

Ooh, excellent point I hadn't considered. That or turning it into a dialplan function which can pull info about the recording just done.

[gtjoseph]

Should be a UserNote (new feature) not UpgradeNote (possible breaking change).

[InterLinked1]

Should be a UserNote (new feature) not UpgradeNote (possible breaking change).

It was requested this be master only because it "could" be breaking, that's why I removed the cherry picks and made it an upgrade note. Should it still be a user note?

[jcolp]

Should be a UserNote (new feature) not UpgradeNote (possible breaking change).

It was requested this be master only because it "could" be breaking, that's why I removed the cherry picks and made it an upgrade note. Should it still be a user note?

Since there could be a conflict in variable usage, I think UpgradeNote is appropriate.

[seanbright]

Is there another way this information could be made available without creating a channel variable? If it was a dialplan function as @jcolp mentioned you could make other information available too (recording format, file size, etc.) and that could be easily extendable in the future.

Edit: And then it would be a candidate for cherry picking which would be good,

Edit edit: Although I suppose a dialplan function have the same naming collision problem. I'm not sure if you can have a variable and a dialplan function with the same name.

[asteriskteam]

This PR has been marked stale because it has been in "Changes Requested" or "submitter-action-required" state for 28 days or more. Please make the requested changes within 14 days or the PR will be closed.

[InterLinked1]

Is there another way this information could be made available without creating a channel variable? If it was a dialplan function as @jcolp mentioned you could make other information available too (recording format, file size, etc.) and that could be easily extendable in the future.

Edit: And then it would be a candidate for cherry picking which would be good,

Edit edit: Although I suppose a dialplan function have the same naming collision problem. I'm not sure if you can have a variable and a dialplan function with the same name.

Yeah, aside from that, a function to get the time feels kind of disjoint (at least to me), a variable is more consistent with the other things that are saved. Making it a function would probably also mean now a datastore is now needed to keep track of that, which seems like overkill for just exposing one piece of information. A function could be useful anyways but seems like it should be its own project to expose different information.

Personally I don't really feel like changing this if it's only a suggestion, but if you feel this should absolutely be a function, then I can do so.

[seanbright]

Edit edit: Although I suppose a dialplan function have the same naming collision problem. I'm not sure if you can have a variable and a dialplan function with the same name.

I just tested this and you can have a variable and a function with the same name - there is no conflict:

same =     n,NoOp(${CURL})
same =     n,Set(CURL=hello-${EXTEN})
same =     n,NoOp(${CURL})
same =     n,Hangup()

Personally I don't really feel like changing this if it's only a suggestion, but if you feel this should absolutely be a function, then I can do so.

This PR is not waiting on my review as far as I can tell.

My opinion is that new functionality should not create channel variables that could trample those set by the user. It's a suggestion.

[jcolp]

I agree regarding not using dialplan variables. Older things use dialplan variables because at the time there was no other way. For inclusion I will require it to be a dialplan function.

[gtjoseph]

UpgradeNotes should be for changes in existing behavior a user should be aware of. This should be a UserNote.
Nevermind. But, the upgrade note should explain the possible conflict.

[asteriskteam]

This PR has been marked stale because it has been in "Changes Requested" or "submitter-action-required" state for 28 days or more. Please make the requested changes within 14 days or the PR will be closed.

N/A

[github-actions]

Successfully merged to branch master.

[seanbright]

Um. What?

[jcolp]

@gtjoseph Yes... what?

[jcolp]

When did I actually approve this... why... looks back

[jcolp]

Oh, I never unapproved when we had our discussion. That explains it. I'm reverting this and reopening.