[FFmpeg-devel] [PATCH] RTSP-MS 14/15: ASF packet parsing

[Michael Niedermayer]

On Sun, Jul 26, 2009 at 11:54:05AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Sat, Jul 25, 2009 at 6:02 AM, Michael Niedermayer<michaelni at gmx.at> wrote:
> > this is still buggy, also it can easily end in an infinite loop
> 
> Hm, I think my brain misunderstood. Here's another attempt, I think
> this in fact fixes the issue you were referring to.
> 
> Ronald

>  Changelog             |    1 
>  libavformat/asfdec.c  |    8 ++
>  libavformat/rtp_asf.c |  194 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  libavformat/rtsp.h    |    4 +
>  4 files changed, 206 insertions(+), 1 deletion(-)
> cbca2710321848e28c2a3f81ea65d213113137dd  rtsp-ms-parse-asf-payload.patch
> Index: ffmpeg-svn/libavformat/rtp_asf.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/rtp_asf.c	2009-07-24 16:32:45.000000000 -0400
> +++ ffmpeg-svn/libavformat/rtp_asf.c	2009-07-26 11:52:38.000000000 -0400
> @@ -27,11 +27,71 @@
>  
>  #include <libavutil/base64.h>
>  #include <libavutil/avstring.h>
> +#include <libavutil/intreadwrite.h>
>  #include "rtp.h"
>  #include "rtp_asf.h"
>  #include "rtsp.h"
>  #include "asf.h"
>  
> +/**
> + * From MSDN 2.2.1.4, we learn that ASF data packets over RTP should not
> + * contain any padding. Unfortunately, the header min/max_pktsize are not
> + * updated (thus making min_pktsize invalid). Here, we "fix" these faulty
> + * min_pktsize values in the ASF file header.
> + */
> +static void
> +rtp_asf_fix_header(uint8_t *buf, int len)
> +{
> +    uint8_t *p = buf, *end = buf + len;
> +
> +    if (len < sizeof(ff_asf_guid) * 2 + 22 ||
> +        memcmp(p, ff_asf_header, sizeof(ff_asf_guid))) {
> +        return;
> +    }
> +    p += sizeof(ff_asf_guid) + 14;
> +    do {
> +        uint64_t len = AV_RL64(p + sizeof(ff_asf_guid));
> +        if (memcmp(p, ff_asf_file_header, sizeof(ff_asf_guid))) {
> +            uint8_t *next_p = p + len;
> +
> +            if (next_p <= p || next_p > end)
> +                break;

this should probably do more than just silently retunring
also
1. please do not use 2 local variable with the name len
2. id write if(len > end - p)


[...]

> Index: ffmpeg-svn/libavformat/asfdec.c
> ===================================================================
> --- ffmpeg-svn.orig/libavformat/asfdec.c	2009-07-24 16:32:45.000000000 -0400
> +++ ffmpeg-svn/libavformat/asfdec.c	2009-07-24 16:33:19.000000000 -0400
> @@ -606,6 +606,14 @@
>      }
>  
>      if (c != 0x82) {
> +        /**
> +         * This code allows handling of -EAGAIN at packet boundaries (i.e.
> +         * if the packet sync code above triggers -EAGAIN). This does not
> +         * imply complete -EAGAIN handling support at random positions in
> +         * the stream.
> +         */
> +        if (url_ferror(pb) == AVERROR(EAGAIN))
> +            return AVERROR(EAGAIN);
>          if (!url_feof(pb))
>              av_log(s, AV_LOG_ERROR, "ff asf bad header %x  at:%"PRId64"\n", c, url_ftell(pb));
>      }

hunk ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090726/e467ec1f/attachment.pgp>