[vlc-devel] Symbian VLC Merge

Rémi Denis-Courmont remi at remlab.net
Thu Jul 15 20:00:03 CEST 2010 

	Hello,

Comments inline...

diff --git a/bin/vlc.c b/bin/vlc.c
index 40fa96f..6f42197 100644
--- a/bin/vlc.c
+++ b/bin/vlc.c
@@ -44,6 +44,22 @@
 #include <string.h>
 #endif
 
+#ifdef SYMBIAN
+#include <vlc_common.h>
+static vlc_mutex_t exit_lock = VLC_STATIC_MUTEX;
+static vlc_cond_t  exiting = VLC_STATIC_COND;

You probably need a static boolean variable here...

+void symbian_libvlc_Wait( libvlc_int_t *p_libvlc );
+void symbian_libvlc_Wait( libvlc_int_t *p_libvlc )
+{
+    vlc_mutex_lock( &exit_lock );
+    /*while( vlc_object_alive( p_libvlc ) )

...to replace this condition...

+	{*/
+        vlc_cond_wait( &exiting, &exit_lock );

...otherwise this might break in case of spurious wake-up.

+        //printf("out\n");

Please leave out your old debug statements.

+	/*}*/
+    vlc_mutex_unlock( &exit_lock );
+}
+#endif

 
 /* Explicit HACK */
 extern void LocaleFree (const char *);
@@ -69,9 +85,15 @@ static bool signal_ignored (int signum)
 
 static void vlc_kill (void *data)
 {
+#ifndef SYMBIAN
     pthread_t *ps = data;
 
     pthread_kill (*ps, SIGTERM);
+#else
+    vlc_mutex_lock( &exit_lock );
+    vlc_cond_broadcast( &exiting );

Set the missing boolean variable here.

+    vlc_mutex_unlock( &exit_lock );
+#endif
 }
 
 
@@ -80,6 +102,7 @@ static void vlc_kill (void *data)
  *****************************************************************************/
 int main( int i_argc, const char *ppsz_argv[] )
 {
+#ifndef SYMBIAN
     /* The so-called POSIX-compliant MacOS X reportedly processes SIGPIPE 
even
      * if it is blocked in all thread. Also some libraries want SIGPIPE 
blocked
      * as they have no clue about signal masks.
@@ -164,7 +187,7 @@ int main( int i_argc, const char *ppsz_argv[] )
 #endif
     /* Block all these signals */
     pthread_sigmask (SIG_BLOCK, &set, NULL);
-
+#endif

I would really keep the empty line for readability.

     /* Note that FromLocale() can be used before libvlc is initialized */
     const char *argv[i_argc + 3];
     int argc = 0;
@@ -190,9 +213,9 @@ int main( int i_argc, const char *ppsz_argv[] )
         if ((argv[argc++] = FromLocale (ppsz_argv[i])) == NULL)
             return 1; // BOOM!
     argv[argc] = NULL;
-
+#ifndef SYMBIAN
     vlc_enable_override ();
-
+#endif

This looks wrong. Why you can't call the dummy vlc_enable_override() from 
bin/override.c ?

diff --git a/include/vlc_codecs.h b/include/vlc_codecs.h
index 6c087da..c5f132f 100644
--- a/include/vlc_codecs.h
+++ b/include/vlc_codecs.h
@@ -42,15 +42,20 @@ typedef struct _GUID
     uint8_t  Data4[8];
 } GUID, *REFGUID, *LPGUID;
 #endif /* GUID_DEFINED */
-

Please keep the empty line.

+#ifndef __WINSCW__

Should not be required...

 #ifdef HAVE_ATTRIBUTE_PACKED
 #   define ATTR_PACKED __attribute__((__packed__))
 #elif defined(__SUNPRO_C)
 #   pragma pack(1)
 #   define ATTR_PACKED
+#elif defined(__WINSCW__)
+#   define ATTR_PACKED

