----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4475/ -----------------------------------------------------------
Review request for Asterisk Developers and Joshua Colp. Repository: Asterisk Description ------- Currently, the Asterisk 12+ testsuite test 'snoop_id' is failing due to a stack overflow on 32-bit machines. This occurred after a patch was made that enabled all formats on originated channels, including 192khz SLIN audio. Since it is theoretically possible to create Local channels along with ChanSpy in Asterisk 11 in a similar fashion, this issue is being fixed in that branch as well. When an audiohook is created (which is used by the various Spy applications and Snoop channel), it initially is given a sample rate of 8kHz. It is expected, however, that this rate may change based on the media that passes through the audiohook. However, the read/write operations on the audiohook behave very differently. When a frame is written to the audiohook, the format of the frame is checked against the internal sample rate. If the rate of the format does not match the internal sample rate, the internal sample rate is updated and a new SLIN format is chosen based on that sample rate. This works just fine. When a frame is read, however, we do somehting quite different. If the format rate matches the internal sample rate, all is fine. However, if the rates don't match, the audiohook attempts to "fix up" the number of samples that were requested. This can result in some seriously large number of samples being requested from the read/write factories. Consider the worst case - 192kHz SLIN. If we attempt to read 20ms worth of audio produced at that rate, we'd request 3840 samples (192000 / (1000 / 20)). However, if the audiohook is still expecting an internal sample rate of 8000, we'll attempt to "fix up" the requested samples to: samples_converted = samples * (ast_format_get_sample_rate(format) / (float) audiohook->hook_internal_samp_rate); or: 92160 = 3840 * (192000 / 8000) This results in us attempting to read 92160 samples from our factories, as opposed to the 3840 that we actually wanted. On a 64-bit machine, this miraculously survives - despite allocating up to two buffers of length 92160 on the stack. The 32-bit machines aren't quite so lucky. Even in the case where this works, we will either (a) get way more samples than we wanted; or (b) get about 3840 samples, assuming the timing is pretty good on the machine. Either way, this is kind of wrong. My first inclination was to allocate the buffers on the heap. As it is, however, there's at least two drawbacks with doing this: (1) It's a bit complicated, as the size of the buffers may change during the lifetime of the audiohook (ew). (2) The stack is faster (yay); the heap is slower (boo). Since our calculation is flat out wrong in the first place, I've opted to fix this issue that way instead. Rather than attempting to 'massage' the samples requested, we now instead re-calculate our internal sample rate based on the format requested. This causes us to read the correct number of samples, and has the added benefit of setting the audihook with the right SLIN format. Diffs ----- /branches/11/main/audiohook.c 432788 Diff: https://reviewboard.asterisk.org/r/4475/diff/ Testing ------- (1) Tested ChanSpy in 11 and 13 manually. Still works. Yay. (2) Ran the chanspy tests in 11/13. Those still work too. (3) Ran the snoop tests. They stopped crashing, and also still work. Thanks, Matt Jordan
-- _____________________________________________________________________ -- 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
