[FFmpeg-devel] [PATCH] avformat: avisynth demuxer rewrite

[Michael Niedermayer]

Hi

On Mon, Feb 25, 2013 at 03:20:23PM -0500, Stephen Hutchinson wrote:
> Over the past week, I've been discussing this patch with d
> s/avxsynth-testing on Github
> (https://github.com/avxsynth/avxsynth/issues/90), and they've given me
> permission to resubmit an updated version of the patch on their
> behalf.  I have it in two patches on Github, and have kept the
> authorship information the same since my contributions to it were
> small - just the ifdef for OS X's .dylib, a correction for the
> pkt->size math, and the stuff regarding AviSynth 2.6's additional
> colorspaces.
> 
> AviSynth demuxer rewrite patch:
> https://github.com/qyot27/FFmpeg/commit/0403ba7389e3a20df31b94886f4ce3b880d3cfb1.patch
> 
> To solve the problem of how to handle the need for AviSynth and
> AvxSynth headers to be available, I've adopted the same tactic used by
> x264, and included local copies of the necessary headers in
> compat/avisynth/.  The versions are the same as those included with
> x264.

please split the addition of compat headers into a seperate patch and
submit all patches as attachments not a link, this makes commenting
on the mailing list alot easier.
Also please use git format-patch or git send-email so the patches
come with commit messages and authorship information

The patchset replaces avisynth.c, maybe it was already discussed and
i forgot but is all functionality from avisynth.c still available in
the new ?
that is would all users of all platforms who could use it before still
use it ?
if not then a seperate new avxsynth.c would probably be a better idea


> 
> 
> 
> 
> atexit/exit changes in ffmpeg.c:
> https://github.com/qyot27/FFmpeg/commit/d69ebce458ea8dd023f197cf9c89a98d5dc652e8.patch
> 
> Their rationale for the second patch is explained in the issues discussion:
> "IIRC, the segfault occurs because of an interaction between dlclose,
> atexit, and C++ objects. I will look into seeing if it can be
> mitigated on the AvxSynth side.
> 
> Edit:
> More details about the segfault on exit problem:
> In ffmpeg.c, exit_program is registered with atexit. C++ static global
> objects are also (secretly) registered with a C++ atexit equivalent.
> The ScriptEnvironment destructor in Avxsynth has been modified from
> the Avisynth version to access some global structures, namely the (now
> vector instead of array) list of built-in functions and the log4cpp
> service. Normally, this would not be an issue because constructed
> static objects are guaranteed by the C++ standard to be destroyed
> after user atexit handlers run. However, because Avxsynth is being
> dynamically loaded, the global objects are actually constructed upon
> calling dlopen through avformat_read_header, which occurs after
> exit_program is registered. As a consequence, the Avxsynth global
> structures are destroyed before calling exit_program, and
> avs_delete_script_environment accesses freed memory when calling the
> ScriptEnvironment destructor."
> 
> "I have sat down and thought about this for a while. Unfortunately, I
> can't think of a way to resolve ffmpeg's atexit usage with AvxSynth's
> usage of global objects. Something has to give, and unfortunately, we
> are not willing to give up logging to avoid this segfault. This patch
> to ffmpeg.c replaces the offending atexit handler and does not appear
> to break anything else:
> 
> http://pastebin.com/L988RtYz

ffmpeg is not the only application that uses libavformat so this
might be a much bigger problem than you think.

what you basically suggest is that no application that uses libavformat
is allowed to use atexit() anymore

also your patch replaces 6 out of 30 occurances of exit() in ffmpeg.c
this does look quite incomplete ...

an alternative solution (that may or may not work) would be to
register your own atexit handler when the avxsynth demuxer is opened
and if the callback gets called do the free / or not do the free from
it but set a global flag that prevents subsequent access from
read_close(). I dont know if that would have issues or how ugly it
would be but i guess you too would prefer if libavformat+avxsynth
could be used from applications that do use atexit in a similar way
to ffmpeg.c

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130226/689b57c8/attachment.asc>