[FFmpeg-devel] [PATCH] fix segfault in mxf demuxer

[Baptiste Coudurier]

On 3/22/11 3:30 AM, Nigel Touati-Evans wrote:
> On 21 March 2011 20:09, Baptiste Coudurier <baptiste.coudurier at gmail.com> wrote:
>> Hi,
>>
>> On 03/21/2011 05:40 AM, Tomas Härdin wrote:
>>> Nigel Touati-Evans skrev 2011-03-21 12:06:
>>>> Index: ffmpeg-dmo-0.5+svn20090508/libavformat/mxfdec.c
>>>> ===================================================================
>>>> --- ffmpeg-dmo-0.5+svn20090508.orig/libavformat/mxfdec.c    2011-03-21
>>>> 10:12:47.000000000 +0000
>>>> +++ ffmpeg-dmo-0.5+svn20090508/libavformat/mxfdec.c    2011-03-21
>>>> 10:17:13.000000000 +0000
>>>> @@ -723,13 +723,16 @@
>>>>                      break;
>>>>                  }
>>>>              }
>>>> -            if (!source_track) {
>>>> -                av_log(mxf->fc, AV_LOG_ERROR, "material track %d: no
>>>> corresponding source track found\n", material_track->track_id);
>>>> +            if (source_track) {
>>>> +                if (j+1 <
>>>> material_track->sequence->structural_components_count)
>>>> +                   av_log(mxf->fc, AV_LOG_WARNING, "material track
>>>> %d: found a source track so ignoring %d potential others\n",
>>>> material_track->track_id,
>>>> material_track->sequence->structural_components_count-j-1);
>>>
>>> Break this long line up a bit.
>>>
>>>>                  break;
>>>>              }
>>>>          }
>>>> -        if (!source_track)
>>>> +        if (!source_track) {
>>>> +            av_log(mxf->fc, AV_LOG_ERROR, "material track %d: no
>>>> corresponding source track found\n", material_track->track_id);
>>>>              continue;
>>>> +        }
>>>>
>>>>          st = av_new_stream(mxf->fc, source_track->track_id);
>>>>          if (!st) {
>>>>
>>>>
>>>
>>> Looks OK otherwise to me. Baptiste?
>>
>> Can we get a sample so I can reproduce the problem ?
>> I'd like to know where it exactly segv.
>>
>> --
>> Baptiste COUDURIER
>> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
>> FFmpeg maintainer                                  http://www.ffmpeg.org
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> 
> I'm not sure I have permission to upload the file - I'll try to find
> out. However its quite simple to see how the segv happens:
> 
> For this track sequence->structural_components_count is 2, the first
> time round (j=0) the loop component is set and a source track found.
> When j=1, mxf_resolve_strong_ref returns null (the component is either
> not found or the not of the correct type), so the loop exits. This
> leaves source_track set and component null, so there is a seg fault
> trying to dereference component to find the duration.
> 
> In general it doesn't look like the loop ensures the source_track
> corresponds to the component, as component is updated every iteration
> and source_track is not necessarily (e.g. if it is not found), so it
> seemed to me that the best bet was to exit the loop once a suitable
> component had been found.
> 
> This doesn't fix the 'TODO: handle multiple source clips' but at least
> makes everything consistent when structural_components_count > 1, and
> prints a warning if we might be missing something.

Thanks, patch ok.

-- 
Baptiste COUDURIER
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org