On 03/24/2016 09:51 PM, Marton Balint wrote:
On Thu, 24 Mar 2016, Jan Sebechlebsky wrote:
Calling close_slave in case error is to be returned from open_slave
will free allocated resources.
Since failure can happen before bsfs array is initialized,
close_slave must check that bsfs is not NULL before accessing
tee_slave->bsfs[i] element.
Signed-off-by: Jan Sebechlebsky <sebechlebsky...@gmail.com>
---
libavformat/tee.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/libavformat/tee.c b/libavformat/tee.c
index 09551b3..e43ef08 100644
--- a/libavformat/tee.c
+++ b/libavformat/tee.c
@@ -141,12 +141,14 @@ static void close_slave(TeeSlave* tee_slave)
unsigned i;
avf = tee_slave->avf;
- for (i=0; i < avf->nb_streams; ++i) {
- AVBitStreamFilterContext *bsf_next, *bsf = tee_slave->bsfs[i];
- while (bsf) {
- bsf_next = bsf->next;
- av_bitstream_filter_close(bsf);
- bsf = bsf_next;
+ if (tee_slave->bsfs) {
+ for (i=0; i < avf->nb_streams; ++i) {
+ AVBitStreamFilterContext *bsf_next, *bsf =
tee_slave->bsfs[i];
+ while (bsf) {
+ bsf_next = bsf->next;
+ av_bitstream_filter_close(bsf);
+ bsf = bsf_next;
+ }
}
}
av_freep(&tee_slave->stream_map);
@@ -198,6 +200,7 @@ static int open_slave(AVFormatContext *avf, char
*slave, TeeSlave *tee_slave)
ret = avformat_alloc_output_context2(&avf2, NULL, format, filename);
if (ret < 0)
goto end;
+ tee_slave->avf = avf2;
av_dict_copy(&avf2->metadata, avf->metadata, 0);
avf2->opaque = avf->opaque;
avf2->io_open = avf->io_open;
@@ -277,7 +280,6 @@ static int open_slave(AVFormatContext *avf, char
*slave, TeeSlave *tee_slave)
goto end;
}
- tee_slave->avf = avf2;
tee_slave->bsfs = av_calloc(avf2->nb_streams, sizeof(TeeSlave));
if (!tee_slave->bsfs) {
ret = AVERROR(ENOMEM);
@@ -292,7 +294,8 @@ static int open_slave(AVFormatContext *avf, char
*slave, TeeSlave *tee_slave)
av_log(avf, AV_LOG_ERROR,
"Specifier separator in '%s' is '%c', but only
characters '%s' "
"are allowed\n", entry->key, *spec,
slave_bsfs_spec_sep);
- return AVERROR(EINVAL);
+ ret = AVERROR(EINVAL);
+ goto end;
}
spec++; /* consume separator */
}
@@ -337,6 +340,9 @@ static int open_slave(AVFormatContext *avf, char
*slave, TeeSlave *tee_slave)
}
end:
+ if ( ret < 0 ){
+ close_slave(tee_slave);
+ }
Do you really need to call close_slave here? As far as I see if
open_slave fails then the failure path of tee_write_header will call
close_slaves, so the streams will be closed, therefore no need to do
it here.
You're right, actually at this point I don't need to call close_slave
here. But it will be needed in the next commit, since close_slaves
function skips already dead slaves and then in case of failure of a
slave which is allowed to fail, this one would not be freed.
I'll move this to the next commit to have consistent changes in each one.
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Have a nice day
Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
