From patchwork Mon Dec 7 10:08:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Jan_Ekstr=C3=B6m?= X-Patchwork-Id: 24378 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 60E3A44B3F9 for ; Mon, 7 Dec 2020 12:08:58 +0200 (EET) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 295516898E4; Mon, 7 Dec 2020 12:08:58 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-lf1-f67.google.com (mail-lf1-f67.google.com [209.85.167.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7C50D6880C3 for ; Mon, 7 Dec 2020 12:08:51 +0200 (EET) Received: by mail-lf1-f67.google.com with SMTP id a9so17236300lfh.2 for ; Mon, 07 Dec 2020 02:08:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; bh=YqVYgz28pTl9gWCaO/4pkggLsXb7WDy1iAH51Qts14E=; b=eJ0hziR4wHhXsDrfyfuzWTwXoeBVGRSZQV72eso7Gxci/Zgm2VlkfvmKSTtDslGxl5 ItMj32gDjxVoY8PSKME3uO0kC1JglT+9yxuaAxwX3JYM7RgKfjwqSJbSj6vmOrUW5s4W zwbi2ZHftG0h+sZhdnnuNzJwQ9RND6MY5Fr1EmYqOpxQ7TKo3ThL4/6NoE7HXhbD57Kr y5yYIzFxN/G3/GqCTMD+3nAfyc9e/zbPQMvlimNJCjaXnNB7P0G4Qps3XN0EM6jTYoc6 3LlBUfhdvRanq7Aw2D1CJsIdDYB97B1TH5t7ovRLDvn7y0ng7EsJms2VinC0DqbypQX5 nzKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=YqVYgz28pTl9gWCaO/4pkggLsXb7WDy1iAH51Qts14E=; b=B8EhpWfR/ZKDxjvyPDe9UKw4WYfHrRHmkCqgHQ/Gos9qCV9a/pNl44PZhlNnwDzJNI 9sex1kcPIrNcnRTGzC24vVlMNjIMxPLrWh29uUJb+5boNlC3BjyHbg7awIE4gH42u7v2 o4L1Z29YcDIG7LYK8v5OrzbBr7QikjHnv7CmD1EcJ3cMFFsziROxjdQwjujBOc7FzxUA wrPdH2HuDU8b+hodOMBrpiyQ1dRYGtGrLmn9UzGIaQt01bw8HFByeVGMT9vVPx0RwOrN 5ArR4B6+0omOcf1LPtnDuv1QpmD+DFWR2KfCICh0jgFb/AG2qZBARHz0F5CJDIX/ZrNQ qFew== X-Gm-Message-State: AOAM530NlDgwifVZ+QhWvi0i+8yQ6WvZ+fprGBraLd0oLq0CffB65HqE /KDIOaVIZ6AIAlUNJlS3T8X4yrOXt7RZFA== X-Google-Smtp-Source: ABdhPJwezMYVVokx1jI1OToWJ755Qbo+pvBJR/GM7vnZNbteZxRrV3nTxlSRrSjYGbVN1iOjKWNHMQ== X-Received: by 2002:a05:6512:3054:: with SMTP id b20mr7702946lfb.45.1607335730215; Mon, 07 Dec 2020 02:08:50 -0800 (PST) Received: from localhost.localdomain (91-159-194-103.elisa-laajakaista.fi. [91.159.194.103]) by smtp.gmail.com with ESMTPSA id v5sm45061ljj.135.2020.12.07.02.08.49 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Dec 2020 02:08:49 -0800 (PST) From: =?utf-8?q?Jan_Ekstr=C3=B6m?= To: ffmpeg-devel@ffmpeg.org Date: Mon, 7 Dec 2020 12:08:41 +0200 Message-Id: <20201207100845.17520-2-jeebjp@gmail.com> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20201207100845.17520-1-jeebjp@gmail.com> References: <20201207100845.17520-1-jeebjp@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/5] avformat/fifo: always wait recovery_wait_time between recoveries X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" From: Bernard Boulay 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 --- 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;