[vlc-devel] [PATCH] RealVideo Support

[王波]

> Date: Thu, 22 May 2008 21:01:14 -0700> From: jb at videolan.org> To: vlc-devel at videolan.org> Subject: Re: [vlc-devel] [PATCH] RealVideo Support> > Hello,> > I just have a few random questions, and many of them are cosmetic. :D
 
my pleasure. :)> > - Can you split the rstp part and the realvideo part or are they linked> too much ? Same for the realvideo decoder and the demuxer> modifications...
 
yes, i can split the patch to 3 part.
> > - Why do you remove the AC_CHECK_FUNCS on uselocale ?
 
configure.ac ?
since i do the work in 0.8.6, maybe it is problem when i switch to git version.
> > - Why is RtspReadAndSkip( ) returning int if you never check the return> value and always return 0 ? Why not void ?
 
design but not impliment, maybe it should return some bytes which skipped.
but in fact the bytes is meaningless.
> > - I think that> > + if( psz ) free( psz );> can be used as free( psz ) in vlc. So, the modifications on that are> useless... This was discussed on this mailing list before, IIRC.> 
 
o, i don't know and not sure. maybe i can remove the useless if.
 
> - Why removing intf_UserFatal ? (this might be a good idea, btw)
 
don't know more about intf_UserFatal. ...
 
> > - Why removing case ACCESS_GET_CONTENT_TYPE: ?
 
maybe it be add in new git version.
i will add it
> > - Some changes in rtsp/real.c are just adding trailing spaces on the> lines. Please remove those.> > - Why this change :> > - ptr1=(uint8_t*)(key+16);> > - ptr2=(uint8_t*)(key+20);> > + ptr1=(key+16);> > + ptr2=(key+20);> > Same for this one:> > - rtsp_send_ok( rtsp_session );> > - free( challenge1 );> > - free( alert );> > - free( buf );> > + rtsp_send_ok(rtsp_session);> > + if( challenge1 ) free(challenge1);> > + if( alert ) free(alert);> > + if( buf ) free(buf);> > And that one ?> > - if( rtsp_read_data(rtsp_session, (uint8_t*)description, size) <= 0)> > + if( rtsp_read_data(rtsp_session, description, size) <= 0)
 
(uint8_t*) maybe is added in git version... my original code a little old.
> > Or that one (cosmetic) ?> > - desc->stream = malloc(sizeof(sdpplin_stream_t*)*desc->stream_count);> > - handled=1;> > - data=nl(data);> > + desc->stream = malloc(sizeof(sdpplin_stream_t*)*desc->stream_count);> > + handled=1;> > + data=nl(data);> > - On realvideo.c there is a realaudio.c header. Copyright dates are> wrong.
 
yes.
> > - on that part, you should look at the realaudio.c file, because it> looks in more parts, but that is a detail.> > +#ifdef WIN32> > +const char *g_decode_path="plugins\\drv43260.dll";> > +#else> > +const char *g_decode_path="../lib/vlc/codec/drv4.so.6.0";> > +#endif> > - Check mallocs ?> > + decoder_sys_t *p_sys = malloc( sizeof( decoder_sys_t ) );
ok
> > - I think you add unnecessary includes in your patch (stdlib, for> example)> - +#if 1 ?> > - Please comment in english...> > + // ����ʹ��Stream��position������ʹ��frame��druationӦ�ø���׼��> > - You have many commented lines that you add, why ?
ok, because the patch not perfect , so i record some comments/codes for futures
improve. maybe i need clean it.
> > This was just randomed questions, I may be wrong, too...> > This patch is looking like an old patch here that was never included> because it wasn't clean enough IIRC, and that I was unable to make work> on linux... If we can make it that would be great.... :D
 
yes, the realvideo's patch is coming from it, and i do improve about rtsp, demux, etc.
> > Thanks a lot for the work.
 
ok, i will review the code and submit again, hehe .
i must agree, i an not too professional as a programmer. :(
 
-- 
wangbo
> > > Best Regards,> > -- > Jean-Baptiste Kempf> http://www.jbkempf.com/
_________________________________________________________________
用手机MSN聊天写邮件看空间，无限沟通，分享精彩！
http://mobile.msn.com.cn/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20080523/86913478/attachment.html>