[svn-commits] dvossel: branch 1.6.1 r260052 - in /branches/1.6.1: ./ include/asterisk/ main/
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Thu Apr 29 10:37:19 CDT 2010
Author: dvossel
Date: Thu Apr 29 10:37:15 2010
New Revision: 260052
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=260052
Log:
Merged revisions 260050 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk
................
r260050 | dvossel | 2010-04-29 10:33:27 -0500 (Thu, 29 Apr 2010) | 21 lines
Merged revisions 260049 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r260049 | dvossel | 2010-04-29 10:31:02 -0500 (Thu, 29 Apr 2010) | 14 lines
Fixes crash in audiohook_write_list
The middle_frame in the audiohook_write_list function was
being freed if a audiohook manipulator returned a failure.
This is incorrect logic. This patch resolves this and
adds detailed descriptions of how this function should work
and why manipulator failures must be ignored.
(closes issue #17052)
Reported by: dvossel
Tested by: dvossel
Review: https://reviewboard.asterisk.org/r/623/
........
................
Modified:
branches/1.6.1/ (props changed)
branches/1.6.1/include/asterisk/audiohook.h
branches/1.6.1/main/audiohook.c
Propchange: branches/1.6.1/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.
Modified: branches/1.6.1/include/asterisk/audiohook.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.1/include/asterisk/audiohook.h?view=diff&rev=260052&r1=260051&r2=260052
==============================================================================
--- branches/1.6.1/include/asterisk/audiohook.h (original)
+++ branches/1.6.1/include/asterisk/audiohook.h Thu Apr 29 10:37:15 2010
@@ -73,9 +73,16 @@
* \param chan Channel
* \param frame Frame of audio to manipulate
* \param direction Direction frame came from
- * \return Returns 0 on success, -1 on failure
- * \note An audiohook does not have any reference to a private data structure for manipulate types. It is up to the manipulate callback to store this data
- * via it's own method. An example would be datastores.
+ * \return Returns 0 on success, -1 on failure.
+ * \note An audiohook does not have any reference to a private data structure for manipulate
+ * types. It is up to the manipulate callback to store this data via it's own method.
+ * An example would be datastores.
+ * \note The input frame should never be freed or corrupted during a manipulate callback.
+ * If the callback has the potential to corrupt the frame's data during manipulation,
+ * local data should be used for the manipulation and only copied to the frame on
+ * success.
+ * \note A failure return value indicates that the frame was not manipulated and that
+ * is being returned in its original state.
*/
typedef int (*ast_audiohook_manipulate_callback)(struct ast_audiohook *audiohook, struct ast_channel *chan, struct ast_frame *frame, enum ast_audiohook_direction direction);
Modified: branches/1.6.1/main/audiohook.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.1/main/audiohook.c?view=diff&rev=260052&r1=260051&r2=260052
==============================================================================
--- branches/1.6.1/main/audiohook.c (original)
+++ branches/1.6.1/main/audiohook.c Thu Apr 29 10:37:15 2010
@@ -572,7 +572,29 @@
return frame;
}
-/*! \brief Pass an AUDIO frame off to be handled by the audiohook core
+/*!
+ * \brief Pass an AUDIO frame off to be handled by the audiohook core
+ *
+ * \details
+ * This function has 3 ast_frames and 3 parts to handle each. At the beginning of this
+ * function all 3 frames, start_frame, middle_frame, and end_frame point to the initial
+ * input frame.
+ *
+ * Part_1: Translate the start_frame into SLINEAR audio if it is not already in that
+ * format. The result of this part is middle_frame is guaranteed to be in
+ * SLINEAR format for Part_2.
+ * Part_2: Send middle_frame off to spies and manipulators. At this point middle_frame is
+ * either a new frame as result of the translation, or points directly to the start_frame
+ * because no translation to SLINEAR audio was required. The result of this part
+ * is end_frame will be updated to point to middle_frame if any audiohook manipulation
+ * took place.
+ * Part_3: Translate end_frame's audio back into the format of start frame if necessary.
+ * At this point if middle_frame != end_frame, we are guaranteed that no manipulation
+ * took place and middle_frame can be freed as it was translated... If middle_frame was
+ * not translated and still pointed to start_frame, it would be equal to end_frame as well
+ * regardless if manipulation took place which would not result in this free. The result
+ * of this part is end_frame is guaranteed to be the format of start_frame for the return.
+ *
* \param chan Channel that the list is coming off of
* \param audiohook_list List of audiohooks
* \param direction Direction frame is coming in from
@@ -587,6 +609,7 @@
struct ast_audiohook *audiohook = NULL;
int samples = frame->samples;
+ /* ---Part_1. translate start_frame to SLINEAR if necessary. */
/* If the frame coming in is not signed linear we have to send it through the in_translate path */
if (frame->subclass != AST_FORMAT_SLINEAR) {
if (in_translate->format != frame->subclass) {
@@ -601,6 +624,7 @@
samples = middle_frame->samples;
}
+ /* ---Part_2: Send middle_frame to spy and manipulator lists. middle_frame is guaranteed to be SLINEAR here.*/
/* Queue up signed linear frame to each spy */
AST_LIST_TRAVERSE_SAFE_BEGIN(&audiohook_list->spy_list, audiohook, list) {
ast_audiohook_lock(audiohook);
@@ -654,20 +678,21 @@
audiohook->manipulate_callback(audiohook, chan, NULL, direction);
continue;
}
- /* Feed in frame to manipulation */
+ /* Feed in frame to manipulation. */
if (audiohook->manipulate_callback(audiohook, chan, middle_frame, direction)) {
- ast_frfree(middle_frame);
- middle_frame = NULL;
+ /* XXX IGNORE FAILURE */
+
+ /* If the manipulation fails then the frame will be returned in its original state.
+ * Since there are potentially more manipulator callbacks in the list, no action should
+ * be taken here to exit early. */
}
ast_audiohook_unlock(audiohook);
}
AST_LIST_TRAVERSE_SAFE_END;
- if (middle_frame) {
- end_frame = middle_frame;
- }
- }
-
- /* Now we figure out what to do with our end frame (whether to transcode or not) */
+ end_frame = middle_frame;
+ }
+
+ /* ---Part_3: Decide what to do with the end_frame (whether to transcode or not) */
if (middle_frame == end_frame) {
/* Middle frame was modified and became the end frame... let's see if we need to transcode */
if (end_frame->subclass != start_frame->subclass) {
@@ -692,9 +717,7 @@
}
} else {
/* No frame was modified, we can just drop our middle frame and pass the frame we got in out */
- if (middle_frame) {
- ast_frfree(middle_frame);
- }
+ ast_frfree(middle_frame);
}
return end_frame;
More information about the svn-commits
mailing list