[FFmpeg-devel] [PATCH] avformat/oggenc: Don't free AVStream's priv_data, fix memleak

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Andreas Rheinhardt:
>> For FLAC, Speed, Opus and VP8 the Ogg muxer allocates two buffers
>> for building the headers: The first for extradata in an Ogg-specific
>> format and the second contains a Vorbiscomment. These buffers are
>> reachable via pointers in the corresponding AVStream's priv_data.
>>
>> If an error happens during building the headers, the AVStream's
>> priv_data would be freed. This is pointless in general as it would be
>> freed generically anyway, but here it is actively harmful: If the second
>> of the aforementioned allocations fails, the first buffer would leak
>> upon freeing priv_data.
>>
>> This commit stops freeing priv_data manually, which allows the muxer to
>> properly clean up in the deinit function.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>> ---
>>  libavformat/oggenc.c | 7 +------
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/libavformat/oggenc.c b/libavformat/oggenc.c
>> index fe89f23e36..fbd14fedf9 100644
>> --- a/libavformat/oggenc.c
>> +++ b/libavformat/oggenc.c
>> @@ -546,7 +546,6 @@ static int ogg_init(AVFormatContext *s)
>>                                               &st->metadata);
>>              if (err) {
>>                  av_log(s, AV_LOG_ERROR, "Error writing FLAC headers\n");
>> -                av_freep(&st->priv_data);
>>                  return err;
>>              }
>>          } else if (st->codecpar->codec_id == AV_CODEC_ID_SPEEX) {
>> @@ -555,7 +554,6 @@ static int ogg_init(AVFormatContext *s)
>>                                                &st->metadata);
>>              if (err) {
>>                  av_log(s, AV_LOG_ERROR, "Error writing Speex headers\n");
>> -                av_freep(&st->priv_data);
>>                  return err;
>>              }
>>          } else if (st->codecpar->codec_id == AV_CODEC_ID_OPUS) {
>> @@ -564,7 +562,6 @@ static int ogg_init(AVFormatContext *s)
>>                                               &st->metadata, s->chapters, s->nb_chapters);
>>              if (err) {
>>                  av_log(s, AV_LOG_ERROR, "Error writing Opus headers\n");
>> -                av_freep(&st->priv_data);
>>                  return err;
>>              }
>>          } else if (st->codecpar->codec_id == AV_CODEC_ID_VP8) {
>> @@ -572,7 +569,6 @@ static int ogg_init(AVFormatContext *s)
>>                                              s->flags & AVFMT_FLAG_BITEXACT);
>>              if (err) {
>>                  av_log(s, AV_LOG_ERROR, "Error writing VP8 headers\n");
>> -                av_freep(&st->priv_data);
>>                  return err;
>>              }
>>          } else {
>> @@ -585,7 +581,7 @@ static int ogg_init(AVFormatContext *s)
>>                                        st->codecpar->codec_id == AV_CODEC_ID_VORBIS ? 30 : 42,
>>                                        (const uint8_t**)oggstream->header, oggstream->header_len) < 0) {
>>                  av_log(s, AV_LOG_ERROR, "Extradata corrupted\n");
>> -                av_freep(&st->priv_data);
>> +                oggstream->header[1] = NULL;
>>                  return AVERROR_INVALIDDATA;
>>              }
>>  
>> @@ -755,7 +751,6 @@ static void ogg_free(AVFormatContext *s)
>>              av_freep(&oggstream->header[0]);
>>          }
>>          av_freep(&oggstream->header[1]);
>> -        av_freep(&st->priv_data);
>>      }
>>  
>>      while (p) {
>>
> Will apply this tomorrow if there are no objections.
> 
> - Andreas
> 
Applied.

- Andreas