[vlc-devel] [PATCH] mp3: id3: support id3v2.4 tag for id3 chapter markers

[Wang Gary]

Hello,

I'd like to re-ask for review since there are no replies or comments after
a week, and here are some additional contexts about this patch:

Previously VLC4 cannot load chapter markers from MP3 files that contain the
ID3v2.4 tag, which caused by ID3TAG_ReadSize will always read the chunk size
in an ID3v2.3 fashion. The syncsafe bit is also not used to determining how we
read the chunk size here.

BTW, I also found in some special cases, it also won't be able to read
the ID3v2.3 tag, although after tracking that bug a little bit founds out
it's not related to this one, but it's still worth mentioning. The original
auphonic_chapters_demo.mp3 [1] usually used to test chapter support,
it contains the ID3v2.3 tag, but cannot be read by VLC.
In modules/demux/mpeg/es.c there is a vlc_stream_Control
STREAM_GET_TAGS check that will not return VLC_SUCCESS,
which will cause ID3TAG_Parse won't get called. I haven't look into that
issue yet, there might be another patch for fixing that issue, this patch
only intended to add ID3v2.4 support.

[1]: You can get it from
https://auphonic.com/blog/2013/07/03/chapter-marks-and-enhanced-podcasts/

To test the patch, you can use the modified version of the
auphonic_chapters_demo.mp3
that contains the ID3v2.4 tag. See attachment for the file. To verify ID3v2.3
tag, you can use the audio file cre197-ipv6.mp3 from
https://trac.videolan.org/vlc/ticket/7485

Let me know if you need any additional information. Thanks.

Gary Wang <wzc782970009 at gmail.com> 于2021年2月11日周四 下午2:56写道：
>
> ---
>  modules/meta_engine/ID3Tag.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/modules/meta_engine/ID3Tag.h b/modules/meta_engine/ID3Tag.h
> index d60e965..b1240c5 100644
> --- a/modules/meta_engine/ID3Tag.h
> +++ b/modules/meta_engine/ID3Tag.h
> @@ -20,9 +20,9 @@
>  #ifndef ID3TAG_H
>  #define ID3TAG_H
>
> -static uint32_t ID3TAG_ReadSize( const uint8_t *p_buffer, bool b_syncsafe )
> +static uint32_t ID3TAG_ReadSize( const uint8_t *p_buffer, bool b_as_u32 )
>  {
> -    if( !b_syncsafe )
> +    if( b_as_u32 )
>          return GetDWBE( p_buffer );
>      return ( (uint32_t)p_buffer[3] & 0x7F ) |
>              (( (uint32_t)p_buffer[2] & 0x7F ) << 7) |
> @@ -45,8 +45,8 @@ static size_t ID3TAG_Parse( const uint8_t *p_peek, size_t i_peek,
>      uint32_t i_ID3size = 0;
>      if( i_peek > 10 && ID3TAG_IsTag( p_peek, false ) )
>      {
> -        const bool b_syncsafe = p_peek[5] & 0x80;
> -        i_ID3size = ID3TAG_ReadSize( &p_peek[6], true );
> +        const uint8_t i_ID3major = p_peek[3];
> +        i_ID3size = ID3TAG_ReadSize( &p_peek[6], false );
>          if( i_ID3size > i_peek - 10 )
>              return 0;
>          i_total_size = i_ID3size + 10;
> @@ -54,7 +54,7 @@ static size_t ID3TAG_Parse( const uint8_t *p_peek, size_t i_peek,
>          while( i_ID3size > 10 )
>          {
>              uint32_t i_tagname = VLC_FOURCC( p_frame[0], p_frame[1], p_frame[2], p_frame[3] );
> -            uint32_t i_framesize = ID3TAG_ReadSize( &p_frame[4], b_syncsafe ) + 10;
> +            uint32_t i_framesize = ID3TAG_ReadSize( &p_frame[4], i_ID3major == 3 ) + 10;
>              if( i_framesize > i_ID3size )
>                  return 0;
>
> --
> 2.25.1
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: auphonic_chapters_demo - 副本.mp3
Type: audio/mpeg
Size: 1739500 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20210218/8d832f75/attachment-0001.mp3>