[FFmpeg-devel] [PATCH 3/4] avformat/hlsenc: use hlsenc_io_* APIs

[Karthick Jeyapal]

On 12/18/17 2:17 PM, Steven Liu wrote:
> Signed-off-by: Steven Liu <lq at 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()
>       }
>       av_freep(&vs->basename);
>       av_freep(&vs->base_output_dirname);