[FFmpeg-devel] [PATCH] mxfenc: ensure mxf->body_partition_offset is not NULL before using it

[tomas.hardin at codemill.se]

On 2015-03-14 18:03, Andreas Cadhalpun wrote:
> On 14.03.2015 02:17, Mark Reid wrote:
>> On Fri, Mar 13, 2015 at 6:02 AM, Andreas Cadhalpun <
>> andreas.cadhalpun at googlemail.com> wrote:
>> 
>>> On 13.03.2015 11:59, Tomas Härdin wrote:
>>>> A better solution would
>>>> be to figure out why mxf->body_partition_offset becomes NULL so that
>>>> index tables and such can be rewritten properly.
>>> 
>>> It can always happen that mxf->body_partition_offset is NULL, e.g. if
>>> no memory is left, or if something else fails. Try e.g.:
>>> ffmpeg -f lavfi -i testsrc -c:v libx264 -f mxf_opatom
>>> 
>>> 
>> mxf->body_partition_offset is NULL because currently only AVC Intra 
>> 50/100
>> h264 is supported.
> 
> Yes.
> 
>> The encoder figures out the h264 format by parsing the
>> h264 packet and doesn't write the body partiton (or even the header
>> partition) untill after it parses the first packet. If the packet is
>> invalid, nothing get written and mxf->body_partition_offset doesn't 
>> get
>> allocated.
> 
> That's correct.
> 
>>  perhaps mxf_write_footer should return a error if
>> mxf->body_partition_offset is NULL or just if mxf->header_written == 0
>>  before doing trying to write anything.
> 
> Well, mxf_write_footer also has to free some allocated memory, which 
> would
> get leaked if one just returns...
> ...like it does in any of the currently present 'return err' cases.
> 
> Attached is a patch fixing the memleaks and another returning an error,
> if no header was written before the footer.
> 
> Best regards,
> Andreas

[PATCH 2/2] mxfenc: don't try to write footer without header:

> +    if (!mxf->header_written ||
> +        (s->oformat == &ff_mxf_opatom_muxer && 
> !mxf->body_partition_offset)) {
> +        err = AVERROR_UNKNOWN;
> +        goto end;
> +    }
> +

AVERROR_UNKNOWN?

Memleak patch is obviously OK.

/Tomas