[FFmpeg-devel] [PATCH] avformat/hlsenc: move init file write code block to hls_write_header

[Andreas Rheinhardt]

Steven Liu:
> Signed-off-by: Steven Liu <lq at chinaffmpeg.org>
> ---
>  libavformat/hlsenc.c | 37 ++++++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 5695c6cc95..a1eddade7b 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -2266,6 +2266,26 @@ static int hls_write_header(AVFormatContext *s)
>                  }
>              }
>          }
> +        if (hls->segment_type == SEGMENT_TYPE_FMP4 && !vs->init_range_length) {
> +            int range_length = 0;
> +            int byterange_mode = (hls->flags & HLS_SINGLE_FILE) || (hls->max_seg_size > 0);
> +            av_write_frame(vs->avf, NULL); /* Flush any buffered data */
> +            avio_tell(vs->avf->pb);
> +            avio_flush(vs->avf->pb);
> +            range_length = avio_close_dyn_buf(vs->avf->pb, &vs->init_buffer);
> +            if (range_length <= 0)
> +                return AVERROR(EINVAL);
> +            avio_write(vs->out, vs->init_buffer, range_length);
> +            if (!hls->resend_init_file)
> +                av_freep(&vs->init_buffer);
> +            vs->init_range_length = range_length;
> +            avio_open_dyn_buf(&vs->avf->pb);
> +            vs->packets_written = 0;
> +            vs->start_pos = range_length;
> +            if (!byterange_mode) {
> +                hlsenc_io_close(s, &vs->out, vs->base_output_dirname);
> +            }
> +        }
>      }
>  
>      return ret;
> @@ -2383,23 +2403,6 @@ static int hls_write_packet(AVFormatContext *s, AVPacket *pkt)
>          new_start_pos = avio_tell(oc->pb);
>          vs->size = new_start_pos - vs->start_pos;
>          avio_flush(oc->pb);
> -        if (hls->segment_type == SEGMENT_TYPE_FMP4) {
> -            if (!vs->init_range_length) {
> -                range_length = avio_close_dyn_buf(oc->pb, &vs->init_buffer);
> -                if (range_length <= 0)
> -                    return AVERROR(EINVAL);
> -                avio_write(vs->out, vs->init_buffer, range_length);
> -                if (!hls->resend_init_file)
> -                    av_freep(&vs->init_buffer);
> -                vs->init_range_length = range_length;
> -                avio_open_dyn_buf(&oc->pb);
> -                vs->packets_written = 0;
> -                vs->start_pos = range_length;
> -                if (!byterange_mode) {
> -                    hlsenc_io_close(s, &vs->out, vs->base_output_dirname);
> -                }
> -            }
> -        }
>          if (!byterange_mode) {
>              if (vs->vtt_avf) {
>                  hlsenc_io_close(s, &vs->vtt_avf->pb, vs->vtt_avf->url);
> 

1. Delaying writing the header was introduced in commit
880b299381de1e66f8248abd6c320c7c490466a2 to fix ticket 6825 (where the
number of channels in the input stream switches). Have you tested
whether your patch breaks said ticket again?
2. You don't need to check for vs->init_range_length being zero when
writing the header as this is automatically fulfilled. (After all, one
calls avformat_write_header() only once.)
3. There is also code in hls_write_trailer() that duplicates the code to
write the init file (should actually have been factored out into a
common function...). It has been added in commit
76b8e42c1f0453215244c45114d5aa302e2add7b to address ticket #7166 and if
you always write the init file in write_header, you should also remove
the code in write_trailer.
4. Commit 880b299381de1e66f8248abd6c320c7c490466a2 also added the
packets_written member; it was initialized to 0 in mp4 mode and to 1 in
non-mp4 mode. Since cff309097a3af4f7f2acb5b5f9fb914ba9ae45c3 this was
set to 1 initially just in order to be reset to 0 a few lines below.
Maybe packets_written should be removed completely? (I made a patch [2]
to remove the currently redundant initializations.)
5. The current way of writing the init header (that you intend to remove
in this commit) has a serious issue: If you have a subtitle and an audio
stream, yet no video stream and if the audio is delayed sufficiently,
then a subtitle packet triggers the output of the mp4 init header. But
subtitles are output directly and not via dynamic buffers and yet the
code presumes that the AVIOContext for output is a dynamic buffer. This
leads to segfaults. (If you remember, this is point 6 in [1].) This
patch seems to fix this; but if you drop this patch because of 1., this
still needs to addressed.

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/254420.html
[2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/262385.html