[FFmpeg-devel] [PATCH] MSN Messenger TCP Webcam streams demuxer

[Michael Niedermayer]

On Tue, Mar 11, 2008 at 06:55:53PM -0300, Ramiro Polla wrote:
> Hello,
>
> Attached patch implements a demuxer for the TCP Webcam streams used by MSN 
> Messenger. The Mimic codec in my previous patch is used for them.
>
> Such files are created by MSN Webcam Recorder, aMSN and Mercury Messenger.
>
> Oh, and before anyone asks me to drop the TCP in the name, MSN Webcam 
> Recorder also records UDP transmissions into a different format. I'll 
> probably implement it when all this gets in...
[...]
> static unsigned int find_valid_start_byteio(ByteIOContext *pb)
> {
>     char buf[HEADER_SIZE];
>     uint64_t pos = 0;
>     int ret;
> 
>     for(;;) {
>         url_fseek(pb, pos, SEEK_SET);
>         if((ret = get_buffer(pb, buf, HEADER_SIZE)) != HEADER_SIZE)
>             return -1;
>         if((ret=find_valid_start(buf, HEADER_SIZE)) != -1)
>             return pos;
>         pos++;
>     }
>     /* Should never be reached, unless format was forced and not probed */
>     return -1;
> }

ret is unused, also this ++, seek back get_buffer() looks quite inefficient.
And will not work on non seekable input.


> 
> static int msnwc_tcp_read_header(AVFormatContext *ctx, AVFormatParameters *ap)
> {
>     ByteIOContext *pb = ctx->pb;
>     AVCodecContext *codec;
>     AVStream *st;
>     uint64_t pos;
> 
>     st = av_new_stream(ctx, 0);
>     if(!st)
>         return AVERROR_NOMEM;
> 
>     codec = st->codec;
>     codec->codec_type = CODEC_TYPE_VIDEO;
>     codec->codec_id = CODEC_ID_MIMIC;

>     codec->codec_tag = CODEC_ID_MIMIC;

isnt that ML20 the codec tag ?


> 
>     codec->extradata_size = MIMIC_HEADER_SIZE;
>     codec->extradata = av_malloc(codec->extradata_size);
>     if(!codec->extradata)
>         return AVERROR_NOMEM;
> 
>     av_set_pts_info(st, 32, 1, 1000);
> 
>     pos = find_valid_start_byteio(pb);
>     if(pos == -1)
>         return -1;
> 
>     if(pos)
>         av_log(ctx, AV_LOG_DEBUG, "skipped %d bytes of trash\n", pos);
> 
>     // Get width/height
>     url_fseek(pb, pos, SEEK_SET);
>     url_fskip(pb, 2);
>     codec->width  = get_le16(pb);
>     codec->height = get_le16(pb);

Please get rid of the backward seeking.


> 
>     // Get extradata
>     url_fseek (pb, pos + HEADER_SIZE, SEEK_SET);
>     get_buffer(pb, codec->extradata, MIMIC_HEADER_SIZE);
>     url_fseek (pb, pos, SEEK_SET);

This looks very suspicious, dont tell me you place the header of the first
packet in extradata this is definitly not ok.


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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20080313/37bdc0a4/attachment.pgp>