[FFmpeg-devel] [PATCH 1/2 v2] avformat/avisynth: remove atexit() handler

[FFmpeg-devel] [PATCH 1/2 v2] avformat/avisynth: remove atexit() handler

[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>
---
Changes compared to v1:
* Added missing dlclose invocation to read_close

 libavformat/avisynth.c | 46 +-----------------------------------------
 1 file changed, 1 insertion(+), 45 deletions(-)

diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 625bdf7e3a..26747671c0 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)
 {
@@ -1134,6 +1089,7 @@ static av_cold int avisynth_read_close(AVFormatContext *s)
         return AVERROR_UNKNOWN;
 
     avisynth_context_destroy(s->priv_data);
+    dlclose(avs_library.library);
     ff_mutex_unlock(&avisynth_mutex);
     return 0;
 }
-- 
2.43.0

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/2 v2] avformat/avisynth: remove mutex lock from avisynth_read_close

[Stephen Hutchinson]

Signed-off-by: Stephen Hutchinson <qyot27@gmail.com>
---
Changes compared to v1:
* Adjusted for patch #1's inclusion of dlclose

 libavformat/avisynth.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 26747671c0..b01263e010 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -1085,12 +1085,8 @@ static int avisynth_read_packet(AVFormatContext *s, AVPacket *pkt)
 
 static av_cold int avisynth_read_close(AVFormatContext *s)
 {
-    if (ff_mutex_lock(&avisynth_mutex))
-        return AVERROR_UNKNOWN;
-
     avisynth_context_destroy(s->priv_data);
     dlclose(avs_library.library);
-    ff_mutex_unlock(&avisynth_mutex);
     return 0;
 }
 
-- 
2.43.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/2 v2] avformat/avisynth: remove mutex lock from avisynth_read_close

[Stephen Hutchinson]

On 7/19/24 12:56 PM, Stephen Hutchinson wrote:
> Signed-off-by: Stephen Hutchinson <qyot27@gmail.com>
> ---
> Changes compared to v1:
> * Adjusted for patch #1's inclusion of dlclose
> 
>   libavformat/avisynth.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index 26747671c0..b01263e010 100644
> --- a/libavformat/avisynth.c
> +++ b/libavformat/avisynth.c
> @@ -1085,12 +1085,8 @@ static int avisynth_read_packet(AVFormatContext *s, AVPacket *pkt)
>   
>   static av_cold int avisynth_read_close(AVFormatContext *s)
>   {
> -    if (ff_mutex_lock(&avisynth_mutex))
> -        return AVERROR_UNKNOWN;
> -
>       avisynth_context_destroy(s->priv_data);
>       dlclose(avs_library.library);
> -    ff_mutex_unlock(&avisynth_mutex);
>       return 0;
>   }
>   

Pushed.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2 v2] avformat/avisynth: remove atexit() handler

[Stephen Hutchinson]

On 7/19/24 12:56 PM, Stephen Hutchinson wrote:
> 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>
> ---
> Changes compared to v1:
> * Added missing dlclose invocation to read_close
> 
>   libavformat/avisynth.c | 46 +-----------------------------------------
>   1 file changed, 1 insertion(+), 45 deletions(-)
> 
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index 625bdf7e3a..26747671c0 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)
>   {
> @@ -1134,6 +1089,7 @@ static av_cold int avisynth_read_close(AVFormatContext *s)
>           return AVERROR_UNKNOWN;
>   
>       avisynth_context_destroy(s->priv_data);
> +    dlclose(avs_library.library);
>       ff_mutex_unlock(&avisynth_mutex);
>       return 0;
>   }

Since there's not been any more feedback, I'll push both of these
refined patches in a day or two.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2 v2] avformat/avisynth: remove atexit() handler

[Ramiro Polla]

On Fri, Jul 19, 2024 at 7:51 PM Stephen Hutchinson <qyot27@gmail.com> wrote:
>
> 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>
> ---
> Changes compared to v1:
> * Added missing dlclose invocation to read_close
>
>  libavformat/avisynth.c | 46 +-----------------------------------------
>  1 file changed, 1 insertion(+), 45 deletions(-)
>
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index 625bdf7e3a..26747671c0 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)
>  {
> @@ -1134,6 +1089,7 @@ static av_cold int avisynth_read_close(AVFormatContext *s)
>          return AVERROR_UNKNOWN;
>
>      avisynth_context_destroy(s->priv_data);
> +    dlclose(avs_library.library);

Maybe it's best to wrap this around an if (avs_library.library).

[...]

Besides that, the patch looks ok.

Ramiro
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2 v2] avformat/avisynth: remove atexit() handler

[Stephen Hutchinson]

On 7/28/24 9:38 AM, Ramiro Polla wrote:
>> @@ -1134,6 +1089,7 @@ static av_cold int avisynth_read_close(AVFormatContext *s)
>>           return AVERROR_UNKNOWN;
>>
>>       avisynth_context_destroy(s->priv_data);
>> +    dlclose(avs_library.library);
> 
> Maybe it's best to wrap this around an if (avs_library.library).
> 

True. I had tried toying around with C11 _Thread_local since we now
use C17 as the default std, and in that case there was what I presume
was a double free happening that required adding a check for whether
avs_library.library still existed.  As that hadn't been happening prior
to that test I didn't really think much of it, but yeah, it would
make sense to check it anyway.

I abandoned that general idea after 1) finding out that while GCC and
Clang are fine, MSVC doesn't seem to yet (or if it does, only the
most recent versions of MSVC 2022 do) 2) C23 renamed it to thread_local,
and most importantly, 3) I probably hadn't quite used it entirely
correctly, because while the script could be parsed and played back just
fine, trying to encode anything from it would segfault.

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2 v2] avformat/avisynth: remove atexit() handler

[Stephen Hutchinson]

