[FFmpeg-devel] [PATCH] http: Return AVERROR_EOF instead of 0 in some EOF conditions

[Jan Ekström]

On Fri, Nov 13, 2020 at 12:32 AM Martin Storsjö <martin at martin.st> wrote:
>
> IO functions are expected to return AVERROR_EOF instead of 0
> nowadays. This is also expected by other higher level layers
> within the http protocol itself (e.g. the reconnect mechanism).
> ---
>  libavformat/http.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 3d25d652d3..528ea046ef 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -1408,12 +1408,12 @@ static int http_buf_read(URLContext *h, uint8_t *buf, int size)
>              if (!s->chunksize && s->multiple_requests) {
>                  http_get_line(s, line, sizeof(line)); // read empty chunk
>                  s->chunkend = 1;
> -                return 0;
> +                return AVERROR_EOF;
>              }

Trying to read into the context of this change, it seems like this
case is supposed to actually cause a retry, and the earlier if
(s->chunkend) check returns EOF on the following call (
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=91a565e20f0c220a6bc37e58c11cea4a3590149c
).

Quoting the commit: <<EOQ
After this patch, EOF is emulated once a 0-byte final chunk is
received by setting a new flag. This flag is reset in ff_http_do_new_request(),
which is used to make additional requests on the open socket.
EOQ

Of course, if this specific case does not actually require this sort
of loop-de-loop any more then this code can be somewhat simplified,
but otherwise it seems like this return 0 is here for a reason (?).

>              else if (!s->chunksize) {
>                  av_log(h, AV_LOG_DEBUG, "Last chunk received, closing conn\n");
>                  ffurl_closep(&s->hd);
> -                return 0;
> +                return AVERROR_EOF;
>              }

This one seems more correct for an EOF.

Although not knowing the implementations of other protocols the choice
of closing the underlying TLS/TCP connection ASAP is an interesting
one (as it's also done in http_close). But that is unrelated to this
change.

Jan