[vlc-devel] [PATCH] realvideo bug fix and document
xxcv
xxcv07 at gmail.com
Tue Feb 24 09:21:22 CET 2009
Jean-Baptiste Kempf wrote:
> 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
>>
>
>
Someone hurry up and commit this code already ... what's the wait about ???
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090224/04d4b0eb/attachment.html>
More information about the vlc-devel
mailing list