[FFmpeg-devel] Dangerous actions in ff_frame_thread_init() or API misuse by me?

[Andrey Utkin]

The scenario is reencoding of video stream, particularly with h264
encoder open failure.
The sequence of actions leading to double free of same memory region
with extradata:

1) avcodec_open2(input_file->streams[i]->codec)
2) avcodec_copy_context(output_file->streams[i]->codec,
input_file->streams[i]->codec)
3) avcodec_open2(output_file->streams[i]->codec) which fails
4) avformat_free_context(output_file)

It turns out that during output_file closing, extradata pointer is
freed for second time. But extradata is released by av_freep()
everywhere, so that pointer is NULLed after freeing memory. Debugging
the code, i found out that AVCodecContext structure is copied
completely in ff_frame_thread_init(), then stored in PerThreadContext
structure, then, on failure of encoder opening, these AVCodecContext
copies are subject to running AVCodec.close function on them.
X264_close() function includes av_freep(&avctx->extradata), thus we
get multiple freeing of same memory pointer: (at least) once in a copy
of AVCodecContext, then, during avformat_free_context() - in
AVCodecContext pointed to by AVStream.

So the question is - is above-mentioned order of actions 1-4
considered legal in terms of API?
If yes, then we have to fix internals. If no, we have to shed some
light on recommended way to do that thing, and still add some guard
against such usage.

Modified doc/examples/remuxing.c to reproduce the issue:
https://gist.github.com/krieger-od/f0fa65ee12fa30c3421a
Parts of Valgrind output on it, ffmpeg sources slightly modified for
more verbosity:
https://gist.github.com/krieger-od/9d891906eb871e52bd45

-- 
Andrey Utkin