[FFmpeg-devel] [RFC][PATCH] avformat/flvdec: avoid reseting eof_reached to 0 silently

[Zhang Rui]

2015-04-12 22:45 GMT+08:00 Michael Niedermayer <michaelni at gmx.at>:
> On Sun, Apr 12, 2015 at 12:00:18PM +0800, Zhang Rui wrote:
>> 2015-04-10 22:04 GMT+08:00 wm4 <nfxjfg at googlemail.com>:
>> > On Fri, 10 Apr 2015 21:17:42 +0800
>> > Zhang Rui <bbcallen at gmail.com> wrote:
>> >>
>> >> This kind of error handling need some more work in aviobuf.c,
>> >> and more advises from ffmpeg developers.
>> >> And i prefer this way than the patch I posted.
>> >
>> > stdio.h does it this way: FILE has an error flag that is set when
>> > something goes wrong.
>>
>> AVIOContext has an error field, too. But I don't think it's enough
>> for EAGAIN situation without some convention.
>> At least, ffplay doesn't show that.
>>
>> >> > Also, why doesn't avio_skip() return an error if the skip count is not
>> >> > 0 and the stream has reached EOF?
>> >>
>> >> The eof handling is quite confusing in ffplay for me. AVERROR_EOF is
>> >> clear enough.
>> >
>> > Well, I have no idea what avio_skip() even returns... it just calls
>> > avio_seek(), which is a goddamn fucked up mess thanks to years of
>> > people adding hacks.
>>
>> Is there any correct direction to fix it?
>>
>> > ffplay probably does it wrong. Wouldn't be surprising. It checks
>> > avio_feof() after a av_read_frame() call, which doesn't look correct.
>> > File EOF has absolutely nothing to do with whether a demuxer still has
>> > data.
>> >
>> > On a side note, I'm not sure whether av_read_frame() returning
>> > AVERROR_EOF is an error at all, or just signals that the end of the
>> > file was reached. The doxygen on this function isn't helpful either.
>>
>> Is there any ideas, or any helpful keywords or threads in mail list archive?
>
> a simple error_count field could be added that way one could easily
> check if the count increased over any series of function call(s)

Good enough for internal use of avio_r8().

> it also could be presented at verbose level by the user application,
> showing how many io errors where encountered which where not fatal

Two problems for application:

1. Which error should be defined as fatal?
For avio_r8(), even an EAGAIN can be a fatal.
The error_count has no more information than error field for application.

2. Nested format, e.g. hls, concatdec.
The error_count field is supposed to be added to AVIOContext.
But if the internal input failed, it's weired to set error to the
outer AVIOContext,
since it has nothing todo with the outer http/file/... protocol.


In my opinion, we could stop returning avio error code directly from
av_read_frame(),
and limit the error code which could return from av_read_frame(), explicitly.
e.g.

// Map various error codes to limited error codes.
int av_read_frame2(AVFormatContext *s, AVPacket *pkt) {
    int ret = av_read_frame(s, pkt);
    switch (ret) {
        case AVERROR_EOS: // end of stream.
        case AVERROR_AGAIN: // error can be recovered.
        case AVERROR_EXIT: // interrupted by user.
        case AVERROR_FAIL: // generic error
            return ret;
        case AVERROR(EAGAIN):
            return AVERROR_AGAIN;
        case AVERROR(EOF):
            return AVERROR_EOS;
        default:
            return AVERROR_FAIL;
    }
}

Detailed error code could be obtained from new API av_get_error(),
as errno/WSAGetLastError() does.