[FFmpeg-devel] [PATCH 1/5] avformat/fifo: always wait recovery_wait_time between recoveries

[Jan Ekström]

From: Bernard Boulay <bernard.boulay at 24i.com>

With various formats/protocols - for example fragmented mp4 over
HTTP - success/failure might not appear right away. The header and
first packet might get successfully written into the muxer, but
a response of failure might appear only N packets later.

In such a case, a recovery attempt would always be the "first time",
and it would thus be executed without delay, causing effectively a
DoS towards the server on the other side of the connection.

Thus, modify the logic of the module so that it takes into account
the last time recovery was attempted - even if it seemed to be
successful:
1. Refactor update of last_recovery_ts to its own helper function.
2. Call this function even if the recovery is successful, thus
   ensuring that the time is always set.
3. Move initialization of last_recovery_ts to be handled together
   with the thread initialization.
4. Do not limit sleeping in recovery based on recovery_nr being
   nonzero ("not the first time of attempted recovery").

Signed-off-by: Jan Ekström <jan.ekstrom at 24i.com>
---
 libavformat/fifo.c | 54 +++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/libavformat/fifo.c b/libavformat/fifo.c
index 17748e94ce5..55cbf14ae93 100644
--- a/libavformat/fifo.c
+++ b/libavformat/fifo.c
@@ -279,15 +279,10 @@ static void free_message(void *msg)
         av_packet_unref(&fifo_msg->pkt);
 }
 
-static int fifo_thread_process_recovery_failure(FifoThreadContext *ctx, AVPacket *pkt,
-                                                int err_no)
+static void fifo_thread_set_last_recovery_ts(FifoThreadContext *ctx, AVPacket *pkt)
 {
     AVFormatContext *avf = ctx->avf;
     FifoContext *fifo = avf->priv_data;
-    int ret;
-
-    av_log(avf, AV_LOG_INFO, "Recovery failed: %s\n",
-           av_err2str(err_no));
 
     if (fifo->recovery_wait_streamtime) {
         if (pkt->pts == AV_NOPTS_VALUE)
@@ -297,6 +292,19 @@ static int fifo_thread_process_recovery_failure(FifoThreadContext *ctx, AVPacket
     } else {
         ctx->last_recovery_ts = av_gettime_relative();
     }
+}
+
+static int fifo_thread_process_recovery_failure(FifoThreadContext *ctx, AVPacket *pkt,
+                                                int err_no)
+{
+    AVFormatContext *avf = ctx->avf;
+    FifoContext *fifo = avf->priv_data;
+    int ret;
+
+    av_log(avf, AV_LOG_INFO, "Recovery failed: %s\n",
+           av_err2str(err_no));
+
+    fifo_thread_set_last_recovery_ts(ctx, pkt);
 
     if (fifo->max_recovery_attempts &&
         ctx->recovery_nr >= fifo->max_recovery_attempts) {
@@ -329,27 +337,22 @@ static int fifo_thread_attempt_recovery(FifoThreadContext *ctx, FifoMessage *msg
         ctx->header_written = 0;
     }
 
-    if (!ctx->recovery_nr) {
-        ctx->last_recovery_ts = fifo->recovery_wait_streamtime ?
-                                AV_NOPTS_VALUE : 0;
-    } else {
-        if (fifo->recovery_wait_streamtime) {
-            if (ctx->last_recovery_ts == AV_NOPTS_VALUE) {
-                AVRational tb = avf->streams[pkt->stream_index]->time_base;
-                time_since_recovery = av_rescale_q(pkt->pts - ctx->last_recovery_ts,
-                                                   tb, AV_TIME_BASE_Q);
-            } else {
-                /* Enforce recovery immediately */
-                time_since_recovery = fifo->recovery_wait_time;
-            }
+    if (fifo->recovery_wait_streamtime) {
+        if (ctx->last_recovery_ts == AV_NOPTS_VALUE) {
+            AVRational tb = avf->streams[pkt->stream_index]->time_base;
+            time_since_recovery = av_rescale_q(pkt->pts - ctx->last_recovery_ts,
+                                               tb, AV_TIME_BASE_Q);
         } else {
-            time_since_recovery = av_gettime_relative() - ctx->last_recovery_ts;
+            /* Enforce recovery immediately */
+            time_since_recovery = fifo->recovery_wait_time;
         }
-
-        if (time_since_recovery < fifo->recovery_wait_time)
-            return AVERROR(EAGAIN);
+    } else {
+        time_since_recovery = av_gettime_relative() - ctx->last_recovery_ts;
     }
 
+    if (time_since_recovery < fifo->recovery_wait_time)
+        return AVERROR(EAGAIN);
+
     ctx->recovery_nr++;
 
     if (fifo->max_recovery_attempts) {
@@ -372,6 +375,7 @@ static int fifo_thread_attempt_recovery(FifoThreadContext *ctx, FifoMessage *msg
         }
     } else {
         av_log(avf, AV_LOG_INFO, "Recovery successful\n");
+        fifo_thread_set_last_recovery_ts(ctx, pkt);
         ctx->recovery_nr = 0;
     }
 
@@ -389,7 +393,7 @@ static int fifo_thread_recover(FifoThreadContext *ctx, FifoMessage *msg, int err
     int ret;
 
     do {
-        if (!fifo->recovery_wait_streamtime && ctx->recovery_nr > 0) {
+        if (!fifo->recovery_wait_streamtime) {
             int64_t time_since_recovery = av_gettime_relative() - ctx->last_recovery_ts;
             int64_t time_to_wait = FFMAX(0, fifo->recovery_wait_time - time_since_recovery);
             if (time_to_wait)
@@ -420,6 +424,8 @@ static void *fifo_consumer_thread(void *data)
     memset(&fifo_thread_ctx, 0, sizeof(FifoThreadContext));
     fifo_thread_ctx.avf = avf;
     fifo_thread_ctx.last_received_dts = AV_NOPTS_VALUE;
+    fifo_thread_ctx.last_recovery_ts = fifo->recovery_wait_streamtime ?
+                                       AV_NOPTS_VALUE : 0;
 
     while (1) {
         uint8_t just_flushed = 0;
-- 
2.29.2