[vlc-devel] [PATCH 2/2] demux: wav: support parsing INFO chunk

Zhao, Gang gang.zhao.42 at gmail.com
Wed Sep 16 10:35:19 CEST 2020


On Wed, Sep 16, 2020 at 3:22 AM Rémi Denis-Courmont <remi at remlab.net> wrote:

>         Hi,
>
> Le tiistaina 15. syyskuuta 2020, 17.26.39 EEST Zhao, Gang a écrit :
> > Parse INFO chunk and save meta data to vlc.
> >
> > Feature ticket: https://trac.videolan.org/vlc/ticket/6587
> >
> > Signed-off-by: Zhao, Gang <gang.zhao.42 at gmail.com>
> > ---
> >  modules/demux/wav.c | 191 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 191 insertions(+)
> >
> > diff --git modules/demux/wav.c modules/demux/wav.c
> > index 52f9639ef9..e09734579a 100644
> > --- modules/demux/wav.c
> > +++ modules/demux/wav.c
> > @@ -36,6 +36,8 @@
> >  #include <vlc_demux.h>
> >  #include <vlc_aout.h>
> >  #include <vlc_codecs.h>
> > +#include <vlc_charset.h>
> > +#include <vlc_meta.h>
> >
> >  #include "windows_audio_commons.h"
> >
> > @@ -58,12 +60,39 @@ typedef struct
> >      uint32_t i_channel_mask;
> >      uint8_t i_chans_to_reorder;            /* do we need channel
> reordering
> > */ uint8_t pi_chan_table[AOUT_CHAN_MAX];
> > +
> > +    vlc_meta_t    *p_meta;
> >  } demux_sys_t;
> >
> >  enum wav_chunk_id {
> > +    wav_chunk_id_list,
> >      wav_chunk_id_data,
> >      wav_chunk_id_ds64,
> >      wav_chunk_id_fmt,
> > +
> > +    wav_info_chunk_id_iarl,
> > +    wav_info_chunk_id_iart,
> > +    wav_info_chunk_id_icms,
> > +    wav_info_chunk_id_icmt,
> > +    wav_info_chunk_id_icop,
> > +    wav_info_chunk_id_icrd,
> > +    wav_info_chunk_id_icrp,
> > +    wav_info_chunk_id_idim,
> > +    wav_info_chunk_id_idpi,
> > +    wav_info_chunk_id_ieng,
> > +    wav_info_chunk_id_ignr,
> > +    wav_info_chunk_id_ikey,
> > +    wav_info_chunk_id_ilgt,
> > +    wav_info_chunk_id_imed,
> > +    wav_info_chunk_id_inam,
> > +    wav_info_chunk_id_iplt,
> > +    wav_info_chunk_id_iprd,
> > +    wav_info_chunk_id_isbj,
> > +    wav_info_chunk_id_isft,
> > +    wav_info_chunk_id_ishp,
> > +    wav_info_chunk_id_isrc,
> > +    wav_info_chunk_id_isrf,
> > +    wav_info_chunk_id_itch,
> >  };
> >
> >  struct wav_chunk_id_key
> > @@ -74,12 +103,54 @@ struct wav_chunk_id_key
> >
> >  static const struct wav_chunk_id_key wav_chunk_id_key_list[] =  {
> >      /* Alphabetical order */
> > +    { wav_chunk_id_list, "LIST" },
> >      { wav_chunk_id_data, "data" },
> >      { wav_chunk_id_ds64, "ds64" },
> >      { wav_chunk_id_fmt,  "fmt " },
> >  };
> >  static const size_t wav_chunk_id_key_count =
> > ARRAY_SIZE(wav_chunk_id_key_list);
> >
> > +static const struct wav_chunk_id_key wav_info_chunk_id_key_list[] =  {
> > +    /* Alphabetical order */
> > +    { wav_info_chunk_id_iarl, "IARL" },
> > +    { wav_info_chunk_id_iart, "IART" },
> > +    { wav_info_chunk_id_icms, "ICMS" },
> > +    { wav_info_chunk_id_icmt, "ICMT" },
> > +    { wav_info_chunk_id_icop, "ICOP" },
> > +    { wav_info_chunk_id_icrd, "ICRD" },
> > +    { wav_info_chunk_id_icrp, "ICRP" },
> > +    { wav_info_chunk_id_idim, "IDIM" },
> > +    { wav_info_chunk_id_idpi, "IDPI" },
> > +    { wav_info_chunk_id_ieng, "IENG" },
> > +    { wav_info_chunk_id_ignr, "IGNR" },
> > +    { wav_info_chunk_id_ikey, "IKEY" },
> > +    { wav_info_chunk_id_ilgt, "ILGT" },
> > +    { wav_info_chunk_id_imed, "IMED" },
> > +    { wav_info_chunk_id_inam, "INAM" },
> > +    { wav_info_chunk_id_iplt, "IPLT" },
> > +    { wav_info_chunk_id_iprd, "IPRD" },
> > +    { wav_info_chunk_id_isbj, "ISBJ" },
> > +    { wav_info_chunk_id_isft, "ISFT" },
> > +    { wav_info_chunk_id_ishp, "ISHP" },
> > +    { wav_info_chunk_id_isrc, "ISRC" },
> > +    { wav_info_chunk_id_isrf, "ISRF" },
> > +    { wav_info_chunk_id_itch, "ITCH" },
> > +};
>
> You could just as well use a table with designated initializers here.
>
> I think you meant this array could be created more easily. Could you
please evaluate it ?

