---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3175/#review10762
---
Ship it!
Ship It!
- Mark Michelson
On Feb. 5, 2014, 6:17
/#comment20310
Another place where you should use AST_VECTOR_SIZE()
/branches/12/main/format_cap_ng.c
https://reviewboard.asterisk.org/r/3178/#comment20311
Should the call to AST_VECTOR_REMOVE_CMP_ORDERED be outside the list
traversal?
- Mark Michelson
On Feb. 4, 2014, 3:57 p.m
/configs/pjsip.conf.sample
https://reviewboard.asterisk.org/r/3180/#comment20247
I would not remove this. It's handy to have this explicitly noted.
- Mark Michelson
On Feb. 4, 2014, 9:13 p.m., rnewton wrote
in the From
header of incoming requests
2) Use an identify section to match based on IP address.
This places like information in the same area.
- Mark Michelson
On Feb. 4, 2014, 11:12 p.m., rnewton wrote
On Feb. 4, 2014, 11:35 p.m., Mark Michelson wrote:
/branches/12/configs/pjsip.conf.sample, lines 95-98
https://reviewboard.asterisk.org/r/3180/diff/2/?file=53435#file53435line95
I think that rather than stating that this is how endpoint matching is
performed and then later
://reviewboard.asterisk.org/r/3180/#comment20255
s/match the the/match the/
- Mark Michelson
On Feb. 4, 2014, 11:12 p.m., rnewton wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3180
for the
testsuite.
Thanks,
Mark Michelson
--
_
-- 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
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3171/#review10740
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 31, 2014, 10:24
/tests/channels/pjsip/mwi/configs/ast1/modules.conf
PRE-CREATION
/asterisk/trunk/lib/python/asterisk/pluggable_modules.py 4618
Diff: https://reviewboard.asterisk.org/r/3151/diff/
Testing
---
The tests pass.
Thanks,
Mark Michelson
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3148/#review10718
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 30, 2014, 5:51
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3145/#review10719
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 28, 2014, 3:55
and Paul will
have advice on what is right or wrong about what you've done. I just spotted a
typo, though :)
branches/12/UPGRADE.txt
https://reviewboard.asterisk.org/r/3167/#comment20215
s/pj_systems/ps_systems/
- Mark Michelson
On Jan. 30, 2014, 5:51 p.m., Kevin Harwell wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3160/#review10723
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 27, 2014, 9:11
/ast1/extensions.conf
https://reviewboard.asterisk.org/r/3153/#comment20228
redacity
- Mark Michelson
On Jan. 24, 2014, 6:09 p.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3153/#review10734
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 30, 2014, 9:53
/trunk/tests/channels/pjsip/mwi/configs/ast1/modules.conf
PRE-CREATION
/asterisk/trunk/lib/python/asterisk/pluggable_modules.py 4618
Diff: https://reviewboard.asterisk.org/r/3151/diff/
Testing
---
The tests pass.
Thanks,
Mark Michelson
On Jan. 24, 2014, 4:49 p.m., Mark Michelson wrote:
branches/12/res/res_pjsip/config_system.c, lines 152-156
https://reviewboard.asterisk.org/r/3148/diff/1/?file=53009#file53009line152
Josh may correct me on this, but since sorcery configs are immutable,
you shouldn't need
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3149/#review10681
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 22, 2014, 7:04
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3144/#review10682
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 21, 2014, 5:34
on which the
message is being sent, then do we really want to be overwriting that with our
local address?
- Mark Michelson
On Jan. 22, 2014, 3:30 a.m., Joshua Colp wrote:
---
This is an automatically generated e-mail. To reply, visit
not load, Asterisk continues operating.
FAILURE: Module does not load, Asterisk shuts down.
- Mark Michelson
On Jan. 24, 2014, 5:46 p.m., Kevin Harwell wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
-sensitively to
Dial.
- Mark Michelson
On Jan. 24, 2014, 6:08 p.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3154
.
- Mark Michelson
On Jan. 24, 2014, 5:51 p.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3152
) A bunch of non-waitmarked users are in the conference chatting away, and
a marked user enters. CONF_SOUND_BEGIN will play when the marked user enters.
You could get around this by not playing the sound if there were no waitmarked
users in the conference when the marked user entered.
- Mark
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3152/#review10691
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 24, 2014, 8:16
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3154/#review10694
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 24, 2014, 7:33
/two_parties/nominal/alice_initiated/requested_capabilities/configs/ast1/extensions.conf
https://reviewboard.asterisk.org/r/3139/#comment20195
These extensions aren't used, so you can remove them.
- Mark Michelson
On Jan. 18, 2014, 5:13 p.m., Joshua Colp wrote
---
On Jan. 23, 2014, 10:52 p.m., Mark Michelson wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3151
/asterisk/pluggable_modules.py 4595
Diff: https://reviewboard.asterisk.org/r/3151/diff/
Testing
---
The tests pass.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk
-CREATION
Diff: https://reviewboard.asterisk.org/r/3150/diff/
Testing
---
In addition to manual testing, I have also posted
https://reviewboard.asterisk.org/r/3151 that contains new tests for the
testsuite.
Thanks,
Mark Michelson
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3125/#review10621
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 13, 2014, 11:26
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3129/#review10622
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 16, 2014, 8:56
On Jan. 16, 2014, 5 p.m., Mark Michelson wrote:
/branches/12/main/http.c, line 813
https://reviewboard.asterisk.org/r/3125/diff/2/?file=52892#file52892line813
chunked_atoh shouldn't be necessary. You should be able to just use:
sscanf(chunk_header, %x, chunk_length
with a ship it! on the
review.
/branches/12/main/format_pref.c
https://reviewboard.asterisk.org/r/3131/#comment20123
And the award for most nit-picky critique on a code review goes to:
drumroll
Mark Michelson! For his comment: The i.e. here should really be an e.g.
since you
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3118/#review10624
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 16, 2014, 8:26
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3130/#review10626
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 16, 2014, 6:33
On Jan. 16, 2014, 3:10 p.m., Joshua Colp wrote:
branches/12/res/res_pjsip_endpoint_identifier_ip.c, lines 285-287
https://reviewboard.asterisk.org/r/3104/diff/1/?file=51197#file51197line285
I'm cracking down on gratuitous RAII_VAR usage these days.
Actually, this use of RAII_VAR
end up returning it.
branches/12/res/res_pjsip_outbound_registration.c
https://reviewboard.asterisk.org/r/3104/#comment20077
More magic numbers here. What is 53?
- Mark Michelson
On Jan. 6, 2014, 9:49 p.m., George Joseph wrote
/mailbox_baseline.py
https://reviewboard.asterisk.org/r/3118/#comment20087
What happens if this is removed?
- Mark Michelson
On Jan. 9, 2014, 11:40 p.m., Jonathan Rose wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
/3125/#comment20092
chunked_atoh shouldn't be necessary. You should be able to just use:
sscanf(chunk_header, %x, chunk_length);
You would need to change chunk_length to be an unsigned int instead of an
int, but that's fine since the length will never be negative.
- Mark
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3126/#review10614
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 13, 2014, 10:13
.
Both of these will eliminate the need to declare the hold_uid variable and
will eliminate extra unnecessary reffing. I prefer option 1, myself.
- Mark Michelson
On Jan. 16, 2014, 6:26 p.m., Jonathan Rose wrote
:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3129/
---
(Updated Jan. 16, 2014, 6:26 p.m.)
Review request for Asterisk Developers and Mark Michelson.
Repository
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3107/#review10584
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 13, 2014, 9:41
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3117/#review10585
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 10, 2014, 11:03
script that printed out the security events as they were
received. Confirmed that the account ID changed from a UUID to unknown
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com
Jordan and Mark Michelson.
Repository: Asterisk
Description
---
This patch adds the ability to interface with res_external_mwi via AMI. The
following commands are implemented:
PUT mailboxes/mailboxName
modifies mailbox state and implicitly creates new mailboxes
GET
you should either return all or
nothing. If an error occurs while creating the array, you should free
everything and return NULL.
- Mark Michelson
On Jan. 9, 2014, 11:38 p.m., Jonathan Rose wrote:
---
This is an automatically generated
/3114/#comment20035
This should explain what silent means if it is specified.
- Mark Michelson
On Jan. 10, 2014, 7:26 p.m., rmudgett wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
changed from a UUID to unknown
Thanks,
Mark Michelson
--
_
-- 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
and not in a way that will
cause disastrous results point of view, this gets a resounding ship it! My
only problem is that since I'm not familiar with the intricacies of Stasis's
inner workings, I don't know if this might have negative consequences in a way
I don't know about.
- Mark Michelson
there is precedent in other
places in the code to unconditionally queue unhold frames.
- Mark Michelson
On Jan. 6, 2014, 10:15 p.m., Jonathan Rose wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
will resolve itself, it's still best to
avoid it. From what I could see, this flag was not set when parking a call.
- Mark Michelson
On Jan. 7, 2014, 6:20 p.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply
to the chan_sip
tests, but I don't think that it's a high-priority change to make.
- Mark Michelson
On Jan. 6, 2014, 9:59 p.m., Jonathan Rose wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
, so I would expect that the old
sip_pvt should have been destroyed by the time the new REGISTER is being sent
out.
- Mark Michelson
On Jan. 7, 2014, 7:04 p.m., Scott Griepentrog wrote:
---
This is an automatically generated e-mail
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3108/#review10525
---
Ship it!
Ship It!
- Mark Michelson
On Jan. 7, 2014, 6:33
likely be more catastrophic than the previously-missing
ao2_iterator_destroy() calls.
- Mark Michelson
On Dec. 20, 2013, 10:47 p.m., rmudgett wrote:
---
This is an automatically generated e-mail. To reply, visit:
https
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3061/#review10518
---
Ship it!
Ship It!
- Mark Michelson
On Dec. 20, 2013, 4:37
essages on
startup to see what may have caused the object not to be created
properly.
What error or warning messages are emitted on the CLI when the call
failure occurs? At what point during the SIP INVITE transaction is the
problem occurring?
Hopefully this is something tha
the patch. I found none on my machine, though I'll admit I did not run
every test.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE
/r/3061/#comment19932
This swath is the only place where you should mention urgent messages since
it is part of the API.
- Mark Michelson
On Dec. 19, 2013, 6:49 p.m., rmudgett wrote:
---
This is an automatically generated e-mail
the patch. I found none on my machine, though I'll admit I did not run
every test.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update
was successful or not.
Having said that, I believe that you need to NULL out *stmt whether
sqlite3_finalize() returns an error or not.
- Mark Michelson
On Dec. 18, 2013, 7:42 p.m., Scott Griepentrog wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3078/#review10456
---
Ship it!
Ship It!
- Mark Michelson
On Dec. 18, 2013, 9:26
(ast_strdupa(mailbox_id), context, mailbox)) {
return 0;
}
/branches/12/configs/chan_dahdi.conf.sample
https://reviewboard.asterisk.org/r/3072/#comment19926
This could use a mention in UPGRADE.txt since previously the command did
not specify a context.
- Mark Michelson
On Dec
the patch. I found none on my machine, though I'll admit I did not run
every test.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3072/#review10459
---
Ship it!
Ship It!
- Mark Michelson
On Dec. 18, 2013, 11:40
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3069/#review10423
---
On Dec. 12, 2013, 9:59 p.m., Mark Michelson wrote
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3046/#review10411
---
Ship it!
Ship It!
- Mark Michelson
On Dec. 13, 2013, 4:28
, and with this patch it no longer occurs. I also ran a bevy of testsuite
tests to see if there were any that did not pass with this patch but did pass
without the patch. I found none on my machine, though I'll admit I did not run
every test.
Thanks,
Mark Michelson
freely do what it needs to. I
think such refactoring could actually be done, but I also feel like since that
sort of refactoring does not directly affect the problem being fixed here, it
should be saved for a separate review.
- Mark Michelson
On Dec. 12, 2013, 9:59 p.m., Mark Michelson wrote
be altered to expect a single mailbox string instead.
- Mark Michelson
On Dec. 9, 2013, 9:51 p.m., rmudgett wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3061
On Dec. 11, 2013, 3:22 p.m., Matt Jordan wrote:
/trunk/res/res_pjsip_path.c, lines 105-126
https://reviewboard.asterisk.org/r/3050/diff/1/?file=49103#file49103line105
Because this is using an ao2_callback to get the contact, it's going to
be rather expensive. If we were searching
, it turned out I was wrong
instead :)
The only suggestion I have for this is that since RTP glue update_peer() method
is now consistently called with the channel locked, I would update its
documentation in rtp_engine.h to note this.
- Mark Michelson
On Dec. 11, 2013, 11:20 p.m., Kevin Harwell
On Dec. 4, 2013, 9:02 p.m., Mark Michelson wrote:
/branches/12/channels/pjsip/dialplan_functions.c, lines 316-327
https://reviewboard.asterisk.org/r/3038/diff/1/?file=48949#file48949line316
These descriptions are inaccurate when used on outgoing channels.
Matt Jordan wrote
On Dec. 4, 2013, 9:02 p.m., Mark Michelson wrote:
/branches/12/channels/pjsip/dialplan_functions.c, lines 338-345
https://reviewboard.asterisk.org/r/3038/diff/1/?file=48949#file48949line338
These descriptions are inaccurate when used on outbound channels.
Matt Jordan wrote
On Dec. 4, 2013, 9:02 p.m., Mark Michelson wrote:
/branches/12/channels/pjsip/dialplan_functions.c, lines 316-327
https://reviewboard.asterisk.org/r/3038/diff/1/?file=48949#file48949line316
These descriptions are inaccurate when used on outgoing channels.
Matt Jordan wrote
, this review deserves a
ship it! really.
- Mark Michelson
On Dec. 10, 2013, 3:51 a.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3038
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3060/#review10367
---
Ship it!
Ship It!
- Mark Michelson
On Dec. 9, 2013, 10:52
://reviewboard.asterisk.org/r/3035/#comment19771
Just curious, but why does disallow get this special treatment?
- Mark Michelson
On Dec. 10, 2013, 2:34 a.m., Matt Jordan wrote:
---
This is an automatically generated e-mail. To reply
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3034/#review10376
---
Ship it!
Ship It!
- Mark Michelson
On Nov. 28, 2013, 4:31
Matthew Jordan
Sunday, December
08, 2013 9:09 AM
Back
when we first added python logging to the Asterisk Test Suite, there
were considerably fewer tests and the log files - while large - were
slightly more manageable. Today, it feels like things have gotten a bit
more
.
Thanks,
Mark Michelson
--
_
-- 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
for MixMonitor, a user can easily manage multiple recordings on a
single channel.
Diffs
-
/trunk/apps/app_mixmonitor.c 402883
Diff: https://reviewboard.asterisk.org/r/3023/diff/
Testing
---
See https://reviewboard.asterisk.org/r/3024/
Thanks,
Mark Michelson
4331
/asterisk/trunk/tests/apps/mixmonitor_func/test-config.yaml PRE-CREATION
/asterisk/trunk/tests/apps/mixmonitor_func/configs/ast1/extensions.conf
PRE-CREATION
Diff: https://reviewboard.asterisk.org/r/3024/diff/
Testing
---
Test consistently passes.
Thanks,
Mark Michelson
://reviewboard.asterisk.org/r/3009/diff/
Testing
---
See review 3015
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit
/python/asterisk/apptest.py 4331
Diff: https://reviewboard.asterisk.org/r/3015/diff/
Testing
---
Ran the test in a while loop and ensured that in 30+ executions, the test
passed each time.
Thanks,
Mark Michelson
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2944/#review10338
---
Ship it!
Ship It!
- Mark Michelson
On Dec. 9, 2013, 4:17
ast_channel_suppress() is called, it results in AST_FRAME_VOICE being
turned into AST_FRAME_NULL.
If there is a jitter buffer on a channel, then AST_FRAME_NULL gets turned into
AST_FRAME_VOICE.
While I don't think this will loop infinitely, it may result in the jitter
buffer being depleted.
- Mark
that the conflict does not apply for this
test.
The other question I have is if you plan to go through the tests that use
chan_sip and chan_pjsip and add the appropriate deps to the properties in their
test-config.yaml files. Were you holding off on that in case this approach was
vetoed?
- Mark Michelson
is scoped to this function, once this function returns,
cfg-default_outbound_endpoint cannot be assumed safe to use since cfg (and its
contents) may get freed.
- Mark Michelson
On Dec. 5, 2013, 8:26 p.m., Kevin Harwell wrote
Jason Parker
Thursday,
December 05, 2013 3:31 PM
Presence State is a feature that was added in Asterisk 11, which
allows for more information to be captured about the state of a
user. There appears to have been a rather large oversight in the
/r/3045/#comment19721
Check for a NULL return here. This will go badly if app_stack.so is not
loaded.
- Mark Michelson
On Dec. 5, 2013, midnight, Jonathan Rose wrote:
---
This is an automatically generated e-mail. To reply, visit
may have purposely not
included the old test since it is not relevant, but since it's not here I just
wanted to make sure it hadn't disappeared.
- Mark Michelson
On Dec. 5, 2013, 11:36 p.m., Jonathan Rose wrote
by the lock.
- Mark Michelson
On Dec. 6, 2013, 9:41 p.m., Scott Griepentrog wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3055
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3053/#review10326
---
Ship it!
Ship It!
- Mark Michelson
On Dec. 6, 2013, 9:01
450 test runs, there were no negative consequences. Prior
to this changeset, running the test 20-50 times would result in a crash.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3041/#review10298
---
Ship it!
Ship It!
- Mark Michelson
On Dec. 3, 2013, 11:04
/#comment19673
Just use ast_copy_string()
/branches/12/channels/pjsip/dialplan_functions.c
https://reviewboard.asterisk.org/r/3038/#comment19674
Print a warning about an unrecognized option here.
- Mark Michelson
On Dec. 1, 2013, 2:08 p.m., Matt Jordan wrote
://reviewboard.asterisk.org/r/3044/diff/
Testing
---
Ran inbound/outbound call and outbound registration tests in the testsuite to
ensure they still pass.
Thanks,
Mark Michelson
--
_
-- Bandwidth and Colocation Provided
801 - 900 of 927 matches
Mail list logo