...as you already have this.

 #else
 #   error FIXME
 #endif
+#else
+#define ATTR_PACKED
+#endif
 
 #ifndef _WAVEFORMATEX_
 #define _WAVEFORMATEX_
diff --git a/include/vlc_common.h b/include/vlc_common.h
index 9a5617c..9407e18 100644
--- a/include/vlc_common.h
+++ b/include/vlc_common.h
@@ -56,6 +56,10 @@
 # include <stdbool.h>
 #endif
 
+#if defined(SYMBIAN) && !defined(__cplusplus)
+typedef int bool;
+#endif

Maybe this should be in vlc_fixups.h instead?


 /* Try to fix format strings for all versions of mingw and mingw64 */
 #if defined( _WIN32 ) && defined( __USE_MINGW_ANSI_STDIO )
  #undef PRId64
@@ -499,6 +503,10 @@ typedef int ( * vlc_callback_t ) ( vlc_object_t *,      
/* variable's object */
 #else
 # define LIBVLC_EXPORT
 #endif
+#ifdef SYMBIAN
+#undef LIBVLC_EXPORT
+#define LIBVLC_EXPORT __declspec(dllexport)
+#endif

Please use #elif defined in the previous #if block, so you won't need #undef.

 #define VLC_EXPORT( type, name, args ) \
                         LIBVLC_EXTERN LIBVLC_EXPORT type name args
 
@@ -906,7 +914,7 @@ VLC_EXPORT( const char *, VLC_Compiler, ( void ) 
LIBVLC_USED );
 #include "vlc_main.h"
 #include "vlc_configuration.h"
 
-#if defined( WIN32 ) || defined( UNDER_CE )
+#if defined( WIN32 ) || defined( UNDER_CE ) || defined(SYMBIAN)
 #   define DIR_SEP_CHAR '\\'
 #   define DIR_SEP "\\"
 #   define PATH_SEP_CHAR ';'
diff --git a/include/vlc_messages.h b/include/vlc_messages.h
index 15ce936..911c5f6 100644
--- a/include/vlc_messages.h
+++ b/include/vlc_messages.h
@@ -31,7 +31,9 @@
  * \file
  * This file defines structures and functions to handle messages and 
statistics gathering
  */
-
+#if defined(SYMBIAN) && defined(__LIBVLC__)
+#define MODULE_STRING "main"
+#endif

Is there really no way to define MODULE_STRING as a precompiler constant from 
the project configuration files? That would cut this patch size a LOT.

 #include <stdarg.h>
 
 /**
diff --git a/include/vlc_threads.h b/include/vlc_threads.h
index 3770e2f..b47769e 100644
--- a/include/vlc_threads.h
+++ b/include/vlc_threads.h
@@ -110,7 +110,19 @@ typedef pthread_mutex_t vlc_mutex_t;
 #define VLC_STATIC_MUTEX PTHREAD_MUTEX_INITIALIZER
 typedef pthread_cond_t  vlc_cond_t;
 #define VLC_STATIC_COND  PTHREAD_COND_INITIALIZER
+#ifndef SYMBIAN
 typedef pthread_rwlock_t vlc_rwlock_t;
+#else
+typedef struct
+{
+    vlc_mutex_t   mutex;
+    vlc_cond_t    read_wait;
+    vlc_cond_t    write_wait;
+    unsigned long readers;
+    unsigned long writers;
+    unsigned long         writer;
+} vlc_rwlock_t;
+#endif
 typedef pthread_key_t   vlc_threadvar_t;
 typedef struct vlc_timer *vlc_timer_t;
 
@@ -216,7 +228,7 @@ VLC_EXPORT( void, vlc_timer_destroy, (vlc_timer_t) );
 VLC_EXPORT( void, vlc_timer_schedule, (vlc_timer_t, bool, mtime_t, mtime_t) 
);
 VLC_EXPORT( unsigned, vlc_timer_getoverrun, (vlc_timer_t) LIBVLC_USED );
 
-#ifndef LIBVLC_USE_PTHREAD_CANCEL
+#if !defined(LIBVLC_USE_PTHREAD_CANCEL) || defined(SYMBIAN)

No! Please don't define LIBVLC_USE_PTHREAD_CANCEL at the beginning of 
vlc_thread.h instead.

 enum {
     VLC_CLEANUP_PUSH,
     VLC_CLEANUP_POP,
@@ -227,7 +239,7 @@ VLC_EXPORT( int, vlc_savecancel, (void) );
 VLC_EXPORT( void, vlc_restorecancel, (int state) );
 VLC_EXPORT( void, vlc_testcancel, (void) );
 
-#if defined (LIBVLC_USE_PTHREAD_CANCEL)
+#if defined (LIBVLC_USE_PTHREAD_CANCEL) &&!defined(SYMBIAN)

Same here.

diff --git a/modules/control/rc.c b/modules/control/rc.c
index fe10a0c..cec9a71 100644
--- a/modules/control/rc.c
+++ b/modules/control/rc.c
@@ -778,7 +787,9 @@ static void Run( intf_thread_t *p_intf )
         /* Command processed */
         i_size = 0; p_buffer[0] = 0;
     }
