<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
<title></title>
</head>
<body bgcolor="#ffffff" text="#000000">
Jean-Baptiste Kempf wrote:
<blockquote cite="mid:20090211011344.GB17667@videolan.org" type="cite">
<pre wrap="">Wang, any updates?
On Thu, Feb 05, 2009 at 08:31:38PM +0100, Laurent Aimar wrote :
</pre>
<blockquote type="cite">
<pre wrap="">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.)
</pre>
<blockquote type="cite">
<pre wrap="">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]);
</pre>
</blockquote>
<pre wrap=""> 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).
</pre>
<blockquote type="cite">
<pre wrap=""> /* 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] ;
</pre>
</blockquote>
<pre wrap="">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.
</pre>
<blockquote type="cite">
<pre wrap="">+ {
+ HKEY h_key;
+ DWORD i_type, i_data = MAX_PATH + 1;
</pre>
</blockquote>
<blockquote type="cite">
<pre wrap="">+ char psz_codecs[MAX_PATH+1];
+ char * p_data ;
+ char * p_end = psz_paths;
</pre>
</blockquote>
<pre wrap=""> With char ppsz_path[3][MAX_PATH+1] they can be removed I think.
</pre>
<blockquote type="cite">
<pre wrap="">+
+ 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"
};
</pre>
</blockquote>
<pre wrap=""> Same here (array).
</pre>
<blockquote type="cite">
<pre wrap="">
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)
</pre>
</blockquote>
<pre wrap=""> Why not storing the ntohl( p_vide[1]) (same with p_vide[0]) values in a local variables ?
</pre>
<blockquote type="cite">
<pre wrap=""> {
- int i, cmsg_cnt;
+ int cmsg_cnt;
+ unsigned char * p_extrahdr = (unsigned char *) p_vide ;
</pre>
</blockquote>
<pre wrap="">uint8_t
</pre>
<blockquote type="cite">
<pre wrap=""> 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;
</pre>
</blockquote>
<pre wrap="">i_ prefix.
</pre>
<blockquote type="cite">
<pre wrap="">+ unsigned char* dp_data=p_block->p_buffer + 2*4*(i_chunks+1) + 1;
</pre>
</blockquote>
<pre wrap="">uint8_t
</pre>
<blockquote type="cite">
<pre wrap="">+ uint32_t* extra=(uint32_t*)(p_block->p_buffer + 1);
</pre>
</blockquote>
<pre wrap="">pi_ prefix.
</pre>
<blockquote type="cite">
<pre wrap="">+ 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;
</pre>
</blockquote>
<pre wrap=""> You can simply avoid the need of a local extra variable.
</pre>
<blockquote type="cite">
<pre wrap="">- 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 ;
</pre>
</blockquote>
<pre wrap=""> Using vlc_memcpy here would probably be faster.
</pre>
<blockquote type="cite">
<pre wrap="">
/* 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;
</pre>
</blockquote>
<pre wrap=""> Using true is prefered.
</pre>
<blockquote type="cite">
<pre wrap="">+ }
}
vlc_mutex_unlock( &rm_mutex );
</pre>
</blockquote>
<pre wrap="">Regards,
--
fenrir
_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a class="moz-txt-link-freetext" href="http://mailman.videolan.org/listinfo/vlc-devel">http://mailman.videolan.org/listinfo/vlc-devel</a>
</pre>
</blockquote>
<pre wrap=""><!---->
</pre>
</blockquote>
<br>
Someone hurry up and commit this code already ... what's the wait about
???<br>
<br>
</body>
</html>