[vlc-devel] [PATCH 5/7] Add support for parsing more Ogg tags

[Timothy B. Terriberry]

Jean-Baptiste Kempf wrote:
> On 02 Sep, Timothy B. Terriberry wrote :
>> -    bool hasAlbum        = false;
>> +    bool hasAlbum        = false;
> ??
>
>> +    bool hasGenre        = false;
>> -    bool hasGenre        = false;
> ???
>
>> -    bool hasTrackTotal   = false;
>> +    bool hasTrackTotal   = false;
> ???

I was sorting them in the same order they're listed in the header file, 
to make it easier to tell which ones are present and which ones are missing.

>> -        else if( !strncasecmp(psz_comment, "TRACKTOTAL=", strlen("TRACKTOTAL=")))
>> -            vlc_meta_Set( p_meta, vlc_meta_TrackTotal, &psz_comment[strlen("TRACKTOTAL=")] );
>> -        else if( !strncasecmp(psz_comment, "TOTALTRACKS=", strlen("TOTALTRACKS=")))
>> -            vlc_meta_Set( p_meta, vlc_meta_TrackTotal, &psz_comment[strlen("TOTALTRACKS=")] );
>> +        else IF_EXTRACT("TRACKTOTAL=", TrackTotal )
>
> Why this change?
> Because I think I disagree.

Well, what do you think correct behavior is here in the presence of 
multiple tag values?

Personally I think it should be first value wins (which is what the 
taglib code does). I think "show all values" is reasonable (it at least 
lets the user know the file is crazy), and was more consistent with the 
other code here. The existing code was "last value wins" for just these 
tag values. Except it also had an IF_EXTRACT("TOTALTRACKS=", TrackTotal 
) which was totally redundant with the tests I deleted.