ad_lavc, vd_lavc: return full error codes to shared decoder loop

ad_lavc and vd_lavc use the lavc_process() helper to translate the
FFmpeg push/pull API to the internal filter API (which completely
mismatch, even though I'm responsible for both, just fucking kill me).

This interface was "slightly" too tight. It returned only a bool
indicating "progress", which was not enough to handle some cases (see
following commit).

While we're at it, move all state into a struct. This is only a single
bool, but we get the chance to add more if needed.

This fixes mpv falling asleep if decoding returns an error during
draining. If decoding fails when we already sent EOF, the state machine
stopped making progress. This left mpv just sitting around and doing
nothing.

A test case can be created with: echo $RANDOM >> image.png

This makes libavformat read a proper packet plus a packet of garbage.
libavcodec will decode a frame, and then return an error code. The
lavc_process() wrapper could not deal with this, because there was no
way to differentiate between "retry" and "send new packet". Normally, it
would send a new packet, so decoding would make progress anyway. If
there was "progress", we couldn't just retry, because it'd retry
forever.

This is made worse by the fact that it tries to decode at least two
frames before starting display, meaning it will "sit around and do
nothing" before the picture is displayed.

Change it so that on error return, "receiving" a frame is retried. This
will make it return the EOF, so everything works properly.

This is a high-risk change, because all these funny bullshit exceptions
for hardware decoding are in the way, and I didn't retest them. For
example, if hardware decoding is enabled, it keeps a list of packets,
that are fed into the decoder again if hardware decoding fails, and a
software fallback is performed. Another case of horrifying accidental
complexity.

Fixes: #6618

audio/decode/ad_lavc.c

@@ -48,7 +48,7 @@ struct priv {
     bool preroll_done;
     double next_pts;
     AVRational codec_timebase;
-    bool eof_returned;
+    struct lavc_state state;
 
     struct mp_decoder public;
 };
@@ -159,10 +159,10 @@ static void reset(struct mp_filter *da)
     ctx->trim_samples = 0;
     ctx->preroll_done = false;
     ctx->next_pts = MP_NOPTS_VALUE;
-    ctx->eof_returned = false;
+    ctx->state = (struct lavc_state){0};
 }
 
-static bool send_packet(struct mp_filter *da, struct demux_packet *mpkt)
+static int send_packet(struct mp_filter *da, struct demux_packet *mpkt)
 {
     struct priv *priv = da->priv;
     AVCodecContext *avctx = priv->avctx;
@@ -177,16 +177,12 @@ static bool send_packet(struct mp_filter *da, struct demux_packet *mpkt)
     mp_set_av_packet(&pkt, mpkt, &priv->codec_timebase);
 
     int ret = avcodec_send_packet(avctx, mpkt ? &pkt : NULL);
-
-    if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
-        return false;
-
     if (ret < 0)
         MP_ERR(da, "Error decoding audio.\n");
-    return true;
+    return ret;
 }
 
-static bool receive_frame(struct mp_filter *da, struct mp_frame *out)
+static int receive_frame(struct mp_filter *da, struct mp_frame *out)
 {
     struct priv *priv = da->priv;
     AVCodecContext *avctx = priv->avctx;
@@ -198,7 +194,7 @@ static bool receive_frame(struct mp_filter *da, struct mp_frame *out)
         // over in case we get new packets at some point in the future.
         // (Dont' reset the filter itself, we want to keep other state.)
         avcodec_flush_buffers(priv->avctx);
-        return false;
+        return ret;
     } else if (ret < 0 && ret != AVERROR(EAGAIN)) {
         MP_ERR(da, "Error decoding audio.\n");
     }
@@ -209,14 +205,14 @@ static bool receive_frame(struct mp_filter *da, struct mp_frame *out)
 #endif
 
     if (!priv->avframe->buf[0])
-        return true;
+        return ret;
 
     double out_pts = mp_pts_from_av(priv->avframe->pts, &priv->codec_timebase);
 
     struct mp_aframe *mpframe = mp_aframe_from_avframe(priv->avframe);
     if (!mpframe) {
         MP_ERR(da, "Converting libavcodec frame to mpv frame failed.\n");
-        return true;
+        return ret;
     }
 
     if (priv->force_channel_map.num)
@@ -264,14 +260,14 @@ static bool receive_frame(struct mp_filter *da, struct mp_frame *out)
 
     av_frame_unref(priv->avframe);
 
-    return true;
+    return ret;
 }
 
 static void process(struct mp_filter *ad)
 {
     struct priv *priv = ad->priv;
 
-    lavc_process(ad, &priv->eof_returned, send_packet, receive_frame);
+    lavc_process(ad, &priv->state, send_packet, receive_frame);
 }
 
 static const struct mp_filter_info ad_lavc_filter = {

filters/f_decoder_wrapper.c

@@ -22,6 +22,7 @@
 #include <assert.h>
 
 #include <libavutil/buffer.h>
+#include <libavutil/common.h>
 #include <libavutil/rational.h>
 
 #include "config.h"
@@ -814,22 +815,23 @@ error:
     return NULL;
 }
 
