[FFmpeg-devel] [RFC] Windows issues with av_destruct_packet_nofree

[Art Clarke]

Hi folks,

I just spent somewhere between 2-3 weeks tracking down a weird bug that
resulted in our code creating corrupted files on Windows but working fine on
Linux and Mac.  The issue, it turns out, affects people who allocate their
own AVPackets, and then try to use av_interleaved_write_frame on Windows.
That's because on Windows, "av_destruct_packet_nofree" outside of the libav
DLL's themselves is NOT the same as "av_destruct_packet_nofree" inside the
libav DLLs.  As a result when using av_interleaved_write_frame on Windows,
packet you might think FFMPEG is copying, aren't actually duplicated and
memory corruption results.

My suggestion is to add a comment to avformat.h above
av_destruct_packet_nofree warning users in Windows to not use this method
outside of libav, and instead set AVPacket->destruct to 0 (or NULL) if they
want to manage their own packet memory.  I'd like people's thoughts on that
(although at the end of this missive I put some other options that I, um,
recommend against).

Why is this happening?  Read on for the gritty details of the issue:

The API doc for av_interleave_packet_per_dts suggest that setting
AVPacket->destruct to av_destruct_packet will result in the packet being
freed inside that function.  Separately there is a function called
av_destruct_packet_nofree without documentation (I'm presuming if something
is in avformat.h it's meant as part of the public API).  Reviewing
libavformat/utils.c, especially av_dup_packet, it suggests if you create
your own AVPacket's, and set AVPacket->destruct to
av_destruct_packet_nofree, then av_dup_packet will copy (malloc/memcpy) your
data if it needs to keep it around for writing interleaved files.  All good
so far.

So here's the issue: av_destruct_packet_nofree on Windows gets exported in
the DLL with a thunking-wrapping function around it (generated by the
compiler).  That means the following code:

// In External Windows executable or library that dynamically links
AVFORMAT.DLL
AVPacket pkt;
av_init_packet(&pkt);
pkt.destruct = av_destruct_packet_nofree;

AVPacket dupPkt;
dupPkt = pkt;
av_dup_packet(&dupPkt);

Will not actually successfully duplicate that packet, because
av_destruct_packet_nofree (in the windows executable linking to
AVFORMAT.DLL) != av_destruct_packet_nofree (inside AVFORMAT.DLL).
av_dup_packet explicitly checks if pkt->destruct ==
av_destruct_packet_nofree (or NULL) before deciding to copy, and in the
above code although they LOOK equal, because of Windows DLL madness,
pkt->destruct != av_destruct_packet_nofree but isn't null either.  As a
result, packets you want to be duplciated are not, and craziness abounds.

Now, the fix is relatively simple in external libraries (once you determine
what's going on):
AVPacket pkt;
av_init_packet(&pkt);
pkt.destruct = 0; // set to NULL, not av_destruct_packet_nofree

AVPacket dupPkt;
dupPkt = pkt;
av_dup_packet(&dupPkt);

My proposed "fix" is to  add a comment to avformat.h warning windows users
that they can't use the _nofree function and instead to set destruct to 0,
but other potential solutions:

1) Change av_dup_packet to check for != av_destruct_packet; this just swaps
the danger and could end up breaking some folks who are using the API
incorrectly in the other way so I recommend against.
2) Rework the FFMPEG build system to __dllimport and __dllexport the
appropriate functions which should fix the issue; I recommend against this
because, well, do I really need to explain why?
3) Tell all Windows users to take a long walk off a very short pier.  I'm
sorely tempted to recommend this, but then 50% of my users get upset.
4) remove av_destruct_packet_nofree from avformat.h with a deprecation.
This might be the right thing to do, but I'll let the FFMPEG gods decide
that.

- Art

-- 
http://www.xuggle.com/
xu?ggle (z?' gl) v. To freely encode, decode, and experience audio and
video.

Use Xuggle to get the power of FFMPEG in Java.