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

[Steven Liu]

> 2020年5月10日 上午3:21，Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道：
> 
> 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).
Ok, I get it, this is a good point.

> 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.
I Get your point. 
Let me think about how to move it out of hls_write_packet,
Maybe hlsenc cannot support LowLatency HLS if it always in hls_write_packet.
Or maybe need modify some code in movenc, I want try only modify in hlsenc.
> 
> - Andreas
> 
> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/254420.html
> [2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/262385.html
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

Thanks

Steven Liu