[MPlayer-dev-eng] Re: [PATCH] asf demuxer changes for dvr-ms
Nico Sabbi
nicola_sabbi at fastwebnet.it
Wed Apr 11 00:23:49 CEST 2007
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
More information about the MPlayer-dev-eng
mailing list