> +static const size_t wav_info_chunk_id_key_count =
> > ARRAY_SIZE(wav_info_chunk_id_key_list); +
> > +/* get string type of INFO chunk id from enum */
> > +static const char *wav_info_chunk_id_to_key(enum wav_chunk_id id)
> > +{
> > +    if ( id < wav_info_chunk_id_iarl || id > wav_info_chunk_id_itch )
> > +        return "";
> > +
> > +    for ( int i = 0; i < wav_info_chunk_id_key_count; i++)
> > +        if (wav_info_chunk_id_key_list[i].id == id)
> > +            return wav_info_chunk_id_key_list[i].key;
>
> Err, why use a linear search, when you could directly index the table?
>

Yes you are right. The for loop can be changed to

return  wav_info_chunk_id_key_list[id - wav_info_chunk_id_iarl]

Will update in the next version.

>
> > +
> > +    return "";
> > +}
> > +
> >  static int
> >  wav_chunk_CompareCb(const void *a, const void *b)
> >  {
> > @@ -320,6 +391,10 @@ static void Close ( vlc_object_t * p_this )
> >      demux_sys_t *p_sys   = p_demux->p_sys;
> >
> >      es_format_Clean( &p_sys->fmt );
> > +
> > +    if( p_sys->p_meta )
> > +        vlc_meta_Delete( p_sys->p_meta );
> > +
> >      free( p_sys );
> >  }
> >
> > @@ -594,6 +669,114 @@ error:
> >      return VLC_EGENERIC;
> >  }
> >
> > +static int ChunkParseINFO( demux_t *p_demux, uint32_t i_size )
> > +{
> > +    demux_sys_t *p_sys = p_demux->p_sys;
> > +    const uint8_t *p_peek;
> > +    enum wav_chunk_id id;
> > +    uint32_t chunk_size;
> > +    int meta_type;
> > +    char *meta_value;
> > +
> > +    if( (p_sys->p_meta = vlc_meta_New()) == NULL )
>
> Don't use assignment as truth value unless necessary.
>

I can change this to:

p_sys->p_meta = vlc_meta_New();
if (p_sys->p_meta == NULL)

If this is what you meant.


> > +    {
> > +        msg_Err( p_demux, "vlc_meta_New failed" );
> > +        return VLC_EGENERIC;
> > +    }
> > +
> > +    while (i_size > 0) {
> > +        int ret = ChunkGetNext( p_demux, &id, &chunk_size,
> > +                                wav_info_chunk_id_key_list,
> > +                                wav_info_chunk_id_key_count );
> > +        if ( ret != VLC_SUCCESS )
> > +            break;
> > +
> > +        /* chunk id(4 bytes) and chunk size(4 bytes) consumed by
> > ChunkGetNext */ +        i_size -= 8;
> > +
> > +        if( chunk_size == 0 )
> > +        {
> > +            msg_Err( p_demux, "invalid chunk with a size 0" );
> > +            return VLC_EGENERIC;
> > +        }
> > +
> > +        if( vlc_stream_Peek( p_demux->s, &p_peek, chunk_size ) !=
> > chunk_size ) +            return VLC_EGENERIC;
> > +
> > +        meta_value = strndup( (const char *)p_peek, chunk_size );
> > +        if (meta_value == NULL)
> > +            return VLC_EGENERIC;
> > +
> > +        EnsureUTF8( meta_value );
> > +
> > +        meta_type = -1;
> > +        switch( id )
> > +        {
> > +        case wav_info_chunk_id_iart:
> > +            meta_type = vlc_meta_Artist;
> > +            break;
> > +        case wav_info_chunk_id_icop:
> > +            meta_type = vlc_meta_Copyright;
> > +            break;
> > +        case wav_info_chunk_id_icrd:
> > +            meta_type = vlc_meta_Date;
> > +            break;
> > +        case wav_info_chunk_id_ignr:
> > +            meta_type = vlc_meta_Genre;
> > +            break;
> > +        case wav_info_chunk_id_inam:
> > +            meta_type = vlc_meta_Title;
> > +            break;
> > +        default:
> > +            /* no relevant meta type in vlc */
> > +            break;
> > +        }
> > +
> > +        msg_Dbg( p_demux, "id: %s meta_type: %d meta_value: %s\n",
> > +                 wav_info_chunk_id_to_key(id), meta_type, meta_value );
> > +
> > +        if (meta_type != -1)
> > +            vlc_meta_Set( p_sys->p_meta, meta_type, meta_value );
> > +        else
> > +            vlc_meta_AddExtra( p_sys->p_meta,
> > +                               wav_info_chunk_id_key_list[id].key,
> > meta_value ); +
> > +        free( meta_value );
> > +        if( ChunkSkip( p_demux, chunk_size ) != VLC_SUCCESS )
> > +            return VLC_EGENERIC;
> > +
> > +        i_size -= chunk_size;
> > +    }
> > +
> > +    return VLC_SUCCESS;
> > +}
> > +
> > +static int ChunkParseLIST( demux_t *p_demux, uint32_t i_size )
> > +{
> > +    const uint8_t *p_peek;
> > +
> > +    /* must have four bytes list type */
> > +    if( i_size < 4 )
> > +    {
> > +        msg_Err( p_demux, "invalid 'LIST' chunk" );
> > +        return VLC_EGENERIC;
> > +    }
> > +
> > +    if( vlc_stream_Peek( p_demux->s, &p_peek, 4 ) != 4 )
> > +        return VLC_EGENERIC;
> > +
> > +    /* only care about INFO chunk */
> > +    if ( memcmp(p_peek, "INFO", 4) != 0 ) {
> > +        return ChunkSkip( p_demux, i_size );
> > +    }
> > +
> > +    if ( vlc_stream_Read( p_demux->s, NULL, 4) != 4 )
> > +        return VLC_EGENERIC;
> > +
> > +    i_size -= 4;
> > +    return ChunkParseINFO( p_demux, i_size );
> > +}
> > +
> >  static int Open( vlc_object_t * p_this )
> >  {
> >      demux_t     *p_demux = (demux_t*)p_this;
> > @@ -684,6 +867,14 @@ static int Open( vlc_object_t * p_this )
> >                  if( ChunkParseFmt( p_demux, i_size ) != VLC_SUCCESS )
> >                      goto error;
> >                  break;
> > +            case wav_chunk_id_list:
> > +                if( ChunkParseLIST( p_demux, i_size ) != VLC_SUCCESS )
> > +                    goto error;
> > +                break;
> > +            default:
> > +                if( ChunkSkip( p_demux, i_size ) != VLC_SUCCESS )
> > +                    goto error;
> > +                break;
> >          }
> >      }
>
>
> --
> レミ・デニ-クールモン
> http://www.remlab.net/
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200916/8e448e39/attachment-0001.html>


More information about the vlc-devel mailing list