Re: [asterisk-dev] Asterisk Beacon Module Proposal
On Mon, Jun 8, 2015 at 9:57 AM, Leif Madsen leif.mad...@avoxi.com wrote: On Mon, Jun 8, 2015 at 9:35 AM, Russell Bryant russ...@russellbryant.net wrote: On Tue, Jun 2, 2015 at 8:05 PM, Matthew Jordan mjor...@digium.com wrote: Personally, I'd like some way to present any user of Asterisk with a one-time only, non-annoying please help the project and opt in question, and then move on forever. Unfortunately, I don't have a good idea on making this suggestion work. If the only way to opt in is to provide a .conf file and set enable = true, then I can't see anywhere near sufficient numbers of people being aware of the configuration choice, much less making the choice to enable it, to justify the creation of the module itself. If someone has a good proposal on making the suggestion work, then I'd love to entertain it further. I feel that opt-out is fundamentally wrong from a privacy perspective. Further, I think the potential backlash and resulting damage to the project is pretty severe. I also don't think opt-in is hard is an acceptable reason not to do it. If it's too hard to make an opt-in solution useful, then maybe it shouldn't be done at all. This sort of thing really doesn't seem very common, and this is probably a big reason why. One alternative would be to issue periodic user surveys that are promoted on the mailing lists, twitter, etc. I don't think you'll ever get a reliable absolute numbers measure. A survey could still produce useful relative numbers and help identify some trends over time. First, I think the idea of a quarterly survey makes a lot of sense. You would probably get a bunch of useful information in one go rather than a slow trickle of information. You could also make this something people do on the website when downloading Asterisk from there. Other projects I've seen this on (Bower for example), do it during the installation process. The way I would picture this working with the Makefile is that you provide a prompt with a Y/N selection asking if you want to opt-in. If someone wants to automate this process, provide a flag option that allows them to --opt-in or --opt-out in order to provide a selection while also skipping human input. Then you can override that compiled in default with the below suggestion. As Matt pointed out, not everyone installs from source. I'd say the vast majority do not. I think any packager that chose to compile Asterisk with this stuff enabled by default should be flamed. It's just plain wrong. Around the configuration file approach (which I think is also useful for those wanting to override the default compile time option, which will be selected during SOME sort of compile time process, whether that be on the machine, or during the package creation process), I would expect it to be a file provided via the 'make samples' option. Then on the console when you connect, you could provide output that looks similar to the following: $ asterisk -r Asterisk 1.8.11-cert9, Copyright (C) 1999 - 2012 Digium, Inc. and others. Created by Mark Spencer marks...@digium.com Asterisk comes with ABSOLUTELY NO WARRANTY; type 'core show warranty' for details. This is free software, with components licensed under the GNU General Public License version 2 and other licenses; you are welcome to redistribute it under certain conditions. Type 'core show license' for details. = Connected to Asterisk 1.8.11-cert9 currently running on localhost (pid = 4364) Verbosity is at least 5 Asterisk Beacon: Enabled (see core show help beacon) *CLI With a separate configuration file and module, you could then provide the following option: beacon set opt in beacon set opt out This would then write out to the configuration beacon.conf with enabled=yes or no and reload the module. So, all of this is about making opt-in easier. That's fine, but folks will have to decide if it's still useful enough to do the work and run the server side infrastructure. I suspect not, personally. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk Beacon Module Proposal
On Tue, Jun 2, 2015 at 8:05 PM, Matthew Jordan mjor...@digium.com wrote: Suggestion 1: Make the module opt in instead of opt out. In most cases, I would normally agree with this proposal, as I personally prefer an opt in model instead of an opt out model. Unfortunately, I'm not sure I can think of a way where this suggestion would work with Asterisk - where work implies that a meaningful number of people are presented with the option to opt in in a consistent fashion. Unlike a number of other projects, Asterisk does not have: * A consistent way in which it is downloaded or deployed. Asterisk is built from source; obtained from upstream distro packages; from the project packages; from ISO distributions; and more. All of these can determine what is presented during the process, and the Asterisk project itself has little control over those mechanisms. I think this is generally true for virtually every open source project. * A UI that can present a user with a single, one-time only option to 'opt in'. This is in contrast to say Operating Systems or anything running with any kind of installation UI. Even operating systems don't have a consistent deployment method. There's lots of ways to do it and the installation UI isn't used for much beyond laptops and one-off test systems. Production systems are generally some sort of automated install or done with pre-built images. Anyway, my point is that I don't think Asterisk is terribly unique here, either. Personally, I'd like some way to present any user of Asterisk with a one-time only, non-annoying please help the project and opt in question, and then move on forever. Unfortunately, I don't have a good idea on making this suggestion work. If the only way to opt in is to provide a .conf file and set enable = true, then I can't see anywhere near sufficient numbers of people being aware of the configuration choice, much less making the choice to enable it, to justify the creation of the module itself. If someone has a good proposal on making the suggestion work, then I'd love to entertain it further. I feel that opt-out is fundamentally wrong from a privacy perspective. Further, I think the potential backlash and resulting damage to the project is pretty severe. I also don't think opt-in is hard is an acceptable reason not to do it. If it's too hard to make an opt-in solution useful, then maybe it shouldn't be done at all. This sort of thing really doesn't seem very common, and this is probably a big reason why. One alternative would be to issue periodic user surveys that are promoted on the mailing lists, twitter, etc. I don't think you'll ever get a reliable absolute numbers measure. A survey could still produce useful relative numbers and help identify some trends over time. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk Beacon Module Proposal
On Mon, May 11, 2015 at 1:31 PM, Jeffrey Ollie j...@ocjtech.us wrote: On Thu, May 7, 2015 at 9:35 PM, Matthew Jordan mjor...@digium.com wrote: * The module must provide the ability for users to opt out of gathering statistics. I believe that this must be opt *in*, not opt out. Yes, you'll get fewer servers reporting data but it's the much friendlier thing to with respect to privacy. +1. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Team branches and gerrit
On Mon, Apr 27, 2015 at 8:20 AM, Olle E. Johansson o...@edvina.net wrote:
On 24 Apr 2015, at 15:42, Russell Bryant russ...@russellbryant.net
wrote:
On Fri, Apr 24, 2015 at 8:31 AM, Joshua Colp jc...@digium.com wrote:
Olle E. Johansson wrote:
Playing around following Matt's wiki page on gerrit usage, I created
a team branch and did two commits. When pushing it with git review
{branch} only the last commit shows up.
Is that the way it's supposed to be? I thought the whole branch was
the review subject, not just a single commit.
Gerrit works on a single commit (what it refers to as a patch set) that
you want included into a specific branch. As a result you need to squash
all commits into a single one, and if review feedback warrants further
changes they also need to be squashed back into a single commit with the
original changes. The single commit you post for review is what is reviewed
and merged into the branch.
Gerrit can also work on a patch series, and tracks dependencies between
those patches.
Define patch series - how do you commit a series of patches for review?
In some cases, it makes sense for a feature or fix to be proposed as
multiple changes in a series. I feel that it makes things easier to review
and understand.
As a completely realistic example, let's assume we want to add support for
toaster control to chan_pjsip. That could be submitted as a series of 3
commits:
1 - ast_toaster: Add core modular API for toaster control.
2 - toastip - Add ast_toaster backend for the toastip protocol.
3 - pjsip_toast - Add chan_pjsip integration with the ast_toaster API.
Those *could* all be one commit, but it's really the development of a
feature in 3 logical chunks, so breaking it up like this is an alternative.
When it comes to what that actually looks like it git.
Create a local branch for a feature:
$ cd asterisk-git
$ git checkout -b example-patch-series origin/master
Commit 1:
$ echo some text foo
$ git add foo
$ git commit -m Add foo
Commit 2:
$ echo some more text foo
$ git commit -a -m Add more text to foo
We now have 2 commits on top of the master branch:
$ git log --oneline origin/master..HEAD
5a53b4f Add more text to foo.
4bf86fa Add foo.
Post series of 2 commits for review:
$ git review
You are about to submit multiple commits. This is expected if you are
submitting a commit that is dependent on one or more in-review
commits. Otherwise you should consider squashing your changes into one
commit before submitting.
The outstanding commits are:
5a53b4f (HEAD, example-patch-series) Add more text to foo.
4bf86fa Add foo.
Do you really want to submit the above commits?
Type 'yes' to confirm, other to cancel:
(You would type 'yes' here because you're submitting 2 changes for review
intentionally.)
Here's a wiki page that I think does a nice job laying out some reasoning
behind best practices for breaking up changes into a series of commits if
anyone is interested:
https://wiki.openstack.org/wiki/GitCommitMessages
--
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Team branches and gerrit
On Fri, Apr 24, 2015 at 8:31 AM, Joshua Colp jc...@digium.com wrote:
Olle E. Johansson wrote:
Playing around following Matt's wiki page on gerrit usage, I created
a team branch and did two commits. When pushing it with git review
{branch} only the last commit shows up.
Is that the way it's supposed to be? I thought the whole branch was
the review subject, not just a single commit.
Gerrit works on a single commit (what it refers to as a patch set) that
you want included into a specific branch. As a result you need to squash
all commits into a single one, and if review feedback warrants further
changes they also need to be squashed back into a single commit with the
original changes. The single commit you post for review is what is reviewed
and merged into the branch.
Gerrit can also work on a patch series, and tracks dependencies between
those patches.
--
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Team branches and gerrit
On Fri, Apr 24, 2015 at 12:17 PM, Matthew Jordan mjor...@digium.com wrote:
On Fri, Apr 24, 2015 at 8:42 AM, Russell Bryant
russ...@russellbryant.net wrote:
On Fri, Apr 24, 2015 at 8:31 AM, Joshua Colp jc...@digium.com wrote:
Olle E. Johansson wrote:
Playing around following Matt's wiki page on gerrit usage, I created
a team branch and did two commits. When pushing it with git review
{branch} only the last commit shows up.
Is that the way it's supposed to be? I thought the whole branch was
the review subject, not just a single commit.
Gerrit works on a single commit (what it refers to as a patch set) that
you want included into a specific branch. As a result you need to
squash all
commits into a single one, and if review feedback warrants further
changes
they also need to be squashed back into a single commit with the
original
changes. The single commit you post for review is what is reviewed and
merged into the branch.
Gerrit can also work on a patch series, and tracks dependencies between
those patches.
I could be mistaken, but in the case where you have a series of
patches, each patch is put up for review separately with its own
Change-Id, correct?
That's right, but it should also track that they are related and in a
particular order.
https://gerrit.asterisk.org/#/c/255
https://gerrit.asterisk.org/#/c/256
See the Related Changes bit. It shows the patches in order.
--
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] A Week with GIT/Gerrit
On Thu, Apr 16, 2015 at 6:00 PM, George Joseph george.jos...@fairview5.com wrote: Something I've noticed though is that dependencies get messed up when cherry-picking a series of dependent patches. Look at my patches 46, 47, 48 for master. If I cherry-pick 48 to my local repo, I get all 3 which is good because I can't test any without all 3. BUT now look at 43, 44, 45 for 13. When I check out 45, I get only 45 which is an issue. Maybe this is just an artifact of this being the first time with this scenario but it's something to look out for. This is likely because for 13, the changes were cherry picked individually as changes against HEAD of 13. They need to be set up as 3 commits in a series on top of 13 and submitted that way. Backporting in that case will need to be done locally to preserve those dependencies instead of clicking the cherry-pick buttons. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk 1.8/12 build system changes
On Tue, Apr 14, 2015 at 1:37 PM, Matthew Jordan mjor...@digium.com wrote: Hello! As you know, 1.8 and 12 are in security fix only mode. Generally, that would normally mean that releases made from those branches would only ever be made from the previous tag, ensuring that the only changes in the versions are specific to the security issues being resolved. However, since we've moved to Git, there are a few things that needed to be merged into those branches for them to be useful. That includes a minimum amount of Git support (.gitignore files, for example). More generally, that also included menuselect, as those branches previously relied on svn:externals to pull menuselect in. That begs the question: the next time we make a release of 1.8/12 for security reasons, what is the release? And what should it be called? I can see a few different options: * The next release is 1.8.32.4/12.8.3, but is made from the branch and contains the menuselect/git changes. * We make a special, one-time only, bug fix release of 1.8/12 containing the menuselect/git changes, with a note of why it was made. Those releases would go out as 1.8.33.0 and 12.9.0. * We don't make a special bug-fix release, but instead the next security release is made as 1.8.33.0/12.9.0 and contains both the menuselect/Git changes as well as the security release changes. Note that in any of the cases mentioned above, the UPGRADE notes will clearly state what has changed in the version, including the menuselect alterations. Thoughts? Suggestions? Flames? The .gitignore and .gitreview files seem like a non-issue. They're hidden files that should be ignored by anyone consuming the release. The menuselect import also seems like a non-issue from a release perspective. While it's very different from a source management perspective, the actual release deliverable shouldn't look any different. menuselect is in the tarball just like it was before. Or am I missing something? -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Subjects for e-mails
On Tue, Apr 14, 2015 at 8:47 AM, Matthew Jordan mjor...@digium.com wrote: On Tue, Apr 14, 2015 at 2:15 AM, Olle E. Johansson o...@edvina.net wrote: Can we possibly have different Subject: lines in e-mails for a commit and for a review. Seems like everything is a change in asterisk now. I would like to be able to prioritize reading the commits. I took a look at the Gerrit settings, and it doesn't appear to support that: https://gerrit-review.googlesource.com/Documentation/user-notify.html and: https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#sendemail It does support having commits go to a separate list than reviews; that should already be set up for the asterisk-commits list (although the last tweaks on it just went in yesterday). With gerrit it's actually pretty easy to subscribe to the messages you want. You can get notified for new reviews, when patches are updated, when patches merge, or when people make comments (all optionally on a per repository basis). So, another option would be to drop sending to the list completely and let people configure their own notifications that suit their needs and interests. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration update
On Mon, Apr 13, 2015 at 6:52 PM, Matthew Jordan mjor...@digium.com wrote: For *right now*, we are going to try cherry-picking the changes to the affected branches when the change is first up for review. This is clearly a pretty big change in process, as the act of merging into other branches was (a) always done by those with commit access and (b) never reviewed. There's at least two good reasons to give this a try: (1) There isn't anyone with commit access. Anyone can post a review up to Gerrit. The plus side is that there are far fewer barriers to getting a patch into Asterisk. The downside is that there isn't a select group of people who have been trusted to do the merges. The only way to ensure that patches actually do get merged into all the branches is to require people to put the patches up with the initial review. (2) Gerrit really, really wants to review things. That's a good thing: we've had plenty of bad merges take out branches in the past - either from compilation issues, subtle bugs that creep in due to API compatibility problems, etc. We've had even more that get merged upstream and fail to take advantage of APIs that exist in later versions of Asterisk. Reviews will help to catch that. This is a trial. We'll re-evaluate how things are working at the end of the week. Instructions for cherry-picking changes are on a draft wiki page here: https://wiki.asterisk.org/wiki/display/AST/Git+Usage One thing that helps tracking backports is using the same Change-Id for the same patch applied to multiple branches. You can use that Id to check the status of a single patch across all branches. For example, I saw this patch that's proposed for both master and 13: https://gerrit.asterisk.org/#/q/Id0ce0528e58014da1324856ea537e7765466044a -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Subjects for e-mails
On Tue, Apr 14, 2015 at 10:55 AM, Matthew Jordan mjor...@digium.com wrote: On Tue, Apr 14, 2015 at 9:43 AM, Dan Jenkins dan.jenkin...@gmail.com wrote: On Tue, Apr 14, 2015 at 3:18 PM, Russell Bryant russ...@russellbryant.net wrote: On Tue, Apr 14, 2015 at 8:47 AM, Matthew Jordan mjor...@digium.com wrote: On Tue, Apr 14, 2015 at 2:15 AM, Olle E. Johansson o...@edvina.net wrote: Can we possibly have different Subject: lines in e-mails for a commit and for a review. Seems like everything is a change in asterisk now. I would like to be able to prioritize reading the commits. I took a look at the Gerrit settings, and it doesn't appear to support that: https://gerrit-review.googlesource.com/Documentation/user-notify.html and: https://gerrit-review.googlesource.com/Documentation/config-gerrit.html#sendemail It does support having commits go to a separate list than reviews; that should already be set up for the asterisk-commits list (although the last tweaks on it just went in yesterday). With gerrit it's actually pretty easy to subscribe to the messages you want. You can get notified for new reviews, when patches are updated, when patches merge, or when people make comments (all optionally on a per repository basis). So, another option would be to drop sending to the list completely and let people configure their own notifications that suit their needs and interests. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev I like this idea Russell Given that the e-mails are now only going to the asterisk-commits and asterisk-code-review lists - and not the asterisk-dev list - I think a combination of both options is probably okay. * If you want to see the spam, subscribe to the lists * If you want personalized e-mail notifications, set it up through Gerrit Yep, makes sense. I missed the update about notifications only going to the new list before posting the above. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Change in asterisk[master]: Add .gitignore and .gitreview files
Russell Bryant has posted comments on this change. Change subject: Add .gitignore and .gitreview files .. Patch Set 2: (1 comment) https://gerrit.asterisk.org/#/c/42/2/.gitignore File .gitignore: Line 21: addons/mp3 I personally think it's nicer to use .gitignore files in subdirectories. Some concrete reasons it helps: if you rename or move directories, the ignore rules follow automatically and you end up with less conflicts with patches updating the same file. -- To view, visit https://gerrit.asterisk.org/42 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69 Gerrit-PatchSet: 2 Gerrit-Project: asterisk Gerrit-Branch: master Gerrit-Owner: George Joseph george.jos...@fairview5.com Gerrit-Reviewer: Corey Farrell g...@cfware.com Gerrit-Reviewer: Matt Jordan mjor...@digium.com Gerrit-Reviewer: Russell Bryant russ...@russellbryant.net Gerrit-HasComments: Yes -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Change in asterisk[master]: Add .gitignore and .gitreview files
Russell Bryant has posted comments on this change. Change subject: Add .gitignore and .gitreview files .. Patch Set 2: (1 comment) https://gerrit.asterisk.org/#/c/42/2//COMMIT_MSG Commit Message: Line 15: Tested-by: George Joseph I think it would be good to move to using the same format for other git headers (include the full name and email address) -- To view, visit https://gerrit.asterisk.org/42 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69 Gerrit-PatchSet: 2 Gerrit-Project: asterisk Gerrit-Branch: master Gerrit-Owner: George Joseph george.jos...@fairview5.com Gerrit-Reviewer: Corey Farrell g...@cfware.com Gerrit-Reviewer: Matt Jordan mjor...@digium.com Gerrit-Reviewer: Russell Bryant russ...@russellbryant.net Gerrit-HasComments: Yes -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Change in asterisk[master]: Add .gitignore and .gitreview files
Russell Bryant has posted comments on this change. Change subject: Add .gitignore and .gitreview files .. Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.asterisk.org/42 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69 Gerrit-PatchSet: 3 Gerrit-Project: asterisk Gerrit-Branch: master Gerrit-Owner: George Joseph george.jos...@fairview5.com Gerrit-Reviewer: Corey Farrell g...@cfware.com Gerrit-Reviewer: George Joseph george.jos...@fairview5.com Gerrit-Reviewer: Matt Jordan mjor...@digium.com Gerrit-Reviewer: Russell Bryant russ...@russellbryant.net Gerrit-HasComments: No -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk to Git Migration Update
On Tue, Mar 31, 2015 at 2:32 PM, Matthew Jordan mjor...@digium.com wrote: Hey everyone: Hi! Just as a brief status update on the Asterisk Git Migration project, here is where we stand today, and what the next steps are: Awesome work so far. :-) There's a large amount of intervening work that has to be done, and some questions that still need to be answered. In no particular order: (1) In test runs of building the Asterisk repo, I've successfully managed to pull in the team repos that are still 'active'. Since Gerrit acts as the canonical Git repo, we can set it up so that direct pushes to team branches are allowed and don't require a code review - although code reviews can be done if someone desires it. For those who use team branches a lot, does this sound acceptable? What's the benefit of trying to keep team branches at all vs. just letting people use their own git trees hosted on one of the many personal git hosting options (github or whatever) ? If it's about several people collaborating on a branch, that makes sense to me to do as a feature branch in gerrit, but it seems like the exception, not the rule. (2) Today, we merge up the branches - 11 = 13, 13 = trunk, etc. After the move, it looks like the best way to handle the merge process is going to be to make patches against trunk, then cherry-pick them back to the currently maintained branches. For long time users of Gerrit, does this seem appropriate? Yes. (3) In Asterisk 13, we pulled in menuselect (yay). On the downside, Asterisk 11 doesn't have menuselect. What's more, Asterisk 1.8 and 12 are still in security fix mode. We really have two options here: (a) Update the build scripts to pull in menuselect as they do now from SVN, and more or less keep the existing process. My fear is that this may turn into a bit of hackery to keep the Git/SVN lines from getting confused. (b) Bite the bullet and just backport menuselect into all the branches. Moving to Git is going to be a breaking change for people using Asterisk 11 anyway, and this way things end up in a reasonably pristine state. Thoughts? (b) sounds the least painful overall to me. People consuming releases shouldn't notice a difference, right? (4) Asterisk records the SVN revision in each file using the special keyword $Revision:. This is then registered in a linked list for retrieval by the CLI/AMI. Unfortunately, Git doesn't support this concept, as adding data into a file after commit would change the checksum . It does allow doing this on checkout via the $Id$ keyword, which may be an acceptable workaround. That whole thing seems questionably useful, anyway. Just removing it is another option. Anyway, the goal is to kick off the move of Asterisk to Git immediately after we get the next batch of releases out. I'll send out an e-mail once we know exactly when that is. \o/ -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Change in testsuite[master]: Add .gitignore files
Russell Bryant has posted comments on this change. Change subject: Add .gitignore files .. Patch Set 1: Code-Review+1 looks sane -- To view, visit https://gerrit.asterisk.org/8 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibedc56ac80bcb1981b66a6b514cf817f7423f6ec Gerrit-PatchSet: 1 Gerrit-Project: testsuite Gerrit-Branch: master Gerrit-Owner: Matt Jordan mjor...@digium.com Gerrit-Reviewer: Russell Bryant russ...@russellbryant.net Gerrit-HasComments: No -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Change in testsuite[master]: Add a .gitreview file for the testsuite
Russell Bryant has posted comments on this change. Change subject: Add a .gitreview file for the testsuite .. Patch Set 1: Code-Review+1 (1 comment) https://gerrit.asterisk.org/#/c/7/1/.gitreview File .gitreview: Line 5: micro-nit: you have an extra blank line :) -- To view, visit https://gerrit.asterisk.org/7 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34162d2d66cb4f80b918c85fdb55304c43aadb28 Gerrit-PatchSet: 1 Gerrit-Project: testsuite Gerrit-Branch: master Gerrit-Owner: Matt Jordan mjor...@digium.com Gerrit-Reviewer: Matt Jordan mjor...@digium.com Gerrit-Reviewer: Russell Bryant russ...@russellbryant.net Gerrit-Reviewer: Samuel Galarneau sgalarn...@digium.com Gerrit-HasComments: Yes -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Change in testsuite[master]: Add a .gitreview file
Russell Bryant has posted comments on this change. Change subject: Add a .gitreview file .. Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.asterisk.org/5 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I057f1cb490c6467637b68f235936a96d2a6a5a49 Gerrit-PatchSet: 1 Gerrit-Project: testsuite Gerrit-Branch: master Gerrit-Owner: Matt Jordan mjor...@digium.com Gerrit-Reviewer: Russell Bryant russ...@russellbryant.net Gerrit-HasComments: No -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk Testsuite: Moving to Git
On Mon, Mar 23, 2015 at 6:20 PM, Matthew Jordan mjor...@digium.com wrote: [2] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage git-review can set up the commit hook, as well. Just run git review -s. Note that for git-review to know where your gerrit is, each repo will need to have a .gitreview file added [1]. [1] https://gerrit.asterisk.org/#/c/3 -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Change in repotools[master]: Add .gitreview file
Russell Bryant has uploaded a new change for review. https://gerrit.asterisk.org/3 Change subject: Add .gitreview file .. Add .gitreview file This file tells 'git review' how to contact the gerrit server. Change-Id: I4f78512b9b82001086916f3d33059a0704881de9 --- A .gitreview 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.asterisk.org:29418/repotools refs/changes/03/3/1 -- To view, visit https://gerrit.asterisk.org/3 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4f78512b9b82001086916f3d33059a0704881de9 Gerrit-PatchSet: 1 Gerrit-Project: repotools Gerrit-Branch: master Gerrit-Owner: Russell Bryant russ...@russellbryant.net -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Change in repotools[master]: .gitignore: Add a .gitignore file that mirrors the previous ...
Russell Bryant has posted comments on this change. Change subject: .gitignore: Add a .gitignore file that mirrors the previous svn:ignore .. Patch Set 2: You can create .gitignore files in subdirectories instead of listing sub-directory ignore rules in the top level file. I suppose it's a matter of preference, though. -- To view, visit https://gerrit.asterisk.org/2 To unsubscribe, visit https://gerrit.asterisk.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbbafde95752e31af573502e8cac4aebac69e825 Gerrit-PatchSet: 2 Gerrit-Project: repotools Gerrit-Branch: master Gerrit-Owner: Matt Jordan mjor...@digium.com Gerrit-Reviewer: Corey Farrell g...@cfware.com Gerrit-Reviewer: Russell Bryant russ...@russellbryant.net Gerrit-Reviewer: Samuel Galarneau sgalarn...@digium.com Gerrit-HasComments: No -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] How to catch the source of a deadlock?
On Tue, Jan 27, 2015 at 7:01 AM, Yousf Ateya y.at...@starkbits.com wrote: Dear, While investigating iax2 bug ASTERISK-24478 https://issues.asterisk.org/jira/browse/ASTERISK-24478?focusedCommentId=224584page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-224584; I found some dead locks. Which happens very often in my setup. I tried to debug the problem to know why this deadlock happens; but each time I got lost in huge logs and many scheduler/astobj2 details. Is there any efficient way to debug deadlocks in asterisk? Have you seen the DEBUG_THREADS compile time option and the core show locks CLI command? https://wiki.asterisk.org/wiki/display/AST/CLI+commands+useful+for+debugging -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] git migration update
On Wed, Dec 24, 2014 at 5:06 PM, Olle E. Johansson o...@edvina.net wrote: You are missing one thing. When committing to the current team branches, the code is contributed under the license agreement. The code in my branches is available for Digium to use at any point in time. If I had to have it in my own GIT or github it would be very different. Certainly feature branches could be created in the main repo on a case by case basis as it makes sense. I think the main point is that in a git world, people have no use for pushing their dev branches into the main repo. It's just a different model. I don't see why anyone would use them for day-to-day work like team branches have been used in the past. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] git migration update
On Mon, Dec 22, 2014 at 3:08 PM, George Joseph george.jos...@fairview5.com
wrote:
On Mon, Dec 22, 2014 at 12:03 PM, Samuel Galarneau sgalarn...@digium.com
wrote:
2 - we have a few options as far as team branches go. We could configure
user branches using refs/heads/team/${username}/* permissions in Gerrit to
allow users to create branches. This would prohibit other users from
pushing to a user branch, but they would still be visible. This would most
likely involve reproducing some sort of automerge/autorebase process. The
other option is to use github as another remote for team branches, with a
remote pointing to Gerrit for code reviews. Is there a preference between
these two approaches, or perhaps a better setup we could follow?
I don't think there's any need for you to host users' repos any more. It
may have made sense for SVN but I don't think it does for GIT. Let users
make their own arrangements be it GitHub or in my case, my own GIT
infrastructure.
+1. I don't think it makes sense with git. github or whatever should work
just fine for that purpose.
--
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] AstriDevCon 2014: Agenda item Deprecate AMI/AGI (Ben Klang)
On Tue, Oct 28, 2014 at 11:24 AM, Phil Mickelson p...@cbasoftware.com wrote: So. After all this I'm tired of hearing the same argument. I've decided to take the other side. I have seen the progression of Asterisk for the last 5+ years. I was at Astricon when it was held in the DC area and the last one in Atlanta (I wish you'd go back there instead of Vegas!). I tried to use Asterisk many times but always found it wanting. Why? I'm a programmer. I like to be able to make tools do what I want. So, I left Asterisk and moved on to FreeSWITCH. After beating my head against the FS wall for quite some time I was just about ready to give up completely. Until I saw the videos from Astricon 2013 talking about ARI. I knew immediately this is what I wanted. Over the last several months I have written a full answering service system using ARI. It is PERFECT! Would I like a few more features in ARI? Of course. I'm I blown away by how complete the new options are?. Without ARI I could've never have done what I did. Should they drop the dialplan? Please. Now. I can't believe this isn't holding back options. You really want to stick with the dialplan? Really? Fine. Do it. You have all the source code. Apparently, you use V1.4 anyway. What difference is it to you? Be honest. This has nothing to do with the Asterisk Community. It has to do with you. What you want. There are many of us who are very thankful they have moved forward. Both ARI and PJSIP. The old SIP problems were many and awful. If all you want is a standard PBX you've got it. What I want is something I can work with without having to program in C again which I gave up many, many years ago. I have no interest in going back. So, from my POV and hopefully many who will discover Asterisk again (I'm talking to you FreeSWITCHers!) ARI (and PJSIP) are the real thing. This is a very exciting time and I'm looking forward to many applications that I will be able to create. Thank you Digium and all the Asterisk developers for going in this direction. I will certainly be at Astricon next year (even if it's in LV) and you will hear me support moving forward as quickly as possible. I LOVE this post. Thanks, Phil. Well said. I said this in my talk last week and will reiterate it here. I think the Asterisk dev community has been doing an amazing job over the last few years. Internal refactorings have been achieved that we used to dream of. A new SIP channel driver finally exists. The new API work just totally rocks. It's an absolutely critical piece of what is needed from Asterisk to fit in to the way future telephony applications should be architected in my view. Well done, dev community. Keep kicking ass. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk 11.9.0 Segmentation fault.
On Tue, Oct 21, 2014 at 3:17 AM, 為近 吉摩（情報システム本部）- Tamechika Yoshikiyo - yoshikiyo.tamech...@g.softbank.co.jp wrote: (gdb) bt full #0 0x003e94230265 in raise () from /lib64/libc.so.6 No symbol table info available. #1 0x2aaab22946b2 in skgesigOSCrash () from /usr/local/lib/libclntsh.so.11.1 No symbol table info available. #2 0x2aaab2532705 in kpeDbgSignalHandler () from /usr/local/lib/libclntsh.so.11.1 No symbol table info available. #3 0x2aaab22948c2 in skgesig_sigactionHandler () from /usr/local/lib/libclntsh.so.11.1 No symbol table info available. #4 signal handler called No symbol table info available. #5 0x2aaaceab8af1 in stop_session_timer (p=0x2aab18b892d8) at chan_sip.c:29206 __PRETTY_FUNCTION__ = stop_session_timer Based on this backtrace: 1) It looks like the crash is down in an oracle client library (libclntsh) and not in Asterisk itself. 2) Based on this backtrace, it shows chan_sip calling into this client library, which doesn't exist in Asterisk code, so it could be a problem specific to modifications made in your version. -- Russell Bryant -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] asterisk-testsuite: linux-only
On Mon, Oct 13, 2014 at 1:58 AM, Tzafrir Cohen tzafrir.co...@xorcom.com wrote: Hi I finally got the asterisk-testsuite accpted into Debian (I know, I need to get it updated). A related question ... what's the value of having it packaged at all? I could see the value if the test suite was designed in such a way that people could run it against their own deployment to do some validation, but quite far from how it actually works. So, what use cases are you aiming to allow exactly? -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] License issue
On Mon, Oct 13, 2014 at 9:28 AM, Olle E. Johansson o...@edvina.net wrote: Hi! A customer showed interest in supporting the SIP GIN standard - registering for multiple contacts according to RFC 6140, part of the SIPconnect work. I remembered an old patch on reviewboard that has an issue in the bug tracker - #18705 The patch was never uploaded to the bug tracker, only to reviewboard. https://reviewboard.asterisk.org/r/1515 Does that mean that the code is licensed to Digium or not? It's clearly licensed under GPL 2, but if I take on this work I need to know whether to start from scratch or if I can use this code and still submit it at some point... Trying to get hold of the original author, but no reply so far. It should be fine. Users have never been able to use reviewboard without an active contributor license agreement. (AFAIK) -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] asterisk-testsuite: linux-only
On Mon, Oct 13, 2014 at 9:44 AM, Tzafrir Cohen tzafrir.co...@xorcom.com wrote: On Mon, Oct 13, 2014 at 09:18:10AM -0400, Russell Bryant wrote: On Mon, Oct 13, 2014 at 1:58 AM, Tzafrir Cohen tzafrir.co...@xorcom.com wrote: Hi I finally got the asterisk-testsuite accpted into Debian (I know, I need to get it updated). A related question ... what's the value of having it packaged at all? For the asterisk package to have tests that could be run by the Debian testing infrastructure. Ah, OK. That makes sense. Thanks. :-) -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Thu, Sep 18, 2014 at 12:29 PM, Russell Bryant russ...@russellbryant.net wrote: On Thu, Sep 18, 2014 at 11:31 AM, Samuel Galarneau sgalarn...@digium.com wrote: On Tue, Sep 16, 2014 at 5:01 PM, Russell Bryant russ...@russellbryant.net wrote: On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan mjor...@digium.com wrote: And there was much rejoicing \o/ But seriously, we all know that a lot of people have wanted to move to Git for some time. For the record, everyone at Digium has wanted to move the project to Git for some time. I swore to myself that we wouldn't do another Standard release on Subversion - after we spent at least six weeks mucking around with merge conflicts during Asterisk 12 - and with Asterisk 14 looming ever closer, the time is now to start getting something done on this. ... -- Team repos I'd recommend just using your own account on github or whatever. ... -- Process Recommendation I discussed this a good bit above, but I'm happy to answer questions. -- Russell Bryant Russell, how does Gerrit deal with submitting reviews? Are all reviews simply topic branches on the repository that Gerrit hosts? Perhaps a real demonstration of workflow would help. I'll use a recent trivial fix that I did. This is a one-liner patch that needed to go into master as well as two stable branches. I headed over to my local git tree and created a branch to do the fix. $ cd openstack/nova $ git checkout -b bug/1370191 origin/master ... Hack code and commit the fix ... $ git commit -a Now I have a single patch on top of upstream master that I want to submit for review. $ git review This created https://review.openstack.org/#/c/121940/ What's actually happening is a git push to gerrit. A git repo in gerrit maintains all revisions of all patches. You can actually fetch the patch and look at it locally in your tree. A couple more comments about the magic happening here ... First, git review knows where to push based on a file checked in to the repo: $ cat .gitreview [gerrit] host=review.openstack.org port=29418 project=openstack/nova.git git review also sets up a local commit hook that adds a Change-Id header to your commit message. That Change-Id is what links multiple revisions of the same change together. So, if you edit your change and push it again, as long as the Change-Id remains the same, gerrit treats it as the same review request and not a new one. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Thu, Sep 18, 2014 at 3:01 PM, Samuel Galarneau sgalarn...@digium.com wrote: A couple more comments about the magic happening here ... First, git review knows where to push based on a file checked in to the repo: $ cat .gitreview [gerrit] host=review.openstack.org port=29418 project=openstack/nova.git git review also sets up a local commit hook that adds a Change-Id header to your commit message. That Change-Id is what links multiple revisions of the same change together. So, if you edit your change and push it again, as long as the Change-Id remains the same, gerrit treats it as the same review request and not a new one. Sounds good to me. So it doesn't really matter from which repo you post a review so long as it's a clone of the original with that .gitreview file. I have another question unrelated to reviews. Does your setup make it easy to mirror a repo? In a more complicated scenario, what if someone had a private fork but they wanted to get public commits to master mirrored to their repo? Would they have to treat the original repo as upstream and manually pull changes and rebase their private branch off of it? Mirroring, sure. I don't remember exactly how we do it, but all OpenStack repos are mirrored to github for convenience, for example. Regarding private branches, git generally makes that kind of thing ***MUCH*** easier than svn. You can easily track the exact commits that are applied on top of upstream. You could either periodically rebase your changes on top of upstream (re-applying the commits, rewriting history but a cleaner history), or periodically merge from upstream. In either case, I personally wouldn't automate it. You want some sanity checking around that stuff. Conflicts happen. Really, I think this is going to be MUCH better no matter what specific infrastructure you go with. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Tue, Sep 16, 2014 at 10:52 PM, Matthew Jordan mjor...@digium.com wrote: On Tue, Sep 16, 2014 at 5:01 PM, Russell Bryant russ...@russellbryant.net wrote: On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan mjor...@digium.com wrote: So, to set what I hope are a few guidelines: (1) I know this is a subject with a lot of opinions, and coming to a concurrence on something that is opinion based is tough. Voice opinions after thinking, would I be willing to spend time working on this? (2) This is a project that can easily suck several developers for a long time if the scope of it expands substantially. The scope should hopefully be kept as tight as possible, as we will have a lot of re-tooling to do once the git move occurs (all CI, code review, releasing, etc.) (3) The Asterisk project has several hard requirements, a few softer ones, that have to be met: * Code submitted has to be assigned to a CLA. We have to verify that people proposing patches have acknowledged that they were licensed to submit said patch in some fashion. * Issue tracking has to be done in JIRA (large investment, huge cost in moving to anything else right now). * Code must be reviewed. * Any substitute for Review Board, Bamboo, or other existing tools should have obvious benefits over the existing tools. Very nice set of guidelines. For the last few years, most of my time has been going into OpenStack [1]. We use git and I have become a big fan of our workflow and infrastructure. It's all open source and reusable. From a high level, all patches go to a code review system. *Every* patch must be peer reviewed (usually by 2 people, but that's a policy decision). *Every* patch must also pass tests. Once a patch passes both tests and peer review, it is automatically merged into the repository. I *love* that workflow for several reasons. If it's appealing, it's probably much easier to do it now while you're doing a big switch anyway. If you're not sold, I'm certainly not hurt. Like I said, I just wanted to offer info. The current plan will be less up front setup for sure. I am a huge fan of this. We've been moving (slowly but surely) towards this model. Code is reviewed more often than not, and with Crowd integration, anyone can propose a patch. Tests are written for the vast majority of new work. We *do* still have a problem with bouncing tests - and while Bamboo may point the finger more at 12+ currently (due to a current spat with native RTP bridges, direct media, and transfers) - the bouncing is actually more pernicious in many ways with the 1.8/11 branches. We'd have to solve that problem if we move to this model. I don't consider that a bad thing at all. OpenStack struggles with this problem, as well. It can be incredibly painful. If a test is just failing some of the time, you get developers confused and frustrated that seemingly unrelated failures are blocking their patch. Worse, you may get a mess of a backlog in the CI system as things have to get re-run. We keep a dashboard of current bugs being hit sorted by frequency so we know what the biggest offenders are. http://status.openstack.org/elastic-recheck/ (Note that running this is another set of infrastructure that I didn't mention, but it's interesting to see I think.) There are things you can do to mitigate the impact while working to improve things over time. You can mark some CI jobs as non-voting, meaning that the job runs and you see the results on the patch, but it doesn't affect the overall pass/fail result. If you split up tests into some subsets, you can have some jobs voting and some non-voting. If you're a hands on kind of person, browse http://review.openstack.org/ for open code reviews. You can also see patches going through CI pipelines on http://status.openstack.org/zuul// The major tools involved are: - gerrit for code review and repository management [2][3] - jenkins for CI [4] - Zuul, A CI job scheduler that automates running things in response to events on gerrit. [5] - CGit, repo hosting [6][7] This would replace Review Board and Bamboo as well, so we'll need to consider the effort involved with that. Yes, for sure. I think it's worthwhile long term, and it's at least largely automated and proven, so that helps. Everything we use is managed via puppet and all of the configuration is in git. It's designed to be reusable. The folks that run it have documented how to re-use it [8] and are quite friendly. You can find them in #openstack-infra on freenode. Awesome. I'll do that - in particular, I'm interested what is involved in the recurring maintenance of gerrit/Jenkins/Zuul. On IRC, jeblair is the infrastructure project lead. He's also an Asterisk fan, so that helps. :-) Depending on the depth of the question(s), it may be better to use the openstack-infra mailing list: http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack
Re: [asterisk-dev] Git Migration
reviews done (avg 700 per day). That gives some sense of scale. From a quick glance at the CI status page (http://status.openstack.org/zuul/), it has been launching about 500-600 jobs per hour today. -- Process Recommendation I discussed this a good bit above, but I'm happy to answer questions. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Tue, Sep 16, 2014 at 6:01 PM, Russell Bryant russ...@russellbryant.net wrote: From a high level, all patches go to a code review system. *Every* patch must be peer reviewed (usually by 2 people, but that's a policy decision). *Every* patch must also pass tests. Once a patch passes both tests and peer review, it is automatically merged into the repository. I just thought of another important bit of the workflow ... the CLA handling. With Asterisk today, all patches go through the issue tracker. The issue tracker handles the CLA. Uploading code to the issue tracker bypasses that, so we had to hack reviewboard to also know about CLAs. OpenStack uses a CLA, as well, and gerrit has built-in CLA handling. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Git Migration
On Tue, Sep 16, 2014 at 6:12 PM, Russell Bryant russ...@russellbryant.net wrote: On Tue, Sep 16, 2014 at 6:01 PM, Russell Bryant russ...@russellbryant.net wrote: From a high level, all patches go to a code review system. *Every* patch must be peer reviewed (usually by 2 people, but that's a policy decision). *Every* patch must also pass tests. Once a patch passes both tests and peer review, it is automatically merged into the repository. I just thought of another important bit of the workflow ... the CLA handling. With Asterisk today, all patches go through the issue tracker. The issue tracker handles the CLA. Uploading code to the issue tracker bypasses that, so we had to hack reviewboard to also know about CLAs. OpenStack uses a CLA, as well, and gerrit has built-in CLA handling. Some more workflow comments, sorry... and then maybe I'll shut up. :-) One thing I really like about gerrit vs review board is that gerrit is focused on git and as a result, has more native git integration. Posting code reviews is just git review from your git tree. git review is really just a helper around a normal git push. You can push a patch series to gerrit and gerrit understands what that is and tracks the patch dependencies. Last I checked, review board still lacked any sort of support for a series of patches related to each other. Also, if you're really attached to doing code reviews in a console and maybe even offline, someone in the OpenStack community made gertty [1], which is a replacement for using the web UI. It's gerrit, but entirely synced locally and in a terminal. I've used it for several hours while offline on an airplane and it's pretty darn amazing. It syncs all the reviews you did back to gerrit once you're back online. [1] http://lists.openstack.org/pipermail/openstack-dev/2014-September/045013.html -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk Beginner learning about Channels
On Wed, Aug 27, 2014 at 4:32 PM, Maiquel Breitenbach maiquel.breitenb...@gmail.com wrote: Hi, I was already here asking about the building of new channel, I use as example chan_alsa, the new channel is work, the write function record a voice in file, but i am having problems with function read, asterisk doesn't call the read function, can someone explain how the asterisk works to call the read function of a channel? I appreciate very much your help. Most channel drivers, including chan_alsa, set a file descriptor on the channel that will be polled. When that fd indicates reading is possible, the read() callback will be called. Search for ast_channel_set_fd() in chan_alsa.c, for example. It's also possible to do reading in your own way in another thread and queue frames to the channel using ast_queue_frame(). chan_iax2 does this. In that case, the read calback isn't used. chan_iax2 has to do it that way as all data for *every* call is arriving on the same UDP socket, so it can't put that one socket fd on every channel. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3952: Add 'rtpbindaddr' setting for chan_sip
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3952/#review13195 --- Ship it! looks sane to me - Russell Bryant On Aug. 27, 2014, 3:44 p.m., Paul Belanger wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3952/ --- (Updated Aug. 27, 2014, 3:44 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Users now have the ability to bind the rtpengine instance to a specific IP address. For example, you want chan_sip (call control) on eth0 but rtp (media) on eth1. Diffs - trunk/configs/samples/sip.conf.sample 422198 trunk/channels/chan_sip.c 422198 trunk/CHANGES 422198 Diff: https://reviewboard.asterisk.org/r/3952/diff/ Testing --- kamailio proxy with rtpengine. Multihomed asterisk system. Thanks, Paul Belanger -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Does asterisk video frame contains video data only or there is RTP headers too ?
On Mon, Aug 25, 2014 at 3:20 AM, Shishir Pokharel shishir.pokha...@on24.com wrote: Hi list, I am developing an application to convert different video format to H.264 and transport it via RTMP protocol to FMS server. I am bit confused about asterisk Video Frame , my confusion is does the Video frame has the RTP headers on it or the just the video data? Going through the asterisk code I figured out the frame structure (ast_frame) has frame- datalen (length of data) frame-data.ptr(pointer to actual data). Does this data contains only video data’s or there is RTP headers information’s too ? Just the video data. For sending, The frames sometimes also have an optimization, frame-offset, which is how many bytes are available ahead of frame-data.ptr. That lets you write headers and use the frame's buffer directly for a socket send instead of having to copy the data to another buffer for every frame you're sending. Good luck with your work! -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] loader.c in asterisk 1.8.15-cert5
On Wed, Aug 13, 2014 at 3:41 AM, Deepak Bhatia dee...@voxomos.com wrote: Hello, We have an application voicefun.so, which we used to load without any issue in asterisk 1.6. But we are facing problem in the loader.c in the asterisk 1.8.15-cert5. Could you share some more detail on the problem you're having? Without more detail, nobody will be able to help you. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Bounty] Stasis bug/finishing implementation - $1, 000
Heh, but you just posted your own patch. Are you still interested in help here? -- Russell Bryant On Wed, Jul 16, 2014 at 11:23 PM, Krandon krandon.br...@gmail.com wrote: Up'ing bounty to $1,000 -- KB On Wednesday, July 16, 2014 at 5:56 AM, Krandon wrote: ARI /continue fails to actually continue into the dialplan. https://issues.asterisk.org/jira/browse/ASTERISK-24043 Mailing list thread here (with some tips): http://lists.digium.com/pipermail/asterisk-app-dev/2014-July/000507.html $500 bounty - let's get this party going! Thanks, -- KB -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3424: (mix)monitor: Add options to enable a periodic beep
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 14, 2014, 7:51 p.m.) Review request for Asterisk Developers. Changes --- Included changes for adding the option to Monitor() as well (was originally just MixMonitor()) Summary (updated) - (mix)monitor: Add options to enable a periodic beep Repository: Asterisk Description (updated) --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. The option is provided for both Monitor() and MixMonitor(). Diffs (updated) - /trunk/res/res_monitor.c 412327 /trunk/include/asterisk/monitor.h 412327 /trunk/include/asterisk/beep.h PRE-CREATION /trunk/funcs/func_periodic_hook.c 412327 /trunk/bridges/bridge_builtin_features.c 412327 /trunk/apps/app_queue.c 412327 /trunk/apps/app_mixmonitor.c 412327 /trunk/CHANGES 412327 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
On April 10, 2014, 3:35 p.m., Mark Michelson wrote: /trunk/apps/app_mixmonitor.c, line 1067 https://reviewboard.asterisk.org/r/3424/diff/2/?file=57155#file57155line1067 Suggestion: Instead of doing ast_module_check(), use ast_custom_function_find() to find the PERIODIC_HOOK function. I have a couple of reasons for this: 1) Currently, if func_periodic_hook.so is loaded, it also means that PERIODIC_HOOK is registered, but that assumption may not always hold. 2) More importantly, getting the ast_custom_function allows for you to bump the module refcount for func_periodic_hook.so so that the module cannot be unloaded while there is an active mixmonitor. You can decrement the module refcount when the mixmonitor is destroyed. I started looking at this, but I'm not sure the alternative is better. They both have downsides. ast_custom_function_find() may be worse. Re: #1, you're right, but ... Re: #2, the module ref count is now always incremented on func_periodic_hook.so when PERIODIC_HOOK() is active. Also, it looks like ast_custom_function_find() isn't safe. There is a race condition between finding a function and incrementing the module ref count where the module could be unloaded and cause a crash. At least with ast_module_check(), ast_func_read() may fail, but it's handled gracefully without crashing. - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11562 --- On April 8, 2014, 7:49 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 8, 2014, 7:49 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs - /trunk/main/app.c 412023 /trunk/include/asterisk/app.h 412023 /trunk/funcs/func_periodic_hook.c 412023 /trunk/apps/app_mixmonitor.c 412023 /trunk/CHANGES 412023 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 11, 2014, 10:32 p.m.) Review request for Asterisk Developers. Changes --- Updated to use optional_api Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs (updated) - /trunk/include/asterisk/beep.h PRE-CREATION /trunk/funcs/func_periodic_hook.c 412280 /trunk/apps/app_mixmonitor.c 412278 /trunk/CHANGES 412278 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
On April 10, 2014, 3:35 p.m., Mark Michelson wrote: /trunk/apps/app_mixmonitor.c, line 1067 https://reviewboard.asterisk.org/r/3424/diff/2/?file=57155#file57155line1067 Suggestion: Instead of doing ast_module_check(), use ast_custom_function_find() to find the PERIODIC_HOOK function. I have a couple of reasons for this: 1) Currently, if func_periodic_hook.so is loaded, it also means that PERIODIC_HOOK is registered, but that assumption may not always hold. 2) More importantly, getting the ast_custom_function allows for you to bump the module refcount for func_periodic_hook.so so that the module cannot be unloaded while there is an active mixmonitor. You can decrement the module refcount when the mixmonitor is destroyed. Russell Bryant wrote: I started looking at this, but I'm not sure the alternative is better. They both have downsides. ast_custom_function_find() may be worse. Re: #1, you're right, but ... Re: #2, the module ref count is now always incremented on func_periodic_hook.so when PERIODIC_HOOK() is active. Also, it looks like ast_custom_function_find() isn't safe. There is a race condition between finding a function and incrementing the module ref count where the module could be unloaded and cause a crash. At least with ast_module_check(), ast_func_read() may fail, but it's handled gracefully without crashing. And actually, converting this over to optional_api removed the module check anyway - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11562 --- On April 11, 2014, 10:32 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 11, 2014, 10:32 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs - /trunk/include/asterisk/beep.h PRE-CREATION /trunk/funcs/func_periodic_hook.c 412280 /trunk/apps/app_mixmonitor.c 412278 /trunk/CHANGES 412278 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3429: monitor: use app options parsing helper code
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3429/ --- (Updated April 10, 2014, 8:13 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 412102 Repository: Asterisk Description --- This app is pretty ancient, so it was never converted to use the option parsing helper code. I'd like to add an option to this app that takes an argument, and that's a pain to do when not using this helper, so start by doing this conversion. Diffs - /trunk/res/res_monitor.c 412023 Diff: https://reviewboard.asterisk.org/r/3429/diff/ Testing --- Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
On April 10, 2014, 4:21 p.m., Corey Farrell wrote: /trunk/apps/app_mixmonitor.c, line 1070 https://reviewboard.asterisk.org/r/3424/diff/2/?file=57155#file57155line1070 Mark's suggestion to use ast_custom_function_find made me think about module references. We don't keep a reference to func_periodic_hook for the channels that have hooks. I missed this in the original review, I guess I expected the channel's datastore and/or the audiohook to reference the module, but they don't appear to. Not sure if you want to add the fix to this review or if we need to do it separate. Valid point, though I think it's a logically separate change from this, so I'll do it separately. - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11564 --- On April 8, 2014, 7:49 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 8, 2014, 7:49 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs - /trunk/main/app.c 412023 /trunk/include/asterisk/app.h 412023 /trunk/funcs/func_periodic_hook.c 412023 /trunk/apps/app_mixmonitor.c 412023 /trunk/CHANGES 412023 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
On April 8, 2014, 1:49 p.m., Corey Farrell wrote: /trunk/main/app.c, line 3109 https://reviewboard.asterisk.org/r/3424/diff/1/?file=57128#file57128line3109 Can we create the context from func_periodic_hook? My complaint is that for anyone not using func_periodic_hook this context is just wasted memory. Another way to deal with this would be for ast_app_get_beep_* to call init_beep_helper and track if we've created the extensions or not. In any case please ensure the internal dialplan is removed at shutdown. Corey Farrell wrote: I was actually hoping that the code in init_beep_helper would move to func_periodic_hook. This is more of a nit, you've addressed my major concern which is to lifetime of the beep context. On the topic of what code goes where, almost all the code could be moved to optional_api functions in func_periodic_hook. One function to start and one to stop beeping on a channel. This would have a few benefits: * De-duplication of code to be used by res_monitor and app_mixmonitor. * No code added to asterisk core. * Beep dial-plan becomes a private implementation detail, and can use the func_periodic_hook context. Good suggestions, thanks! I'll work on this some more. - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11515 --- On April 8, 2014, 7:49 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 8, 2014, 7:49 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs - /trunk/main/app.c 412023 /trunk/include/asterisk/app.h 412023 /trunk/funcs/func_periodic_hook.c 412023 /trunk/apps/app_mixmonitor.c 412023 /trunk/CHANGES 412023 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3423: Internal timing: Add notice message about the -I and internal_timing option no longer needed.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3423/#review11511 --- Ship it! lgtm - Russell Bryant On April 8, 2014, 12:03 a.m., rmudgett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3423/ --- (Updated April 8, 2014, 12:03 a.m.) Review request for Asterisk Developers. Bugs: ASTERISK-22846 https://issues.asterisk.org/jira/browse/ASTERISK-22846 Repository: Asterisk Description --- Add notice messages during execution that the -I command line option and the astersik.conf internal_timing option are no longer needed. The internal timing functionality is now always enabled if there is a timing module loaded. NOTE: Since the command line options and the asterisk.conf config file are processed before the logging system is enabled, the messages are output to stderr. Diffs - /branches/1.8/main/asterisk.c 411882 Diff: https://reviewboard.asterisk.org/r/3423/diff/ Testing --- Messages are displayed. Thanks, rmudgett -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
On April 8, 2014, 5:49 p.m., Corey Farrell wrote: Does this module need to be tagged with optional dependency func_periodic_hook? I forgot about use for menuselect. Unfortunately, it doesn't seem to work for module dependencies. It turns it into the same thing as depend for another module. - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/#review11515 --- On April 8, 2014, 1:28 a.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 8, 2014, 1:28 a.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs - /trunk/main/app.c 411908 /trunk/include/asterisk/app.h 411908 /trunk/apps/app_mixmonitor.c 411908 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- (Updated April 8, 2014, 11:49 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs (updated) - /trunk/main/app.c 412023 /trunk/include/asterisk/app.h 412023 /trunk/funcs/func_periodic_hook.c 412023 /trunk/apps/app_mixmonitor.c 412023 /trunk/CHANGES 412023 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3300: Don't crash on lack of bridged rtp instance
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3300/ --- (Updated April 8, 2014, 7:59 p.m.) Status -- This change has been discarded. Review request for Asterisk Developers and leifmadsen. Bugs: ASTERISK-23419 https://issues.asterisk.org/jira/browse/ASTERISK-23419 Repository: Asterisk Description --- This bug shows a crash in bridge_p2p_rtp_write(). There is no bridged rtp instance so it goes boom. The patch just catches the case and returns. I'm not really sure *why* there's no bridged instance, but this seems like a pretty safe function input sanity check. Diffs - /tags/1.8.18.1/res/res_rtp_asterisk.c 405155 Diff: https://reviewboard.asterisk.org/r/3300/diff/ Testing --- Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 2684: Fix exposure of template-only config sections
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2684/
---
(Updated April 8, 2014, 8:04 p.m.)
Status
--
This change has been discarded.
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
While working on a deployment, I had to change the [directories] section of
asterisk.conf from the defaults. That worked. Later I noticed that the
directories section was defined as a template-only section like so:
[directories](!)
which means my changes should *not* have taken effect. This one line change
fixes that.
As a side note, while looking at this, I noticed multiple cases of comparing
against a category name like this throughout the file, which seems wrong:
from ast_category_delete()
if (cat-name == category) {
from ast_variable_browse()
if (config-last_browse (config-last_browse-name == category)) {
etc.
Diffs
-
/trunk/main/config.c 394685
Diff: https://reviewboard.asterisk.org/r/2684/diff/
Testing
---
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 2684: Fix exposure of template-only config sections
On Aug. 5, 2013, 10:45 a.m., wdoekes wrote:
Can't reproduce.
My asterisk-trunk skips [directories](!).
I broke a path, and it starts only when I added (!), which leads me to
believe that it doesn't read it then.
Yeah, I can't reproduce this anymore. *shrug*
I'll just close for now
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2684/#review9325
---
On July 18, 2013, 9:44 a.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2684/
---
(Updated July 18, 2013, 9:44 a.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
While working on a deployment, I had to change the [directories] section of
asterisk.conf from the defaults. That worked. Later I noticed that the
directories section was defined as a template-only section like so:
[directories](!)
which means my changes should *not* have taken effect. This one line change
fixes that.
As a side note, while looking at this, I noticed multiple cases of comparing
against a category name like this throughout the file, which seems wrong:
from ast_category_delete()
if (cat-name == category) {
from ast_variable_browse()
if (config-last_browse (config-last_browse-name == category)) {
etc.
Diffs
-
/trunk/main/config.c 394685
Diff: https://reviewboard.asterisk.org/r/2684/diff/
Testing
---
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3426: Fix build failure on SmartOS / Illumos / SunOS
On April 8, 2014, 6:31 p.m., Tzafrir Cohen wrote: I know I really shouldn't be writing this, as I added one of the defined() in that line, but: shouldn't this test be done in autoconf? Ideally, yes, but seems like a reasonable improvement that could be made later, beyond the scope of this immediate fix - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3426/#review11520 --- On April 8, 2014, 3:31 p.m., Sebastian Wiedenroth wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3426/ --- (Updated April 8, 2014, 3:31 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23576 https://issues.asterisk.org/jira/browse/ASTERISK-23576 Repository: Asterisk Description --- this fixes two issues when building on SmartOS: - channels/chan_oss.c: it makes sure soundcard.h is found - main/Makefile: only use -Wl,--version-script when GNU LD is used as the Sun Linker doesn't support that. Similar checks are already used elswhere in the Makefile Diffs - /branches/11/main/Makefile 411655 /branches/11/channels/chan_oss.c 411655 Diff: https://reviewboard.asterisk.org/r/3426/diff/ Testing --- With these modifications Asterisk builds fine when using pkgsrc-2014Q1 on SmartOS Thanks, Sebastian Wiedenroth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3426: Fix build failure on SmartOS / Illumos / SunOS
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3426/#review11524 --- Ship it! Looks sane ... and at least shouldn't break primary platforms - Russell Bryant On April 8, 2014, 3:31 p.m., Sebastian Wiedenroth wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3426/ --- (Updated April 8, 2014, 3:31 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23576 https://issues.asterisk.org/jira/browse/ASTERISK-23576 Repository: Asterisk Description --- this fixes two issues when building on SmartOS: - channels/chan_oss.c: it makes sure soundcard.h is found - main/Makefile: only use -Wl,--version-script when GNU LD is used as the Sun Linker doesn't support that. Similar checks are already used elswhere in the Makefile Diffs - /branches/11/main/Makefile 411655 /branches/11/channels/chan_oss.c 411655 Diff: https://reviewboard.asterisk.org/r/3426/diff/ Testing --- With these modifications Asterisk builds fine when using pkgsrc-2014Q1 on SmartOS Thanks, Sebastian Wiedenroth -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3429: monitor: use app options parsing helper code
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3429/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- This app is pretty ancient, so it was never converted to use the option parsing helper code. I'd like to add an option to this app that takes an argument, and that's a pain to do when not using this helper, so start by doing this conversion. Diffs - /trunk/res/res_monitor.c 412023 Diff: https://reviewboard.asterisk.org/r/3429/diff/ Testing --- Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3424: mixmonitor: Add option to enable a periodic beep
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3424/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Add an option to enable a periodic beep to be played into a call if it is being recorded. If enabled, it uses the PERIODIC_HOOK() function internally to play the 'beep' prompt into the call at a specified interval. Diffs - /trunk/main/app.c 411908 /trunk/include/asterisk/app.h 411908 /trunk/apps/app_mixmonitor.c 411908 Diff: https://reviewboard.asterisk.org/r/3424/diff/ Testing --- exten = 103,1,Answer() same = n,MixMonitor(test.gsm,B(5)) same = n,MusicOnHold() exten = 104,1,Answer() same = n,MixMonitor(test.gsm,B) same = n,MusicOnHold() exten = 105,1,Answer() same = n,MixMonitor(test.gsm,B(3)) same = n,StartMusicOnHold() same = n,Wait(15) same = n,StopMusicOnHold() same = n,StopMixMonitor() same = n,Wait(5) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 5, 2014, 8:06 a.m.)
Status
--
This change has been marked as submitted.
Review request for Asterisk Developers.
Changes
---
Committed in revision 411768
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411684
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [svn-commits] rmudgett: branch 1.8 r411715 - in /branches/1.8: ./ channels/ configs/ includ...
On Sat, Apr 5, 2014 at 4:46 PM, Paul Belanger paul.belan...@polybeacon.comwrote: On Sat, Apr 5, 2014 at 2:58 AM, Olle E. Johansson o...@edvina.net wrote: On 04 Apr 2014, at 20:32, SVN commits to the Digium repositories svn-comm...@lists.digium.com wrote: - case 'I': - ast_set_flag(ast_options, AST_OPT_FLAG_INTERNAL_TIMING); - break; Just checking... I would rather add a NOTICE log here that i is not needed any more. Please make sure that configurations starting with - i will not suddenly fail. I agree with Olle here, this seems to be a massive change mid-release. Removing a command-line option is certainly going to break some peoples boxes. Why not a deprecated warning and then removal from trunk to give people time to react? FWIW, specifying this command line option or asterisk.conf option, even after it has been removed, should be fine. It will just be ignored and no new warnings will be generated, AFAICT. On the surface, this looks like a change that shouldn't be made in a release branch. However, this really is an option that should have never existed. It's never the right thing to turn it off. The change to make it the only way it works is really the right thing to do. It was equivalent to an option called make_things_work_properly=yes. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 4, 2014, 3:42 p.m.)
Review request for Asterisk Developers.
Changes
---
updated based on reviews and re-tested
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411684
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On April 2, 2014, 7:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 48-57
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line48
Document the (On write) hook_id.
Is this the first function that has different parameters for read and
write?
I don't know of another one off hand that has different args.
On April 2, 2014, 7:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 85-87
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line85
These should not be uppercase. That indicates they are macros.
Hrm, I think of UPPERCASE() as a macro and UPPERCASE as a constant. That's why
I did it this way.
On April 2, 2014, 7:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 301-302
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line301
You need to check if args.interval is NULL. Also the sscanf should be
using %30u instead. Also passing a NULL string pointer to printf will
crash on Solaris.
Missing \n
Ah, I thought args.interval would be guaranteed to at least be an empty string.
That doesn't seem to be the case.
On April 2, 2014, 7:44 p.m., rmudgett wrote:
/trunk/funcs/func_periodic_hook.c, lines 384-386
https://reviewboard.asterisk.org/r/3362/diff/8/?file=56955#file56955line384
Maybe use ast_true() and ast_false() instead.
Done
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11476
---
On April 1, 2014, 4:43 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 1, 2014, 4:43 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411583
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 4, 2014, 12:05 a.m.)
Review request for Asterisk Developers.
Changes
---
Thanks for the reviews!
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411684
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk Test Suite Git Migration
On Wed, Apr 2, 2014 at 10:34 AM, Kevin Harwell kharw...@digium.com wrote: Hello everyone, Moving Asterisk to git has been talked about for some time now. However, before doing that we thought it might be easier to move the Asterisk test suite over first. The current plan is to keep everything as is, as far as the structure and layout of the test suite, within a single git repository. This should make the migration much simpler. See the following for more information: https://wiki.asterisk.org/wiki/display/AST/Asterisk+Test+Suite+Git +Migration Comments/thoughts always welcome and more information to come once we begin the actual migration. +1! :-) -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Russell Bryant wrote:
Awesome. That sounds like the API call needed here.
Well ... it's not *that* straight forward. GoSub (or Macro) is being used as
one side of an originated call. I'd have to add a new application to make it
work. GoSubInternal (for internal use only) or something.
Any preference between just using Macro vs adding a new GoSubInternal app?
I know Macro is considered deprecated but it does work fine here. Besides,
Macro has been around *so* long (10+ years?), removing it would just be evil.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Russell Bryant wrote:
Awesome. That sounds like the API call needed here.
Russell Bryant wrote:
Well ... it's not *that* straight forward. GoSub (or Macro) is being
used as one side of an originated call. I'd have to add a new application to
make it work. GoSubInternal (for internal use only) or something.
Any preference between just using Macro vs adding a new GoSubInternal app?
I know Macro is considered deprecated but it does work fine here.
Besides, Macro has been around *so* long (10+ years?), removing it would just
be evil.
rmudgett wrote:
Macro has been removed from being a requirement of Asterisk core and is
deprecated. It needs to go away. It sounds like you are originating to an
application which is a Macro. Why not just originate to an exten since that
seems to be what Macro is doing for you.
It's originating with dialplan on both ends of the call. Macro is used on one
end as a way to run dialplan, but also pass arguments (variables) to that
dialplan. Earlier revs of the patch originated to an extension directly, but
with that I couldn't pass variables to that dialplan (channel name and hook ID
right now).
BTW, somewhat unrelated, I think removing Macro() is a terrible idea. I'd be
all for re-implementing it using GoSub internally, but removing a syntax that
has been supported for 10+ years, breaking countless examples on the internet,
does not seem worth it.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Russell Bryant wrote:
Awesome. That sounds like the API call needed here.
Russell Bryant wrote:
Well ... it's not *that* straight forward. GoSub (or Macro) is being
used as one side of an originated call. I'd have to add a new application to
make it work. GoSubInternal (for internal use only) or something.
Any preference between just using Macro vs adding a new GoSubInternal app?
I know Macro is considered deprecated but it does work fine here.
Besides, Macro has been around *so* long (10+ years?), removing it would just
be evil.
rmudgett wrote:
Macro has been removed from being a requirement of Asterisk core and is
deprecated. It needs to go away. It sounds like you are originating to an
application which is a Macro. Why not just originate to an exten since that
seems to be what Macro is doing for you.
Russell Bryant wrote:
It's originating with dialplan on both ends of the call. Macro is used
on one end as a way to run dialplan, but also pass arguments (variables) to
that dialplan. Earlier revs of the patch originated to an extension
directly, but with that I couldn't pass variables to that dialplan (channel
name and hook ID right now).
BTW, somewhat unrelated, I think removing Macro() is a terrible idea.
I'd be all for re-implementing it using GoSub internally, but removing a
syntax that has been supported for 10+ years, breaking countless examples on
the internet, does not seem worth it.
Actually ... maybe this should work without using GoSub or Macro? I'm passing
variables in already via the originate API call, but I was only expecting them
to be available to the dialplan running behind the Local channel for some
reason. Maybe they're available on both sides of the Local channel? I'll have
to test it out.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Russell Bryant wrote:
Awesome. That sounds like the API call needed here.
Russell Bryant wrote:
Well ... it's not *that* straight forward. GoSub (or Macro) is being
used as one side of an originated call. I'd have to add a new application to
make it work. GoSubInternal (for internal use only) or something.
Any preference between just using Macro vs adding a new GoSubInternal app?
I know Macro is considered deprecated but it does work fine here.
Besides, Macro has been around *so* long (10+ years?), removing it would just
be evil.
rmudgett wrote:
Macro has been removed from being a requirement of Asterisk core and is
deprecated. It needs to go away. It sounds like you are originating to an
application which is a Macro. Why not just originate to an exten since that
seems to be what Macro is doing for you.
Russell Bryant wrote:
It's originating with dialplan on both ends of the call. Macro is used
on one end as a way to run dialplan, but also pass arguments (variables) to
that dialplan. Earlier revs of the patch originated to an extension
directly, but with that I couldn't pass variables to that dialplan (channel
name and hook ID right now).
BTW, somewhat unrelated, I think removing Macro() is a terrible idea.
I'd be all for re-implementing it using GoSub internally, but removing a
syntax that has been supported for 10+ years, breaking countless examples on
the internet, does not seem worth it.
Russell Bryant wrote:
Actually ... maybe this should work without using GoSub or Macro? I'm
passing variables in already via the originate API call, but I was only
expecting them to be available to the dialplan running behind the Local
channel for some reason. Maybe they're available on both sides of the Local
channel? I'll have to test it out.
Um, yes. It works. I can drop the usage of Macro() or GoSub() completely.
Oops.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated April 1, 2014, 4:43 p.m.)
Review request for Asterisk Developers.
Changes
---
Fixed to no longer use Macro() and just run the extension directly
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411583
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing (updated)
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Verbose(1,Channel name: ${HOOK_CHANNEL})
same = n,Verbose(1,Hook ID: ${HOOK_ID})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
.
It's not a technical limitation of ChanSpy, more just built in sanity checking
/ rate limiting.
On March 30, 2014, 12:12 a.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 68-71
https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line68
Can we get a documented variable for the original channel name or
uniqueid? I'd prefer something defined by this module (so not SPY_CHANNEL).
Oops, this shouldn't have been using SPY_CHANNEL, as it's totally unrelated to
what ChanSpy uses SPY_CHANNEL for. I changed that.
As for setting a variable for the original channel in the hook, that's a good
idea. I suppose this should be reworked to exec GoSub with arguments instead
of run an extension directly. That's the only way I can see to pass variables
to the hook.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11441
---
On March 29, 2014, 8:27 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 29, 2014, 8:27 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 12:56 p.m.)
Review request for Asterisk Developers.
Changes
---
Updated to address comments.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 1:14 p.m.)
Review request for Asterisk Developers.
Changes
---
updated testing extensions
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing (updated)
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 30, 2014, 12:12 a.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, line 206
https://reviewboard.asterisk.org/r/3362/diff/4/?file=56930#file56930line206
state-last_hook is never pre-initialized to ast_tvnow(), so hooks
would always trigger on first check after being created or re-enabled.
Unless this behaviour is documented I would expect the first run after
create or resume to be delayed state-interval seconds.
Russell Bryant wrote:
Fair. I had originally intended for it to run immediately, but I think
it makes more sense to delay.
jamuel wrote:
So is the implication then if I were to set the interval to 30 seconds
that the hook wouldn't run for 30 seconds and then run every 30 seconds?
That seems more counter intuitive. I'd think that it should run and then
repeat every interval seconds.
Yes, that's right. I keep going back and forth but I think I agree with you
that running immediately makes the most sense.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11441
---
On March 31, 2014, 1:14 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 1:14 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 4:11 p.m.)
Review request for Asterisk Developers.
Changes
---
moved back to original behavior of running hook immediately, and then every
interval seconds.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 136-139
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line136
I'm unsure the purpose of beep in macro_arg.
Also think chan_name + hook_id can never be close to 1024 chars
(AST_CHANNEL_NAME is defined as 80).
Oops. It's supposed to be arg-macro_name. This was just a mistake from hard
coding something while testing.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
I tried that. GoSub() doesn't actually work in this case. Perhaps because
there's no PBX running on the channel? In any case, that's why.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 4:11 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 4:11 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 p.m.)
Review request for Asterisk Developers.
Changes
---
Fixed left over hard coded macro name from testing. Re-tested to make sure
macro name passed in is used as expected.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
On March 31, 2014, 5:58 p.m., Corey Farrell wrote:
/trunk/funcs/func_periodic_hook.c, lines 141-143
https://reviewboard.asterisk.org/r/3362/diff/6/?file=56951#file56951line141
Macro is deprecated, why not use Gosub?
Russell Bryant wrote:
I tried that. GoSub() doesn't actually work in this case. Perhaps
because there's no PBX running on the channel? In any case, that's why.
rmudgett wrote:
I haven't checked, but would calling this be appropriate here to avoid
the adding the need for Macro back into the code:
int ast_app_exec_sub(struct ast_channel *autoservice_chan, struct
ast_channel *sub_chan, const char *sub_args, int ignore_hangup)
Awesome. That sounds like the API call needed here.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/#review11455
---
On March 31, 2014, 8:33 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 31, 2014, 8:33 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[macro-beep]
exten = s,1,Answer()
same = n,Verbose(1,Channel name: ${ARG1})
same = n,Verbose(1,Hook ID: ${ARG2})
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 29, 2014, 8:06 p.m.)
Review request for Asterisk Developers.
Changes
---
Update diff against CHANGES to current trunk
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
Playback+ChanSpy to accomplish the task.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 29, 2014, 8:11 p.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description (updated)
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_periodic_hook: New function for periodic hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3362/
---
(Updated March 29, 2014, 8:27 p.m.)
Review request for Asterisk Developers.
Changes
---
Added ability turn on the hook again later after it has been turned off.
Repository: Asterisk
Description
---
This commit introduces a new dialplan function, PERIODIC_HOOK().
It allows you run to a dialplan hook on a channel periodically. The
original use case that inspired this was the ability to play a beep
periodically into a call being recorded. The implementation is much
more generic though and could be used for many other things.
The implementation makes heavy use of existing Asterisk components.
It uses a combination of Local channels and ChanSpy() to run some
custom dialplan and inject any audio it generates into an active call.
The other important bit of the implementation is how it figures out
when to trigger the beep playback. This implementation uses the
audiohook API, even though it's not actually touching the audio in any
way. It's a convenient way to get a callback and check if it's time
to kick off another beep. It would be nice if this was timer event
based instead of polling based, but unfortunately I don't see a way to
do it that won't interfere with other things.
Diffs (updated)
-
/trunk/funcs/func_periodic_hook.c PRE-CREATION
/trunk/CHANGES 411572
Diff: https://reviewboard.asterisk.org/r/3362/diff/
Testing (updated)
---
Called the following extension (100@test), both letting it run all the way
through, as well as hanging up at various points in the middle.
[hooks]
exten = beep,1,Answer()
same = n,Playback(beep)
[test]
exten = 100,1,Answer()
same = n,Set(BEEP_ID=${PERIODIC_HOOK(hooks,beep,5)})
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=off)
same = n,Wait(20)
same = n,Set(PERIODIC_HOOK(${BEEP_ID})=on)
same = n,Wait(20)
same = n,Hangup()
Thanks,
Russell Bryant
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] g722 package for extras sounds went missing
Just FYI, I'm pretty sure there used to be an asterisk-sounds-extra-en-g722 package on packages.asterisk.org for centos. It seems to have gone missing. I noticed it due to a puppet error. g722 is still present for other sounds packages. http://packages.asterisk.org/centos/6/current/i386/RPMS/ -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Alembic in 12 SVN
On Mon, Mar 17, 2014 at 1:01 PM, Joshua Colp jc...@digium.com wrote: Matthew Jordan wrote: snip Technically, it's probably not hard. I wonder how horrible it'd be for end users, however. Probably pretty horrible, since it's basically changing everything. Well ... you completely drop support for anyone that has already run the scripts. How advertised / documented is it? It may be new enough that nobody is truly relying on it yet. That's my guess, honestly. You could probably provide some scripts to manually fix an existing database to deal with the change. That sounds like a pain and probably not worth it, though. If you want to rework it, I would just do it ASAP and cover it in release notes. If you go through this effort, I would also add an automated test that verifies that all migrations are linear to catch these problems ASAP in the future. You could do something like the existing validate-docs Makefile target that only runs if dev mode is on. Add a validate-linear-migrations target that runs in dev mode or something like that. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Alembic in 12 SVN
On Tue, Mar 18, 2014 at 10:07 AM, Matthew Jordan mjor...@digium.com wrote: On Tue, Mar 18, 2014 at 8:31 AM, Russell Bryant russ...@russellbryant.net wrote: On Mon, Mar 17, 2014 at 1:01 PM, Joshua Colp jc...@digium.com wrote: Matthew Jordan wrote: snip Technically, it's probably not hard. I wonder how horrible it'd be for end users, however. Probably pretty horrible, since it's basically changing everything. Well ... you completely drop support for anyone that has already run the scripts. How advertised / documented is it? It may be new enough that nobody is truly relying on it yet. That's my guess, honestly. You could probably provide some scripts to manually fix an existing database to deal with the change. That sounds like a pain and probably not worth it, though. If you want to rework it, I would just do it ASAP and cover it in release notes. If you go through this effort, I would also add an automated test that verifies that all migrations are linear to catch these problems ASAP in the future. You could do something like the existing validate-docs Makefile target that only runs if dev mode is on. Add a validate-linear-migrations target that runs in dev mode or something like that. I'd prefer to do it now and get it done. Better to suffer the pain now rather than during an LTS - but a quick e-mail to the -users list may be in order. Having a make target that tests this is a good idea - we already have something similar in mkrelease when it generates the SQL scripts for the tarballs, and we have a Bamboo plan that just tests make targets. I'll add that to the list. Back to the original issue though ... the problem was that there wasn't a way to catch non-linear migrations. That can be fixed with some quick validation. This also led to potentially wanting to split up migrations per module. The reason it was all together in the first place was based on the assumption that if someone was using realtime, they're likely going to want it for *all* modules. Also, having a separate database per module for just config seems pretty excessive, and honestly a bit of a pain. When doing installs, upgrades, and backups you will have to make a list of all of the different databases you need to mess with. After thinking about it more, it sounds like a worse user experience for just a little bit of developer convenience. -- Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3362: func_beep: New function for periodic beeps.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- This commit introduces a new dialplan function, BEEP(). It allows you to enable playing a beep tone at a regular interval into a call. The most common use case for this is to use during call recording to notify and remind the callers that the call is being recorded. A future commit will update the call recording applications in Asterisk to use this automatically if an option is specified for convenience. The implementation makes heavy use of existing Asterisk components. Instead of replicating logic required to load a sound file, transcode it if necessary, and do audio mixing, it makes use of Playback+ChanSpy to accomplish the task. The other important bit of the implementation is how it figures out when to trigger the beep playback. This implementation uses the audiohook API, even though it's not actually touching the audio in any way. It's a convenient way to get a callback and check if it's time to kick off another beep. It would be nice if this was timer event based instead of polling based, but unfortunately I don't see a way to do it that won't interfere with other things. Diffs - /trunk/funcs/func_beep.c PRE-CREATION /trunk/CHANGES 410649 Diff: https://reviewboard.asterisk.org/r/3362/diff/ Testing --- Called the following extension, both letting it run all the way through, as well as hanging up at various points in the middle. [test] exten = 100,1,Answer() same = n,Set(BEEP(5)=on) same = n,Wait(20) same = n,Set(BEEP()=off) same = n,Wait(20) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3362: func_beep: New function for periodic beeps.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/#review11230 --- So, after talking to pabelanger about it, this could be a bit more generic without a lot of effort. Right now it's hard coded to do a Playback(beep) into the call. It could pretty easily changed to just be a periodic dialplan hook. So you could have: [dialplan_hooks] exten = beep,1,Playback(beep) [something] ; Run the beep hook for this call every 30 seconds exten = foo,1,Set(PERIODIC_HOOK(30,beep@dialplan_hooks) Does that sound better than the current BEEP()? - Russell Bryant On March 15, 2014, 6:15 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3362/ --- (Updated March 15, 2014, 6:15 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- This commit introduces a new dialplan function, BEEP(). It allows you to enable playing a beep tone at a regular interval into a call. The most common use case for this is to use during call recording to notify and remind the callers that the call is being recorded. A future commit will update the call recording applications in Asterisk to use this automatically if an option is specified for convenience. The implementation makes heavy use of existing Asterisk components. Instead of replicating logic required to load a sound file, transcode it if necessary, and do audio mixing, it makes use of Playback+ChanSpy to accomplish the task. The other important bit of the implementation is how it figures out when to trigger the beep playback. This implementation uses the audiohook API, even though it's not actually touching the audio in any way. It's a convenient way to get a callback and check if it's time to kick off another beep. It would be nice if this was timer event based instead of polling based, but unfortunately I don't see a way to do it that won't interfere with other things. Diffs - /trunk/funcs/func_beep.c PRE-CREATION /trunk/CHANGES 410649 Diff: https://reviewboard.asterisk.org/r/3362/diff/ Testing --- Called the following extension, both letting it run all the way through, as well as hanging up at various points in the middle. [test] exten = 100,1,Answer() same = n,Set(BEEP(5)=on) same = n,Wait(20) same = n,Set(BEEP()=off) same = n,Wait(20) same = n,Hangup() Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3282: Fix a refcount error with realtime MOH
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3282/ --- (Updated March 7, 2014, 1:49 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Repository: Asterisk Description --- I observed a crash in res_musiconhold on an Asterisk 11 system using realtime MOH. Investigation of the backtrace showed a corrupt mohclass, implying that it got destroyed before the code expected it to. I went looking for reference counting errors that could have caused this crash and this patch this result. It contains 2 changes. 1) Remove a usless block of code that was impossible to reach. There was even a comment indicating that it was impossible to reach. The conditional includes !ast_test_flag(global_flags, MOH_CACHERTCLASSES) and it's inside of an if block with the opposite check ast_test_flag(global_flags, MOH_CACHERTCLASSES). There's no good reason to keep it around. 2) A similar block to #1 contained a reference counting error. It stores state-class in the local variable mohclass without increasing its reference count. The reference count on mohclass is decremented at the end of the function. This block of code probably very rarely runs, which would help explain why this system was working fine for many months before experiencing a crash. Diffs - /branches/11/res/res_musiconhold.c 409286 Diff: https://reviewboard.asterisk.org/r/3282/diff/ Testing --- Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3300: Don't crash on lack of bridged rtp instance
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3300/ --- Review request for Asterisk Developers and leifmadsen. Bugs: ASTERISK-23419 https://issues.asterisk.org/jira/browse/ASTERISK-23419 Repository: Asterisk Description --- This bug shows a crash in bridge_p2p_rtp_write(). There is no bridged rtp instance so it goes boom. The patch just catches the case and returns. I'm not really sure *why* there's no bridged instance, but this seems like a pretty safe function input sanity check. Diffs - /tags/1.8.18.1/res/res_rtp_asterisk.c 405155 Diff: https://reviewboard.asterisk.org/r/3300/diff/ Testing --- Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3282: Fix a refcount error with realtime MOH
On March 3, 2014, 5:44 p.m., wdoekes wrote: Why is this limited to 11 and not 1.8+? 11 is just where I saw the crash. I can apply to 1.8+ though - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3282/#review11024 --- On March 2, 2014, 8:45 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3282/ --- (Updated March 2, 2014, 8:45 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- I observed a crash in res_musiconhold on an Asterisk 11 system using realtime MOH. Investigation of the backtrace showed a corrupt mohclass, implying that it got destroyed before the code expected it to. I went looking for reference counting errors that could have caused this crash and this patch this result. It contains 2 changes. 1) Remove a usless block of code that was impossible to reach. There was even a comment indicating that it was impossible to reach. The conditional includes !ast_test_flag(global_flags, MOH_CACHERTCLASSES) and it's inside of an if block with the opposite check ast_test_flag(global_flags, MOH_CACHERTCLASSES). There's no good reason to keep it around. 2) A similar block to #1 contained a reference counting error. It stores state-class in the local variable mohclass without increasing its reference count. The reference count on mohclass is decremented at the end of the function. This block of code probably very rarely runs, which would help explain why this system was working fine for many months before experiencing a crash. Diffs - /branches/11/res/res_musiconhold.c 409286 Diff: https://reviewboard.asterisk.org/r/3282/diff/ Testing --- Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3282: Fix a refcount error with realtime MOH
On March 3, 2014, 3:30 p.m., Michael Young wrote: ASTERISK-21775 is still open that had this same fix in it because the testers were still reporting a problem. Is the crash similar to what the reporters on that issue have been reporting? Thanks The crash I saw is different - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3282/#review11020 --- On March 2, 2014, 8:45 p.m., Russell Bryant wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3282/ --- (Updated March 2, 2014, 8:45 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- I observed a crash in res_musiconhold on an Asterisk 11 system using realtime MOH. Investigation of the backtrace showed a corrupt mohclass, implying that it got destroyed before the code expected it to. I went looking for reference counting errors that could have caused this crash and this patch this result. It contains 2 changes. 1) Remove a usless block of code that was impossible to reach. There was even a comment indicating that it was impossible to reach. The conditional includes !ast_test_flag(global_flags, MOH_CACHERTCLASSES) and it's inside of an if block with the opposite check ast_test_flag(global_flags, MOH_CACHERTCLASSES). There's no good reason to keep it around. 2) A similar block to #1 contained a reference counting error. It stores state-class in the local variable mohclass without increasing its reference count. The reference count on mohclass is decremented at the end of the function. This block of code probably very rarely runs, which would help explain why this system was working fine for many months before experiencing a crash. Diffs - /branches/11/res/res_musiconhold.c 409286 Diff: https://reviewboard.asterisk.org/r/3282/diff/ Testing --- Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3282: Fix a refcount error with realtime MOH
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3282/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- I observed a crash in res_musiconhold on an Asterisk 11 system using realtime MOH. Investigation of the backtrace showed a corrupt mohclass, implying that it got destroyed before the code expected it to. I went looking for reference counting errors that could have caused this crash and this patch this result. It contains 2 changes. 1) Remove a usless block of code that was impossible to reach. There was even a comment indicating that it was impossible to reach. The conditional includes !ast_test_flag(global_flags, MOH_CACHERTCLASSES) and it's inside of an if block with the opposite check ast_test_flag(global_flags, MOH_CACHERTCLASSES). There's no good reason to keep it around. 2) A similar block to #1 contained a reference counting error. It stores state-class in the local variable mohclass without increasing its reference count. The reference count on mohclass is decremented at the end of the function. This block of code probably very rarely runs, which would help explain why this system was working fine for many months before experiencing a crash. Diffs - /branches/11/res/res_musiconhold.c 409286 Diff: https://reviewboard.asterisk.org/r/3282/diff/ Testing --- Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3279: Iterate through logger.conf [general] section
On Feb. 28, 2014, 12:51 p.m., Matt Jordan wrote:
I'm not sure I understand the need for this patch.
Setting a configuration option twice - when that option doesn't support
being set multiple times - would generally have undefined behaviour. Your
patch changes it so that Asterisk reads the last defined value, as opposed
to the first. How is that better?
Leif Madsen wrote:
It's better when you want to deploy Asterisk in a DevOps environment.
What happens is you define the default behavior in the main file that gets
deployed. From there you then override that behavior in an included file
which is defined and built via DevOps (usually through a template of some
sort that contains information for the particular machine you're deploying).
The example is not a good one, because obviously you would never deploy
the file in the example provided.
; logger.conf
[general]
queue_log=no
#include logger.conf.d/logger.conf.local
; logger.conf.d/logger.conf.local
[general]
queue_log=yes ; we've deployed a queue server, so enable queue logging
Primary example of it in use available at
https://github.com/kickstandproject/kickstandproject-asterisk/tree/master/templates/etc/asterisk
wdoekes wrote:
+1 on iterating over the configs options. Most modules do, some don't.
Consistency is nice.
Please do add a note to the upgrade file though. Perhaps someone has
already worked around this by placing the #include logger.conf.d statement at
the top of the file.
Matt Jordan wrote:
I can see how this might be better for that deployment scenario.
I compared this approach against the Config Framework (config_options.c).
It uses an ast_variable_browse as well (which is not surprising, since it has
to parse through a list of key/value pairs), so this does take this into line
with the recommended (tm) way of doing things:
int aco_process_category_options(struct aco_type *type, struct ast_config
*cfg, const char *cat, void *obj)
{
struct ast_variable *var;
for (var = ast_variable_browse(cfg, cat); var; var = var-next) {
if (aco_process_var(type, cat, var, obj)) {
return -1;
}
}
return 0;
}
That being said, why not just bite the bullet and use the configuration
framework for logger.conf?
Patch looks fine to me. It's an improvement over what's there, for sure.
Using new-style config handling sounds nice, but this is at least a step in a
better direction.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3279/#review10991
---
On Feb. 28, 2014, 1:44 a.m., Paul Belanger wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3279/
---
(Updated Feb. 28, 2014, 1:44 a.m.)
Review request for Asterisk Developers.
Repository: Asterisk
Description
---
This patch allows you to override the [general] section of logger.conf,
making it the same functionality as the [logfiles] sections.
Diffs
-
trunk/main/logger.c 409111
Diff: https://reviewboard.asterisk.org/r/3279/diff/
Testing
---
local development. Setup
[general]
queue_log = no
queue_log = yes
Queue logfiles were created.
Thanks,
Paul Belanger
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3159: Allow nested #includes in extconfig.conf
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3159/ --- Review request for Asterisk Developers and Paul Belanger. Bugs: ASTERISK-17837 https://issues.asterisk.org/jira/browse/ASTERISK-17837 Repository: Asterisk Description --- extconfig.conf was hard-coded to not allow nested includes for some reason. The code has been this way since a patch was merged for ASTERISK- (revision 4889), which was a significant update to this code (Merge config updates). I can't figure out any good reason why this should be limited. This patch just removes the limit and uses the default nesting depth limit. Diffs - /branches/1.8/main/config.c 406640 Diff: https://reviewboard.asterisk.org/r/3159/diff/ Testing --- Created some nested includes in extconfig.conf. Confirmed error before the patch and loading fine afterwards. Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3135: Protect ast_filestream object when on a channel
and chan-timingdata have now been reset since this code ran. That probably also means that the ast_filestream got destroyed before the code in ast_readaudio_callback was done using it. The only way I can think of this happening is via something in another thread. Something like AMI running on a channel doing something that affects audio. I'm basically speculating at this point. If this is the case, it seems like the filestream's reference count needs to be bumped while it's on the channel. Otherwise, it seems possible that it could disappear at just the wrong time. Diffs - /branches/1.8/main/file.c 405876 /branches/1.8/main/channel.c 405876 /branches/1.8/include/asterisk/channel.h 405876 Diff: https://reviewboard.asterisk.org/r/3135/diff/ Testing --- This issue was originally reported by Leif Madsen. The patch was given to him for further testing. They have done some basic sanity tests with it, but it's not yet running in production or anything like that. It at least doesn't blow up immediately ... I wanted to get the patch and analysis up to get some more eyes on it. Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3148: res_pjsip: Config option to enable PJSIP logger at load time
On Jan. 22, 2014, 1:14 a.m., Paul Belanger wrote: branches/12/contrib/ast-db-manage/config/versions/2fc7930b41b3_add_pjsip_endpoint_options_for_12_1.py, line 1 https://reviewboard.asterisk.org/r/3148/diff/1/?file=53005#file53005line1 You should be creating new alembic scripts not modifying committed ones. This is going to breaks peoples systems that have already invoked the scripts. right. each script should stay unmodified. any db schema changes should be a new script, which is a migration from the old schema to the updated one. That way at each release, people can re-run alembic to apply all of the updates since the version of the schema they currently have. - Russell --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3148/#review10653 --- On Jan. 21, 2014, 11:01 p.m., Kevin Harwell wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3148/ --- (Updated Jan. 21, 2014, 11:01 p.m.) Review request for Asterisk Developers. Bugs: ASTERISK-23038 https://issues.asterisk.org/jira/browse/ASTERISK-23038 Repository: Asterisk Description --- Added a debug configuration option for res_pjsip that when set to yes enables SIP messages to be logged. It is specified under the system type. Also updated the alembic 12.1 script to include this option as well as a few others that were missing. Also updated the _adding_extensions script in order to make the id column on the table a primary key because mysql needed it to be as such. Diffs - branches/12/res/res_pjsip_logger.c 405906 branches/12/res/res_pjsip/config_system.c 405906 branches/12/res/res_pjsip.c 405906 branches/12/include/asterisk/res_pjsip.h 405906 branches/12/contrib/ast-db-manage/config/versions/581a4264e537_adding_extensions.py 405906 branches/12/contrib/ast-db-manage/config/versions/2fc7930b41b3_add_pjsip_endpoint_options_for_12_1.py 405906 branches/12/configs/pjsip.conf.sample 405906 Diff: https://reviewboard.asterisk.org/r/3148/diff/ Testing --- Set the debug option in the pjsip.conf file and observed SIP debug messages on the console. Also, tested the modified alembic scripts against postgres and mysql database servers. Thanks, Kevin Harwell -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3135: Protect ast_filestream object when on a channel
that the ast_filestream got destroyed before the code in ast_readaudio_callback was done using it. The only way I can think of this happening is via something in another thread. Something like AMI running on a channel doing something that affects audio. I'm basically speculating at this point. If this is the case, it seems like the filestream's reference count needs to be bumped while it's on the channel. Otherwise, it seems possible that it could disappear at just the wrong time. Diffs - /branches/1.8/main/file.c 405876 /branches/1.8/main/channel.c 405876 /branches/1.8/include/asterisk/channel.h 405876 Diff: https://reviewboard.asterisk.org/r/3135/diff/ Testing --- This issue was originally reported by Leif Madsen. The patch was given to him for further testing. They have done some basic sanity tests with it, but it's not yet running in production or anything like that. It at least doesn't blow up immediately ... I wanted to get the patch and analysis up to get some more eyes on it. Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3135: Protect ast_filestream object when on a channel
On Jan. 17, 2014, 11:50 p.m., rmudgett wrote:
/branches/1.8/main/channel.c, lines 3576-3579
https://reviewboard.asterisk.org/r/3135/diff/1/?file=52962#file52962line3576
This should not be done at all.
You are dropping a reference when timingdata doesn't really hold the
reference. In the only case where timingdata is an identified ao2 object,
the timingdata pointer is sharing the reference with c-stream. The
reference is dropped by ast_closestream() after clearing the timingdata
with its own call to ast_settimeout(). Otherwise, you will need to give
timingdata a reference when setting an identified ao2 object.
I intended to have a corresponding ao2_ref(obj, 1) in ast_settimeout() when it
gets stored on the channel. I'll fix that.
- Russell
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3135/#review10631
---
On Jan. 17, 2014, 10:18 p.m., Russell Bryant wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3135/
---
(Updated Jan. 17, 2014, 10:18 p.m.)
Review request for Asterisk Developers and leifmadsen.
Repository: Asterisk
Description
---
The ast_filestream object gets tacked on to a channel via
chan-timingdata. It's a reference counted object, but the reference
count isn't used when putting it on a channel. It's theoretically
possible for another thread to interfere with the channel while it's
unlocked and cause the filestream to get destroyed.
Use the astobj2 reference count to make sure that as long as this code
path is holding on the ast_filestream and passing it into the file.c
playback code, that it knows it's valid.
-
More detail:
A crash occurs in voicemail. Here is the backtrace:
#0 0x004d396a in ast_readaudio_callback (s=0x7f260856f580) at
file.c:779
#1 0x004d3b8a in ast_fsread_audio (data=0x7f260856f580) at file.c:806
#2 0x0047599d in __ast_read (chan=0x7f2644c49460, dropaudio=0) at
channel.c:3865
#3 0x00477d81 in ast_read (chan=0x7f2644c49460) at channel.c:4325
#4 0x004d5231 in waitstream_core (c=0x7f2644c49460,
breakon=0x7f262ed56f00 #*, forward=0x64c4a2 , reverse=0x64c4a2 ,
skip_ms=0, audiofd=-1, cmdfd=-1, context=0x0) at file.c:1253
#5 0x004d5783 in ast_waitstream (c=0x7f2644c49460,
breakon=0x7f262ed56f00 #*) at file.c:1344
#6 0x7f266d34c22b in leave_voicemail (chan=0x7f2644c49460,
ext=0x7f2609908c20 redacted, options=0x7f262ed56ff0) at
app_voicemail.c:5773
#7 0x7f266d35eb71 in vm_exec (chan=0x7f2644c49460, data=0x7f262ed59720
redacted) at app_voicemail.c:10722
(gdb) frame 0
#0 0x004d396a in ast_readaudio_callback (s=0x7f260856f580) at
file.c:779
779 if (s-owner-timingfd -1) {
(gdb) p s
$6 = (struct ast_filestream *) 0x7f260856f580
The crash occurs here because the contents of the ast_filestream are garbage.
(gdb) p *s
$7 = {fmt = 0x6372756f530a0d74, flags = 924858981, mode = 875902769,
open_filename = 0x440a0d3330393734 Address 0x440a0d3330393734 out of
bounds, filename = 0x6974616e69747365 Address 0x6974616e69747365 out of
bounds,
realfilename = 0x440a0d73203a6e6f Address 0x440a0d73203a6e6f out of
bounds, vfs = 0x6974616e69747365, trans = 0x7865746e6f436e6f, tr =
0x2d7473786e203a74, lastwriteformat = 762475363, lasttimeout = 1969583473,
owner = 0x656c6c61430a0d65, f = 0x313722203a444972, fr = {frametype =
875836727, subclass = {integer = 926687266, codec = 3761973748257529890},
datalen = 809056052, samples = 168640051, mallocd = 1851877443,
mallocd_hdr_len = 7881689877736674080, offset = 762148397, src =
0xd61633832313030 Address 0xd61633832313030 out of bounds, data = {ptr =
0x616e69747365440a, uint32 = 1936016394, pad = \nDestina}, delivery = {
tv_sec = 7953753055737899380, tv_usec = 5785246594218354030},
frame_list = {next = 0x36702d5453584e2f}, flags = 758134073, ts =
7010989772577650738, len = 7163375912484959347, seqno = 1869182049},
buf = 0x614c0a0d65756575 Address 0x614c0a0d65756575 out of bounds,
_private = 0x203a617461447473, orig_chan_name = 0x7273632d7473786e Address
0x7273632d7473786e out of bounds,
write_buffer = 0x38312c2c2c2c712d Address 0x38312c2c2c2c712d out of
bounds}
(gdb) p s-owner
$8 = (struct ast_channel *) 0x656c6c61430a0d65
(gdb) p *s-owner
Cannot access memory at address 0x656c6c61430a0d65
s-owner should have been 0x7f2644c49460, from higher up in the backtrace.
Here is where things get quite interesting ...
(gdb) frame 2
#2 0x0047599d in __ast_read (chan=0x7f2644c49460, dropaudio=0) at
channel.c:3865
3865
Re: [asterisk-dev] [Code Review] 3135: Protect ast_filestream object when on a channel
-timingfunc and chan-timingdata have now been reset since this code ran. That probably also means that the ast_filestream got destroyed before the code in ast_readaudio_callback was done using it. The only way I can think of this happening is via something in another thread. Something like AMI running on a channel doing something that affects audio. I'm basically speculating at this point. If this is the case, it seems like the filestream's reference count needs to be bumped while it's on the channel. Otherwise, it seems possible that it could disappear at just the wrong time. Diffs (updated) - /branches/1.8/main/file.c 405876 /branches/1.8/main/channel.c 405876 /branches/1.8/include/asterisk/channel.h 405876 Diff: https://reviewboard.asterisk.org/r/3135/diff/ Testing --- This issue was originally reported by Leif Madsen. The patch was given to him for further testing. They have done some basic sanity tests with it, but it's not yet running in production or anything like that. It at least doesn't blow up immediately ... I wanted to get the patch and analysis up to get some more eyes on it. Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3062: a systemd service
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3062/#review10506 --- I'm not sure this belongs in Asterisk itself, personally. In any case, Fedora has had a systemd unit for Asterisk for quite some time. For reference, you can view it here: http://pkgs.fedoraproject.org/cgit/asterisk.git/tree/asterisk.service - Russell Bryant On Dec. 24, 2013, 4:49 p.m., Tzafrir Cohen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3062/ --- (Updated Dec. 24, 2013, 4:49 p.m.) Review request for Asterisk Developers. Repository: Asterisk Description --- Installs a systemd service file for Asterisk. Systeemd is the new one daemon to rule them all for Linux: http://www.freedesktop.org/wiki/Software/systemd/ On systems without systemd this should be just a harmless (though maybe annoying) text file. This is aimed at replacing safe_asterisk with a more reliable main loop. It almost does that. Is still fails to handle failures, as it seems that systemd's ExecPostStop command does not get the exist status of the stopped command. Diffs - /trunk/contrib/asterisk.service PRE-CREATION /trunk/Makefile 404563 Diff: https://reviewboard.asterisk.org/r/3062/diff/ Testing --- Thanks, Tzafrir Cohen -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3065: Reset peer outboundproxy on sip.conf reload
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3065/ --- Review request for Asterisk Developers. Bugs: ASTERISK-19454 https://issues.asterisk.org/jira/browse/ASTERISK-19454 Repository: Asterisk Description --- If you set a peer's outboundproxy and then removed it from the config, this would not get picked up in a config reload. This patch fixes that by resetting it in set_peer_defaults(). Note that the fix is the same for 1.8, 11, 12, and trunk branches. Diffs - /branches/1.8/channels/chan_sip.c 403633 Diff: https://reviewboard.asterisk.org/r/3065/diff/ Testing --- 1) Created a peer with outboundproxy set, observed with 'sip show peer foo' 2) removed outboundproxy from the config 3) executed *CLI sip reload 4) Repeat step 1, observe outboundproxy no longer set when the patch is applied I also verified that step 4 fails without the patch Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3065: Reset peer outboundproxy on sip.conf reload
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3065/ --- (Updated Dec. 11, 2013, 7:19 p.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Bugs: ASTERISK-19454 https://issues.asterisk.org/jira/browse/ASTERISK-19454 Repository: Asterisk Description --- If you set a peer's outboundproxy and then removed it from the config, this would not get picked up in a config reload. This patch fixes that by resetting it in set_peer_defaults(). Note that the fix is the same for 1.8, 11, 12, and trunk branches. Diffs - /branches/1.8/channels/chan_sip.c 403633 Diff: https://reviewboard.asterisk.org/r/3065/diff/ Testing --- 1) Created a peer with outboundproxy set, observed with 'sip show peer foo' 2) removed outboundproxy from the config 3) executed *CLI sip reload 4) Repeat step 1, observe outboundproxy no longer set when the patch is applied I also verified that step 4 fails without the patch Thanks, Russell Bryant -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [svn-commits] murf: branch murf/bug11210 r103686 - /team/murf/bug11210/doc/
Johansson Olle E wrote: Maybe we can start creating a collection of SIPP tests to run various scenarious. I would like to test how registrations and subscriptions affect the stack too. Imagine your PVT list with 500 subscriptions, 50 per extension. Then place 200 calls and you will see asterisk having a lot of fun delivering call states to 50 subscribers per extension. That would be a good stress test for your branch, since it not only involves INVITE/BYE but also traversing the list for SUBSCRIBEs and having a much larger list of PVT's than just the calls. I think all of this stuff is really great to have. I would also encourage anyone else that comes up with anything more extensive like this for testing to help get it into the tree somewhere. As it starts to pile up, we will start to have a nice set of tests available ... -- Russell Bryant Senior Software Engineer Open Source Team Lead Digium, Inc. ___ --Bandwidth and Colocation Provided by http://www.api-digital.com-- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
