Memory/Resource Leak in avcodec_open2() on failure

[Jose Santiago]

When calling avcodec_open2(), memory and other resources can leak if the encoder init fails or other steps within the codec open function fails. This includes memory and threads that can be created, but not destroyed in the case of a failure.

For instance, if you attempt to intialize an MPEG2 Video Encoder and enable slice based threaded encoding. If you do not set the resolution, fps, or the pixel format to a valid value. The encoder inititialization will fail, indicating that the codec context parameter is invalid for the selected encoder. It properly returns an error. But it does not destroy the threads it created. Nor does it destory all of the memory that the encoder init allocated before returning the failure.

For instance you can run valgrind with the following program to demonstrate the problem:

#include <libavcodec/avcodec.h>
#include <libavutil/avassert.h>

int main(void)
{
   AVCodec * codec;
   AVCodecContext * avctx;

   codec = avcodec_find_encoder(AV_CODEC_ID_MPEG2VIDEO);
   av_assert0(codec != NULL);
   avctx = avcodec_alloc_context3(codec);
   av_assert0(avctx != NULL);

   // Basic configuration
   // You can get the results by setting the pix_fmt
   // and/or resolution to some invalid or unsupported value.
   avctx->pix_fmt = AV_PIX_FMT_YUV420P;
   avctx->width = 640;
   avctx->height = 480;
   // Set some ffps that is not supported by MPEG2 Video
   avctx->time_base.num = 91;
   avctx->time_base.den = 2201;
   avctx->ticks_per_frame = 1;
   // Enable slice thread encoding
   avctx->thread_count = 10;
   avctx->thread_type = 2;
   avctx->active_thread_type = 2;

   // Open the Codec for encoding
   avcodec_open2(avctx, codec, NULL);

   // close and free the Codec
   avcodec_free_context(&avctx);
}

NOTE: If you put contents of main() in a loop to run forever, it will eventually make your system unstable as the number of threads created and not destroyed by the program exceed the number of supported threads in the system.

[Jose Santiago]

Valgrind output

[Jose Santiago]

Program that demonstrates the problem

[Jose Santiago]

Patch that resolves the issue

[James]

Please, send your patches to the ffmpeg-devel mailing list where they can be properly reviewed.
See https://ffmpeg.org/contact.html#MailingLists

Also, AVCodecContext->active_thread_type is not a field that should be set by the API user, as stated in the doxy.

[Carl Eugen Hoyos]

To simplify the reviewing process please use the same bracing style as the rest of the source file (visible twice in your patch file).

[Jose Santiago]

Updated patch fixing style issues

[Jose Santiago]

Modified the patch, fixing the brace style and submitted it to the libav-user mailing list.

[Carl Eugen Hoyos]

Send the patch to the development mailing list instead, no review can be done on the libav-user mailing list: ​https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel/

[Carl Eugen Hoyos]

Please provide a patch made with git format-patch, nothing else is accepted, sorry for not realizing this earlier!

[Balling]

Wonderful, another patch.

[Carl Eugen Hoyos]

Not reproducible anymore, possibly fixed in 39ff027fd81185d0f8b4dfd49e709f4d0d44e2de