On Thu, Oct 16, 2014 at 06:06:39PM +0200, Benoit Fouet wrote:
> Hi,
>
> On 16 October 2014 17:10:38 CEST, Michael Niedermayer <michaelni at gmx.at> wrote:
> >On Thu, Oct 16, 2014 at 11:44:47AM +0200, Benoit Fouet wrote:
> >> Some encoders do not use syncsafe sizes in v2.4 id3 tags. Check the
> >next
> >> tag to try to choose between the two.
> >>
> >> Fixes ticket #4003
> >> ---
> >> libavformat/id3v2.c | 42 +++++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 41 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> >> index 5469e0a..3bccd76 100644
> >> --- a/libavformat/id3v2.c
> >> +++ b/libavformat/id3v2.c
> >> @@ -170,6 +170,23 @@ static unsigned int get_size(AVIOContext *s, int
> >len)
> >> return v;
> >> }
> >>
> >> +/* No real verification, only check that the tag consists of
> >> + * a combination of capital alpha-numerical characters */
> >> +static int is_tag(const char *buf, int len)
> >> +{
> >> + if (!len)
> >> + return 0;
> >> +
> >> + while (len--)
> >> + if ((buf[len] < 'A' ||
> >> + buf[len] > 'Z') &&
> >> + (buf[len] < '0' ||
> >> + buf[len] > '9'))
> >> + return 0;
> >> +
> >> + return 1;
> >> +}
> >> +
> >> /**
> >> * Free GEOB type extra metadata.
> >> */
> >> @@ -734,8 +751,31 @@ static void id3v2_parse(AVIOContext *pb,
> >AVDictionary **metadata,
> >> tag[4] = 0;
> >> if (version == 3) {
> >> tlen = avio_rb32(pb);
> >> - } else
> >> + } else {
> >> tlen = get_size(pb, 4);
> >> + if (tlen > 0x7f) {
> >
> >i think that check isnt sufficient to detect that the 2 readings
> >differ. For example 0xFF would be read as 0xFF by one and 0x7F by
> >the other and would not trigger this
> >
>
> True, although I guess that could be handled by just making this test a >= instead of >.
that wouldnt work with 0x81
>
> >also maybe len could be used to distingish a subset of cases
> >
>
> Not sure I understand what you mean here... The tlen check seems to be enough to me.
i was thinking of avoiding the seek for cases where one is clearly
invalid like tlen > len IIRC the variable names
>
> >and there is the problem with seekable=0 streams where seeking
> >back might fail, not sure what to do in that case ...
> >
>
> We could theoretically buffer the data instead of seeking back, as the syncsafe size will always be smaller than the other one. Is this something that we want?
> I can work on something like that.
probably that is needed, also see ffio_ensure_seekback()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141017/2ab1f335/attachment.asc>