[FFmpeg-devel] [PATCH] avformat/mov: fix hang while seek on a kind of fragmented mp4.

[Michael Niedermayer]

On Mon, Feb 25, 2019 at 07:07:27PM +0800, C.H.Liu wrote:
>  Would you mind share your input file? I still can’t reproduce the OOM.

i belive i cannot share this sample, but the patch really needs to be
fully reviewed not just one bug that was found by chance fixed otherwise
we might be missing other bugs ...


> 
> 
> I have tried several types of mp4.
> 
> I also tried to create a file like yours. According to the snapshot, seems
> it has no ‘sidx’ box and enable ‘use_mfra_for’.
> 
> Massif didn’t report update_frag_index() function. I didn’t see an extra
> heap as big as here, either.

looking at the sample that triggers the OOM the do/while loop you add 
to update_frag_index() is entered and never exits while doing allocations
in it.

also the patch as is is not ok. You for example have a list of unrelated
changes in the commit message. Each should be in its own patch
also changes that are functional should be splited from non functional
changes. For example here:

     // If moof_offset already exists in frag_index, return index to it
-    index = search_frag_moof_offset(&c->frag_index, offset);
-    if (index < c->frag_index.nb_items &&
-        c->frag_index.item[index].moof_offset == offset)
+    index = search_frag_moof_offset(frag_index, offset);
+    if (index < frag_index->nb_items &&
+        frag_index->item[index].moof_offset == offset) {
+        frag_index->current = index;
         return index;
+    }

you change frag_index to a local variable this is non functional and
should not be in a patch that does a functional change ideally.
Now sure if a functional change is trivial and clear such things are
sometimes done together but this patch is not clear, at least its not
clear to me ...

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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/20190227/867a5692/attachment.sig>