[FFmpeg-devel] [PATCH] avformat/avidec: fix demuxing of all keyframes of sample 200707170736151.avi

[Michael Niedermayer]

Hi

On Tue, Aug 14, 2018 at 08:09:47PM +0200, Paul B Mahol wrote:
> Hi,
> 
> very important patch attached.

>  avidec.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> cafdde2706e703bcac31ddfada057ccce61518c9  0001-avformat-avidec-fix-demuxing-of-all-keyframes-of-sam.patch
> From 52e92ad5b608c93690b5fceeebc306f094b1905f Mon Sep 17 00:00:00 2001
> From: Paul B Mahol <onemda at gmail.com>
> Date: Tue, 14 Aug 2018 20:06:32 +0200
> Subject: [PATCH] avformat/avidec: fix demuxing of all keyframes of sample
>  200707170736151.avi
> 
> ---
>  libavformat/avidec.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> index bafe1dc8da..61aedb6354 100644
> --- a/libavformat/avidec.c
> +++ b/libavformat/avidec.c
> @@ -1279,9 +1279,9 @@ start_sync:
>                  goto start_sync;
>              } else if (((ast->prefix_count < 5 || sync + 9 > i) &&
>                          d[2] < 128 && d[3] < 128) ||
> -                       d[2] * 256 + d[3] == ast->prefix /* ||
> -                       (d[2] == 'd' && d[3] == 'c') ||
> -                       (d[2] == 'w' && d[3] == 'b') */) {
> +                       d[2] * 256 + d[3] == ast->prefix ||
> +                       (d[2] == 'd' && (d[3] == 'c' || d[3] == 'b')) ||
> +                       (d[2] == 'w' && d[3] == 'b')) {

This change is ok but it is not a correct fix for the bug

looking at the sample file
it contains "wc" chunks, which the avi demuxer fails to parse (they also
look like they violate the RIFF spec but that doesnt matter here)
now after failing to parse a wc chunk the demuxer goes into "error resync mode"
and searches for the next chunk. 
And this is why i think this is not correct: It searches the apparent inside
of the wc chunk for the next chunk. This then continues after the wc chunk.
But fails as the chunks oscilate within streams. You patch improves error resync
for these oscilating chunk types. But it still searches the "wc inside" and
that could fail if by bad luck a chunk like sequence of bytes occurs in it

Please somehow detect these special odd wc chunks and skip over them so this data
is not searched for other chunk matches.
This alone should fix the issue but your patch here LGTM too in addition

thanks

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180815/e8fcf20f/attachment.sig>