[vlc-devel] [PATCH 4/4] gstdecode: Eliminate warnings

[Tristan Matthews]

Hi,

On Sun, Jun 28, 2020 at 8:37 AM Vikram Fugro <vikram.fugro at gmail.com> wrote:
>
> Eliminate warnings related to typecasting
> ---
>  modules/codec/gstreamer/gstdecode.c           |  5 +-
>  .../gstreamer/gstvlcpictureplaneallocator.c   | 56 ++++++++++---------
>  2 files changed, 34 insertions(+), 27 deletions(-)
>
> diff --git a/modules/codec/gstreamer/gstdecode.c b/modules/codec/gstreamer/gstdecode.c
> index 118138dcbe..c442c332db 100644
> --- a/modules/codec/gstreamer/gstdecode.c
> +++ b/modules/codec/gstreamer/gstdecode.c
> @@ -333,11 +333,12 @@ static bool vlc_gst_registered = false;
>  static void vlc_gst_init_once(void)
>  {
>      gst_init( NULL, NULL );
> +
> +    vlc_gst_build_fourcc_table();
> +
>      vlc_gst_registered = gst_plugin_register_static( 1, 0, "videolan",
>                  "VLC Gstreamer plugins", vlc_gst_plugin_init,
>                  "1.0.0", "LGPL", "NA", "vlc", "NA" );
> -
> -    vlc_gst_build_fourcc_table();
>  }
>
>  /* gst_init( ) is not thread-safe, hence a thread-safe wrapper */
> diff --git a/modules/codec/gstreamer/gstvlcpictureplaneallocator.c b/modules/codec/gstreamer/gstvlcpictureplaneallocator.c
> index 580a9f931b..80b525e10e 100644
> --- a/modules/codec/gstreamer/gstvlcpictureplaneallocator.c
> +++ b/modules/codec/gstreamer/gstvlcpictureplaneallocator.c
> @@ -134,32 +134,38 @@ static GstMemory* gst_vlc_picture_plane_copy(
>
>  void vlc_gst_build_fourcc_table ( void )
>  {
> +    if( p_fourcc_map )
> +        g_hash_table_destroy( p_fourcc_map );
> +
>      p_fourcc_map = g_hash_table_new( g_str_hash, g_str_equal );
>
> -    g_hash_table_insert( p_fourcc_map, "I420_9LE",  "I09L");
> -    g_hash_table_insert( p_fourcc_map, "I420_9BE",  "I09B");
> -    g_hash_table_insert( p_fourcc_map, "I420_10LE", "I0AL");
> -    g_hash_table_insert( p_fourcc_map, "I420_10BE", "I0AB");
> -    g_hash_table_insert( p_fourcc_map, "I420_12LE", "I0CL");
> -    g_hash_table_insert( p_fourcc_map, "I420_12BE", "I0CB");
> -    g_hash_table_insert( p_fourcc_map, "I420_16LE", "I0FL");
> -    g_hash_table_insert( p_fourcc_map, "I420_16BE", "I0FB");
> -    g_hash_table_insert( p_fourcc_map, "I422_9LE",  "I29L");
> -    g_hash_table_insert( p_fourcc_map, "I422_9BE",  "I29B");
> -    g_hash_table_insert( p_fourcc_map, "I422_10LE", "I2AL");
> -    g_hash_table_insert( p_fourcc_map, "I422_10BE", "I2AB");
> -    g_hash_table_insert( p_fourcc_map, "I422_12LE", "I2CL");
> -    g_hash_table_insert( p_fourcc_map, "I422_12BE", "I2CB");
> -    g_hash_table_insert( p_fourcc_map, "I422_16LE", "I2FL");
> -    g_hash_table_insert( p_fourcc_map, "I422_16BE", "I2FB");
> -    g_hash_table_insert( p_fourcc_map, "I444_9LE",  "I49L");
> -    g_hash_table_insert( p_fourcc_map, "I444_9BE",  "I49B");
> -    g_hash_table_insert( p_fourcc_map, "I444_10LE", "I4AL");
> -    g_hash_table_insert( p_fourcc_map, "I444_10BE", "I4AB");
> -    g_hash_table_insert( p_fourcc_map, "I444_12LE", "I4CL");
> -    g_hash_table_insert( p_fourcc_map, "I444_12BE", "I4CB");
> -    g_hash_table_insert( p_fourcc_map, "I444_16LE", "I4FL");
> -    g_hash_table_insert( p_fourcc_map, "I444_16BE", "I4FB");
> +#define INSERT_KEY_VALUE( map, key, val ) \
> +    g_hash_table_insert( map, (gpointer)key,  (gpointer)val )
> +
> +    INSERT_KEY_VALUE( p_fourcc_map, "I420_9LE", "I09L" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I420_9BE", "I09B" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I420_10LE", "I0AL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I420_10BE", "I0AB" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I420_12LE", "I0CL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I420_12BE", "I0CB" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I420_16LE", "I0FL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I420_16BE", "I0FB" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I422_9LE", "I29L" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I422_9BE", "I29B" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I422_10LE", "I2AL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I422_10BE", "I2AB" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I422_12LE", "I2CL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I422_12BE", "I2CB" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I422_16LE", "I2FL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I422_16BE", "I2FB" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I444_9LE", "I49L" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I444_9BE", "I49B" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I444_10LE", "I4AL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I444_10BE", "I4AB" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I444_12LE", "I4CL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I444_12BE", "I4CB" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I444_16LE", "I4FL" );
> +    INSERT_KEY_VALUE( p_fourcc_map, "I444_16BE", "I4FB" );
>  }
>
>  void gst_vlc_picture_plane_allocator_release(
> @@ -253,7 +259,7 @@ bool gst_vlc_set_vout_fmt( GstVideoInfo *p_info, GstVideoAlignment *p_align,
>      vlc_fourcc_t i_chroma;
>      int i_padded_width, i_padded_height;
>
> -    char* psz_fourcc = gst_structure_get_string(p_str, "format");
> +    const char* psz_fourcc = gst_structure_get_string( p_str, "format" );
>      if( psz_fourcc )
>      {
>          if( strlen( psz_fourcc ) != 4 )
> --

Couple things here,
- Can you squash patches before sending them to the list? It doesn't
make sense to have patch 4/4 fix warnings that were introduced in
patch 1/4.
- Why use a g_hashtable (that will get leaked at program exit) for a
table that is effectively immutable? You probably just want to do
something like we're already doing to map avcodec fourcc's to VLC's
here:
http://git.videolan.org/?p=vlc.git;a=blob;f=modules/codec/avcodec/fourcc.c;h=bd849f485851a755fa13f5d94ee4c39f1ffee7da;hb=refs/heads/master
(Note the use of static tables)

Best,
-t