-
+#ifdef SYMBIAN
+	config_SaveConfigFile( p_intf, NULL );
+#endif

Is this really intended?? Also please keep the empty line.

     msg_rc( STATUS_CHANGE "( stop state: 0 )" );
     msg_rc( STATUS_CHANGE "( quit )" );
 
@@ -1555,12 +1566,12 @@ static int VolumeMove( vlc_object_t *p_this, char 
const *psz_cmd,
 
     if ( !strcmp(psz_cmd, "volup") )
     {
-        if ( aout_VolumeUp( p_this, i_nb_steps, &i_volume ) < 0 )
+        if ( aout_VolumeUp( p_intf->p_sys->p_playlist, i_nb_steps, &i_volume 
) < 0 )

I don't understand why you need that hack.

@@ -2005,6 +2016,13 @@ bool ReadCommand( intf_thread_t *p_intf, char 
*p_buffer, int *pi_size )
                        0 /*STDIN_FILENO*/ : p_intf->p_sys->i_socket, NULL,
                   (uint8_t *)p_buffer + *pi_size, 1, false ) ) > 0 )
     {
+#ifdef SYMBIAN
+    /*RC Interface needs some work for Other Platforms also.Here is What 
Symbian Non-Qwerty Keyboards Demand.*/
+    if((*(p_buffer+(*pi_size)))=='\b')
+	{
+    *pi_size-=2;
+	}
+#endif

Mind the indentation. What happens if pi_size equals 1 ??

diff --git a/src/audio_output/intf.c b/src/audio_output/intf.c
index 017ece2..cab2b4a 100644
--- a/src/audio_output/intf.c
+++ b/src/audio_output/intf.c
@@ -346,7 +346,9 @@ int aout_VolumeSoftGet( aout_instance_t * p_aout, 
audio_volume_t * pi_volume )
 /* Placeholder for pf_volume_set(). */
 int aout_VolumeSoftSet( aout_instance_t * p_aout, audio_volume_t i_volume )
 {
+#ifndef SYMBIAN
     aout_MixerMultiplierSet( p_aout, (float)i_volume / AOUT_VOLUME_DEFAULT );
+#endif

I am not sure this is a good idea.

diff --git a/src/config/cmdline.c b/src/config/cmdline.c
index d01c133..0f89a24 100644
--- a/src/config/cmdline.c
+++ b/src/config/cmdline.c
@@ -200,7 +200,7 @@ int config_LoadCmdLine( vlc_object_t *p_this, int i_argc,
      * Parse the command line options
      */
     vlc_getopt_t state;
-    state.optind = 0 ; /* set to 0 to tell GNU getopt to reinitialize */
+    state.opt_optind = 0 ; /* set to 0 to tell GNU getopt to reinitialize */

I second-guess that optind and optarg are evil macros on Symbian. In my 
opinion, it would be nicer to rename the members to 'ind' or 'opt_ind' than 
'opt_optind' and the like, in this case.

I would also suggest making that change as a separate patch for clarity.

diff --git a/src/control/core.c b/src/control/core.c
index 942212d..56292fb 100644
--- a/src/control/core.c
+++ b/src/control/core.c
@@ -48,14 +48,21 @@ libvlc_instance_t * libvlc_new( int argc, const char 
*const *argv )
     const char *my_argv[argc + 2];
     my_argv[0] = "libvlc"; /* dummy arg0, skipped by getopt() et al */
     for( int i = 0; i < argc; i++ )
+#ifndef __WINSCW__
          my_argv[i + 1] = argv[i];
+#else
+    my_argv[i + 1] = (char *) argv[i];
+#endif

I don't understand the problem here. my_argv is declared as const char*[] 
right above.

 {
+#ifndef SYMBIAN
     return vlc_threadvar_get(queue(p_em)->is_asynch_dispatch_thread_var);
+#else
+    return (bool)vlc_threadvar_get(queue(p_em)-
>is_asynch_dispatch_thread_var);
+#endif
 }

Just compare: != NULL on all platforms. No need to make an ifdef here.
 
diff --git a/src/input/es_out_timeshift.c b/src/input/es_out_timeshift.c
index 3ac19d5..82965a2 100644
--- a/src/input/es_out_timeshift.c
+++ b/src/input/es_out_timeshift.c
@@ -55,7 +55,9 @@
  *****************************************************************************/
 
 /* XXX attribute_packed is (and MUST be) used ONLY to reduce memory usage */
-#ifdef HAVE_ATTRIBUTE_PACKED
+/*WINSCW is a Nokia Compiler For Running Symbian C++ Code on x86 Machines*/
+
+#if defined(HAVE_ATTRIBUTE_PACKED) && !defined(__WINSCW__)
 #   define attribute_packed __attribute__((__packed__))

You should not define HAVE_ATTRIBUTE_PACKED if you don't have it.

diff --git a/src/input/input_internal.h b/src/input/input_internal.h
index 7cd39f7..3d6649a 100644
--- a/src/input/input_internal.h
+++ b/src/input/input_internal.h
@@ -32,8 +32,11 @@
 #include <vlc_demux.h>
 #include <vlc_input.h>
 #include <libvlc.h>
+#ifndef SYMBIAN
 #include "input_interface.h"
-
+#else
+#include "input/input_interface.h"
+#endif

Shouldn't this be corrected in the project file instead?

diff --git a/src/libvlc.c b/src/libvlc.c
index 68fa418..2360f69 100644
--- a/src/libvlc.c
+++ b/src/libvlc.c
@@ -667,7 +673,10 @@ int libvlc_InternalInit( libvlc_int_t *p_libvlc, int 
i_argc,
         priv->i_verbose = -1;
     }
     vlc_threads_setup( p_libvlc );
-
+#ifdef SYMBIAN
+    /*This is the Best Place to Start a Thread Controller.As We Init a Libvlc 
*/
+    ret=pthread_create(&threadid,NULL,ThreadController,&dead);
+#endif

Probably the really best place is system_Init() or system_Configure(), and 
then system_End() not libvlc_InternalInit().

diff --git a/src/libvlc.h b/src/libvlc.h
index 69996de..4cd02c8 100644
--- a/src/libvlc.h
+++ b/src/libvlc.h
@@ -26,7 +26,9 @@
 # define LIBVLC_LIBVLC_H 1
 
 #include<vlc_media_library.h>
-
+#ifdef SYMBIAN
+void * ThreadController (void * );/*Symbian Needs a Thread Controller 
Thread.This is the Entry Routine of Thread*/
+#endif
 typedef struct variable_t variable_t;

This shouldn't (need to) be defined in a common header.

diff --git a/src/misc/messages.c b/src/misc/messages.c
index 063ae60..9ae783a 100644
--- a/src/misc/messages.c
+++ b/src/misc/messages.c
@@ -510,7 +510,7 @@ static void PrintMsg ( vlc_object_t * p_this, msg_item_t * 
p_item )
     libvlc_priv_t *priv = libvlc_priv (p_this->p_libvlc);
     msg_bank_t *bank = priv->msg_bank;
     int i_type = p_item->i_type;
-
+#ifndef SYMBIAN

Please leave the empty lines.

diff --git a/src/network/udp.c b/src/network/udp.c
index 5af1c0b..22afddf 100644
--- a/src/network/udp.c
+++ b/src/network/udp.c
@@ -90,6 +90,10 @@
 # define UDPLITE_RECV_CSCOV     11
 #endif
 
+#ifdef SYMBIAN
+		extern const struct in6_addr in6addr_any=IN6ADDR_ANY_INIT;
+#endif
+

Don't indent here.

diff --git a/src/playlist/playlist_internal.h 
b/src/playlist/playlist_internal.h
index 959b8e3..1a22d87 100644
--- a/src/playlist/playlist_internal.h
+++ b/src/playlist/playlist_internal.h
@@ -37,9 +37,15 @@
 #include "input/input_interface.h"
 #include <assert.h>
 
+#ifndef INCLUDED_FROM_LIBVLC_C
 #include "art.h"
 #include "fetcher.h"
 #include "preparser.h"
+#else
+#include "playlist/art.h"
+#include "playlist/fetcher.h"
+#include "playlist/preparser.h"
+#endif

Should really change the include path in the project file.
 
diff --git a/src/text/unicode.c b/src/text/unicode.c
index 77b7684..d5cc0c1 100644
--- a/src/text/unicode.c
+++ b/src/text/unicode.c
@@ -42,7 +42,7 @@
 #endif
 #include <errno.h>
 
-#if defined (__APPLE__) || defined (HAVE_MAEMO)
+#if defined (__APPLE__) || defined (HAVE_MAEMO) || defined (SYMBIAN)
 /* Define this if the OS always use UTF-8 internally */
 # define ASSUME_UTF8 1
 #endif

Hmm... is this really so=??

diff --git a/src/video_output/display.c b/src/video_output/display.c
index af73be8..4dbc401 100644
--- a/src/video_output/display.c
+++ b/src/video_output/display.c
@@ -1210,8 +1210,8 @@ static vout_display_t *DisplayNew(vout_thread_t *vout,
         cfg_windowed.is_fullscreen  = false;
         cfg_windowed.display.width  = 0;
         cfg_windowed.display.height = 0;
-        vout_display_GetDefaultDisplaySize(&osys->width_saved,
-                                           &osys->height_saved,
+        vout_display_GetDefaultDisplaySize((unsigned *)&osys->width_saved,
+                                           (unsigned *)&osys->height_saved,
                                            source_org, &cfg_windowed);

Not Symbian specific. Also, this is not the correct way to solve the real 
problem. Just slap Laurent until he fixes the code :P

diff --git a/src/video_output/snapshot.c b/src/video_output/snapshot.c
index 0f2c938..c844801 100644
--- a/src/video_output/snapshot.c
+++ b/src/video_output/snapshot.c
@@ -42,8 +42,11 @@
 void vout_snapshot_Init(vout_snapshot_t *snap)
 {
     vlc_mutex_init(&snap->lock);
+#ifndef SYMBIAN
     vlc_cond_init(&snap->wait);
-
+#else
+    vlc_cond_init(&snap->snapshot_wait);
+#endif

Renaming the whole thing on all platforms, say to 'wake' or 'cond' would be 
far simpler and less ugly.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

More information about the vlc-devel mailing list