From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] avformat/avisynth: remove atexit() handler
Date: Sun, 7 Jul 2024 15:50:37 +0200
Message-ID: <AS8P250MB0744D0153076A88334F5C6E08FD92@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20240707032511.2714-1-qyot27@gmail.com>
Stephen Hutchinson:
> The atexit() handler in the avisynth demuxer was added because
> there was a conflict in AvxSynth that arose due to their use
> of C++ global objects, particularly in relation to having
> added a logging function relying on log4cpp.
>
> This conflict was responsible for causing a segfault on exit.
> It did not affect Windows with the (at the time) upstream
> AviSynth 2.5 and 2.6, nor does it affect AviSynth+.
>
> Unfortunately, none of this was actually shielded by ifdefs
> indicating the fact it was only needed for AvxSynth, so four
> years ago when AviSynth+ replaced AvxSynth as the handler
> for AviSynth scripts on Unix-like OSes, the fact that the
> atexit handler was no longer necessary was overlooked.
>
> Signed-off-by: Stephen Hutchinson <qyot27@gmail.com>
> ---
> libavformat/avisynth.c | 45 ------------------------------------------
> 1 file changed, 45 deletions(-)
>
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index 625bdf7e3a..5d1ecc0bf6 100644
> --- a/libavformat/avisynth.c
> +++ b/libavformat/avisynth.c
> @@ -115,9 +115,6 @@ typedef struct AviSynthContext {
> int error;
>
> uint32_t flags;
> -
> - /* Linked list pointers. */
> - struct AviSynthContext *next;
> } AviSynthContext;
>
> static const int avs_planes_packed[1] = { 0 };
> @@ -133,15 +130,7 @@ static const int avs_planes_rgba[4] = { AVS_PLANAR_G, AVS_PLANAR_B,
>
> static AVMutex avisynth_mutex = AV_MUTEX_INITIALIZER;
>
> -/* A conflict between C++ global objects, atexit, and dynamic loading requires
> - * us to register our own atexit handler to prevent double freeing. */
> static AviSynthLibrary avs_library;
> -static int avs_atexit_called = 0;
> -
> -/* Linked list of AviSynthContexts. An atexit handler destroys this list. */
> -static AviSynthContext *avs_ctx_list = NULL;
> -
> -static av_cold void avisynth_atexit_handler(void);
>
> static av_cold int avisynth_load_library(void)
> {
> @@ -185,7 +174,6 @@ static av_cold int avisynth_load_library(void)
> LOAD_AVS_FUNC(avs_get_env_property, 1);
> #undef LOAD_AVS_FUNC
>
> - atexit(avisynth_atexit_handler);
> return 0;
>
> fail:
> @@ -214,30 +202,11 @@ static av_cold int avisynth_context_create(AVFormatContext *s)
> }
> }
>
> - if (!avs_ctx_list) {
> - avs_ctx_list = avs;
> - } else {
> - avs->next = avs_ctx_list;
> - avs_ctx_list = avs;
> - }
> -
> return 0;
> }
>
> static av_cold void avisynth_context_destroy(AviSynthContext *avs)
> {
> - if (avs_atexit_called)
> - return;
> -
> - if (avs == avs_ctx_list) {
> - avs_ctx_list = avs->next;
> - } else {
> - AviSynthContext *prev = avs_ctx_list;
> - while (prev->next != avs)
> - prev = prev->next;
> - prev->next = avs->next;
> - }
> -
> if (avs->clip) {
> avs_library.avs_release_clip(avs->clip);
> avs->clip = NULL;
> @@ -248,20 +217,6 @@ static av_cold void avisynth_context_destroy(AviSynthContext *avs)
> }
> }
>
> -static av_cold void avisynth_atexit_handler(void)
> -{
> - AviSynthContext *avs = avs_ctx_list;
> -
> - while (avs) {
> - AviSynthContext *next = avs->next;
> - avisynth_context_destroy(avs);
> - avs = next;
> - }
> - dlclose(avs_library.library);
> -
> - avs_atexit_called = 1;
> -}
> -
> /* Create AVStream from audio and video data. */
> static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
> {
avisynth_context_destroy() is currently always called while holding the
lock (i.e. avisynth_mutex). Is this even necessary? It is clear that
avisynth_load_library() (and the check for whether it should be called)
need the lock, but does anything else (like
avs_create_script_environment) really need it?
- 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".
next prev parent reply other threads:[~2024-07-07 13:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-07 3:25 Stephen Hutchinson
2024-07-07 13:50 ` Andreas Rheinhardt [this message]
2024-07-07 17:46 ` Stephen Hutchinson
2024-07-17 22:10 ` Stephen Hutchinson
2024-07-17 22:05 ` [FFmpeg-devel] [PATCH 2/2] avformat/avisynth: remove mutex lock from read_close Stephen Hutchinson
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=AS8P250MB0744D0153076A88334F5C6E08FD92@AS8P250MB0744.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