| Message ID | 20180412084835.26565-1-lq@chinaffmpeg.org |
|---|---|
| State | Accepted |
| Commit | 6fbfb20faf22d15c0c92d124e22eb8298fb10dcb |
| Headers | show |
On 2018/4/12 16:48, Steven Liu wrote: > Signed-off-by: Steven Liu <lq@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 ? > 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.
2018-04-13 16:19 GMT+08:00 Jun Zhao <mypopydev@gmail.com>: > > > On 2018/4/12 16:48, Steven Liu wrote: >> Signed-off-by: Steven Liu <lq@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. >> 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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 2018/4/13 20:29, Steven Liu wrote: > 2018-04-13 16:19 GMT+08:00 Jun Zhao <mypopydev@gmail.com>: >> >> On 2018/4/12 16:48, Steven Liu wrote: >>> Signed-off-by: Steven Liu <lq@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. >>> 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@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> On 16 Apr 2018, at 08:37, Jun Zhao <mypopydev@gmail.com> wrote: > > > > On 2018/4/13 20:29, Steven Liu wrote: >> 2018-04-13 16:19 GMT+08:00 Jun Zhao <mypopydev@gmail.com>: >>> >>> On 2018/4/12 16:48, Steven Liu wrote: >>>> Signed-off-by: Steven Liu <lq@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@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Thanks Steven
On Mon, Apr 16, 2018 at 10:24 PM, Steven Liu <lq@chinaffmpeg.org> wrote: > > > > On 16 Apr 2018, at 08:37, Jun Zhao <mypopydev@gmail.com> wrote: > > > > > > > > On 2018/4/13 20:29, Steven Liu wrote: > >> 2018-04-13 16:19 GMT+08:00 Jun Zhao <mypopydev@gmail.com>: > >>> > >>> On 2018/4/12 16:48, Steven Liu wrote: > >>>> Signed-off-by: Steven Liu <lq@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@ffmpeg.org > >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@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? -Richard > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
> On 18 Apr 2018, at 09:01, Richard Shaffer <rshaffer@tunein.com> wrote: > > On Mon, Apr 16, 2018 at 10:24 PM, Steven Liu <lq@chinaffmpeg.org> wrote: > >> >> >>> On 16 Apr 2018, at 08:37, Jun Zhao <mypopydev@gmail.com> wrote: >>> >>> >>> >>> On 2018/4/13 20:29, Steven Liu wrote: >>>> 2018-04-13 16:19 GMT+08:00 Jun Zhao <mypopydev@gmail.com>: >>>>> >>>>> On 2018/4/12 16:48, Steven Liu wrote: >>>>>> Signed-off-by: Steven Liu <lq@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@ffmpeg.org >>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@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@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Thanks Steven
On 18 April 2018 at 03:46, Steven Liu <lq@chinaffmpeg.org> wrote: > > > > On 18 Apr 2018, at 09:01, Richard Shaffer <rshaffer@tunein.com> wrote: > > > > On Mon, Apr 16, 2018 at 10:24 PM, Steven Liu <lq@chinaffmpeg.org> wrote: > > > >> > >> > >>> On 16 Apr 2018, at 08:37, Jun Zhao <mypopydev@gmail.com> wrote: > >>> > >>> > >>> > >>> On 2018/4/13 20:29, Steven Liu wrote: > >>>> 2018-04-13 16:19 GMT+08:00 Jun Zhao <mypopydev@gmail.com>: > >>>>> > >>>>> On 2018/4/12 16:48, Steven Liu wrote: > >>>>>> Signed-off-by: Steven Liu <lq@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@ffmpeg.org > >>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >>>> _______________________________________________ > >>>> ffmpeg-devel mailing list > >>>> ffmpeg-devel@ffmpeg.org > >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >>> > >>> _______________________________________________ > >>> ffmpeg-devel mailing list > >>> ffmpeg-devel@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@ffmpeg.org > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > Thanks > Steven > > > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@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.
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); 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
Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavformat/hls.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)