[FFmpeg-devel] [PATCH] Workaround for MPEG-TS crashes

[Baptiste Coudurier]

Hi,

On 09/14/2009 10:25 AM, Dan Dennedy wrote:
> Hi
>
> On Sun, Sep 13, 2009 at 3:00 AM, Ivan Schreter<schreter at gmx.net>  wrote:
>> Hi Baptiste,
>>
>> several people reported crashes when working with MPEG-TS files. I suppose,
>> those files are either really or subtly broken and MPEG-TS format handler
>> doesn't handle them gracefully.
>>
>> With this patch, the crashes in the samples I have seem to be gone:
>>
>
> In addition, I ran into some small negative lengths passed to memcpy:
>
> Index: libavformat/mpegts.c
> ===================================================================
> --- libavformat/mpegts.c        (revision 19804)
> +++ libavformat/mpegts.c        (working copy)
> @@ -915,10 +915,12 @@
>               len = PES_START_SIZE - pes->data_index;
>               if (len>  buf_size)
>                   len = buf_size;
> -            memcpy(pes->header + pes->data_index, p, len);
> -            pes->data_index += len;
> -            p += len;
> -            buf_size -= len;
> +            if (len>  0) {
> +                memcpy(pes->header + pes->data_index, p, len);
> +                pes->data_index += len;
> +                p += len;
> +                buf_size -= len;
> +            }

Well, len < 0 is an error IMHO. I'd like to have a sample though,
this should not happen.

 > [...]
 >
>               if (pes->data_index == PES_START_SIZE) {
>                   /* we got all the PES or section header. We can now
>                      decide */
>
>> However, it's IMHO just a workaround and the real root cause of the problem
>> should be found. At this time, pes->buffer is NULL, but data for PES packet
>> are coming in =>  crash.
>>
>> Crashing sample is here:
>> http://dennedy.org/~ddennedy/dvgrab-2009.03.28_19-07-22.m2t
>> Playable sample is here:
>> http://dennedy.org/~ddennedy/dvgrab-2009.03.28_19-06-41.m2t
>>
>> Both samples seem to be seriously broken at the beginning. They originated
>> from a HDV-capable camcorder (MPEG-TS containing MPEG-2 video).
>
> Last night, after some serious testing and soul searching, I
> discovered some file system corruption or bad file transfers when
> archiving this material to external HDD. There were some HDV test
> samples that were failing that I _know_ had worked well in the past. I
> had a backup of those on my network, and when I tested them over the
> network, they worked fine! An e2fsck on that disk did indeed find some
> problems that the "automatic repair" option could not fix.
>
> So, the samples above are definitely not to be considered playable,
> but I can confirm older versions of ffmpeg (0.5) at least did not
> segfault on this corrupt input. With these two changes, it is much
> more stable.
>

These 2 files can be played if MAX_RESYNC_SIZE is increased to 65536.
Note that mplayer reads them.

Is 65k reasonable ?

Besides, these files do not segv with latest svn

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