[MPlayer-dev-eng] [PATCH] - fix for audio and video in dvr-ms asf

[John Donaghy]

Hi

Thanks for reviewing my changes
>
> > +static int find_backwards_asf_guid(char *buf, const char *guid, int cur_pos)
> > +{
> > +  int i;
> > +  for (i=cur_pos; i>0; i--) {
> > +    if (memcmp(&buf[i], guid, 16) == 0)
> > +      return i + 16 + 8; // point after guid + length
> > +  }
> > +  return -1;
> > +}
> > +
>
> I wonder if either a buf_len parameter or starting from cur_pos -
> 16 might be a good idea, but I guess you know better.
> But this reminds reminds me: please add doxygen comments as described in
> DOCS/tech/code-documentation.txt, so at least assumptions like "cur_pos
> point at most to a position 16 bytes before the end of the buffer" are
> documentation, because otherwise I'm sure somebody will sooner or later
> miss that little detail.

I agree it's better to start at cur_pos - 16 so I'll add that to the
next submission. In that case do you still want me to document the new
function (and can you give me an example of how this should look)?

> cosmetics (and a bad one at that *g*), you added trailing whitespace to
> the empty line.

> cosmetics as well, what did you change in that { line?
Not sure how the trailing space got there. I will deal with these too.

>
> > @@ -30,6 +35,28 @@
> >                        ((unsigned char*)(p))[1]<<8)
> >  #endif
> >
> > +#if defined(ARCH_X86) || defined(ARCH_X86_64)
> > +#    define unaligned32(a) (*(const uint32_t*)(a))
> > +#else
> > +#    ifdef __GNUC__
> > +static inline uint32_t unaligned32(const void *v) {
> > +    struct Unaligned {
> > +        uint32_t i;
> > +    } __attribute__((packed));
> > +
> > +    return ((const struct Unaligned *) v)->i;
> > +}
> > +#    elif defined(__DECC)
> > +static inline uint32_t unaligned32(const void *v) {
> > +    return *(const __unaligned uint32_t *) v;
> > +}
> > +#    else
> > +static inline uint32_t unaligned32(const void *v) {
> > +    return *(const uint32_t *) v;
> > +}
> > +#    endif
> > +#endif //!ARCH_X86
> > +
>
> Ugh. Scary. Why can't you use the LOAD_LE32 macros? Or alternatively
> make similar LOAD_BE32 macros? This at least looks like both a
> portability and maintainability macros - at best acceptable in highly
> performance-critical code (if it makes a performance difference at all).
>

This piece of code is pretty much copied from ffmpeg. I didn't change
it because I'm not sure I fully understand the implications of doing
so on all the different architectures. There is one call to
'unaligned32' in the 'find_start_code' function (which itself
originates in ffmpeg):

  *state=  be2me_32(unaligned32(p));

If you can tell me how I could do it with the MPlayer macros in an
'architecture-safe' way I'd be grateful.

Also, what about the ASFMIN macro I added? - is there one already
defined somewhere else that I can pick up (sorry, but I don't know the
mplayer code at all well)

Regards,

John