[vlc-devel] [PATCH] RealVideo Support

[Jean-Baptiste Kempf]

Hello,

I just have a few random questions, and many of them are cosmetic. :D

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

- Why do you remove the AC_CHECK_FUNCS on uselocale ?

- Why is RtspReadAndSkip( ) returning int if you never check the return
  value and always return 0 ? Why not void ?

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

- Why removing intf_UserFatal ? (this might be a good idea, btw)

- Why removing  case ACCESS_GET_CONTENT_TYPE: ?

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

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.

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

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

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

Thanks a lot for the work.


Best Regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/