> On 19 Dec 2017, at 12:11, Karthick Jeyapal <kjeya...@akamai.com> wrote: > > > > On 12/19/17 9:29 AM, 刘歧 wrote: >> >>> On 19 Dec 2017, at 11:55, Karthick Jeyapal <kjeya...@akamai.com> wrote: >>> >>> >>> >>> On 12/18/17 2:17 PM, Steven Liu wrote: >>>> Signed-off-by: Steven Liu <l...@chinaffmpeg.org> >>>> --- >>>> libavformat/hlsenc.c | 23 ++++++++++++----------- >>>> 1 file changed, 12 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c >>>> index 0eebcb4462..0cb75ff198 100644 >>>> --- a/libavformat/hlsenc.c >>>> +++ b/libavformat/hlsenc.c >>>> @@ -440,7 +440,7 @@ static int hls_delete_old_segments(AVFormatContext *s, >>>> HLSContext *hls, >>>> av_dict_set(&options, "method", "DELETE", 0); >>>> if ((ret = vs->avf->io_open(vs->avf, &out, path, >>>> AVIO_FLAG_WRITE, &options)) < 0) >>>> goto fail; >>>> - ff_format_io_close(vs->avf, &out); >>>> + hlsenc_io_close(vs->avf, &out, path); >>> Will not actually close, when http_persistent is 1. I think it is better to >>> leave this as ff_format_io_close >>>> } else if (unlink(path) < 0) { >>>> av_log(hls, AV_LOG_ERROR, "failed to delete old segment %s: >>>> %s\n", >>>> path, strerror(errno)); >>>> @@ -463,7 +463,7 @@ static int hls_delete_old_segments(AVFormatContext *s, >>>> HLSContext *hls, >>>> av_free(sub_path); >>>> goto fail; >>>> } >>>> - ff_format_io_close(vs->avf, &out); >>>> + hlsenc_io_close(vs->avf, &out, sub_path); >>> Will not actually close, when http_persistent is 1. >>>> } else if (unlink(sub_path) < 0) { >>>> av_log(hls, AV_LOG_ERROR, "failed to delete old segment >>>> %s: %s\n", >>>> sub_path, strerror(errno)); >>>> @@ -556,8 +556,10 @@ static int do_encrypt(AVFormatContext *s, >>>> VariantStream *vs) >>>> } >>>> ff_data_to_hex(hls->key_string, key, sizeof(key), 0); >>>> - if ((ret = s->io_open(s, &pb, hls->key_file, AVIO_FLAG_WRITE, >>>> NULL)) < 0) >>>> - return ret; >>>> + ret = hlsenc_io_open(s, &pb, hls->key_file, NULL); >>> We needn't call hlsenc_io_open if we are not planning to use a persistent >>> connection for it. In this case pb is uninitialized and hlsenc_io_open will >>> most probably cause a crash or undefined behavior. You can get around that >>> issue by initializing pb to NULL. But I think that is unnecessary and are >>> better placed with s->io_open(). >>>> + if (ret < 0) { >>>> + return ret;; >>> Extra semicolon >>>> + } >>>> avio_seek(pb, 0, SEEK_CUR); >>>> avio_write(pb, key, KEYSIZE); >>>> avio_close(pb); >>>> @@ -588,7 +590,7 @@ static int hls_encryption_start(AVFormatContext *s) >>>> ff_get_line(pb, hls->iv_string, sizeof(hls->iv_string)); >>>> hls->iv_string[strcspn(hls->iv_string, "\r\n")] = '\0'; >>>> - ff_format_io_close(s, &pb); >>>> + hlsenc_io_close(s, &pb, hls->key_info_file); >>>> if (!*hls->key_uri) { >>>> av_log(hls, AV_LOG_ERROR, "no key URI specified in key info >>>> file\n"); >>>> @@ -606,7 +608,7 @@ static int hls_encryption_start(AVFormatContext *s) >>>> } >>>> ret = avio_read(pb, key, sizeof(key)); >>>> - ff_format_io_close(s, &pb); >>>> + hlsenc_io_close(s, &pb, hls->key_file); >>> Will not actually close, when http_persistent is 1. >>>> if (ret != sizeof(key)) { >>>> av_log(hls, AV_LOG_ERROR, "error reading key file %s\n", >>>> hls->key_file); >>>> if (ret >= 0 || ret == AVERROR_EOF) >>>> @@ -1812,7 +1814,6 @@ static int hls_write_packet(AVFormatContext *s, >>>> AVPacket *pkt) >>>> vs->init_range_length = range_length; >>>> avio_open_dyn_buf(&oc->pb); >>>> vs->packets_written = 0; >>>> - ff_format_io_close(s, &vs->out); >>>> hlsenc_io_close(s, &vs->out, vs->base_output_dirname); >>>> } >>>> } else { >>>> @@ -1845,7 +1846,7 @@ static int hls_write_packet(AVFormatContext *s, >>>> AVPacket *pkt) >>>> if (ret < 0) { >>>> return ret; >>>> } >>>> - ff_format_io_close(s, &vs->out); >>>> + hlsenc_io_close(s, &vs->out, vs->avf->filename); >>>> } >>>> ret = hls_append_segment(s, hls, vs, vs->duration, vs->start_pos, >>>> vs->size); >>>> vs->start_pos = new_start_pos; >>>> @@ -1925,14 +1926,14 @@ static int hls_write_trailer(struct >>>> AVFormatContext *s) >>>> if (ret < 0) { >>>> return ret; >>>> } >>>> - ff_format_io_close(s, &vs->out); >>>> + hlsenc_io_close(s, &vs->out, vs->avf->filename); >>> Will not actually close, when http_persistent is 1. hls_write_trailer >>> should always call ff_format_io_close() >>>> } >>>> av_write_trailer(oc); >>>> if (oc->pb) { >>>> vs->size = avio_tell(vs->avf->pb) - vs->start_pos; >>>> if (hls->segment_type != SEGMENT_TYPE_FMP4) >>>> - ff_format_io_close(s, &oc->pb); >>>> + hlsenc_io_close(s, &oc->pb, oc->filename); >>> Will not actually close, when http_persistent is 1. hls_write_trailer >>> should always call ff_format_io_close() >>>> if ((hls->flags & HLS_TEMP_FILE) && oc->filename[0]) { >>>> hls_rename_temp_file(s, oc); >>>> @@ -1948,7 +1949,7 @@ static int hls_write_trailer(struct AVFormatContext >>>> *s) >>>> if (vtt_oc->pb) >>>> av_write_trailer(vtt_oc); >>>> vs->size = avio_tell(vs->vtt_avf->pb) - vs->start_pos; >>>> - ff_format_io_close(s, &vtt_oc->pb); >>>> + hlsenc_io_close(s, &vtt_oc->pb, vtt_oc->filename); >>> Will not actually close, when http_persistent is 1. hls_write_trailer >>> should always call ff_format_io_close() >> I think, indent code to only one is better than more, so remove the >> hlsenc_io_* and all use hlsenc_io_*, >> I choose use hlsenc_io_* and improve hlsenc_io_* > I agree having a common function for io_* is better. But I think in this case > trying to make this common func, could make the code for hlsenc_io_close() > and its callers a bit more complicated. Maybe I am wrong here. You can try a > shot at it. > But if you choose that case, then hlsenc_io_close should be first improved to > handle the all the use-cases, before pushing this patch. Otherwise this patch > will break the existing behavior and cause resource/memory leaks.
I see you call hlsenc_io_open and hlsenc_io_close in m3u8 file and http_persistent 1, Is that have no resource/memory leaks? > > Thanks and regards, > Karthick >> >> >> Thanks >> >> Steven >>>> } >>>> av_freep(&vs->basename); >>>> av_freep(&vs->base_output_dirname); >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel