[FFmpeg-devel] [PATCH] avformat/hls: remove redundant code
Steven Liu
lq at chinaffmpeg.org
Wed Apr 18 05:46:49 EEST 2018
> 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
More information about the ffmpeg-devel
mailing list