diff options
| -rw-r--r-- | include/asterisk/file.h | 15 | ||||
| -rw-r--r-- | include/asterisk/frame.h | 4 | ||||
| -rw-r--r-- | main/file.c | 84 | ||||
| -rw-r--r-- | main/frame.c | 3 |
4 files changed, 38 insertions, 68 deletions
diff --git a/include/asterisk/file.h b/include/asterisk/file.h index ed75c1f9b..0f33b5340 100644 --- a/include/asterisk/file.h +++ b/include/asterisk/file.h @@ -315,21 +315,6 @@ off_t ast_tellstream(struct ast_filestream *fs); */ struct ast_frame *ast_readframe(struct ast_filestream *s); -/*!\brief destroy a filestream using an ast_frame as input - * - * This is a hack that is used also by the ast_trans_pvt and - * ast_dsp structures. When a structure contains an ast_frame - * pointer as one of its fields. It may be that the frame is - * still used after the outer structure is freed. This leads to - * invalid memory accesses. This function allows for us to hold - * off on destroying the ast_filestream until we are done using - * the ast_frame pointer that is part of it - * - * \param fr The ast_frame that is part of an ast_filestream we wish - * to free. - */ -void ast_filestream_frame_freed(struct ast_frame *fr); - #if defined(__cplusplus) || defined(c_plusplus) } #endif diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h index 9d90a083f..b1e7344d2 100644 --- a/include/asterisk/frame.h +++ b/include/asterisk/frame.h @@ -134,10 +134,6 @@ enum { * The dsp cannot be free'd if the frame inside of it still has * this flag set. */ AST_FRFLAG_FROM_DSP = (1 << 2), - /*! This frame came from a filestream and is still the original frame. - * The filestream cannot be free'd if the frame inside of it still has - * this flag set. */ - AST_FRFLAG_FROM_FILESTREAM = (1 << 3), }; /*! \brief Data structure associated with a single frame of data diff --git a/main/file.c b/main/file.c index 5063b7356..853bdc797 100644 --- a/main/file.c +++ b/main/file.c @@ -686,17 +686,36 @@ struct ast_filestream *ast_openvstream(struct ast_channel *chan, const char *fil return NULL; } -struct ast_frame *ast_readframe(struct ast_filestream *s) +static struct ast_frame *read_frame(struct ast_filestream *s, int *whennext) { - struct ast_frame *f = NULL; - int whennext = 0; - if (s && s->fmt) - f = s->fmt->read(s, &whennext); - if (f) { - ast_set_flag(f, AST_FRFLAG_FROM_FILESTREAM); - ao2_ref(s, +1); + struct ast_frame *fr, *new_fr; + + if (!s || !s->fmt) { + return NULL; + } + + if (!(fr = s->fmt->read(s, whennext))) { + return NULL; + } + + if (!(new_fr = ast_frisolate(fr))) { + ast_frfree(fr); + return NULL; + } + + if (new_fr != fr) { + ast_frfree(fr); + fr = new_fr; } - return f; + + return fr; +} + +struct ast_frame *ast_readframe(struct ast_filestream *s) +{ + int whennext = 0; + + return read_frame(s, &whennext); } enum fsread_res { @@ -713,15 +732,13 @@ static enum fsread_res ast_readaudio_callback(struct ast_filestream *s) while (!whennext) { struct ast_frame *fr; - - if (s->orig_chan_name && strcasecmp(s->owner->name, s->orig_chan_name)) + + if (s->orig_chan_name && strcasecmp(s->owner->name, s->orig_chan_name)) { goto return_failure; - - fr = s->fmt->read(s, &whennext); - if (fr) { - ast_set_flag(fr, AST_FRFLAG_FROM_FILESTREAM); - ao2_ref(s, +1); } + + fr = read_frame(s, &whennext); + if (!fr /* stream complete */ || ast_write(s->owner, fr) /* error writing */) { if (fr) { ast_log(LOG_WARNING, "Failed to write frame\n"); @@ -729,10 +746,12 @@ static enum fsread_res ast_readaudio_callback(struct ast_filestream *s) } goto return_failure; } + if (fr) { ast_frfree(fr); } } + if (whennext != s->lasttimeout) { #ifdef HAVE_DAHDI if (s->owner->timingfd > -1) { @@ -782,11 +801,8 @@ static enum fsread_res ast_readvideo_callback(struct ast_filestream *s) int whennext = 0; while (!whennext) { - struct ast_frame *fr = s->fmt->read(s, &whennext); - if (fr) { - ast_set_flag(fr, AST_FRFLAG_FROM_FILESTREAM); - ao2_ref(s, +1); - } + struct ast_frame *fr = read_frame(s, &whennext); + if (!fr /* stream complete */ || ast_write(s->owner, fr) /* error writing */) { if (fr) { ast_log(LOG_WARNING, "Failed to write frame\n"); @@ -795,6 +811,7 @@ static enum fsread_res ast_readvideo_callback(struct ast_filestream *s) s->owner->vstreamid = -1; return FSREAD_FAILURE; } + if (fr) { ast_frfree(fr); } @@ -886,20 +903,6 @@ int ast_closestream(struct ast_filestream *f) } } - if (ast_test_flag(&f->fr, AST_FRFLAG_FROM_FILESTREAM)) { - /* If this flag is still set, it essentially means that the reference - * count of f is non-zero. We can't destroy this filestream until - * whatever is using the filestream's frame has finished. - * - * Since this was called, however, we need to remove the reference from - * when this filestream was first allocated. That way, when the embedded - * frame is freed, the refcount will reach 0 and we can finish destroying - * this filestream properly. - */ - ao2_ref(f, -1); - return 0; - } - ao2_ref(f, -1); return 0; } @@ -1315,17 +1318,6 @@ int ast_waitstream_exten(struct ast_channel *c, const char *context) -1, -1, context); } -void ast_filestream_frame_freed(struct ast_frame *fr) -{ - struct ast_filestream *fs; - - ast_clear_flag(fr, AST_FRFLAG_FROM_FILESTREAM); - - fs = (struct ast_filestream *) (((char *) fr) - offsetof(struct ast_filestream, fr)); - - ao2_ref(fs, -1); -} - /* * if the file name is non-empty, try to play it. * Return 0 if success, -1 if error, digit if interrupted by a digit. diff --git a/main/frame.c b/main/frame.c index 1bc4dca76..a3795bd70 100644 --- a/main/frame.c +++ b/main/frame.c @@ -334,8 +334,6 @@ static void __frame_free(struct ast_frame *fr, int cache) ast_translate_frame_freed(fr); } else if (ast_test_flag(fr, AST_FRFLAG_FROM_DSP)) { ast_dsp_frame_freed(fr); - } else if (ast_test_flag(fr, AST_FRFLAG_FROM_FILESTREAM)) { - ast_filestream_frame_freed(fr); } if (!fr->mallocd) @@ -424,7 +422,6 @@ struct ast_frame *ast_frisolate(struct ast_frame *fr) } else { ast_clear_flag(fr, AST_FRFLAG_FROM_TRANSLATOR); ast_clear_flag(fr, AST_FRFLAG_FROM_DSP); - ast_clear_flag(fr, AST_FRFLAG_FROM_FILESTREAM); out = fr; } |