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

[John Donaghy]

>
> 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


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

- 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?

- 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?

since this code is executed only for dvr-ms I'm not opposed to commit
> this patch when those issues are fixed && if nothing breaks.


Great. Thanks again.

John