> On 18 Apr 2018, at 09:01, Richard Shaffer <[email protected]> wrote: > > On Mon, Apr 16, 2018 at 10:24 PM, Steven Liu <[email protected]> wrote: > >> >> >>> On 16 Apr 2018, at 08:37, Jun Zhao <[email protected]> wrote: >>> >>> >>> >>> On 2018/4/13 20:29, Steven Liu wrote: >>>> 2018-04-13 16:19 GMT+08:00 Jun Zhao <[email protected]>: >>>>> >>>>> On 2018/4/12 16:48, Steven Liu wrote: >>>>>> Signed-off-by: Steven Liu <[email protected]> >>>>>> --- >>>>>> 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 >>>>> [email protected] >>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> [email protected] >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> [email protected] >>> 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 >> [email protected] >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > _______________________________________________ > ffmpeg-devel mailing list > [email protected] > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Thanks Steven _______________________________________________ ffmpeg-devel mailing list [email protected] http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
