[MPlayer-dev-eng] Re: [PATCH] asf demuxer changes for dvr-ms

[Nico Sabbi]

John Donaghy wrote:
>>
>> uhm the most traumatic patch first, eh? :)
> 
> 
> 
> Sorry to put you through this but thanks for reviewing it.
> 
> Premitting that I know nothing of asf in general I noticed the
> 
>> following issues:
>> - in get_ext_stream_properties() I'd prefer to have some more
>> boundary checks against buf_len
> 
> 

your quoting has the strangest look :)

> 
> I'm not quite sure what you mean by this.

I mean that you increase buffer+=N without checking
that buffer don't overflow &buf[buf_len-1]

> 
> - init_priv() can be made easier with calloc() + some assignments
> 
> 
> It appears that the private data is already allocated using calloc in
> asf_check_header. So do you mean that init_priv should only bother to set
> anything that is non-zero?

yes

> 
> - in get_payload_entension_data() defaulting to 50 fps is
> 
>>    a bit nonsense (no one uses 50 fps, better default to 60
>>    that is surely much more used)
> 
> 
> 
> Actually Jon Elwood discovered that there are some 50fps dvr-ms files out
> there and he sent me an update to my patch which I'll include when I resend
> (if (((segment_marker & 0XF0) >> 4) == 6) then it's 50fps). I will change
> the else clause to default to 60fps though as you suggest.
> 
> - did you notice if the three frame-type bits are always in the range
> 
>> [1..3] ?
> 
> 
> 
> I did - I assume these indicate I, P and B frames and I'll add a comment to
> say this. Is this information useful for anything at the minute though?

no, I asked out of pure curiosity