diff mbox series

[FFmpeg-devel,v1] avformat/dashdec: remove xmlCleanupParser call in dashdec

Message ID 20241010151409.2138582-1-lq@chinaffmpeg.org
State New
Headers show
Series [FFmpeg-devel,v1] avformat/dashdec: remove xmlCleanupParser call in dashdec | expand

Checks

Context Check Description
yinshiyou/commit_msg_loongarch64 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/commit_msg_x86 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Steven Liu Oct. 10, 2024, 3:14 p.m. UTC
Because there have global varible in libxml2, it cleans up memory allocated by
the library itself. It is a cleanup function for the XML library.
It tries to reclaim all related global memory allocated for the library processing.
It doesn't deallocate any document related memory.
One should call xmlCleanupParser() only when the process has finished using the library
and all XML/HTML documents built with it.
See also xmlInitParser() which has the opposite function of preparing the library
for operations. if there are multithreaded or has plugin support calling
this may crash the application if another thread or a plugin is still using libxml2.
So give a warning message to user for call xmlCleanupParser by themselves.

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/dashdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marvin Scholz Oct. 10, 2024, 3:16 p.m. UTC | #1
On 10 Oct 2024, at 17:14, Steven Liu wrote:

> Because there have global varible in libxml2, it cleans up memory allocated by
> the library itself. It is a cleanup function for the XML library.
> It tries to reclaim all related global memory allocated for the library processing.
> It doesn't deallocate any document related memory.
> One should call xmlCleanupParser() only when the process has finished using the library
> and all XML/HTML documents built with it.
> See also xmlInitParser() which has the opposite function of preparing the library
> for operations. if there are multithreaded or has plugin support calling
> this may crash the application if another thread or a plugin is still using libxml2.
> So give a warning message to user for call xmlCleanupParser by themselves.
>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavformat/dashdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 99ac6197be..c33ddb9ea4 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -1362,7 +1362,6 @@ static int parse_manifest(AVFormatContext *s, const char *url, AVIOContext *in)
>  cleanup:
>          /*free the document */
>          xmlFreeDoc(doc);
> -        xmlCleanupParser();
>          xmlFreeNode(mpd_baseurl_node);
>      }
>
> @@ -2026,6 +2025,7 @@ static int dash_read_header(AVFormatContext *s)
>      if ((ret = ffio_copy_url_options(s->pb, &c->avio_opts)) < 0)
>          return ret;
>
> +    av_log(s, AV_LOG_WARNING, "Have not call xmlCleanupParser yet, you should call it by yourself\n");

Ffmpeg CLI users will get this and be very confused, no? (I would be too…)

And in general how would application code even go about calling this, given they probably only depend
directly on FFmpeg and not necessarily on libxml2 too…

Even when applications call this properly, they (and indirectly users) would get this warning
anyway possibly thinking there is a bug even when the application did everything right.

This is essentially the same issue libcurl has about global initialisation and I do not think there is a
satisfying solution for this for Ffmpeg unfortunately.

>      if ((ret = parse_manifest(s, s->url, s->pb)) < 0)
>          return ret;
>
> -- 
> 2.39.3 (Apple Git-146)
>
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 99ac6197be..c33ddb9ea4 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1362,7 +1362,6 @@  static int parse_manifest(AVFormatContext *s, const char *url, AVIOContext *in)
 cleanup:
         /*free the document */
         xmlFreeDoc(doc);
-        xmlCleanupParser();
         xmlFreeNode(mpd_baseurl_node);
     }
 
@@ -2026,6 +2025,7 @@  static int dash_read_header(AVFormatContext *s)
     if ((ret = ffio_copy_url_options(s->pb, &c->avio_opts)) < 0)
         return ret;
 
+    av_log(s, AV_LOG_WARNING, "Have not call xmlCleanupParser yet, you should call it by yourself\n");
     if ((ret = parse_manifest(s, s->url, s->pb)) < 0)
         return ret;