Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Cc: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Subject: [FFmpeg-devel] [PATCH v2 06/27] avcodec/vp9: Fix race when attaching side-data for show-existing frame
Date: Mon,  8 Apr 2024 22:13:44 +0200
Message-ID: <GV1P250MB073711693E037A6ED4C41C208F002@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <GV1P250MB073755270DAD40B30BE913B78F002@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM>

When outputting a show-existing frame, the VP9 decoder simply
created a reference to said frame and returned it immediately to
the caller, without waiting for it to have finished decoding.
In case of frame-threading it is possible for the frame to
only be decoded while it was waiting to be output.
This is normally benign.

But there is one case where it is not: If the user wants
video encoding parameters to be exported, said side data
will only be attached to the src AVFrame at the end of
decoding the frame that is actually being shown. Without
synchronisation adding said side data in the decoder thread
and the reads in av_frame_ref() in the output thread
constitute a data race. This happens e.g. when using the
venc_data_dump tool with vp90-2-10-show-existing-frame.webm
from the FATE-suite.

Fix this by actually waiting for the frame to be output.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/vp9.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index bd52478ce7..e0bc313301 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -1569,6 +1569,8 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
             av_log(avctx, AV_LOG_ERROR, "Requested reference %d not available\n", ref);
             return AVERROR_INVALIDDATA;
         }
+        ff_progress_frame_await(&s->s.refs[ref], INT_MAX);
+
         if ((ret = av_frame_ref(frame, s->s.refs[ref].f)) < 0)
             return ret;
         frame->pts     = pkt->pts;
@@ -1715,10 +1717,8 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
 #endif
         {
             ret = decode_tiles(avctx, data, size);
-            if (ret < 0) {
-                ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
-                return ret;
-            }
+            if (ret < 0)
+                goto fail;
         }
 
         // Sum all counts fields into td[0].counts for tile threading
@@ -1732,18 +1732,19 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
             ff_thread_finish_setup(avctx);
         }
     } while (s->pass++ == 1);
-    ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
 
     if (s->td->error_info < 0) {
         av_log(avctx, AV_LOG_ERROR, "Failed to decode tile data\n");
         s->td->error_info = 0;
-        return AVERROR_INVALIDDATA;
+        ret = AVERROR_INVALIDDATA;
+        goto fail;
     }
     if (avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) {
         ret = vp9_export_enc_params(s, &s->s.frames[CUR_FRAME]);
         if (ret < 0)
-            return ret;
+            goto fail;
     }
+    ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
 
 finish:
     // ref frame setup
@@ -1757,6 +1758,9 @@ finish:
     }
 
     return pkt->size;
+fail:
+    ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
+    return ret;
 }
 
 static void vp9_decode_flush(AVCodecContext *avctx)
-- 
2.40.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  parent reply	other threads:[~2024-04-08 20:15 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API Andreas Rheinhardt
2024-04-09  1:40   ` Michael Niedermayer
2024-04-09  6:35     ` Andreas Rheinhardt
2024-04-10  7:01   ` Anton Khirnov
2024-04-10  7:09     ` Andreas Rheinhardt
2024-04-10  7:59       ` Anton Khirnov
2024-04-10  8:02         ` Andreas Rheinhardt
2024-04-10  8:06           ` Anton Khirnov
2024-04-10  8:09             ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 03/27] avcodec/mimic: Switch to ProgressFrames Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 04/27] avcodec/vp3: " Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 05/27] avcodec/vp9: " Andreas Rheinhardt
2024-04-08 20:13 ` Andreas Rheinhardt [this message]
2024-04-10  7:06   ` [FFmpeg-devel] [PATCH v2 06/27] avcodec/vp9: Fix race when attaching side-data for show-existing frame Anton Khirnov
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 07/27] avcodec/vp9: Reduce wait times Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 08/27] avcodec/vp9: Simplify replacing VP9Frame Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 09/27] avcodec/vp9: Replace atomic_store() by atomic_init() Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 10/27] avcodec/pthread_frame: Add API to share RefStruct refs just once Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 11/27] avcodec/wavpack: Use ThreadProgress API Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 12/27] avcodec/wavpack: Move initializing DSD data to a better place Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 13/27] avcodec/wavpack: Only reset DSD context upon parameter change Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 14/27] avcodec/wavpack: Optimize always-false comparison away Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 15/27] avcodec/wavpack: Move transient variable from context to stack Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 16/27] avcodec/vp8: Convert to ProgressFrame API Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 17/27] avcodec/vp8: Mark flushing functions as av_cold Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 18/27] avcodec/codec_internal: Remove FF_CODEC_CAP_ALLOCATE_PROGRESS Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 19/27] avcodec/hevcdec: Switch to ProgressFrames Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 20/27] avcodec/pngdec: " Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 21/27] avcodec/ffv1dec: " Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 22/27] avcodec/qsv: Use RefStruct API for memory id (mids) array Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 23/27] avcodec/rkmppdec: Fix double-free on error Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 24/27] avcodec/rkmppdec: Check av_buffer_ref() Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 25/27] avcodec/rkmppdec: Use RefStruct API for references to decoder itself Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 26/27] avcodec/rkmppdec: Allocate AVDRMFrameDescriptor and frame ctx jointly Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 27/27] avcodec/v4l2_(m2m|buffers): Use RefStruct API for context references Andreas Rheinhardt
2024-04-09  6:33 ` [FFmpeg-devel] [PATCH v3 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
2024-04-10  6:38 ` [FFmpeg-devel] [PATCH v2 " Anton Khirnov
2024-04-18 20:40 ` Andreas Rheinhardt
2024-04-18 21:30   ` epirat07

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=GV1P250MB073711693E037A6ED4C41C208F002@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM \
    --to=andreas.rheinhardt@outlook.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git