[vlc-devel] [PATCH 5/7] Add support for parsing more Ogg tags
Timothy B. Terriberry
tterribe at xiph.org
Mon Sep 2 21:00:42 CEST 2013
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.
More information about the vlc-devel