[asterisk-commits] russell: branch russell/translator_frame_fix-1.4 r98853 - in /team/russell/tr...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon Jan 14 15:21:42 CST 2008
Author: russell
Date: Mon Jan 14 15:21:42 2008
New Revision: 98853
URL: http://svn.digium.com/view/asterisk?view=rev&rev=98853
Log:
The bug being addressed here is related to issue #11698.
The issue is that it is possible for a translation frame to still be in use
when the translation path gets destroyed. Specifically, this can happen if
a masquerade happens in between a call to ast_read() and ast_write(), since
the channel is not locked between these two operations.
This hack of a fix is written in such a way that it is API and ABI compatible
with previous versions of Asterisk 1.4.
First, the "has_timing_info" field in ast_frame was change to be a flags field.
A flag was added that gets set when a translator outputs an internal frame.
Then, when a frame gets free'd, if it has this flag set, a hint is given to
the translation core. So, if the translator was requested to be destroyed while
this frame was still in use, it can be destroyed once the frame is sent to
ast_frame_free(), meaning that it is no longer needed.
Modified:
team/russell/translator_frame_fix-1.4/channels/chan_iax2.c
team/russell/translator_frame_fix-1.4/codecs/codec_zap.c
team/russell/translator_frame_fix-1.4/include/asterisk/frame.h
team/russell/translator_frame_fix-1.4/include/asterisk/translate.h
team/russell/translator_frame_fix-1.4/main/abstract_jb.c
team/russell/translator_frame_fix-1.4/main/frame.c
team/russell/translator_frame_fix-1.4/main/rtp.c
team/russell/translator_frame_fix-1.4/main/translate.c
Modified: team/russell/translator_frame_fix-1.4/channels/chan_iax2.c
URL: http://svn.digium.com/view/asterisk/team/russell/translator_frame_fix-1.4/channels/chan_iax2.c?view=diff&rev=98853&r1=98852&r2=98853
==============================================================================
--- team/russell/translator_frame_fix-1.4/channels/chan_iax2.c (original)
+++ team/russell/translator_frame_fix-1.4/channels/chan_iax2.c Mon Jan 14 15:21:42 2008
@@ -1801,7 +1801,7 @@
the IAX thread with the iaxsl lock held. */
struct iax_frame *fr = data;
fr->retrans = -1;
- fr->af.has_timing_info = 0;
+ ast_clear_flag(&fr->af, AST_FRFLAG_HAS_TIMING_INFO);
if (iaxs[fr->callno] && !ast_test_flag(iaxs[fr->callno], IAX_ALREADYGONE))
iax2_queue_frame(fr->callno, &fr->af);
/* Free our iax frame */
Modified: team/russell/translator_frame_fix-1.4/codecs/codec_zap.c
URL: http://svn.digium.com/view/asterisk/team/russell/translator_frame_fix-1.4/codecs/codec_zap.c?view=diff&rev=98853&r1=98852&r2=98853
==============================================================================
--- team/russell/translator_frame_fix-1.4/codecs/codec_zap.c (original)
+++ team/russell/translator_frame_fix-1.4/codecs/codec_zap.c Mon Jan 14 15:21:42 2008
@@ -114,7 +114,6 @@
int lasttotalms;
#endif
struct zt_transcode_header *hdr;
- struct ast_frame f;
};
static int transcoder_show(int fd, int argc, char **argv)
@@ -186,13 +185,14 @@
if (ztp->fake == 2) {
ztp->fake = 1;
- ztp->f.frametype = AST_FRAME_VOICE;
- ztp->f.subclass = 0;
- ztp->f.samples = 160;
- ztp->f.data = NULL;
- ztp->f.offset = 0;
- ztp->f.datalen = 0;
- ztp->f.mallocd = 0;
+ pvt->f.frametype = AST_FRAME_VOICE;
+ pvt->f.subclass = 0;
+ pvt->f.samples = 160;
+ pvt->f.data = NULL;
+ pvt->f.offset = 0;
+ pvt->f.datalen = 0;
+ pvt->f.mallocd = 0;
+ ast_set_flag(&pvt->f, AST_FRFLAG_FROM_TRANSLATOR);
pvt->samples = 0;
} else if (ztp->fake == 1) {
return NULL;
@@ -205,14 +205,15 @@
ztp->lasttotalms = ztp->totalms;
}
#endif
- ztp->f.frametype = AST_FRAME_VOICE;
- ztp->f.subclass = hdr->dstfmt;
- ztp->f.samples = hdr->dstsamples;
- ztp->f.data = hdr->dstdata + hdr->dstoffset;
- ztp->f.offset = hdr->dstoffset;
- ztp->f.datalen = hdr->dstlen;
- ztp->f.mallocd = 0;
- pvt->samples -= ztp->f.samples;
+ pvt->f.frametype = AST_FRAME_VOICE;
+ pvt->f.subclass = hdr->dstfmt;
+ pvt->f.samples = hdr->dstsamples;
+ pvt->f.data = hdr->dstdata + hdr->dstoffset;
+ pvt->f.offset = hdr->dstoffset;
+ pvt->f.datalen = hdr->dstlen;
+ pvt->f.mallocd = 0;
+ ast_set_flag(&pvt->f, AST_FRFLAG_FROM_TRANSLATOR);
+ pvt->samples -= pvt->f.samples;
hdr->dstlen = 0;
} else {
@@ -226,7 +227,7 @@
}
}
- return &ztp->f;
+ return &pvt->f;
}
static void zap_destroy(struct ast_trans_pvt *pvt)
Modified: team/russell/translator_frame_fix-1.4/include/asterisk/frame.h
URL: http://svn.digium.com/view/asterisk/team/russell/translator_frame_fix-1.4/include/asterisk/frame.h?view=diff&rev=98853&r1=98852&r2=98853
==============================================================================
--- team/russell/translator_frame_fix-1.4/include/asterisk/frame.h (original)
+++ team/russell/translator_frame_fix-1.4/include/asterisk/frame.h Mon Jan 14 15:21:42 2008
@@ -123,6 +123,15 @@
};
#define AST_FRAME_DTMF AST_FRAME_DTMF_END
+enum {
+ /*! This frame contains valid timing information */
+ AST_FRFLAG_HAS_TIMING_INFO = (1 << 0),
+ /*! This frame came from a translator and is still the original frame.
+ * The translator can not be free'd if the frame inside of it still has
+ * this flag set. */
+ AST_FRFLAG_FROM_TRANSLATOR = (1 << 1),
+};
+
/*! \brief Data structure associated with a single frame of data
*/
struct ast_frame {
@@ -148,8 +157,8 @@
struct timeval delivery;
/*! For placing in a linked list */
AST_LIST_ENTRY(ast_frame) frame_list;
- /*! Timing data flag */
- int has_timing_info;
+ /*! Misc. frame flags */
+ unsigned int flags;
/*! Timestamp in milliseconds */
long ts;
/*! Length in milliseconds */
Modified: team/russell/translator_frame_fix-1.4/include/asterisk/translate.h
URL: http://svn.digium.com/view/asterisk/team/russell/translator_frame_fix-1.4/include/asterisk/translate.h?view=diff&rev=98853&r1=98852&r2=98853
==============================================================================
--- team/russell/translator_frame_fix-1.4/include/asterisk/translate.h (original)
+++ team/russell/translator_frame_fix-1.4/include/asterisk/translate.h Mon Jan 14 15:21:42 2008
@@ -135,7 +135,14 @@
struct ast_translator *t;
struct ast_frame f; /*!< used in frameout */
int samples; /*!< samples available in outbuf */
- int datalen; /*!< actual space used in outbuf */
+ /*!
+ * \brief actual space used in outbuf
+ *
+ * Also, for the sake of ABI compatability, a magic value of -1 in this
+ * field means that the pvt has been requested to be destroyed, but is
+ * pending destruction until ast_translate_frame_freed() gets called.
+ */
+ int datalen;
void *pvt; /*!< more private data, if any */
char *outbuf; /*!< the useful portion of the buffer */
plc_state_t *plc; /*!< optional plc pointer */
@@ -245,6 +252,20 @@
*/
unsigned int ast_translate_available_formats(unsigned int dest, unsigned int src);
+/*!
+ * \brief Hint that a frame from a translator has been freed
+ *
+ * This is sort of a hack. This function gets called when ast_frame_free() gets
+ * called on a frame that has the AST_FRFLAG_FROM_TRANSLATOR flag set. This is
+ * because it is possible for a translation path to be destroyed while a frame
+ * from a translator is still in use. Specifically, this happens if a masquerade
+ * happens in between calls to ast_read() and ast_write(), since the channel lock
+ * is not held in between those two operations.
+ *
+ * \return nothing
+ */
+void ast_translate_frame_freed(struct ast_frame *fr);
+
#if defined(__cplusplus) || defined(c_plusplus)
}
#endif
Modified: team/russell/translator_frame_fix-1.4/main/abstract_jb.c
URL: http://svn.digium.com/view/asterisk/team/russell/translator_frame_fix-1.4/main/abstract_jb.c?view=diff&rev=98853&r1=98852&r2=98853
==============================================================================
--- team/russell/translator_frame_fix-1.4/main/abstract_jb.c (original)
+++ team/russell/translator_frame_fix-1.4/main/abstract_jb.c Mon Jan 14 15:21:42 2008
@@ -317,10 +317,10 @@
}
/* We consider an enabled jitterbuffer should receive frames with valid timing info. */
- if (!f->has_timing_info || f->len < 2 || f->ts < 0) {
+ if (!ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO) || f->len < 2 || f->ts < 0) {
ast_log(LOG_WARNING, "%s recieved frame with invalid timing info: "
"has_timing_info=%d, len=%ld, ts=%ld, src=%s\n",
- chan->name, f->has_timing_info, f->len, f->ts, f->src);
+ chan->name, ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO), f->len, f->ts, f->src);
return -1;
}
Modified: team/russell/translator_frame_fix-1.4/main/frame.c
URL: http://svn.digium.com/view/asterisk/team/russell/translator_frame_fix-1.4/main/frame.c?view=diff&rev=98853&r1=98852&r2=98853
==============================================================================
--- team/russell/translator_frame_fix-1.4/main/frame.c (original)
+++ team/russell/translator_frame_fix-1.4/main/frame.c Mon Jan 14 15:21:42 2008
@@ -43,6 +43,7 @@
#include "asterisk/utils.h"
#include "asterisk/threadstorage.h"
#include "asterisk/linkedlists.h"
+#include "asterisk/translate.h"
#ifdef TRACE_FRAMES
static int headers;
@@ -368,6 +369,9 @@
#endif
free(fr);
}
+
+ if (ast_test_flag(fr, AST_FRFLAG_FROM_TRANSLATOR))
+ ast_translate_frame_freed(fr);
}
/*!
@@ -379,7 +383,9 @@
{
struct ast_frame *out;
void *newdata;
-
+
+ ast_clear_flag(fr, AST_FRFLAG_FROM_TRANSLATOR);
+
if (!(fr->mallocd & AST_MALLOCD_HDR)) {
/* Allocate a new header if needed */
if (!(out = ast_frame_header_new()))
@@ -391,8 +397,8 @@
out->offset = fr->offset;
out->data = fr->data;
/* Copy the timing data */
- out->has_timing_info = fr->has_timing_info;
- if (fr->has_timing_info) {
+ ast_copy_flags(out, fr, AST_FRFLAG_HAS_TIMING_INFO);
+ if (ast_test_flag(fr, AST_FRFLAG_HAS_TIMING_INFO)) {
out->ts = fr->ts;
out->len = fr->len;
out->seqno = fr->seqno;
@@ -495,7 +501,7 @@
/* Must have space since we allocated for it */
strcpy((char *)out->src, f->src);
}
- out->has_timing_info = f->has_timing_info;
+ ast_copy_flags(out, f, AST_FRFLAG_HAS_TIMING_INFO);
out->ts = f->ts;
out->len = f->len;
out->seqno = f->seqno;
Modified: team/russell/translator_frame_fix-1.4/main/rtp.c
URL: http://svn.digium.com/view/asterisk/team/russell/translator_frame_fix-1.4/main/rtp.c?view=diff&rev=98853&r1=98852&r2=98853
==============================================================================
--- team/russell/translator_frame_fix-1.4/main/rtp.c (original)
+++ team/russell/translator_frame_fix-1.4/main/rtp.c Mon Jan 14 15:21:42 2008
@@ -1310,7 +1310,7 @@
ast_frame_byteswap_be(&rtp->f);
calc_rxstamp(&rtp->f.delivery, rtp, timestamp, mark);
/* Add timing data to let ast_generic_bridge() put the frame into a jitterbuf */
- rtp->f.has_timing_info = 1;
+ ast_set_flag(&rtp->f, AST_FRFLAG_HAS_TIMING_INFO);
rtp->f.ts = timestamp / 8;
rtp->f.len = rtp->f.samples / 8;
} else {
@@ -2657,7 +2657,7 @@
if (rtp->lastts > rtp->lastdigitts)
rtp->lastdigitts = rtp->lastts;
- if (f->has_timing_info)
+ if (ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO))
rtp->lastts = f->ts * 8;
/* Get a pointer to the header */
Modified: team/russell/translator_frame_fix-1.4/main/translate.c
URL: http://svn.digium.com/view/asterisk/team/russell/translator_frame_fix-1.4/main/translate.c?view=diff&rev=98853&r1=98852&r2=98853
==============================================================================
--- team/russell/translator_frame_fix-1.4/main/translate.c (original)
+++ team/russell/translator_frame_fix-1.4/main/translate.c Mon Jan 14 15:21:42 2008
@@ -140,6 +140,18 @@
{
struct ast_translator *t = pvt->t;
+ if (ast_test_flag(&pvt->f, AST_FRFLAG_FROM_TRANSLATOR)) {
+ /* If this flag is still set, that means that the translation path has
+ * been torn down, while we still have a frame out there being used.
+ * When ast_frfree() gets called on that frame, this ast_trans_pvt
+ * will get destroyed, too. */
+
+ /* Set the magic hint that this has been requested to be destroyed. */
+ pvt->datalen = -1;
+
+ return;
+ }
+
if (t->destroy)
t->destroy(pvt);
free(pvt);
@@ -154,7 +166,7 @@
int samples = pvt->samples; /* initial value */
/* Copy the last in jb timing info to the pvt */
- pvt->f.has_timing_info = f->has_timing_info;
+ ast_copy_flags(&pvt->f, f, AST_FRFLAG_HAS_TIMING_INFO);
pvt->f.ts = f->ts;
pvt->f.len = f->len;
pvt->f.seqno = f->seqno;
@@ -233,6 +245,8 @@
f->src = pvt->t->name;
f->data = pvt->outbuf;
+ ast_set_flag(f, AST_FRFLAG_FROM_TRANSLATOR);
+
return f;
}
@@ -311,7 +325,7 @@
long len;
int seqno;
- has_timing_info = f->has_timing_info;
+ has_timing_info = ast_test_flag(f, AST_FRFLAG_HAS_TIMING_INFO);
ts = f->ts;
len = f->len;
seqno = f->seqno;
@@ -361,7 +375,7 @@
path->nextout = ast_tvadd(path->nextout, ast_samp2tv(out->samples, format_rate(out->subclass)));
} else {
out->delivery = ast_tv(0, 0);
- out->has_timing_info = has_timing_info;
+ ast_set2_flag(out, has_timing_info, AST_FRFLAG_HAS_TIMING_INFO);
if (has_timing_info) {
out->ts = ts;
out->len = len;
@@ -948,3 +962,23 @@
return res;
}
+
+void ast_translate_frame_freed(struct ast_frame *fr)
+{
+ struct ast_trans_pvt *pvt, tmp_pvt;
+ int num_bytes;
+ char *p;
+
+ ast_clear_flag(fr, AST_FRFLAG_FROM_TRANSLATOR);
+
+ num_bytes = ((char *) &tmp_pvt.f) - ((char *) &tmp_pvt);
+
+ p = ((char *) fr) - num_bytes;
+
+ pvt = (struct ast_trans_pvt *) p;
+
+ if (pvt->datalen != -1)
+ return;
+
+ destroy(pvt);
+}
More information about the asterisk-commits
mailing list