[FFmpeg-devel] [PATCH] avformat/hls: remove redundant code
Rostislav Pehlivanov
atomnuker at gmail.com
Wed Apr 18 16:50:22 EEST 2018
On 18 April 2018 at 03:46, Steven Liu <lq at chinaffmpeg.org> wrote:
>
>
> > On 18 Apr 2018, at 09:01, Richard Shaffer <rshaffer at tunein.com> wrote:
> >
> > On Mon, Apr 16, 2018 at 10:24 PM, Steven Liu <lq at chinaffmpeg.org> wrote:
> >
> >>
> >>
> >>> On 16 Apr 2018, at 08:37, Jun Zhao <mypopydev at gmail.com> wrote:
> >>>
> >>>
> >>>
> >>> On 2018/4/13 20:29, Steven Liu wrote:
> >>>> 2018-04-13 16:19 GMT+08:00 Jun Zhao <mypopydev at gmail.com>:
> >>>>>
> >>>>> On 2018/4/12 16:48, Steven Liu wrote:
> >>>>>> Signed-off-by: Steven Liu <lq at chinaffmpeg.org>
> >>>>>> ---
> >>>>>> libavformat/hls.c | 27 +++++++++------------------
> >>>>>> 1 file changed, 9 insertions(+), 18 deletions(-)
> >>>>>>
> >>>>>> diff --git a/libavformat/hls.c b/libavformat/hls.c
> >>>>>> index ae0545a086..74f0c2ccc5 100644
> >>>>>> --- a/libavformat/hls.c
> >>>>>> +++ b/libavformat/hls.c
> >>>>>> @@ -945,14 +945,8 @@ static struct segment *next_segment(struct
> >> playlist *pls)
> >>>>>> return pls->segments[n];
> >>>>>> }
> >>>>>>
> >>>>>> -enum ReadFromURLMode {
> >>>>>> - READ_NORMAL,
> >>>>>> - READ_COMPLETE,
> >>>>>> -};
> >>>>>> -
> >>>>>> static int read_from_url(struct playlist *pls, struct segment *seg,
> >>>>>> - uint8_t *buf, int buf_size,
> >>>>>> - enum ReadFromURLMode mode)
> >>>>>> + uint8_t *buf, int buf_size)
> >>>>>> {
> >>>>>> int ret;
> >>>>>>
> >>>>>> @@ -960,12 +954,9 @@ static int read_from_url(struct playlist *pls,
> >> struct segment *seg,
> >>>>>> if (seg->size >= 0)
> >>>>>> buf_size = FFMIN(buf_size, seg->size - pls->cur_seg_offset);
> >>>>>>
> >>>>>> - if (mode == READ_COMPLETE) {
> >>>>>> - ret = avio_read(pls->input, buf, buf_size);
> >>>>>> - if (ret != buf_size)
> >>>>>> - av_log(NULL, AV_LOG_ERROR, "Could not read complete
> >> segment.\n");
> >>>>>> - } else
> >>>>>> - ret = avio_read(pls->input, buf, buf_size);
> >>>>>> + ret = avio_read(pls->input, buf, buf_size);
> >>>>>> + if (ret != buf_size)
> >>>>>> + av_log(NULL, AV_LOG_ERROR, "Could not read complete
> >> segment.\n");
> >>>>>>
> >>>>>> if (ret > 0)
> >>>>>> pls->cur_seg_offset += ret;
> >>>>>> @@ -1085,7 +1076,7 @@ static void intercept_id3(struct playlist
> *pls,
> >> uint8_t *buf,
> >>>>>> while (1) {
> >>>>>> /* see if we can retrieve enough data for ID3 header */
> >>>>>> if (*len < ID3v2_HEADER_SIZE && buf_size >=
> >> ID3v2_HEADER_SIZE) {
> >>>>>> - bytes = read_from_url(pls, seg, buf + *len,
> >> ID3v2_HEADER_SIZE - *len, READ_COMPLETE);
> >>>>>> + bytes = read_from_url(pls, seg, buf + *len,
> >> ID3v2_HEADER_SIZE - *len);
> >>>>>> if (bytes > 0) {
> >>>>>>
> >>>>>> if (bytes == ID3v2_HEADER_SIZE - *len)
> >>>>>> @@ -1137,7 +1128,7 @@ static void intercept_id3(struct playlist
> *pls,
> >> uint8_t *buf,
> >>>>>>
> >>>>>> if (remaining > 0) {
> >>>>>> /* read the rest of the tag in */
> >>>>>> - if (read_from_url(pls, seg, pls->id3_buf +
> >> id3_buf_pos, remaining, READ_COMPLETE) != remaining)
> >>>>>> + if (read_from_url(pls, seg, pls->id3_buf +
> >> id3_buf_pos, remaining) != remaining)
> >>>>>> break;
> >>>>>> id3_buf_pos += remaining;
> >>>>>> av_log(pls->ctx, AV_LOG_DEBUG, "Stripped additional
> >> %d HLS ID3 bytes\n", remaining);
> >>>>>> @@ -1151,7 +1142,7 @@ static void intercept_id3(struct playlist
> *pls,
> >> uint8_t *buf,
> >>>>>>
> >>>>>> /* re-fill buffer for the caller unless EOF */
> >>>>>> if (*len >= 0 && (fill_buf || *len == 0)) {
> >>>>>> - bytes = read_from_url(pls, seg, buf + *len, buf_size -
> *len,
> >> READ_NORMAL);
> >>>>>> + bytes = read_from_url(pls, seg, buf + *len, buf_size -
> *len);
> >>>>>>
> >>>>>> /* ignore error if we already had some data */
> >>>>>> if (bytes >= 0)
> >>>>>> @@ -1311,7 +1302,7 @@ static int update_init_section(struct playlist
> >> *pls, struct segment *seg)
> >>>>>> av_fast_malloc(&pls->init_sec_buf, &pls->init_sec_buf_size,
> >> sec_size);
> >>>>>>
> >>>>>> ret = read_from_url(pls, seg->init_section, pls->init_sec_buf,
> >>>>>> - pls->init_sec_buf_size, READ_COMPLETE);
> >>>>>> + pls->init_sec_buf_size);
> >>>>> Didn't care ret < pls->init_sec_buf_size ?
> >>>> avio_read is full size read, so it will return error, or
> >>>> init_sec_buf_size, as your question, maybe it will happen then the
> >>>> read_from_url called avio_read_partiall.
> >>> Thanks the clarify, I think the patch is Ok now.
> >> pushed
> >>>>>> ff_format_io_close(pls->parent, &pls->input);
> >>>>>>
> >>>>>> if (ret < 0)
> >>>>>> @@ -1506,7 +1497,7 @@ reload:
> >>>>>> }
> >>>>>>
> >>>>>> seg = current_segment(v);
> >>>>>> - ret = read_from_url(v, seg, buf, buf_size, READ_NORMAL);
> >>>>>> + ret = read_from_url(v, seg, buf, buf_size);
> >>>>>> if (ret > 0) {
> >>>>>> if (just_opened && v->is_id3_timestamped != 0) {
> >>>>>> /* Intercept ID3 tags here, elementary audio streams are
> >> required
> >>>>> LGTM except one question.
> >>>>> _______________________________________________
> >>>>> ffmpeg-devel mailing list
> >>>>> ffmpeg-devel at ffmpeg.org
> >>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>> _______________________________________________
> >>>> ffmpeg-devel mailing list
> >>>> ffmpeg-devel at ffmpeg.org
> >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>>
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel at ffmpeg.org
> >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> Thanks
> >> Steven
> >>
> >>
> >> I notice that after this change, the message "Could not read complete
> > segment" appears when reading the end of an HLS segment. This makes
> sense,
> > since the end of the segment will often not align with the buffer size.
> >
> > The code seems to work fine, but the message is annoying. The log message
> > seems to be left over from when the code was changed to use the avio_ API
> > functions instead of ffurl_ functions. (
> > https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/
> 6fbfb20faf22d15c0c92d124e22eb8298fb10dcb)
> > I think maybe the check for ret != buf_size and the log message are no
> > longer useful. Should we remove them?
> ok , i will remove it.
> >
> > -Richard
> >
> >>
> >>
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel at ffmpeg.org
> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> Thanks
> Steven
>
>
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
That code is still present in the release/4.0 branch. You should port this
commit if it needs to be there.
More information about the ffmpeg-devel
mailing list