On 7/28/24 11:59 AM, Stephen Hutchinson wrote:
> On 7/28/24 9:38 AM, Ramiro Polla wrote:
>>> @@ -1134,6 +1089,7 @@ static av_cold int 
>>> avisynth_read_close(AVFormatContext *s)
>>>           return AVERROR_UNKNOWN;
>>>
>>>       avisynth_context_destroy(s->priv_data);
>>> +    dlclose(avs_library.library);
>>
>> Maybe it's best to wrap this around an if (avs_library.library).
>>
> 
> True. I had tried toying around with C11 _Thread_local since we now
> use C17 as the default std, and in that case there was what I presume
> was a double free happening that required adding a check for whether
> avs_library.library still existed.  As that hadn't been happening prior
> to that test I didn't really think much of it, but yeah, it would
> make sense to check it anyway.
> 
> I abandoned that general idea after 1) finding out that while GCC and
> Clang are fine, MSVC doesn't seem to yet (or if it does, only the
> most recent versions of MSVC 2022 do) 2) C23 renamed it to thread_local,
> and most importantly, 3) I probably hadn't quite used it entirely
> correctly, because while the script could be parsed and played back just
> fine, trying to encode anything from it would segfault.
> 

I had been ready to push the first two patches two weeks ago, but
the addition of #s 3 and 4 delayed that so I could test a bit more.
I didn't run into any issues with the two newer patches either, so
I'll wait another couple days for any additional comments before
pushing the whole set.
_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/4] avformat/avisynth: remove library allocation from global state

[Stephen Hutchinson]

As part of this, the mutexes are no longer necessary, and
avisynth_read_close needs to check that avs->avs_library.library
still exists before it attempts to call avisynth_context_destroy
and dlclose.

Signed-off-by: Stephen Hutchinson <qyot27@gmail.com>
---
 libavformat/avisynth.c | 133 ++++++++++++++++++++---------------------
 1 file changed, 64 insertions(+), 69 deletions(-)

diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index b01263e010..b91d77b8d8 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -115,6 +115,7 @@ typedef struct AviSynthContext {
     int error;
 
     uint32_t flags;
+    struct AviSynthLibrary avs_library;
 } AviSynthContext;
 
 static const int avs_planes_packed[1] = { 0 };
