[FFmpeg-devel] [PATCH 2/3] nutdec: stop skipping bytes at EOF

[Andreas Cadhalpun]

On 20.05.2015 17:02, Michael Niedermayer wrote:
> On Wed, May 20, 2015 at 04:35:10PM +0200, Andreas Cadhalpun wrote:
>> On 20.05.2015 03:15, Michael Niedermayer wrote:
>>> On Wed, May 20, 2015 at 12:49:55AM +0200, Andreas Cadhalpun wrote:
>>>> This can unnecessarily waste a lot of time.
>>>>
>>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>>>> ---
>>>>  libavformat/nutdec.c | 8 +++++---
>>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
>>>> index a75587f..e979ee6 100644
>>>> --- a/libavformat/nutdec.c
>>>> +++ b/libavformat/nutdec.c
>>>> @@ -47,6 +47,8 @@ static int get_str(AVIOContext *bc, char *string, unsigned int maxlen)
>>>>      while (len > maxlen) {
>>>>          avio_r8(bc);
>>>>          len--;
>>>> +        if (bc->eof_reached)
>>>> +            len = maxlen;
>>>>      }
>>>
>>> maybe this would be better as avio_skip()
>>> but ok either way
>>
>> I prefer to avoid the additional complexity of avio_skip (see below...).
>>
>>>>      if (maxlen)
>>>> @@ -211,7 +213,7 @@ static int skip_reserved(AVIOContext *bc, int64_t pos)
>>>>          avio_seek(bc, pos, SEEK_CUR);
>>>>          return AVERROR_INVALIDDATA;
>>>>      } else {
>>>> -        while (pos--)
>>>> +        while (pos-- && !bc->eof_reached)
>>>>              avio_r8(bc);
>>>>          return 0;
>>>>      }
>>>> @@ -291,7 +293,7 @@ static int decode_main_header(NUTContext *nut)
>>>>          if (tmp_fields > 7)
>>>>              tmp_head_idx = ffio_read_varlen(bc);
>>>>  
>>>> -        while (tmp_fields-- > 8)
>>>> +        while (tmp_fields-- > 8 && !bc->eof_reached)
>>>>              ffio_read_varlen(bc);
>>>>  
>>>>          if (count <= 0 || count > 256 - (i <= 'N') - i) {
>>>> @@ -990,7 +992,7 @@ static int decode_frame_header(NUTContext *nut, int64_t *pts, int *stream_id,
>>>>          *header_idx = ffio_read_varlen(bc);
>>>>      if (flags & FLAG_RESERVED)
>>>>          reserved_count = ffio_read_varlen(bc);
>>>> -    for (i = 0; i < reserved_count; i++)
>>>> +    for (i = 0; i < reserved_count && !bc->eof_reached; i++)
>>>>          ffio_read_varlen(bc);
>>>
>>> these should return an error in the eof case
>>
>> OK, patch updated.
>>
>>> the first of the 3 could use a seek/skip as well possibly
>>
>> I tried this, but it caused weird crashes in av_crc, so I reverted that.
>>
> 
>>> also if you want some of these things could also be limited by te
>>> packet end from get_packetheader and not just EOF
>>
>> I'm not sure this would help much, because get_packetheader reads the
>> size from the file, so it could be an arbitrary value as well. 
> 
> yes for a mallicious file but one that is just damaged
> could recover quickly and contiue playback if the header size was
> used while otherwise it might read till EOF rendering it unwatchable

I see what you mean.

> but thats orthogonal to the patch 

Indeed.

> patch LGTM

Applied.

Best regards,
Andreas