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
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel