[FFmpeg-devel] [PATCH] movenc.c fix (Was: Can av_write_frame() modify pkt.data?)

[Michael Niedermayer]

On Tue, Jun 17, 2008 at 03:41:19PM +0200, Luca Abeni wrote:
> Hi Michael,
>
> Michael Niedermayer wrote:
>>> I just discovered that the mov muxer can modify the
>>> "data" field of the AVPacket structure that it receives
>>> as parameter (in case of H.264 video, mov_write_packet()
>>> calls ff_avc_parse_nal_units(pkt->data, &pkt->data, &pkt->size);
>>> which frees pkt->data and allocates a new buffer).
> [...]
>>> So, should output_example.c be fixed, or should movenc.c
>>> be fixed? (or is everything ok, since output_example does
>>> not write H.264 in mov unless you modify it?)
>> movenc.c looks buggy, and a quick grep hints towards that its not
>> the only one.
>> pkt.data does not need to be a malloced() buffer. Thus a blind av_free()
>> is always wrong. The correct way to kill a packet is with pkt->destruct()
>> though that is of course not the muxers job to do.
>> Besides a packet could be passed to more than 1 muxer and thus its content
>> must not be changed by any muxer.
>
> Ok; I tried to fix this bug, but I do not know if the attached patch
> is the best solution. It seems to work well here.

Iam not sure if its the best either ... maybe we should change
ff_avc_parse_nal_units to take a ByteIOContext ...
Of course only if it would end up simpler, if its not simpler than
your patch here is ok

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

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080618/a6c40db1/attachment.pgp>