[vlc-devel] [PATCH] realvideo bug fix and document
Jean-Baptiste Kempf
jb at videolan.org
Wed Feb 11 02:13:44 CET 2009
Wang, any updates?
On Thu, Feb 05, 2009 at 08:31:38PM +0100, Laurent Aimar wrote :
> Hi,
>
> It seems to me that your patch should be splitted in two:
> 1. Correction to make the plugin work after the demuxer changes
> 2. The dll/so path changes.
>
> I would prefer it as it is easier to review/comments and then eases
> regression searches.
>
> (Some of the following comments apply to the code that you may not have
> been responsible for.)
>
> > diff --git a/modules/codec/realvideo.c b/modules/codec/realvideo.c
> > index 6e5c512..92e9da1 100644
> > --- a/modules/codec/realvideo.c
> > +++ b/modules/codec/realvideo.c
> > @@ -31,6 +31,8 @@
> > #include <vlc_vout.h>
> > #include <vlc_codec.h>
> >
> > +#include <vlc_network.h>
> > +
> > #ifdef LOADER
> > /* Need the w32dll loader from mplayer */
> > # include <wine/winerror.h>
> > @@ -89,9 +91,9 @@ struct rv_init_t
> > short h;
> > short unk3;
> > int unk2;
> > - unsigned int * subformat;
> > + int subformat;
> > int unk5;
> > - unsigned int * format;
> > + int format;
> > } rv_init_t;
> >
> > struct decoder_sys_t
> > @@ -241,19 +243,85 @@ static int InitVideo(decoder_t *p_dec)
> > init_data.h = p_dec->fmt_in.video.i_height ;
> > init_data.unk3 = 0;
> > init_data.unk2 = 0;
> > - init_data.subformat = (unsigned int*)p_vide[0];
> > + init_data.subformat = ntohl( p_vide[0] );
> > init_data.unk5 = 1;
> > - init_data.format = (unsigned int*)p_vide[1];
> > + init_data.format = ntohl( p_vide[1]);
>
> Using GetDWBE would be better and would avoid vlc_network.h inclusion.
> You have to use it directky on p_dec->fmt_in.p_extra and compute yourself the
> offset (it would also remove the implicit ugly unsigned int * cast).
>
> > /* first try to load linux dlls, if failed and we're supporting win32 dlls,
> > then try to load the windows ones */
> > - bool b_so_opened = false;
> > +
> > + p_sys->rv_handle = NULL;
> >
> > #ifdef WIN32
> > - g_decode_path="plugins\\drv43260.dll";
> >
> > - if( (p_sys->rv_handle = load_syms(p_dec, g_decode_path)) )
> > - b_so_opened = true;
> > + const char psz_paths[MAX_PATH*3] ;
> The const should be removed as you modify it.
> Now using an array would be really better, like
> char ppsz_path[3][MAX_PATH+1]
> as it would simplify a lot the code.
>
> > + {
> > + HKEY h_key;
> > + DWORD i_type, i_data = MAX_PATH + 1;
>
> > + char psz_codecs[MAX_PATH+1];
> > + char * p_data ;
> > + char * p_end = psz_paths;
> With char ppsz_path[3][MAX_PATH+1] they can be removed I think.
> > +
> > + p_data = psz_codecs;
> > + if ( RegOpenKeyEx( HKEY_CLASSES_ROOT,
> > + _T("Software\\RealNetworks\\Preferences\\DT_Codecs"),
> > + 0, KEY_READ, &h_key ) == ERROR_SUCCESS )
> > + {
> > + if ( RegQueryValueEx( h_key, _T(""), 0, &i_type,
> > + (LPBYTE) p_data, &i_data ) == ERROR_SUCCESS
> > + && i_type == REG_SZ )
> > + {
> > + int i_len = strlen( p_data );
> > + if ( i_len && p_data[i_len-1] == '\\' )
> > + p_data[i_len - 1] = '\0';
> > + memcpy( p_end, p_data, strlen( p_data) + 1 );
> > + p_end += strlen( p_data ) + 1;
> > + }
> > + RegCloseKey( h_key );
> > + }
> > +
> > + p_data = psz_codecs;
> > + if ( RegOpenKeyEx( HKEY_CLASSES_ROOT,
> > + _T("Helix\\HelixSDK\\10.0\\Preferences\\DT_Codecs"),
> > + 0, KEY_READ, &h_key ) == ERROR_SUCCESS )
> > + {
> > + if ( RegQueryValueEx( h_key, _T(""), 0, &i_type,
> > + (LPBYTE) p_data, &i_data ) == ERROR_SUCCESS
> > + && i_type == REG_SZ )
> > + {
> > + int i_len = strlen( p_data );
> > + if ( i_len && p_data[i_len-1] == '\\' )
> > + p_data[i_len - 1] = '\0';
> > + memcpy( p_end, p_data, strlen( p_data) + 1 );
> > + p_end += strlen( p_data ) + 1;
> > + }
> > + RegCloseKey( h_key );
> > + }
> > +
> > + memcpy( p_end, ".\0codecs\0plugins\0\0\0", 20 );
> > + }
> > +
> > + for( size_t i = 0; psz_paths[i]; i += strlen( psz_paths + i ) + 1 )
> > + {
> > + if( asprintf( &g_decode_path, "%s/drv4.dll", psz_paths + i ) != -1 )
> > + {
> > + p_sys->rv_handle = load_syms(p_dec, g_decode_path);
> > + free( g_decode_path );
> > + }
> > + if( p_sys->rv_handle )
> > + break;
> > +
> > + if( asprintf( &g_decode_path, "%s/drv43260.dll", psz_paths + i ) != -1 )
> > + {
> > + p_sys->rv_handle = load_syms(p_dec, g_decode_path);
> > + free( g_decode_path );
> > + }
> > + if( p_sys->rv_handle )
> > + break;
> > +
> > + /*msg_Dbg( p_dec, "Cannot load real decoder library: %s", g_decode_path);*/
> > + }
> > #else
> > static const char psz_paths[] =
> > {
> > @@ -272,7 +340,10 @@ static int InitVideo(decoder_t *p_dec)
> > "/usr/lib64/RealPlayer10/codecs\0"
> > "/usr/lib64/RealPlayer10GOLD/codecs\0"
> > "/usr/local/lib/codecs\0"
> > - "\0"
> > + ".\0"
> > + "codecs\0"
> > + "plugins\0"
> > + "\0\0"
> > };
> Same here (array).
> >
> > for( size_t i = 0; psz_paths[i]; i += strlen( psz_paths + i ) + 1 )
> > @@ -283,10 +354,7 @@ static int InitVideo(decoder_t *p_dec)
> > free( g_decode_path );
> > }
> > if( p_sys->rv_handle )
> > - {
> > - b_so_opened = true;
> > break;
> > - }
> >
> > if( asprintf( &g_decode_path, "%s/drv3.so.6.0", psz_paths + i ) != -1 )
> > {
> > @@ -294,16 +362,13 @@ static int InitVideo(decoder_t *p_dec)
> > free( g_decode_path );
> > }
> > if( p_sys->rv_handle )
> > - {
> > - b_so_opened = true;
> > break;
> > - }
> >
> > - msg_Dbg( p_dec, "Cannot load real decoder library: %s", g_decode_path);
> > + /*msg_Dbg( p_dec, "Cannot load real decoder library: %s", g_decode_path);*/
> > }
> > #endif
> >
> > - if(!b_so_opened )
> > + if( p_sys->rv_handle == NULL )
> > {
> > msg_Err( p_dec, "Cannot any real decoder library" );
> > free( p_sys );
> > @@ -321,24 +386,27 @@ static int InitVideo(decoder_t *p_dec)
> > result=(*rvyuv_init)(&init_data, &p_sys->handle);
> > if (result)
> > {
> > - msg_Err( p_dec, "Cannot Init real decoder library: %s", g_decode_path);
> > + msg_Err( p_dec, "Cannot Init real decoder, error code: %X", result );
> > free( p_sys );
> > return VLC_EGENERIC;
> > }
> >
> > + msg_Dbg( p_dec, "codec id: 0x%8X subid: 0x%8X\n", ntohl(p_vide[1]), ntohl(p_vide[0]) );
> > +
> > /* setup rv30 codec (codec sub-type and image dimensions): */
> > /*if ( p_dec->fmt_in.i_codec == VLC_FOURCC('R','V','3','0') )*/
> > - if (p_vide[1]>=0x20200002)
> > + if ( ntohl( p_vide[1])>=0x20200002)
> Why not storing the ntohl( p_vide[1]) (same with p_vide[0]) values in a local variables ?
>
> > {
> > - int i, cmsg_cnt;
> > + int cmsg_cnt;
> > + unsigned char * p_extrahdr = (unsigned char *) p_vide ;
> uint8_t
>
> > uint32_t cmsg24[16]={p_dec->fmt_in.video.i_width,p_dec->fmt_in.video.i_height};
> > - cmsg_data_t cmsg_data={0x24,1+(p_vide[1]&7), &cmsg24[0]};
> > - cmsg_cnt = (p_vide[1]&7)*2;
> > + cmsg_data_t cmsg_data={0x24,1+(p_extrahdr[1]&7), &cmsg24[0]};
> > + cmsg_cnt = (p_extrahdr[1]&7)*2;
> > if (i_vide - 8 < cmsg_cnt) {
> > cmsg_cnt = i_vide - 8;
> > }
> > - for (i = 0; i < cmsg_cnt; i++)
> > - cmsg24[2+i] = p_vide[8+i]*4;
> > + for (int i = 0; i < cmsg_cnt; i++)
> > + cmsg24[2+i] = p_extrahdr[8+i]*4;
> > #ifdef WIN32
> > if (dll_type == 1)
> > (*wrvyuv_custom_message)(&cmsg_data,p_sys->handle);
> > @@ -349,7 +417,7 @@ static int InitVideo(decoder_t *p_dec)
> > /*
> > es_format_Init( &p_dec->fmt_out, VIDEO_ES, VLC_FOURCC( 'Y','V','1','2' ));
> > es_format_Init( &p_dec->fmt_out, VIDEO_ES, VLC_FOURCC( 'Y','U','Y','2' ));
> > - */
> > + */
> > es_format_Init( &p_dec->fmt_out, VIDEO_ES, VLC_FOURCC( 'I', '4', '2', '0'));
> >
> > p_dec->fmt_out.video.i_width = p_dec->fmt_in.video.i_width;
> > @@ -467,25 +535,20 @@ static picture_t *DecodeVideo( decoder_t *p_dec, block_t **pp_block )
> > if ( p_pic )
> > {
> > unsigned int transform_out[5];
> > - dp_hdr_t dp_hdr;
> > transform_in_t transform_in;
> > - uint32_t pkg_len = ((uint32_t*)p_block->p_buffer)[0];
> > - unsigned char* dp_data=((unsigned char*)p_block->p_buffer)+8;
> > - uint32_t* extra=(uint32_t*)(((char*)p_block->p_buffer)+8+pkg_len);
> > - uint32_t img_size;
> > -
> >
> > - dp_hdr.len = pkg_len;
> > - dp_hdr.chunktab = 8 + pkg_len;
> > - dp_hdr.chunks = ((uint32_t*)p_block->p_buffer)[1]-1;
> > - dp_hdr.timestamp = i_pts;
> > + uint8_t i_chunks = p_block->p_buffer[0];
> > + uint32_t pkg_len = p_block->i_buffer - 2*4*(i_chunks + 1) - 1;
> i_ prefix.
>
> > + unsigned char* dp_data=p_block->p_buffer + 2*4*(i_chunks+1) + 1;
> uint8_t
>
> > + uint32_t* extra=(uint32_t*)(p_block->p_buffer + 1);
> pi_ prefix.
> > + uint32_t img_size;
> >
> > memset(&transform_in, 0, sizeof(transform_in_t));
> >
> > - transform_in.len = dp_hdr.len;
> > + transform_in.len = pkg_len;
> > transform_in.extra = extra;
> You can simply avoid the need of a local extra variable.
>
> > - transform_in.chunks = dp_hdr.chunks;
> > - transform_in.timestamp = dp_hdr.timestamp;
> > + transform_in.chunks = i_chunks;
> > + transform_in.timestamp = i_pts;
> >
> > memset (p_sys->plane, 0, p_dec->fmt_in.video.i_width * p_dec->fmt_in.video.i_height *3/2 );
> >
> > @@ -496,17 +559,7 @@ static picture_t *DecodeVideo( decoder_t *p_dec, block_t **pp_block )
> > #endif
> > result=(*rvyuv_transform)(dp_data, p_sys->plane, &transform_in, transform_out, p_sys->handle);
> >
> > - /* msg_Warn(p_dec, "Real Size %d X %d", transform_out[3],
> > - transform_out[4]); */
> > - /* some bug rm file will print the messages :
> > - [00000551] realvideo decoder warning: Real Size 320 X 240
> > - [00000551] realvideo decoder warning: Real Size 480 X 272
> > - [00000551] realvideo decoder warning: Real Size 320 X 240
> > - [00000551] realvideo decoder warning: Real Size 320 X 240
> > - ...
> > - so it needs fixing!
> > - */
> > - if ( p_sys->inited == 0 )
> > + if ( result == 0 && p_sys->inited == 0 )
> > {
> > /* fix and get the correct image size! */
> > if ( p_dec->fmt_in.video.i_width != transform_out[3]
> > @@ -540,16 +593,19 @@ static picture_t *DecodeVideo( decoder_t *p_dec, block_t **pp_block )
> > p_sys->inited = 1;
> > }
> >
> > - img_size = p_dec->fmt_in.video.i_width * p_dec->fmt_in.video.i_height;
> > - memcpy( p_pic->p[0].p_pixels, p_sys->plane, img_size);
> > - memcpy( p_pic->p[1].p_pixels, p_sys->plane + img_size, img_size/4);
> > - memcpy( p_pic->p[2].p_pixels, p_sys->plane + img_size * 5/4, img_size/4);
> > - p_pic->date = i_pts ;
> > + if ( result == 0 )
> > + {
> > + img_size = p_dec->fmt_in.video.i_width * p_dec->fmt_in.video.i_height;
> > + memcpy( p_pic->p[0].p_pixels, p_sys->plane, img_size);
> > + memcpy( p_pic->p[1].p_pixels, p_sys->plane + img_size, img_size/4);
> > + memcpy( p_pic->p[2].p_pixels, p_sys->plane + img_size * 5/4, img_size/4);
> > + p_pic->date = i_pts ;
> Using vlc_memcpy here would probably be faster.
>
> >
> > /* real video frame is small( frame and frame's time-shift is short),
> > so it will become late picture easier (when render-time changed)and
> > droped by video-output.*/
> > - p_pic->b_force = 1;
> > + p_pic->b_force = 1;
> Using true is prefered.
> > + }
> > }
> >
> > vlc_mutex_unlock( &rm_mutex );
>
> Regards,
>
> --
> fenrir
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
--
Best Regards,
--
Jean-Baptiste Kempf
http://www.jbkempf.com/
More information about the vlc-devel
mailing list