[FFmpeg-devel] [PATCH V1] lavf/oggparsevorbis: Fix change the case of metadata keys issue

[mypopy at gmail.com]

On Wed, Apr 17, 2019 at 6:41 AM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Tue, Apr 16, 2019 at 09:33:08AM +0800, mypopy at gmail.com wrote:
> > On Tue, Apr 16, 2019 at 5:51 AM Michael Niedermayer
> > <michael at niedermayer.cc> wrote:
> > >
> > > On Mon, Apr 15, 2019 at 09:58:30AM +0800, Jun Zhao wrote:
> > > > From: Jun Zhao <barryjzhao at tencent.com>
> > > >
> > > > The spec in https://xiph.org/vorbis/doc/v-comment.html states that
> > > > the metadata keys are case-insensitive, so don't change the case
> > > > and update the fate test case.
> > > >
> > > > Fix #7784
> > > >
> > > > Signed-off-by: Jun Zhao <barryjzhao at tencent.com>
> > > > ---
> > > >  libavformat/oggparsevorbis.c             |    9 ++++-----
> > > >  tests/ref/fate/limited_input_seek        |    2 +-
> > > >  tests/ref/fate/limited_input_seek-copyts |    2 +-
> > > >  3 files changed, 6 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/libavformat/oggparsevorbis.c b/libavformat/oggparsevorbis.c
> > > > index bcfd246..c3c8d38 100644
> > > > --- a/libavformat/oggparsevorbis.c
> > > > +++ b/libavformat/oggparsevorbis.c
> > > > @@ -44,7 +44,7 @@ static int ogm_chapter(AVFormatContext *as, uint8_t *key, uint8_t *val)
> > > >      int i, cnum, h, m, s, ms, keylen = strlen(key);
> > > >      AVChapter *chapter = NULL;
> > > >
> > > > -    if (keylen < 9 || sscanf(key, "CHAPTER%03d", &cnum) != 1)
> > > > +    if (keylen < 9 || (av_strcasecmp(key, "CHAPTER")!=0 && sscanf(key+7, "%03d", &cnum) != 1))
> > >
> > > this looks a bit odd, shouldnt this use av_strncasecmp() ?
> > >
> > It's  in order to be compatible with the fate test case
> > vorbis-1833-chapthers, in this test,
> > we use lower case in test clip but check upper case in the code.
>
> av_strcasecmp() will only succeed if the string ends after "CHAPTER" and
> contains no number which is why av_strncasecmp() feels like more effective.
> The problem is ultimatle that keylen is at this point >= 9 while
> the only case where av_strcasecmp with "CHAPTER" == 0 is with a
> keylen of 7. Thats tricky to achieve ;)
>
> Or said differently this code is not a case insensitive version of the
> previous
Thanks the comments, will update the patch