[FFmpeg-devel] [libav-devel] [PATCH 2/4] mov: check for positive sample->size

[Andreas Cadhalpun]

On 25.05.2015 23:41, Luca Barbato wrote:
> On 25/05/15 23:22, Andreas Cadhalpun wrote:
>> On 25.05.2015 21:38, Michael Niedermayer wrote:
>>> On Mon, May 25, 2015 at 05:25:18PM +0200, Andreas Cadhalpun wrote:
>>> [...]
>>>
>>>>   mov.c |    7 +++++++
>>>>   1 file changed, 7 insertions(+)
>>>> 7ff306f094f2ecd47b720deb20ea318c24efaf4d  0002-mov-reject-zero-bytes_per_frame-with-non-zero-sample.patch
>>>>  From 42c8b0c216b39fd2cb8b329669737ce771ecdd20 Mon Sep 17 00:00:00 2001
>>>> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>>>> Date: Mon, 25 May 2015 17:17:39 +0200
>>>> Subject: [PATCH 2/2] mov: reject zero bytes_per_frame with non-zero
>>>>   samples_per_frame
>>>>
>>>> In this case the mov demuxer can return a large number of empty packets.
>>>
>>> patch should be ok, maybe add avpriv_request_sample()
> 
> The patch seems to silently stop indexing at the first packet not matching
> some sanity check.
> 
> That whole function is kind of bad to begin with since it silently stops
> even if it does because it is out of memory...
> 
> I do iterate my request to just mark the packets as corrupted and return
> them and/or provide an explode option to just stop there assuming the user
> is this concerned since this patch doesn't look exactly great to me.

This check happens beneath mov_read_header, not mov_read_packet.
If the condition is true, the index created by mov_build_index consists
only of entries with size zero, which seems rather useless, because it
causes mov_read_packet to always return empty packets.
Thus I don't understand what you don't like about discarding such an index.

> I'm not sure if I have time to write to write one myself soon, but I would
> appreciate if you get me a sample if you are happy with what you have here already.

Unfortunately I can't, because all the samples I have take a different branch
in Libav, because commit 1892052f8 is not present.

Best regards,
Andreas