-void lavc_process(struct mp_filter *f, bool *eof_flag,
+void lavc_process(struct mp_filter *f, struct lavc_state *state,
-                  bool (*send)(struct mp_filter *f, struct demux_packet *pkt),
+                  int (*send)(struct mp_filter *f, struct demux_packet *pkt),
-                  bool (*receive)(struct mp_filter *f, struct mp_frame *res))
+                  int (*receive)(struct mp_filter *f, struct mp_frame *res))
 {
     if (!mp_pin_in_needs_data(f->ppins[1]))
         return;
 
     struct mp_frame frame = {0};
-    if (!receive(f, &frame)) {
+    int ret_recv = receive(f, &frame);
-        if (!*eof_flag)
+    if (ret_recv == AVERROR_EOF) {
+        if (!state->eof_returned)
             mp_pin_in_write(f->ppins[1], MP_EOF_FRAME);
-        *eof_flag = true;
+        state->eof_returned = true;
     } else if (frame.type) {
-        *eof_flag = false;
+        state->eof_returned = false;
         mp_pin_in_write(f->ppins[1], frame);
-    } else {
+    } else if (ret_recv == AVERROR(EAGAIN)) {
         // Need to feed a packet.
         frame = mp_pin_out_read(f->ppins[0]);
         struct demux_packet *pkt = NULL;
@@ -843,7 +845,8 @@ void lavc_process(struct mp_filter *f, bool *eof_flag,
             }
             return;
         }
-        if (!send(f, pkt)) {
+        int ret_send = send(f, pkt);
+        if (ret_send == AVERROR(EAGAIN)) {
             // Should never happen, but can happen with broken decoders.
             MP_WARN(f, "could not consume packet\n");
             mp_pin_out_unread(f->ppins[0], frame);
@@ -852,5 +855,8 @@ void lavc_process(struct mp_filter *f, bool *eof_flag,
         }
         talloc_free(pkt);
         mp_filter_internal_mark_progress(f);
+    } else {
+        // Just try again.
+        mp_filter_internal_mark_progress(f);
     }
 }

filters/f_decoder_wrapper.h

@@ -109,11 +109,14 @@ extern const struct mp_decoder_fns vd_lavc;
 extern const struct mp_decoder_fns ad_lavc;
 extern const struct mp_decoder_fns ad_spdif;
 
-// Convenience wrapper for lavc based decoders. eof_flag must be set to false
+// Convenience wrapper for lavc based decoders. Treat lavc_state as private;
-// on init and resets.
+// init to all-0 on init and resets.
-void lavc_process(struct mp_filter *f, bool *eof_flag,
+struct lavc_state {
-                  bool (*send)(struct mp_filter *f, struct demux_packet *pkt),
+    bool eof_returned;
-                  bool (*receive)(struct mp_filter *f, struct mp_frame *res));
+};
+void lavc_process(struct mp_filter *f, struct lavc_state *state,
+                  int (*send)(struct mp_filter *f, struct demux_packet *pkt),
+                  int (*receive)(struct mp_filter *f, struct mp_frame *res));
 
 // ad_spdif.c
 struct mp_decoder_list *select_spdif_codec(const char *codec, const char *pref);

video/decode/vd_lavc.c

@@ -172,7 +172,7 @@ typedef struct lavc_ctx {
     AVRational codec_timebase;
     enum AVDiscard skip_frame;
     bool flushing;
-    bool eof_returned;
+    struct lavc_state state;
     const char *decoder;
     bool hwdec_requested;
     bool hwdec_failed;
@@ -974,17 +974,17 @@ static bool do_send_packet(struct mp_filter *vd, struct demux_packet *pkt)
     AVCodecContext *avctx = ctx->avctx;
 
     if (!prepare_decoding(vd))
-        return false;
+        return AVERROR_UNKNOWN;
 
     if (avctx->skip_frame == AVDISCARD_ALL)
-        return true;
+        return AVERROR(EAGAIN);
 
     AVPacket avpkt;
     mp_set_av_packet(&avpkt, pkt, &ctx->codec_timebase);
 
     int ret = avcodec_send_packet(avctx, pkt ? &avpkt : NULL);
     if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
-        return false;
+        return ret;
 
     if (ctx->hw_probing && ctx->num_sent_packets < 32) {
         pkt = pkt ? demux_copy_packet(pkt) : NULL;
@@ -993,38 +993,41 @@ static bool do_send_packet(struct mp_filter *vd, struct demux_packet *pkt)
 
     if (ret < 0)
         handle_err(vd);
-    return true;
+    return ret;
 }
 
-static bool send_queued(struct mp_filter *vd)
+static int send_queued(struct mp_filter *vd)
 {
     vd_ffmpeg_ctx *ctx = vd->priv;
 
-    while (ctx->num_requeue_packets && do_send_packet(vd, ctx->requeue_packets[0]))
+    while (ctx->num_requeue_packets) {
-    {
+        int ret = do_send_packet(vd, ctx->requeue_packets[0]);
+        if (ret < 0)
+            return ret;
         talloc_free(ctx->requeue_packets[0]);
         MP_TARRAY_REMOVE_AT(ctx->requeue_packets, ctx->num_requeue_packets, 0);
     }
 
-    return ctx->num_requeue_packets == 0;
+    return 0;
 }
 
-static bool send_packet(struct mp_filter *vd, struct demux_packet *pkt)
+static int send_packet(struct mp_filter *vd, struct demux_packet *pkt)
 {
-    if (!send_queued(vd))
+    int ret = send_queued(vd);
+    if (ret < 0)
         return false;
 
     return do_send_packet(vd, pkt);
 }
 
 // Returns whether decoder is still active (!EOF state).
-static bool decode_frame(struct mp_filter *vd)
+static int decode_frame(struct mp_filter *vd)
 {
     vd_ffmpeg_ctx *ctx = vd->priv;
     AVCodecContext *avctx = ctx->avctx;
 
     if (!prepare_decoding(vd))
-        return true;
+        return AVERROR(EAGAIN);
 
     int ret = avcodec_receive_frame(avctx, ctx->pic);
     if (ret == AVERROR_EOF) {
@@ -1034,20 +1037,20 @@ static bool decode_frame(struct mp_filter *vd)
         // the delay queue has been drained.
         if (!ctx->num_delay_queue)
             reset_avctx(vd);
-        return false;
+        return ret;
     } else if (ret < 0 && ret != AVERROR(EAGAIN)) {
         handle_err(vd);
     }
 
     if (!ctx->pic->buf[0])
-        return true;
+        return ret;
 
     ctx->hwdec_fail_count = 0;
 
     struct mp_image *mpi = mp_image_from_av_frame(ctx->pic);
     if (!mpi) {
         av_frame_unref(ctx->pic);
-        return true;
+        return ret;
     }
     assert(mpi->planes[0] || mpi->planes[3]);
     mpi->pts = mp_pts_from_av(ctx->pic->pts, &ctx->codec_timebase);
@@ -1061,14 +1064,14 @@ static bool decode_frame(struct mp_filter *vd)
     av_frame_unref(ctx->pic);
 
     MP_TARRAY_APPEND(ctx, ctx->delay_queue, ctx->num_delay_queue, mpi);
-    return true;
+    return ret;
 }
 
-static bool receive_frame(struct mp_filter *vd, struct mp_frame *out_frame)
+static int receive_frame(struct mp_filter *vd, struct mp_frame *out_frame)
 {
     vd_ffmpeg_ctx *ctx = vd->priv;
 
-    bool progress = decode_frame(vd);
+    int ret = decode_frame(vd);
 
     if (ctx->hwdec_failed) {
         // Failed hardware decoding? Try again in software.
@@ -1083,21 +1086,21 @@ static bool receive_frame(struct mp_filter *vd, struct mp_frame *out_frame)
         ctx->num_requeue_packets = num_pkts;
 
         send_queued(vd);
-        progress = decode_frame(vd);
+        ret = decode_frame(vd);
     }
 
     if (!ctx->num_delay_queue)
-        return progress;
+        return ret;
 
-    if (ctx->num_delay_queue <= ctx->max_delay_queue && progress)
+    if (ctx->num_delay_queue <= ctx->max_delay_queue && ret >= 0)
-        return true;
+        return AVERROR(EAGAIN);
 
     struct mp_image *res = ctx->delay_queue[0];
     MP_TARRAY_REMOVE_AT(ctx->delay_queue, ctx->num_delay_queue, 0);
 
     res = res ? mp_img_swap_to_native(res) : NULL;
     if (!res)
-        return progress;
+        return ret;
 
     if (ctx->use_hwdec && ctx->hwdec.copying && res->hwctx) {
         struct mp_image *sw = mp_image_hw_download(res, ctx->hwdec_swpool);
@@ -1107,7 +1110,7 @@ static bool receive_frame(struct mp_filter *vd, struct mp_frame *out_frame)
             MP_ERR(vd, "Could not copy back hardware decoded frame.\n");
             ctx->hwdec_fail_count = INT_MAX - 1; // force fallback
             handle_err(vd);
-            return true;
+            return ret;
         }
     }
 
@@ -1129,7 +1132,7 @@ static bool receive_frame(struct mp_filter *vd, struct mp_frame *out_frame)
     }
 
     *out_frame = MAKE_FRAME(MP_FRAME_VIDEO, res);
-    return true;
+    return ret;
 }
 
 static int control(struct mp_filter *vd, enum dec_ctrl cmd, void *arg)
@@ -1169,7 +1172,7 @@ static void process(struct mp_filter *vd)
 {
     vd_ffmpeg_ctx *ctx = vd->priv;
 
-    lavc_process(vd, &ctx->eof_returned, send_packet, receive_frame);
+    lavc_process(vd, &ctx->state, send_packet, receive_frame);
 }
 
 static void reset(struct mp_filter *vd)
@@ -1178,7 +1181,7 @@ static void reset(struct mp_filter *vd)
 
     flush_all(vd);
 
-    ctx->eof_returned = false;
+    ctx->state = (struct lavc_state){0};
     ctx->framedrop_flags = 0;
 }