<!DOCTYPE html><html><head><title></title><style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body><div><br></div><div><br></div><div>On Tue, Sep 15, 2020, at 09:25, Zhao, Gang wrote:<br></div><blockquote type="cite" id="qt" style=""><div dir="ltr"><div dir="ltr">On Mon, Sep 14, 2020 at 6:54 PM Thomas Guillem <<a href="mailto:thomas@gllm.fr">thomas@gllm.fr</a>> wrote:<br></div><div class="qt-gmail_quote"><blockquote class="qt-gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>Hello,<br></div><div> <br></div></blockquote><div>Thanks for your review.<br></div><div> <br></div><blockquote class="qt-gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>On Sun, Sep 13, 2020, at 04:20, Zhao, Gang wrote:<br></div><div> > Parse INFO chunk and save meta data to vlc.<br></div><div> > <br></div><div> > Feature ticket: <a href="https://trac.videolan.org/vlc/ticket/6587" rel="noreferrer" target="_blank">https://trac.videolan.org/vlc/ticket/6587</a><br></div><div> > <br></div><div> > Signed-off-by: Zhao, Gang <<a href="mailto:gang.zhao.42@gmail.com" target="_blank">gang.zhao.42@gmail.com</a>><br></div><div> > ---<br></div><div> > Tested this patch with the four wav files in ticket 6587. It can get and save<br></div><div> > meta data from INFO chunk.<br></div><div> > <br></div><div> > v2:<br></div><div> > Fixed a typo in commit log<br></div><div> > <br></div><div> > modules/demux/wav.c | 216 ++++++++++++++++++++++++++++++++++++++++++--<br></div><div> > 1 file changed, 207 insertions(+), 9 deletions(-)<br></div><div> > <br></div><div> > diff --git modules/demux/wav.c modules/demux/wav.c<br></div><div> > index 4b0623412f..75b37d5be0 100644<br></div><div> > --- modules/demux/wav.c<br></div><div> > +++ modules/demux/wav.c<br></div><div> > @@ -36,6 +36,8 @@<br></div><div> > #include <vlc_demux.h><br></div><div> > #include <vlc_aout.h><br></div><div> > #include <vlc_codecs.h><br></div><div> > +#include <vlc_charset.h><br></div><div> > +#include <vlc_meta.h><br></div><div> > <br></div><div> > #include "windows_audio_commons.h"<br></div><div> > <br></div><div> > @@ -58,26 +60,96 @@ typedef struct<br></div><div> > uint32_t i_channel_mask;<br></div><div> > uint8_t i_chans_to_reorder; /* do we need channel reordering */<br></div><div> > uint8_t pi_chan_table[AOUT_CHAN_MAX];<br></div><div> > +<br></div><div> > + vlc_meta_t *p_meta;<br></div><div> > } demux_sys_t;<br></div><div> > <br></div><div> > enum wav_chunk_id {<br></div><div> > + wav_chunk_id_list,<br></div><div> > wav_chunk_id_data,<br></div><div> > wav_chunk_id_ds64,<br></div><div> > wav_chunk_id_fmt,<br></div><div> > +<br></div><div> > + wav_info_chunk_id_iarl,<br></div><div> > + wav_info_chunk_id_iart,<br></div><div> > + wav_info_chunk_id_icms,<br></div><div> > + wav_info_chunk_id_icmt,<br></div><div> > + wav_info_chunk_id_icop,<br></div><div> > + wav_info_chunk_id_icrd,<br></div><div> > + wav_info_chunk_id_icrp,<br></div><div> > + wav_info_chunk_id_idim,<br></div><div> > + wav_info_chunk_id_idpi,<br></div><div> > + wav_info_chunk_id_ieng,<br></div><div> > + wav_info_chunk_id_ignr,<br></div><div> > + wav_info_chunk_id_ikey,<br></div><div> > + wav_info_chunk_id_ilgt,<br></div><div> > + wav_info_chunk_id_imed,<br></div><div> > + wav_info_chunk_id_inam,<br></div><div> > + wav_info_chunk_id_iplt,<br></div><div> > + wav_info_chunk_id_iprd,<br></div><div> > + wav_info_chunk_id_isbj,<br></div><div> > + wav_info_chunk_id_isft,<br></div><div> > + wav_info_chunk_id_ishp,<br></div><div> > + wav_info_chunk_id_isrc,<br></div><div> > + wav_info_chunk_id_isrf,<br></div><div> > + wav_info_chunk_id_itch,<br></div><div> > };<br></div><div> > <br></div><div> > -static const struct wav_chunk_id_key<br></div><div> > +struct wav_chunk_id_key<br></div><div> > {<br></div><div> > enum wav_chunk_id id;<br></div><div> > char key[5];<br></div><div> > -} wav_chunk_id_key_list[] = {<br></div><div> > +};<br></div><div> > +<br></div><div> > +static const struct wav_chunk_id_key wav_chunk_id_key_list[] = {<br></div><div> > /* Alphabetical order */<br></div><div> > + { wav_chunk_id_list, "LIST" },<br></div><div> > { wav_chunk_id_data, "data" },<br></div><div> > { wav_chunk_id_ds64, "ds64" },<br></div><div> > { wav_chunk_id_fmt, "fmt " },<br></div><div> > };<br></div><div> > static const size_t wav_chunk_id_key_count = ARRAY_SIZE(wav_chunk_id_key_list);<br></div><div> > <br></div><div> > +static const struct wav_chunk_id_key wav_info_chunk_id_key_list[] = {<br></div><div> > + /* Alphabetical order */<br></div><div> > + { wav_info_chunk_id_iarl, "IARL" },<br></div><div> > + { wav_info_chunk_id_iart, "IART" },<br></div><div> > + { wav_info_chunk_id_icms, "ICMS" },<br></div><div> > + { wav_info_chunk_id_icmt, "ICMT" },<br></div><div> > + { wav_info_chunk_id_icop, "ICOP" },<br></div><div> > + { wav_info_chunk_id_icrd, "ICRD" },<br></div><div> > + { wav_info_chunk_id_icrp, "ICRP" },<br></div><div> > + { wav_info_chunk_id_idim, "IDIM" },<br></div><div> > + { wav_info_chunk_id_idpi, "IDPI" },<br></div><div> > + { wav_info_chunk_id_ieng, "IENG" },<br></div><div> > + { wav_info_chunk_id_ignr, "IGNR" },<br></div><div> > + { wav_info_chunk_id_ikey, "IKEY" },<br></div><div> > + { wav_info_chunk_id_ilgt, "ILGT" },<br></div><div> > + { wav_info_chunk_id_imed, "IMED" },<br></div><div> > + { wav_info_chunk_id_inam, "INAM" },<br></div><div> > + { wav_info_chunk_id_iplt, "IPLT" },<br></div><div> > + { wav_info_chunk_id_iprd, "IPRD" },<br></div><div> > + { wav_info_chunk_id_isbj, "ISBJ" },<br></div><div> > + { wav_info_chunk_id_isft, "ISFT" },<br></div><div> > + { wav_info_chunk_id_ishp, "ISHP" },<br></div><div> > + { wav_info_chunk_id_isrc, "ISRC" },<br></div><div> > + { wav_info_chunk_id_isrf, "ISRF" },<br></div><div> > + { wav_info_chunk_id_itch, "ITCH" },<br></div><div> > +};<br></div><div> > +static const size_t wav_info_chunk_id_key_count = <br></div><div> > ARRAY_SIZE(wav_info_chunk_id_key_list);<br></div><div> > +<br></div><div> > +static const char *wav_info_chunk_id_to_key(enum wav_chunk_id id)<br></div><div> > +{<br></div><div> > + if ( id < wav_info_chunk_id_iarl || id > wav_info_chunk_id_itch )<br></div><div> > + return "";<br></div><div> <br></div><div> Your fonction is taking an enum in argument but is checking if the enum is out of bound. This seems weird.<br></div><div> You could check the validity of the id after ChunkGetNext() in ChunkParseINFO().<br></div><div> <br></div></blockquote><div>This function is used mostly for debug purpose. It'll print the relevant string representation of an enum. <br></div><div><br></div><div>Ids like wav_chunk_id_list, their relevant string can be easily got as wav_chunk_id_key_list[wav_chunk_id_list].<br></div><div>But ids like wav_info_chunk_id_iarl, their relevant string can't be got as wav_info_chunk_id_key_list[wav_info_chunk_id_iarl], <br></div><div>because there's an offset to it. Function wav_info_chunk_id_to_key is a helper function to print the string representation of these ids.<br></div><div><br></div><div>As this function is only useful for info chunk ids, it will check if the enum is in the range of info chunk ids.<br></div><div><br></div><blockquote class="qt-gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>> +<br></div><div> > + for ( int i = 0; i < wav_info_chunk_id_key_count; i++)<br></div><div> > + if (wav_info_chunk_id_key_list[i].id == id)<br></div><div> > + return wav_info_chunk_id_key_list[i].key;<br></div><div> > +<br></div><div> > + return "";<br></div><div> > +}<br></div><div> > +<br></div><div> > static int<br></div><div> > wav_chunk_CompareCb(const void *a, const void *b)<br></div><div> > {<br></div><div> > @@ -190,13 +262,14 @@ static int ChunkSkip( demux_t *p_demux, uint32_t <br></div><div> > i_size )<br></div><div> > }<br></div><div> > <br></div><div> > static int ChunkGetNext( demux_t *p_demux, enum wav_chunk_id *p_id,<br></div><div> > - uint32_t *pi_size )<br></div><div> > + uint32_t *pi_size,<br></div><div> > + const struct wav_chunk_id_key *id_key_list, <br></div><div> > size_t count )<br></div><div> > {<br></div><div> > #ifndef NDEBUG<br></div><div> > /* assert that keys are in alphabetical order */<br></div><div> > - for( size_t i = 0; i < wav_chunk_id_key_count - 1; ++i )<br></div><div> > - assert( strcmp( wav_chunk_id_key_list[i].key,<br></div><div> > - wav_chunk_id_key_list[i + 1].key ) < 0 );<br></div><div> > + for( size_t i = 0; i < count - 1; ++i )<br></div><div> > + assert( strcmp( id_key_list[i].key,<br></div><div> > + id_key_list[i + 1].key ) < 0 );<br></div><div> > #endif<br></div><div> > <br></div><div> > for( ;; )<br></div><div> > @@ -206,8 +279,8 @@ static int ChunkGetNext( demux_t *p_demux, enum <br></div><div> > wav_chunk_id *p_id,<br></div><div> > return VLC_EGENERIC;<br></div><div> > <br></div><div> > const struct wav_chunk_id_key *id =<br></div><div> > - bsearch( p_peek, wav_chunk_id_key_list, wav_chunk_id_key_count,<br></div><div> > - sizeof(*wav_chunk_id_key_list), wav_chunk_CompareCb );<br></div><div> > + bsearch( p_peek, id_key_list, count,<br></div><div> > + sizeof(*id_key_list), wav_chunk_CompareCb );<br></div><div> > uint32_t i_size = GetDWLE( p_peek + 4 );<br></div><div> <br></div><div> This could go in a separate function to improve readability.<br></div><div> <br></div></blockquote><div>I'm not sure what you meant here. The bsearch is needed to get the chunk id, which is an out parameter of<br></div><div>function ChunkGetNext. I added two parameters to ChunkGetNext so it can search in different chunk id arrays.<br></div><div>When we start parsing the wav file, we care about chunk ids in wav_chunk_id_key_list array. When we parsing INFO<br></div><div>chunk, we care about chunk ids in wav_info_chunk_id_key_list array. My change made this two process can both use<br></div><div>ChunkGetNext function to get chunk data.<br></div></div></div></blockquote><div><br></div><div>Sorry, I wanted to say that it could go in a *separate* commit.<br></div><div><br></div><div>First commit: add change the ChunkGetNext() and adapt the whole code.<br></div><div>Second commit: add the INFO parsing.<br></div><div><br></div><blockquote type="cite" id="qt" style=""><div dir="ltr"><div class="qt-gmail_quote"><div><br></div><blockquote class="qt-gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;"><div>> <br></div><div> > if( id == NULL )<br></div><div> > @@ -317,6 +390,10 @@ static void Close ( vlc_object_t * p_this )<br></div><div> > demux_sys_t *p_sys = p_demux->p_sys;<br></div><div> > <br></div><div> > es_format_Clean( &p_sys->fmt );<br></div><div> > +<br></div><div> > + if( p_sys->p_meta )<br></div><div> > + vlc_meta_Delete( p_sys->p_meta );<br></div><div> > +<br></div><div> > free( p_sys );<br></div><div> > }<br></div><div> > <br></div><div> > @@ -591,6 +668,114 @@ error:<br></div><div> > return VLC_EGENERIC;<br></div><div> > }<br></div><div> > <br></div><div> > +static int ChunkParseINFO( demux_t *p_demux, uint32_t i_size )<br></div><div> > +{<br></div><div> > + demux_sys_t *p_sys = p_demux->p_sys;<br></div><div> > + const uint8_t *p_peek;<br></div><div> > + enum wav_chunk_id id;<br></div><div> > + uint32_t chunk_size;<br></div><div> > + int meta_type;<br></div><div> > + char *meta_value;<br></div><div> > +<br></div><div> > + if( (p_sys->p_meta = vlc_meta_New()) == NULL )<br></div><div> > + {<br></div><div> > + msg_Err( p_demux, "vlc_meta_New failed" );<br></div><div> > + return VLC_EGENERIC;<br></div><div> > + }<br></div><div> > +<br></div><div> > + while (i_size > 0) {<br></div><div> > + int ret = ChunkGetNext( p_demux, &id, &chunk_size,<br></div><div> > + wav_info_chunk_id_key_list,<br></div><div> > + wav_info_chunk_id_key_count );<br></div><div> > + if ( ret != VLC_SUCCESS )<br></div><div> > + break;<br></div><div> > +<br></div><div> > + /* chunk id(4 bytes) and chunk size(4 bytes) consumed by <br></div><div> > ChunkGetNext */<br></div><div> > + i_size -= 8;<br></div><div> > +<br></div><div> > + if( chunk_size == 0 )<br></div><div> > + {<br></div><div> > + msg_Err( p_demux, "invalid chunk with a size 0" );<br></div><div> > + return VLC_EGENERIC;<br></div><div> > + }<br></div><div> > +<br></div><div> > + if( vlc_stream_Peek( p_demux->s, &p_peek, chunk_size ) != <br></div><div> > chunk_size )<br></div><div> > + return VLC_EGENERIC;<br></div><div> > +<br></div><div> > + meta_value = strndup( (const char *)p_peek, chunk_size );<br></div><div> > + if (meta_value == NULL)<br></div><div> > + return VLC_EGENERIC;<br></div><div> > +<br></div><div> > + EnsureUTF8( meta_value );<br></div><div> > +<br></div><div> > + meta_type = -1;<br></div><div> > + switch( id )<br></div><div> > + {<br></div><div> > + case wav_info_chunk_id_iart:<br></div><div> > + meta_type = vlc_meta_Artist;<br></div><div> > + break;<br></div><div> > + case wav_info_chunk_id_icop:<br></div><div> > + meta_type = vlc_meta_Copyright;<br></div><div> > + break;<br></div><div> > + case wav_info_chunk_id_icrd:<br></div><div> > + meta_type = vlc_meta_Date;<br></div><div> > + break;<br></div><div> > + case wav_info_chunk_id_ignr:<br></div><div> > + meta_type = vlc_meta_Genre;<br></div><div> > + break;<br></div><div> > + case wav_info_chunk_id_inam:<br></div><div> > + meta_type = vlc_meta_Title;<br></div><div> > + break;<br></div><div> > + default:<br></div><div> > + /* no relevant meta type in vlc */<br></div><div> > + break;<br></div><div> > + }<br></div><div> > +<br></div><div> > + msg_Dbg( p_demux, "id: %s meta_type: %d meta_value: %s\n",<br></div><div> > + wav_info_chunk_id_to_key(id), meta_type, meta_value );<br></div><div> > +<br></div><div> > + if (meta_type != -1)<br></div><div> > + vlc_meta_Set( p_sys->p_meta, meta_type, meta_value );<br></div><div> > + else<br></div><div> > + vlc_meta_AddExtra( p_sys->p_meta,<br></div><div> > + wav_info_chunk_id_key_list[id].key, <br></div><div> > meta_value );<br></div><div> > +<br></div><div> > + free( meta_value );<br></div><div> > + if( ChunkSkip( p_demux, chunk_size ) != VLC_SUCCESS )<br></div><div> > + return VLC_EGENERIC;<br></div><div> > +<br></div><div> > + i_size -= chunk_size;<br></div><div> > + }<br></div><div> > +<br></div><div> > + return VLC_SUCCESS;<br></div><div> > +}<br></div><div> > +<br></div><div> > +static int ChunkParseLIST( demux_t *p_demux, uint32_t i_size )<br></div><div> > +{<br></div><div> > + const uint8_t *p_peek;<br></div><div> > +<br></div><div> > + /* must have four bytes list type */<br></div><div> > + if( i_size < 4 )<br></div><div> > + {<br></div><div> > + msg_Err( p_demux, "invalid 'LIST' chunk" );<br></div><div> > + return VLC_EGENERIC;<br></div><div> > + }<br></div><div> > +<br></div><div> > + if( vlc_stream_Peek( p_demux->s, &p_peek, 4 ) != 4 )<br></div><div> > + return VLC_EGENERIC;<br></div><div> > +<br></div><div> > + /* only care about INFO chunk */<br></div><div> > + if ( memcmp(p_peek, "INFO", 4) != 0 ) {<br></div><div> > + return ChunkSkip( p_demux, i_size );<br></div><div> > + }<br></div><div> > +<br></div><div> > + if ( vlc_stream_Read( p_demux->s, NULL, 4) != 4 )<br></div><div> > + return VLC_EGENERIC;<br></div><div> > +<br></div><div> > + i_size -= 4;<br></div><div> > + return ChunkParseINFO( p_demux, i_size );<br></div><div> > +}<br></div><div> > +<br></div><div> > static int Open( vlc_object_t * p_this )<br></div><div> > {<br></div><div> > demux_t *p_demux = (demux_t*)p_this;<br></div><div> > @@ -629,8 +814,13 @@ static int Open( vlc_object_t * p_this )<br></div><div> > <br></div><div> > bool eof = false;<br></div><div> > enum wav_chunk_id id;<br></div><div> > - while( !eof && ( ChunkGetNext( p_demux, &id, &i_size ) ) == <br></div><div> > VLC_SUCCESS )<br></div><div> > + while( !eof )<br></div><div> > {<br></div><div> > + int ret = ChunkGetNext( p_demux, &id, &i_size,<br></div><div> > + wav_chunk_id_key_list, <br></div><div> > wav_chunk_id_key_count );<br></div><div> > + if (ret != VLC_SUCCESS )<br></div><div> > + break;<br></div><div> > +<br></div><div> > if( i_size == 0 )<br></div><div> > {<br></div><div> > msg_Err( p_demux, "invalid chunk with a size 0");<br></div><div> > @@ -676,6 +866,14 @@ static int Open( vlc_object_t * p_this )<br></div><div> > if( ChunkParseFmt( p_demux, i_size ) != VLC_SUCCESS )<br></div><div> > goto error;<br></div><div> > break;<br></div><div> > + case wav_chunk_id_list:<br></div><div> > + if( ChunkParseLIST( p_demux, i_size ) != VLC_SUCCESS )<br></div><div> > + goto error;<br></div><div> > + break;<br></div><div> > + default:<br></div><div> > + if( ChunkSkip( p_demux, i_size ) != VLC_SUCCESS )<br></div><div> > + goto error;<br></div><div> > + break;<br></div><div> > }<br></div><div> > }<br></div><div> > <br></div><div> > -- <br></div><div> > 2.25.1<br></div><div> > <br></div><div> > _______________________________________________<br></div><div> > vlc-devel mailing list<br></div><div> > To unsubscribe or modify your subscription options:<br></div><div> > <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div><div> _______________________________________________<br></div><div> vlc-devel mailing list<br></div><div> To unsubscribe or modify your subscription options:<br></div><div> <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote></div></div><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div><br></div></body></html>