> 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_*
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
