| Message ID | 20200507104749.61667-1-lq@chinaffmpeg.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/hlsenc: move init file write code block to hls_write_header | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Steven Liu: > Signed-off-by: Steven Liu <lq@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
> 2020年5月10日 上午3:21，Andreas Rheinhardt <andreas.rheinhardt@gmail.com> 写道： > > Steven Liu: >> Signed-off-by: Steven Liu <lq@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". Thanks Steven Liu
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);
Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavformat/hlsenc.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-)