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
Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavc/threadframe: allow decoders to attach buffers to ThreadFrame
Date: Tue, 15 Mar 2022 14:59:14 +0100
Message-ID: <AS1PR01MB95645E43889E39694E86D0648F109@AS1PR01MB9564.eurprd01.prod.exchangelabs.com> (raw)
In-Reply-To: <20220315104932.25496-1-anton@khirnov.net>

Anton Khirnov:
> This may be useful for synchronizing side data that only becomes
> available after ff_thread_finish_setup() is called.
> ---
>  libavcodec/pthread_frame.c | 1 +
>  libavcodec/threadframe.h   | 3 +++
>  libavcodec/utils.c         | 7 +++++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index 33b5a2e628..4da3832942 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -1138,6 +1138,7 @@ fail:
>  void ff_thread_release_ext_buffer(AVCodecContext *avctx, ThreadFrame *f)
>  {
>      av_buffer_unref(&f->progress);
> +    av_buffer_unref(&f->priv_buf);
>      f->owner[0] = f->owner[1] = NULL;
>      ff_thread_release_buffer(avctx, f->f);
>  }
> diff --git a/libavcodec/threadframe.h b/libavcodec/threadframe.h
> index dea4dadc6d..c2ddc2969f 100644
> --- a/libavcodec/threadframe.h
> +++ b/libavcodec/threadframe.h
> @@ -30,6 +30,9 @@ typedef struct ThreadFrame {
>      // progress->data is an array of 2 ints holding progress for top/bottom
>      // fields
>      AVBufferRef *progress;
> +
> +    /* arbitrary user data propagated along with the frame */
> +    AVBufferRef *priv_buf;
>  } ThreadFrame;
>  
>  /**
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 066da76e16..cc2c2715b3 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -881,6 +881,12 @@ int ff_thread_ref_frame(ThreadFrame *dst, const ThreadFrame *src)
>          return AVERROR(ENOMEM);
>      }
>  
> +    if (src->priv_buf) {
> +        dst->priv_buf = av_buffer_ref(src->priv_buf);
> +        if (!dst->priv_buf)
> +            return AVERROR(ENOMEM);
> +    }
> +
>      return 0;
>  }
>  
> @@ -913,6 +919,7 @@ void ff_thread_release_ext_buffer(AVCodecContext *avctx, ThreadFrame *f)
>      f->owner[0] = f->owner[1] = NULL;
>      if (f->f)
>          av_frame_unref(f->f);
> +    av_buffer_unref(&f->priv_buf);
>  }
>  
>  void ff_thread_finish_setup(AVCodecContext *avctx)

This approach has the downside that you have to add the priv_buf before
ff_thread_finish_setup(). So in case it is not apparent initially
whether one needs this one is forced to add it (even if it turns out not
to be needed); it will also necessitate two av_buffer_ref() in. A better
approach would be to replace the progress array (of two atomic ints)
with a struct containing these two atomic ints and whatever data needs
to be shared. The owner should logically also be part of this struct,
yet I could not figure out if this is compatible with current h264dec
last time I looked at this (when I wrote the patchset containing
02220b88fc38ef9dd4f2d519f5d3e4151258b60c); the current way of doing
things allows different threads to have different opinions about the
ownership of the frames.

(My actual aim with this patchset was to move the AVFrame into the
aforementioned structure like so:

struct {
    atomic_int progress[2];
    AVFrame *f;
};

This would avoid the need for the av_frame_ref() in
ff_thread_ref_frame(); therefore all the frame's properties could be set
directly on the AVFrame by its owner as long as the frame is not
finished. The non-owners could read the data subject to the current
limitations (i.e. they have to wait for progress) and could read
anything after the frame is finished (progress == INT_MAX; codecs could
of course use their own semantics here if they wished).
There were two reasons why I didn't finish this approach: 1. How to
synchronize in case of two owners? (Happens only for h264dec.) 2. This
would add an av_frame_alloc() for every frame, even when not using frame
threading. The latter can be easily avoided, but avoiding this with
frame-threading would require a smarter pool-implementation. And I hate
to use/extend the AVBuffer-API for something for which it is simply the
wrong tool and use something that does not require an allocation for
every ref. (It would nevertheless have been advantageous
allocation-wise, because one saves the allocations implicit in
av_frame_ref().))

- Andreas
_______________________________________________
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:[~2022-03-15 13:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-15 10:49 Anton Khirnov
2022-03-15 10:49 ` [FFmpeg-devel] [PATCH 2/2] lavc/vp9: fix the race in exporting video enc params Anton Khirnov
2022-03-15 13:59 ` Andreas Rheinhardt [this message]

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=AS1PR01MB95645E43889E39694E86D0648F109@AS1PR01MB9564.eurprd01.prod.exchangelabs.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