@@ -128,20 +129,16 @@ static const int avs_planes_yuva[4]   = { AVS_PLANAR_Y, AVS_PLANAR_U,
 static const int avs_planes_rgba[4]   = { AVS_PLANAR_G, AVS_PLANAR_B,
                                           AVS_PLANAR_R, AVS_PLANAR_A };
 
-static AVMutex avisynth_mutex = AV_MUTEX_INITIALIZER;
-
-static AviSynthLibrary avs_library;
-
-static av_cold int avisynth_load_library(void)
+static av_cold int avisynth_load_library(AviSynthContext *avs)
 {
-    avs_library.library = dlopen(AVISYNTH_LIB, RTLD_NOW | RTLD_LOCAL);
-    if (!avs_library.library)
+    avs->avs_library.library = dlopen(AVISYNTH_LIB, RTLD_NOW | RTLD_LOCAL);
+    if (!avs->avs_library.library)
         return AVERROR_UNKNOWN;
 
 #define LOAD_AVS_FUNC(name, continue_on_fail)                          \
-        avs_library.name = (name ## _func)                             \
-                           dlsym(avs_library.library, #name);          \
-        if (!continue_on_fail && !avs_library.name)                    \
+        avs->avs_library.name = (name ## _func)                             \
+                           dlsym(avs->avs_library.library, #name);          \
+        if (!continue_on_fail && !avs->avs_library.name)                    \
             goto fail;
 
     LOAD_AVS_FUNC(avs_bit_blt, 0);
@@ -177,7 +174,7 @@ static av_cold int avisynth_load_library(void)
     return 0;
 
 fail:
-    dlclose(avs_library.library);
+    dlclose(avs->avs_library.library);
     return AVERROR_UNKNOWN;
 }
 
@@ -189,13 +186,13 @@ static av_cold int avisynth_context_create(AVFormatContext *s)
     AviSynthContext *avs = s->priv_data;
     int ret;
 
-    if (!avs_library.library)
-        if (ret = avisynth_load_library())
+    if (!avs->avs_library.library)
+        if (ret = avisynth_load_library(avs))
             return ret;
 
-    avs->env = avs_library.avs_create_script_environment(3);
-    if (avs_library.avs_get_error) {
-        const char *error = avs_library.avs_get_error(avs->env);
+    avs->env = avs->avs_library.avs_create_script_environment(3);
+    if (avs->avs_library.avs_get_error) {
+        const char *error = avs->avs_library.avs_get_error(avs->env);
         if (error) {
             av_log(s, AV_LOG_ERROR, "%s\n", error);
             return AVERROR_UNKNOWN;
@@ -208,11 +205,11 @@ static av_cold int avisynth_context_create(AVFormatContext *s)
 static av_cold void avisynth_context_destroy(AviSynthContext *avs)
 {
     if (avs->clip) {
-        avs_library.avs_release_clip(avs->clip);
+        avs->avs_library.avs_release_clip(avs->clip);
         avs->clip = NULL;
     }
     if (avs->env) {
-        avs_library.avs_delete_script_environment(avs->env);
+        avs->avs_library.avs_delete_script_environment(avs->env);
         avs->env = NULL;
     }
 }
@@ -486,17 +483,17 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
      * version 9 at the minimum.  Technically, 8.1 works, but the time
      * distance between 8.1 and 9 is very small, so just restrict it to 9. */
 
-    if (avs_library.avs_get_version(avs->clip) >= 9) {
+    if (avs->avs_library.avs_get_version(avs->clip) >= 9) {
 
-        frame  = avs_library.avs_get_frame(avs->clip, 0);
-        avsmap = avs_library.avs_get_frame_props_ro(avs->env, frame);
+        frame  = avs->avs_library.avs_get_frame(avs->clip, 0);
+        avsmap = avs->avs_library.avs_get_frame_props_ro(avs->env, frame);
 
         /* Field order */
         if(avs->flags & AVISYNTH_FRAMEPROP_FIELD_ORDER) {
-            if(avs_library.avs_prop_get_type(avs->env, avsmap, "_FieldBased") == AVS_PROPTYPE_UNSET) {
+            if(avs->avs_library.avs_prop_get_type(avs->env, avsmap, "_FieldBased") == AVS_PROPTYPE_UNSET) {
                 st->codecpar->field_order = AV_FIELD_UNKNOWN;
             } else {
-                switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_FieldBased", 0, &error)) {
+                switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_FieldBased", 0, &error)) {
                 case 0:
                     st->codecpar->field_order = AV_FIELD_PROGRESSIVE;
                     break;
@@ -514,10 +511,10 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
 
         /* Color Range */
         if(avs->flags & AVISYNTH_FRAMEPROP_RANGE) {
-            if(avs_library.avs_prop_get_type(avs->env, avsmap, "_ColorRange") == AVS_PROPTYPE_UNSET) {
+            if(avs->avs_library.avs_prop_get_type(avs->env, avsmap, "_ColorRange") == AVS_PROPTYPE_UNSET) {
                 st->codecpar->color_range = AVCOL_RANGE_UNSPECIFIED;
             } else {
-                switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_ColorRange", 0, &error)) {
+                switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_ColorRange", 0, &error)) {
                 case 0:
                     st->codecpar->color_range = AVCOL_RANGE_JPEG;
                     break;
@@ -532,7 +529,7 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
 
         /* Color Primaries */
         if(avs->flags & AVISYNTH_FRAMEPROP_PRIMARIES) {
-            switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Primaries", 0, &error)) {
+            switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_Primaries", 0, &error)) {
             case 1:
                 st->codecpar->color_primaries = AVCOL_PRI_BT709;
                 break;
@@ -576,7 +573,7 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
 
         /* Color Transfer Characteristics */
         if(avs->flags & AVISYNTH_FRAMEPROP_TRANSFER) {
-            switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Transfer", 0, &error)) {
+            switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_Transfer", 0, &error)) {
             case 1:
                 st->codecpar->color_trc = AVCOL_TRC_BT709;
                 break;
@@ -635,10 +632,10 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
 
         /* Matrix coefficients */
         if(avs->flags & AVISYNTH_FRAMEPROP_MATRIX) {
-            if(avs_library.avs_prop_get_type(avs->env, avsmap, "_Matrix") == AVS_PROPTYPE_UNSET) {
+            if(avs->avs_library.avs_prop_get_type(avs->env, avsmap, "_Matrix") == AVS_PROPTYPE_UNSET) {
                 st->codecpar->color_space = AVCOL_SPC_UNSPECIFIED;
             } else {
-                switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Matrix", 0, &error)) {
+                switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_Matrix", 0, &error)) {
                 case 0:
                     st->codecpar->color_space = AVCOL_SPC_RGB;
                     break;
@@ -689,10 +686,10 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
 
         /* Chroma Location */
         if(avs->flags & AVISYNTH_FRAMEPROP_CHROMA_LOCATION) {
-            if(avs_library.avs_prop_get_type(avs->env, avsmap, "_ChromaLocation") == AVS_PROPTYPE_UNSET) {
+            if(avs->avs_library.avs_prop_get_type(avs->env, avsmap, "_ChromaLocation") == AVS_PROPTYPE_UNSET) {
                 st->codecpar->chroma_location = AVCHROMA_LOC_UNSPECIFIED;
             } else {
-                switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_ChromaLocation", 0, &error)) {
+                switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_ChromaLocation", 0, &error)) {
                 case 0:
                     st->codecpar->chroma_location = AVCHROMA_LOC_LEFT;
                     break;
@@ -719,12 +716,12 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
 
         /* Sample aspect ratio */
         if(avs->flags & AVISYNTH_FRAMEPROP_SAR) {
-            sar_num = avs_library.avs_prop_get_int(avs->env, avsmap, "_SARNum", 0, &error);
-            sar_den = avs_library.avs_prop_get_int(avs->env, avsmap, "_SARDen", 0, &error);
+            sar_num = avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_SARNum", 0, &error);
+            sar_den = avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_SARDen", 0, &error);
             st->sample_aspect_ratio = (AVRational){ sar_num, sar_den };
         }
 
-        avs_library.avs_release_video_frame(frame);
+        avs->avs_library.avs_release_video_frame(frame);
     } else {
         st->codecpar->field_order = AV_FIELD_UNKNOWN;
         /* AviSynth works with frame-based video, detecting field order can
@@ -753,9 +750,9 @@ static int avisynth_create_stream_audio(AVFormatContext *s, AVStream *st)
     st->duration                        = avs->vi->num_audio_samples;
     avpriv_set_pts_info(st, 64, 1, avs->vi->audio_samples_per_second);
 
-    if (avs_library.avs_get_version(avs->clip) >= 10)
+    if (avs->avs_library.avs_get_version(avs->clip) >= 10)
         av_channel_layout_from_mask(&st->codecpar->ch_layout,
-                                    avs_library.avs_get_channel_mask(avs->vi));
+                                    avs->avs_library.avs_get_channel_mask(avs->vi));
 
     switch (avs->vi->sample_type) {
     case AVS_SAMPLE_INT8:
@@ -817,13 +814,13 @@ static int avisynth_open_file(AVFormatContext *s)
     if (ret = avisynth_context_create(s))
         return ret;
 
-    if (!avs_library.avs_check_version(avs->env, 7)) {
+    if (!avs->avs_library.avs_check_version(avs->env, 7)) {
         AVS_Value args[] = {
             avs_new_value_string(s->url),
             avs_new_value_bool(1) // filename is in UTF-8
         };
-        val = avs_library.avs_invoke(avs->env, "Import",
-                                     avs_new_value_array(args, 2), 0);
+        val = avs->avs_library.avs_invoke(avs->env, "Import",
+                                          avs_new_value_array(args, 2), 0);
     } else {
         AVS_Value arg;
 #ifdef _WIN32
@@ -837,7 +834,7 @@ static int avisynth_open_file(AVFormatContext *s)
 #else
         arg = avs_new_value_string(s->url);
 #endif
-        val = avs_library.avs_invoke(avs->env, "Import", arg, 0);
+        val = avs->avs_library.avs_invoke(avs->env, "Import", arg, 0);
 #ifdef _WIN32
         av_free(filename_ansi);
 #endif
@@ -854,14 +851,14 @@ static int avisynth_open_file(AVFormatContext *s)
         goto fail;
     }
 
-    avs->clip = avs_library.avs_take_clip(val, avs->env);
-    avs->vi   = avs_library.avs_get_video_info(avs->clip);
+    avs->clip = avs->avs_library.avs_take_clip(val, avs->env);
+    avs->vi   = avs->avs_library.avs_get_video_info(avs->clip);
 
     /* On Windows, FFmpeg supports AviSynth interface version 6 or higher.
      * This includes AviSynth 2.6 RC1 or higher, and AviSynth+ r1718 or higher,
      * and excludes 2.5 and the 2.6 alphas. */
 
-    if (avs_library.avs_get_version(avs->clip) < 6) {
+    if (avs->avs_library.avs_get_version(avs->clip) < 6) {
         av_log(s, AV_LOG_ERROR,
                "AviSynth version is too old. Please upgrade to either AviSynth 2.6 >= RC1 or AviSynth+ >= r1718.\n");
         ret = AVERROR_UNKNOWN;
@@ -869,7 +866,7 @@ static int avisynth_open_file(AVFormatContext *s)
     }
 
     /* Release the AVS_Value as it will go out of scope. */
-    avs_library.avs_release_value(val);
+    avs->avs_library.avs_release_value(val);
 
     if (ret = avisynth_create_stream(s))
         goto fail;
@@ -917,7 +914,7 @@ static int avisynth_read_packet_video(AVFormatContext *s, AVPacket *pkt,
     if (discard)
         return 0;
 
-    bits = avs_library.avs_bits_per_pixel(avs->vi);
+    bits = avs->avs_library.avs_bits_per_pixel(avs->vi);
 
     /* Without the cast to int64_t, calculation overflows at about 9k x 9k
      * resolution. */
@@ -934,8 +931,8 @@ static int avisynth_read_packet_video(AVFormatContext *s, AVPacket *pkt,
     pkt->duration = 1;
     pkt->stream_index = avs->curr_stream;
 
-    frame = avs_library.avs_get_frame(avs->clip, n);
-    error = avs_library.avs_clip_get_error(avs->clip);
+    frame = avs->avs_library.avs_get_frame(avs->clip, n);
+    error = avs->avs_library.avs_clip_get_error(avs->clip);
     if (error) {
         av_log(s, AV_LOG_ERROR, "%s\n", error);
         avs->error = 1;
@@ -946,26 +943,26 @@ static int avisynth_read_packet_video(AVFormatContext *s, AVPacket *pkt,
     dst_p = pkt->data;
     for (i = 0; i < avs->n_planes; i++) {
         plane = avs->planes[i];
-        src_p = avs_library.avs_get_read_ptr_p(frame, plane);
-        pitch = avs_library.avs_get_pitch_p(frame, plane);
+        src_p = avs->avs_library.avs_get_read_ptr_p(frame, plane);
+        pitch = avs->avs_library.avs_get_pitch_p(frame, plane);
 
-        rowsize     = avs_library.avs_get_row_size_p(frame, plane);
-        planeheight = avs_library.avs_get_height_p(frame, plane);
+        rowsize     = avs->avs_library.avs_get_row_size_p(frame, plane);
+        planeheight = avs->avs_library.avs_get_height_p(frame, plane);
 
         /* Flip RGB video. */
-        if (avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR)   ||
-            avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR48) ||
-            avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR64)) {
+        if (avs->avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR)   ||
+            avs->avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR48) ||
+            avs->avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR64)) {
             src_p = src_p + (planeheight - 1) * pitch;
             pitch = -pitch;
         }
 
-        avs_library.avs_bit_blt(avs->env, dst_p, rowsize, src_p, pitch,
-                                 rowsize, planeheight);
+        avs->avs_library.avs_bit_blt(avs->env, dst_p, rowsize, src_p, pitch,
+                                     rowsize, planeheight);
         dst_p += rowsize * planeheight;
     }
 
-    avs_library.avs_release_video_frame(frame);
+    avs->avs_library.avs_release_video_frame(frame);
     return 0;
 }
 
@@ -1025,8 +1022,8 @@ static int avisynth_read_packet_audio(AVFormatContext *s, AVPacket *pkt,
     pkt->duration = samples;
     pkt->stream_index = avs->curr_stream;
 
-    avs_library.avs_get_audio(avs->clip, pkt->data, n, samples);
-    error = avs_library.avs_clip_get_error(avs->clip);
+    avs->avs_library.avs_get_audio(avs->clip, pkt->data, n, samples);
+    error = avs->avs_library.avs_clip_get_error(avs->clip);
     if (error) {
         av_log(s, AV_LOG_ERROR, "%s\n", error);
         avs->error = 1;
@@ -1040,16 +1037,9 @@ static av_cold int avisynth_read_header(AVFormatContext *s)
 {
     int ret;
 
-    // Calling library must implement a lock for thread-safe opens.
-    if (ff_mutex_lock(&avisynth_mutex))
-        return AVERROR_UNKNOWN;
-
-    if (ret = avisynth_open_file(s)) {
-        ff_mutex_unlock(&avisynth_mutex);
+    if (ret = avisynth_open_file(s))
         return ret;
-    }
 
-    ff_mutex_unlock(&avisynth_mutex);
     return 0;
 }
 
@@ -1085,8 +1075,13 @@ static int avisynth_read_packet(AVFormatContext *s, AVPacket *pkt)
 
 static av_cold int avisynth_read_close(AVFormatContext *s)
 {
-    avisynth_context_destroy(s->priv_data);
-    dlclose(avs_library.library);
+    AviSynthContext *avs = s->priv_data;
+
+    if (avs->avs_library.library) {
+        avisynth_context_destroy(s->priv_data);
+        dlclose(avs->avs_library.library);
+    }
+
     return 0;
 }
 
-- 
2.43.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/4] avformat/avisynth: remove library allocation from global state

[Stephen Hutchinson]

On 7/29/24 12:43 AM, Stephen Hutchinson wrote:
> As part of this, the mutexes are no longer necessary, and
> avisynth_read_close needs to check that avs->avs_library.library
> still exists before it attempts to call avisynth_context_destroy
> and dlclose.
> 
> Signed-off-by: Stephen Hutchinson <qyot27@gmail.com>
> ---
>   libavformat/avisynth.c | 133 ++++++++++++++++++++---------------------
>   1 file changed, 64 insertions(+), 69 deletions(-)
> 
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index b01263e010..b91d77b8d8 100644
> --- a/libavformat/avisynth.c
> +++ b/libavformat/avisynth.c
> @@ -115,6 +115,7 @@ typedef struct AviSynthContext {
>       int error;
>   
>       uint32_t flags;
> +    struct AviSynthLibrary avs_library;
>   } AviSynthContext;
>   
>   static const int avs_planes_packed[1] = { 0 };
> @@ -128,20 +129,16 @@ static const int avs_planes_yuva[4]   = { AVS_PLANAR_Y, AVS_PLANAR_U,
>   static const int avs_planes_rgba[4]   = { AVS_PLANAR_G, AVS_PLANAR_B,
>                                             AVS_PLANAR_R, AVS_PLANAR_A };
>   
> -static AVMutex avisynth_mutex = AV_MUTEX_INITIALIZER;
> -
> -static AviSynthLibrary avs_library;
> -
> -static av_cold int avisynth_load_library(void)
> +static av_cold int avisynth_load_library(AviSynthContext *avs)
>   {
> -    avs_library.library = dlopen(AVISYNTH_LIB, RTLD_NOW | RTLD_LOCAL);
> -    if (!avs_library.library)
> +    avs->avs_library.library = dlopen(AVISYNTH_LIB, RTLD_NOW | RTLD_LOCAL);
> +    if (!avs->avs_library.library)
>           return AVERROR_UNKNOWN;
>   
>   #define LOAD_AVS_FUNC(name, continue_on_fail)                          \
> -        avs_library.name = (name ## _func)                             \
> -                           dlsym(avs_library.library, #name);          \
> -        if (!continue_on_fail && !avs_library.name)                    \
> +        avs->avs_library.name = (name ## _func)                             \
> +                           dlsym(avs->avs_library.library, #name);          \
> +        if (!continue_on_fail && !avs->avs_library.name)                    \
>               goto fail;
>   
>       LOAD_AVS_FUNC(avs_bit_blt, 0);
> @@ -177,7 +174,7 @@ static av_cold int avisynth_load_library(void)
>       return 0;
>   
>   fail:
> -    dlclose(avs_library.library);
> +    dlclose(avs->avs_library.library);
>       return AVERROR_UNKNOWN;
>   }
>   
> @@ -189,13 +186,13 @@ static av_cold int avisynth_context_create(AVFormatContext *s)
>       AviSynthContext *avs = s->priv_data;
>       int ret;
>   
> -    if (!avs_library.library)
> -        if (ret = avisynth_load_library())
> +    if (!avs->avs_library.library)
> +        if (ret = avisynth_load_library(avs))
>               return ret;
>   
> -    avs->env = avs_library.avs_create_script_environment(3);
> -    if (avs_library.avs_get_error) {
> -        const char *error = avs_library.avs_get_error(avs->env);
> +    avs->env = avs->avs_library.avs_create_script_environment(3);
> +    if (avs->avs_library.avs_get_error) {
> +        const char *error = avs->avs_library.avs_get_error(avs->env);
>           if (error) {
>               av_log(s, AV_LOG_ERROR, "%s\n", error);
>               return AVERROR_UNKNOWN;
> @@ -208,11 +205,11 @@ static av_cold int avisynth_context_create(AVFormatContext *s)
>   static av_cold void avisynth_context_destroy(AviSynthContext *avs)
>   {
>       if (avs->clip) {
> -        avs_library.avs_release_clip(avs->clip);
> +        avs->avs_library.avs_release_clip(avs->clip);
>           avs->clip = NULL;
>       }
>       if (avs->env) {
> -        avs_library.avs_delete_script_environment(avs->env);
> +        avs->avs_library.avs_delete_script_environment(avs->env);
>           avs->env = NULL;
>       }
>   }
> @@ -486,17 +483,17 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
>        * version 9 at the minimum.  Technically, 8.1 works, but the time
>        * distance between 8.1 and 9 is very small, so just restrict it to 9. */
>   
> -    if (avs_library.avs_get_version(avs->clip) >= 9) {
> +    if (avs->avs_library.avs_get_version(avs->clip) >= 9) {
>   
> -        frame  = avs_library.avs_get_frame(avs->clip, 0);
> -        avsmap = avs_library.avs_get_frame_props_ro(avs->env, frame);
> +        frame  = avs->avs_library.avs_get_frame(avs->clip, 0);
> +        avsmap = avs->avs_library.avs_get_frame_props_ro(avs->env, frame);
>   
>           /* Field order */
>           if(avs->flags & AVISYNTH_FRAMEPROP_FIELD_ORDER) {
> -            if(avs_library.avs_prop_get_type(avs->env, avsmap, "_FieldBased") == AVS_PROPTYPE_UNSET) {
> +            if(avs->avs_library.avs_prop_get_type(avs->env, avsmap, "_FieldBased") == AVS_PROPTYPE_UNSET) {
>                   st->codecpar->field_order = AV_FIELD_UNKNOWN;
>               } else {
> -                switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_FieldBased", 0, &error)) {
> +                switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_FieldBased", 0, &error)) {
>                   case 0:
>                       st->codecpar->field_order = AV_FIELD_PROGRESSIVE;
>                       break;
> @@ -514,10 +511,10 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
>   
>           /* Color Range */
>           if(avs->flags & AVISYNTH_FRAMEPROP_RANGE) {
> -            if(avs_library.avs_prop_get_type(avs->env, avsmap, "_ColorRange") == AVS_PROPTYPE_UNSET) {
> +            if(avs->avs_library.avs_prop_get_type(avs->env, avsmap, "_ColorRange") == AVS_PROPTYPE_UNSET) {
>                   st->codecpar->color_range = AVCOL_RANGE_UNSPECIFIED;
>               } else {
> -                switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_ColorRange", 0, &error)) {
> +                switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_ColorRange", 0, &error)) {
>                   case 0:
>                       st->codecpar->color_range = AVCOL_RANGE_JPEG;
>                       break;
> @@ -532,7 +529,7 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
>   
>           /* Color Primaries */
>           if(avs->flags & AVISYNTH_FRAMEPROP_PRIMARIES) {
> -            switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Primaries", 0, &error)) {
> +            switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_Primaries", 0, &error)) {
>               case 1:
>                   st->codecpar->color_primaries = AVCOL_PRI_BT709;
>                   break;
> @@ -576,7 +573,7 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
>   
>           /* Color Transfer Characteristics */
>           if(avs->flags & AVISYNTH_FRAMEPROP_TRANSFER) {
> -            switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Transfer", 0, &error)) {
> +            switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_Transfer", 0, &error)) {
>               case 1:
>                   st->codecpar->color_trc = AVCOL_TRC_BT709;
>                   break;
> @@ -635,10 +632,10 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
>   
>           /* Matrix coefficients */
>           if(avs->flags & AVISYNTH_FRAMEPROP_MATRIX) {
> -            if(avs_library.avs_prop_get_type(avs->env, avsmap, "_Matrix") == AVS_PROPTYPE_UNSET) {
> +            if(avs->avs_library.avs_prop_get_type(avs->env, avsmap, "_Matrix") == AVS_PROPTYPE_UNSET) {
>                   st->codecpar->color_space = AVCOL_SPC_UNSPECIFIED;
>               } else {
> -                switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Matrix", 0, &error)) {
> +                switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_Matrix", 0, &error)) {
>                   case 0:
>                       st->codecpar->color_space = AVCOL_SPC_RGB;
>                       break;
> @@ -689,10 +686,10 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
>   
>           /* Chroma Location */
>           if(avs->flags & AVISYNTH_FRAMEPROP_CHROMA_LOCATION) {
> -            if(avs_library.avs_prop_get_type(avs->env, avsmap, "_ChromaLocation") == AVS_PROPTYPE_UNSET) {
> +            if(avs->avs_library.avs_prop_get_type(avs->env, avsmap, "_ChromaLocation") == AVS_PROPTYPE_UNSET) {
>                   st->codecpar->chroma_location = AVCHROMA_LOC_UNSPECIFIED;
>               } else {
> -                switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_ChromaLocation", 0, &error)) {
> +                switch (avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_ChromaLocation", 0, &error)) {
>                   case 0:
>                       st->codecpar->chroma_location = AVCHROMA_LOC_LEFT;
>                       break;
> @@ -719,12 +716,12 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
>   
>           /* Sample aspect ratio */
>           if(avs->flags & AVISYNTH_FRAMEPROP_SAR) {
> -            sar_num = avs_library.avs_prop_get_int(avs->env, avsmap, "_SARNum", 0, &error);
> -            sar_den = avs_library.avs_prop_get_int(avs->env, avsmap, "_SARDen", 0, &error);
> +            sar_num = avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_SARNum", 0, &error);
> +            sar_den = avs->avs_library.avs_prop_get_int(avs->env, avsmap, "_SARDen", 0, &error);
>               st->sample_aspect_ratio = (AVRational){ sar_num, sar_den };
>           }
>   
> -        avs_library.avs_release_video_frame(frame);
> +        avs->avs_library.avs_release_video_frame(frame);
>       } else {
>           st->codecpar->field_order = AV_FIELD_UNKNOWN;
>           /* AviSynth works with frame-based video, detecting field order can
> @@ -753,9 +750,9 @@ static int avisynth_create_stream_audio(AVFormatContext *s, AVStream *st)
>       st->duration                        = avs->vi->num_audio_samples;
>       avpriv_set_pts_info(st, 64, 1, avs->vi->audio_samples_per_second);
>   
> -    if (avs_library.avs_get_version(avs->clip) >= 10)
> +    if (avs->avs_library.avs_get_version(avs->clip) >= 10)
>           av_channel_layout_from_mask(&st->codecpar->ch_layout,
> -                                    avs_library.avs_get_channel_mask(avs->vi));
> +                                    avs->avs_library.avs_get_channel_mask(avs->vi));
>   
>       switch (avs->vi->sample_type) {
>       case AVS_SAMPLE_INT8:
> @@ -817,13 +814,13 @@ static int avisynth_open_file(AVFormatContext *s)
>       if (ret = avisynth_context_create(s))
>           return ret;
>   
> -    if (!avs_library.avs_check_version(avs->env, 7)) {
> +    if (!avs->avs_library.avs_check_version(avs->env, 7)) {
>           AVS_Value args[] = {
>               avs_new_value_string(s->url),
>               avs_new_value_bool(1) // filename is in UTF-8
>           };
> -        val = avs_library.avs_invoke(avs->env, "Import",
> -                                     avs_new_value_array(args, 2), 0);
> +        val = avs->avs_library.avs_invoke(avs->env, "Import",
> +                                          avs_new_value_array(args, 2), 0);
>       } else {
>           AVS_Value arg;
>   #ifdef _WIN32
> @@ -837,7 +834,7 @@ static int avisynth_open_file(AVFormatContext *s)
>   #else
>           arg = avs_new_value_string(s->url);
>   #endif
> -        val = avs_library.avs_invoke(avs->env, "Import", arg, 0);
> +        val = avs->avs_library.avs_invoke(avs->env, "Import", arg, 0);
>   #ifdef _WIN32
>           av_free(filename_ansi);
>   #endif
> @@ -854,14 +851,14 @@ static int avisynth_open_file(AVFormatContext *s)
>           goto fail;
>       }
>   
> -    avs->clip = avs_library.avs_take_clip(val, avs->env);
> -    avs->vi   = avs_library.avs_get_video_info(avs->clip);
> +    avs->clip = avs->avs_library.avs_take_clip(val, avs->env);
> +    avs->vi   = avs->avs_library.avs_get_video_info(avs->clip);
>   
>       /* On Windows, FFmpeg supports AviSynth interface version 6 or higher.
>        * This includes AviSynth 2.6 RC1 or higher, and AviSynth+ r1718 or higher,
>        * and excludes 2.5 and the 2.6 alphas. */
>   
> -    if (avs_library.avs_get_version(avs->clip) < 6) {
> +    if (avs->avs_library.avs_get_version(avs->clip) < 6) {
>           av_log(s, AV_LOG_ERROR,
>                  "AviSynth version is too old. Please upgrade to either AviSynth 2.6 >= RC1 or AviSynth+ >= r1718.\n");
>           ret = AVERROR_UNKNOWN;
> @@ -869,7 +866,7 @@ static int avisynth_open_file(AVFormatContext *s)
>       }
>   
>       /* Release the AVS_Value as it will go out of scope. */
> -    avs_library.avs_release_value(val);
> +    avs->avs_library.avs_release_value(val);
>   
>       if (ret = avisynth_create_stream(s))
>           goto fail;
> @@ -917,7 +914,7 @@ static int avisynth_read_packet_video(AVFormatContext *s, AVPacket *pkt,
>       if (discard)
>           return 0;
>   
> -    bits = avs_library.avs_bits_per_pixel(avs->vi);
> +    bits = avs->avs_library.avs_bits_per_pixel(avs->vi);
>   
>       /* Without the cast to int64_t, calculation overflows at about 9k x 9k
>        * resolution. */
> @@ -934,8 +931,8 @@ static int avisynth_read_packet_video(AVFormatContext *s, AVPacket *pkt,
>       pkt->duration = 1;
>       pkt->stream_index = avs->curr_stream;
>   
> -    frame = avs_library.avs_get_frame(avs->clip, n);
> -    error = avs_library.avs_clip_get_error(avs->clip);
> +    frame = avs->avs_library.avs_get_frame(avs->clip, n);
> +    error = avs->avs_library.avs_clip_get_error(avs->clip);
>       if (error) {
>           av_log(s, AV_LOG_ERROR, "%s\n", error);
>           avs->error = 1;
> @@ -946,26 +943,26 @@ static int avisynth_read_packet_video(AVFormatContext *s, AVPacket *pkt,
>       dst_p = pkt->data;
>       for (i = 0; i < avs->n_planes; i++) {
>           plane = avs->planes[i];
> -        src_p = avs_library.avs_get_read_ptr_p(frame, plane);
> -        pitch = avs_library.avs_get_pitch_p(frame, plane);
> +        src_p = avs->avs_library.avs_get_read_ptr_p(frame, plane);
> +        pitch = avs->avs_library.avs_get_pitch_p(frame, plane);
>   
> -        rowsize     = avs_library.avs_get_row_size_p(frame, plane);
> -        planeheight = avs_library.avs_get_height_p(frame, plane);
> +        rowsize     = avs->avs_library.avs_get_row_size_p(frame, plane);
> +        planeheight = avs->avs_library.avs_get_height_p(frame, plane);
>   
>           /* Flip RGB video. */
> -        if (avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR)   ||
> -            avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR48) ||
> -            avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR64)) {
> +        if (avs->avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR)   ||
> +            avs->avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR48) ||
> +            avs->avs_library.avs_is_color_space(avs->vi, AVS_CS_BGR64)) {
>               src_p = src_p + (planeheight - 1) * pitch;
>               pitch = -pitch;
>           }
>   
> -        avs_library.avs_bit_blt(avs->env, dst_p, rowsize, src_p, pitch,
> -                                 rowsize, planeheight);
> +        avs->avs_library.avs_bit_blt(avs->env, dst_p, rowsize, src_p, pitch,
> +                                     rowsize, planeheight);
>           dst_p += rowsize * planeheight;
>       }
>   
> -    avs_library.avs_release_video_frame(frame);
> +    avs->avs_library.avs_release_video_frame(frame);
>       return 0;
>   }
>   
> @@ -1025,8 +1022,8 @@ static int avisynth_read_packet_audio(AVFormatContext *s, AVPacket *pkt,
>       pkt->duration = samples;
>       pkt->stream_index = avs->curr_stream;
>   
> -    avs_library.avs_get_audio(avs->clip, pkt->data, n, samples);
> -    error = avs_library.avs_clip_get_error(avs->clip);
> +    avs->avs_library.avs_get_audio(avs->clip, pkt->data, n, samples);
> +    error = avs->avs_library.avs_clip_get_error(avs->clip);
>       if (error) {
>           av_log(s, AV_LOG_ERROR, "%s\n", error);
>           avs->error = 1;
> @@ -1040,16 +1037,9 @@ static av_cold int avisynth_read_header(AVFormatContext *s)
>   {
>       int ret;
>   
> -    // Calling library must implement a lock for thread-safe opens.
> -    if (ff_mutex_lock(&avisynth_mutex))
> -        return AVERROR_UNKNOWN;
> -
> -    if (ret = avisynth_open_file(s)) {
> -        ff_mutex_unlock(&avisynth_mutex);
> +    if (ret = avisynth_open_file(s))
>           return ret;
> -    }
>   
> -    ff_mutex_unlock(&avisynth_mutex);
>       return 0;
>   }
>   
> @@ -1085,8 +1075,13 @@ static int avisynth_read_packet(AVFormatContext *s, AVPacket *pkt)
>   
>   static av_cold int avisynth_read_close(AVFormatContext *s)
>   {
> -    avisynth_context_destroy(s->priv_data);
> -    dlclose(avs_library.library);
> +    AviSynthContext *avs = s->priv_data;
> +
> +    if (avs->avs_library.library) {
> +        avisynth_context_destroy(s->priv_data);
> +        dlclose(avs->avs_library.library);
> +    }
> +
>       return 0;
>   }
>   

Pushed.
_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/4] avformat/avisynth: move avs_planes* consts into relevant function

[Stephen Hutchinson]

These consts are only used in the switch(planar) case located in
avisynth_create_stream_video and nowhere else in the demuxer,
so move them into that function directly.

Signed-off-by: Stephen Hutchinson <qyot27@gmail.com>
---
 libavformat/avisynth.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index b91d77b8d8..cb2be10925 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -118,17 +118,6 @@ typedef struct AviSynthContext {
     struct AviSynthLibrary avs_library;
 } AviSynthContext;
 
-static const int avs_planes_packed[1] = { 0 };
-static const int avs_planes_grey[1]   = { AVS_PLANAR_Y };
-static const int avs_planes_yuv[3]    = { AVS_PLANAR_Y, AVS_PLANAR_U,
-                                          AVS_PLANAR_V };
-static const int avs_planes_rgb[3]    = { AVS_PLANAR_G, AVS_PLANAR_B,
-                                          AVS_PLANAR_R };
-static const int avs_planes_yuva[4]   = { AVS_PLANAR_Y, AVS_PLANAR_U,
-                                          AVS_PLANAR_V, AVS_PLANAR_A };
-static const int avs_planes_rgba[4]   = { AVS_PLANAR_G, AVS_PLANAR_B,
-                                          AVS_PLANAR_R, AVS_PLANAR_A };
-
 static av_cold int avisynth_load_library(AviSynthContext *avs)
 {
     avs->avs_library.library = dlopen(AVISYNTH_LIB, RTLD_NOW | RTLD_LOCAL);
@@ -225,6 +214,17 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
     int sar_num = 1;
     int sar_den = 1;
 
+    static const int avs_planes_packed[1] = { 0 };
+    static const int avs_planes_grey[1]   = { AVS_PLANAR_Y };
+    static const int avs_planes_yuv[3]    = { AVS_PLANAR_Y, AVS_PLANAR_U,
+                                              AVS_PLANAR_V };
+    static const int avs_planes_rgb[3]    = { AVS_PLANAR_G, AVS_PLANAR_B,
+                                              AVS_PLANAR_R };
+    static const int avs_planes_yuva[4]   = { AVS_PLANAR_Y, AVS_PLANAR_U,
+                                              AVS_PLANAR_V, AVS_PLANAR_A };
+    static const int avs_planes_rgba[4]   = { AVS_PLANAR_G, AVS_PLANAR_B,
+                                              AVS_PLANAR_R, AVS_PLANAR_A };
+
     st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
     st->codecpar->codec_id   = AV_CODEC_ID_RAWVIDEO;
     st->codecpar->width      = avs->vi->width;
-- 
2.43.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/4] avformat/avisynth: move avs_planes* consts into relevant function

[Stephen Hutchinson]

On 7/29/24 12:43 AM, Stephen Hutchinson wrote:
> These consts are only used in the switch(planar) case located in
> avisynth_create_stream_video and nowhere else in the demuxer,
> so move them into that function directly.
> 
> Signed-off-by: Stephen Hutchinson <qyot27@gmail.com>
> ---
>   libavformat/avisynth.c | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index b91d77b8d8..cb2be10925 100644
> --- a/libavformat/avisynth.c
> +++ b/libavformat/avisynth.c
> @@ -118,17 +118,6 @@ typedef struct AviSynthContext {
>       struct AviSynthLibrary avs_library;
>   } AviSynthContext;
>   
> -static const int avs_planes_packed[1] = { 0 };
> -static const int avs_planes_grey[1]   = { AVS_PLANAR_Y };
> -static const int avs_planes_yuv[3]    = { AVS_PLANAR_Y, AVS_PLANAR_U,
> -                                          AVS_PLANAR_V };
> -static const int avs_planes_rgb[3]    = { AVS_PLANAR_G, AVS_PLANAR_B,
> -                                          AVS_PLANAR_R };
> -static const int avs_planes_yuva[4]   = { AVS_PLANAR_Y, AVS_PLANAR_U,
> -                                          AVS_PLANAR_V, AVS_PLANAR_A };
> -static const int avs_planes_rgba[4]   = { AVS_PLANAR_G, AVS_PLANAR_B,
> -                                          AVS_PLANAR_R, AVS_PLANAR_A };
> -
>   static av_cold int avisynth_load_library(AviSynthContext *avs)
>   {
>       avs->avs_library.library = dlopen(AVISYNTH_LIB, RTLD_NOW | RTLD_LOCAL);
> @@ -225,6 +214,17 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
>       int sar_num = 1;
>       int sar_den = 1;
>   
> +    static const int avs_planes_packed[1] = { 0 };
> +    static const int avs_planes_grey[1]   = { AVS_PLANAR_Y };
> +    static const int avs_planes_yuv[3]    = { AVS_PLANAR_Y, AVS_PLANAR_U,
> +                                              AVS_PLANAR_V };
> +    static const int avs_planes_rgb[3]    = { AVS_PLANAR_G, AVS_PLANAR_B,
> +                                              AVS_PLANAR_R };
> +    static const int avs_planes_yuva[4]   = { AVS_PLANAR_Y, AVS_PLANAR_U,
> +                                              AVS_PLANAR_V, AVS_PLANAR_A };
> +    static const int avs_planes_rgba[4]   = { AVS_PLANAR_G, AVS_PLANAR_B,
> +                                              AVS_PLANAR_R, AVS_PLANAR_A };
> +
>       st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>       st->codecpar->codec_id   = AV_CODEC_ID_RAWVIDEO;
>       st->codecpar->width      = avs->vi->width;

Pushed.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2 v2] avformat/avisynth: remove atexit() handler

[Stephen Hutchinson]

On 7/19/24 12:56 PM, Stephen Hutchinson wrote:
> 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>
> ---
> Changes compared to v1:
> * Added missing dlclose invocation to read_close
> 
>   libavformat/avisynth.c | 46 +-----------------------------------------
>   1 file changed, 1 insertion(+), 45 deletions(-)
> 
> diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
> index 625bdf7e3a..26747671c0 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)
>   {
> @@ -1134,6 +1089,7 @@ static av_cold int avisynth_read_close(AVFormatContext *s)
>           return AVERROR_UNKNOWN;
>   
>       avisynth_context_destroy(s->priv_data);
> +    dlclose(avs_library.library);
>       ff_mutex_unlock(&avisynth_mutex);
>       return 0;
>   }

Pushed.
_______________________________________________
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".