| Message ID | 20180827195709.25396-3-jstebbins@jetheaddev.com |
|---|---|
| State | Superseded |
| Headers | show |
On 8/27/2018 4:57 PM, John Stebbins wrote: > Fixes ticket #6897 > --- > libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- > 1 file changed, 29 insertions(+), 11 deletions(-) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index 8a3b651514..dd6281d210 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { > { AV_CODEC_ID_NONE, 0 }, > }; > > +static int validate_codec_tag(const AVCodecTag *const *tags, > + unsigned int tag, int codec_id) > +{ > + int i; > + > + /** > + * Check that tag + id is in the table > + */ > + for (i = 0; tags && tags[i]; i++) { > + const AVCodecTag *codec_tags = tags[i]; > + while (codec_tags->id != AV_CODEC_ID_NONE) { > + if (codec_tags->tag == tag && codec_tags->id == codec_id) { Make both tag checks case insensitive using avpriv_toupper4(), then return codec_tags->tag instead of 1 if the check succeeds. > + return 1; > + } > + codec_tags++; > + } > + } > + return 0; > +} > + > static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) > { > int tag; Take the opportunity to change this, the ones in mov_get_codec_tag() and in validate_codec_tag() to unsigned int, including the return types. Codec tags in AVCodecTag are unsigned after all. > @@ -1596,23 +1616,21 @@ static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) > if (is_cover_image(track->st)) > return ff_codec_get_tag(codec_cover_image_tags, track->par->codec_id); > > - if (track->mode == MODE_MP4 || track->mode == MODE_PSP) > - tag = track->par->codec_tag; > - else if (track->mode == MODE_ISM) > - tag = track->par->codec_tag; > - else if (track->mode == MODE_IPOD) { > + if (track->mode == MODE_IPOD) > if (!av_match_ext(s->url, "m4a") && > !av_match_ext(s->url, "m4v") && > !av_match_ext(s->url, "m4b")) > av_log(s, AV_LOG_WARNING, "Warning, extension is not .m4a nor .m4v " > "Quicktime/Ipod might not play the file\n"); > - tag = track->par->codec_tag; > - } else if (track->mode & MODE_3GP) > - tag = track->par->codec_tag; > - else if (track->mode == MODE_F4V) > - tag = track->par->codec_tag; > - else > + > + if (track->mode == MODE_MOV) > tag = mov_get_codec_tag(s, track); > + else > + if (!validate_codec_tag(s->oformat->codec_tag, track->par->codec_tag, > + track->par->codec_id)) > + tag = 0; > + else > + tag = track->par->codec_tag; And of course make this simply tag = validate_codec_tag(...); Thanks. > > return tag; > } >
On 08/27/2018 01:29 PM, James Almer wrote: > On 8/27/2018 4:57 PM, John Stebbins wrote: >> Fixes ticket #6897 >> --- >> libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- >> 1 file changed, 29 insertions(+), 11 deletions(-) >> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >> index 8a3b651514..dd6281d210 100644 >> --- a/libavformat/movenc.c >> +++ b/libavformat/movenc.c >> @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { >> { AV_CODEC_ID_NONE, 0 }, >> }; >> >> +static int validate_codec_tag(const AVCodecTag *const *tags, >> + unsigned int tag, int codec_id) >> +{ >> + int i; >> + >> + /** >> + * Check that tag + id is in the table >> + */ >> + for (i = 0; tags && tags[i]; i++) { >> + const AVCodecTag *codec_tags = tags[i]; >> + while (codec_tags->id != AV_CODEC_ID_NONE) { >> + if (codec_tags->tag == tag && codec_tags->id == codec_id) { > Make both tag checks case insensitive using avpriv_toupper4(), then > return codec_tags->tag instead of 1 if the check succeeds. I've never seen mismatched case in these tags, but sure, why not... there's plenty I haven't seen. > >> + return 1; >> + } >> + codec_tags++; >> + } >> + } >> + return 0; >> +} >> + >> static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) >> { >> int tag; > Take the opportunity to change this, the ones in mov_get_codec_tag() and > in validate_codec_tag() to unsigned int, including the return types. > Codec tags in AVCodecTag are unsigned after all. > >> @@ -1596,23 +1616,21 @@ static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) >> if (is_cover_image(track->st)) >> return ff_codec_get_tag(codec_cover_image_tags, track->par->codec_id); >> >> - if (track->mode == MODE_MP4 || track->mode == MODE_PSP) >> - tag = track->par->codec_tag; >> - else if (track->mode == MODE_ISM) >> - tag = track->par->codec_tag; >> - else if (track->mode == MODE_IPOD) { >> + if (track->mode == MODE_IPOD) >> if (!av_match_ext(s->url, "m4a") && >> !av_match_ext(s->url, "m4v") && >> !av_match_ext(s->url, "m4b")) >> av_log(s, AV_LOG_WARNING, "Warning, extension is not .m4a nor .m4v " >> "Quicktime/Ipod might not play the file\n"); >> - tag = track->par->codec_tag; >> - } else if (track->mode & MODE_3GP) >> - tag = track->par->codec_tag; >> - else if (track->mode == MODE_F4V) >> - tag = track->par->codec_tag; >> - else >> + >> + if (track->mode == MODE_MOV) >> tag = mov_get_codec_tag(s, track); >> + else >> + if (!validate_codec_tag(s->oformat->codec_tag, track->par->codec_tag, >> + track->par->codec_id)) >> + tag = 0; >> + else >> + tag = track->par->codec_tag; > And of course make this simply > > tag = validate_codec_tag(...); > > Thanks. > >> >> return tag; >> } >> All good suggestions, thanks.
On 8/27/2018 5:48 PM, John Stebbins wrote: > On 08/27/2018 01:29 PM, James Almer wrote: >> On 8/27/2018 4:57 PM, John Stebbins wrote: >>> Fixes ticket #6897 >>> --- >>> libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- >>> 1 file changed, 29 insertions(+), 11 deletions(-) >>> >>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>> index 8a3b651514..dd6281d210 100644 >>> --- a/libavformat/movenc.c >>> +++ b/libavformat/movenc.c >>> @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { >>> { AV_CODEC_ID_NONE, 0 }, >>> }; >>> >>> +static int validate_codec_tag(const AVCodecTag *const *tags, >>> + unsigned int tag, int codec_id) >>> +{ >>> + int i; >>> + >>> + /** >>> + * Check that tag + id is in the table >>> + */ >>> + for (i = 0; tags && tags[i]; i++) { >>> + const AVCodecTag *codec_tags = tags[i]; >>> + while (codec_tags->id != AV_CODEC_ID_NONE) { >>> + if (codec_tags->tag == tag && codec_tags->id == codec_id) { >> Make both tag checks case insensitive using avpriv_toupper4(), then >> return codec_tags->tag instead of 1 if the check succeeds. > > I've never seen mismatched case in these tags, but sure, why not... there's plenty I haven't seen. AV1 in IVF is AV01, wheres in mp4 it's av01. That's the case i had in mind when requesting this. > >> >>> + return 1; >>> + } >>> + codec_tags++; >>> + } >>> + } >>> + return 0; >>> +} >>> + >>> static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) >>> { >>> int tag; >> Take the opportunity to change this, the ones in mov_get_codec_tag() and >> in validate_codec_tag() to unsigned int, including the return types. >> Codec tags in AVCodecTag are unsigned after all. >> >>> @@ -1596,23 +1616,21 @@ static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) >>> if (is_cover_image(track->st)) >>> return ff_codec_get_tag(codec_cover_image_tags, track->par->codec_id); >>> >>> - if (track->mode == MODE_MP4 || track->mode == MODE_PSP) >>> - tag = track->par->codec_tag; >>> - else if (track->mode == MODE_ISM) >>> - tag = track->par->codec_tag; >>> - else if (track->mode == MODE_IPOD) { >>> + if (track->mode == MODE_IPOD) >>> if (!av_match_ext(s->url, "m4a") && >>> !av_match_ext(s->url, "m4v") && >>> !av_match_ext(s->url, "m4b")) >>> av_log(s, AV_LOG_WARNING, "Warning, extension is not .m4a nor .m4v " >>> "Quicktime/Ipod might not play the file\n"); >>> - tag = track->par->codec_tag; >>> - } else if (track->mode & MODE_3GP) >>> - tag = track->par->codec_tag; >>> - else if (track->mode == MODE_F4V) >>> - tag = track->par->codec_tag; >>> - else >>> + >>> + if (track->mode == MODE_MOV) >>> tag = mov_get_codec_tag(s, track); >>> + else >>> + if (!validate_codec_tag(s->oformat->codec_tag, track->par->codec_tag, >>> + track->par->codec_id)) >>> + tag = 0; >>> + else >>> + tag = track->par->codec_tag; >> And of course make this simply >> >> tag = validate_codec_tag(...); >> >> Thanks. >> >>> >>> return tag; >>> } >>> > > > All good suggestions, thanks. > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 08/27/2018 02:03 PM, James Almer wrote: > On 8/27/2018 5:48 PM, John Stebbins wrote: >> On 08/27/2018 01:29 PM, James Almer wrote: >>> On 8/27/2018 4:57 PM, John Stebbins wrote: >>>> Fixes ticket #6897 >>>> --- >>>> libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- >>>> 1 file changed, 29 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>>> index 8a3b651514..dd6281d210 100644 >>>> --- a/libavformat/movenc.c >>>> +++ b/libavformat/movenc.c >>>> @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { >>>> { AV_CODEC_ID_NONE, 0 }, >>>> }; >>>> >>>> +static int validate_codec_tag(const AVCodecTag *const *tags, >>>> + unsigned int tag, int codec_id) >>>> +{ >>>> + int i; >>>> + >>>> + /** >>>> + * Check that tag + id is in the table >>>> + */ >>>> + for (i = 0; tags && tags[i]; i++) { >>>> + const AVCodecTag *codec_tags = tags[i]; >>>> + while (codec_tags->id != AV_CODEC_ID_NONE) { >>>> + if (codec_tags->tag == tag && codec_tags->id == codec_id) { >>> Make both tag checks case insensitive using avpriv_toupper4(), then >>> return codec_tags->tag instead of 1 if the check succeeds. >> I've never seen mismatched case in these tags, but sure, why not... there's plenty I haven't seen. > AV1 in IVF is AV01, wheres in mp4 it's av01. That's the case i had in > mind when requesting this. Hmm, I should probably return codec_tags->tag in this case rather than tag since it is the expected capitalization for the container. I'll have to fix that (again). > >>>> + return 1; >>>> + } >>>> + codec_tags++; >>>> + } >>>> + } >>>> + return 0; >>>> +} >>>> + >>>> static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) >>>> { >>>> int tag; >>> Take the opportunity to change this, the ones in mov_get_codec_tag() and >>> in validate_codec_tag() to unsigned int, including the return types. >>> Codec tags in AVCodecTag are unsigned after all. >>> >>>> @@ -1596,23 +1616,21 @@ static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) >>>> if (is_cover_image(track->st)) >>>> return ff_codec_get_tag(codec_cover_image_tags, track->par->codec_id); >>>> >>>> - if (track->mode == MODE_MP4 || track->mode == MODE_PSP) >>>> - tag = track->par->codec_tag; >>>> - else if (track->mode == MODE_ISM) >>>> - tag = track->par->codec_tag; >>>> - else if (track->mode == MODE_IPOD) { >>>> + if (track->mode == MODE_IPOD) >>>> if (!av_match_ext(s->url, "m4a") && >>>> !av_match_ext(s->url, "m4v") && >>>> !av_match_ext(s->url, "m4b")) >>>> av_log(s, AV_LOG_WARNING, "Warning, extension is not .m4a nor .m4v " >>>> "Quicktime/Ipod might not play the file\n"); >>>> - tag = track->par->codec_tag; >>>> - } else if (track->mode & MODE_3GP) >>>> - tag = track->par->codec_tag; >>>> - else if (track->mode == MODE_F4V) >>>> - tag = track->par->codec_tag; >>>> - else >>>> + >>>> + if (track->mode == MODE_MOV) >>>> tag = mov_get_codec_tag(s, track); >>>> + else >>>> + if (!validate_codec_tag(s->oformat->codec_tag, track->par->codec_tag, >>>> + track->par->codec_id)) >>>> + tag = 0; >>>> + else >>>> + tag = track->par->codec_tag; >>> And of course make this simply >>> >>> tag = validate_codec_tag(...); >>> >>> Thanks. >>> >>>> >>>> return tag; >>>> } >>>> >> >> All good suggestions, thanks. >> >> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 8/27/2018 6:07 PM, John Stebbins wrote: > On 08/27/2018 02:03 PM, James Almer wrote: >> On 8/27/2018 5:48 PM, John Stebbins wrote: >>> On 08/27/2018 01:29 PM, James Almer wrote: >>>> On 8/27/2018 4:57 PM, John Stebbins wrote: >>>>> Fixes ticket #6897 >>>>> --- >>>>> libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- >>>>> 1 file changed, 29 insertions(+), 11 deletions(-) >>>>> >>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>>>> index 8a3b651514..dd6281d210 100644 >>>>> --- a/libavformat/movenc.c >>>>> +++ b/libavformat/movenc.c >>>>> @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { >>>>> { AV_CODEC_ID_NONE, 0 }, >>>>> }; >>>>> >>>>> +static int validate_codec_tag(const AVCodecTag *const *tags, >>>>> + unsigned int tag, int codec_id) >>>>> +{ >>>>> + int i; >>>>> + >>>>> + /** >>>>> + * Check that tag + id is in the table >>>>> + */ >>>>> + for (i = 0; tags && tags[i]; i++) { >>>>> + const AVCodecTag *codec_tags = tags[i]; >>>>> + while (codec_tags->id != AV_CODEC_ID_NONE) { >>>>> + if (codec_tags->tag == tag && codec_tags->id == codec_id) { >>>> Make both tag checks case insensitive using avpriv_toupper4(), then >>>> return codec_tags->tag instead of 1 if the check succeeds. >>> I've never seen mismatched case in these tags, but sure, why not... there's plenty I haven't seen. >> AV1 in IVF is AV01, wheres in mp4 it's av01. That's the case i had in >> mind when requesting this. > > Hmm, I should probably return codec_tags->tag in this case rather than tag since it is the expected capitalization for > the container. I'll have to fix that (again). What's the status of this? With that change i think it should be good to go in. > >> >>>>> + return 1; >>>>> + } >>>>> + codec_tags++; >>>>> + } >>>>> + } >>>>> + return 0; >>>>> +} >>>>> + >>>>> static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) >>>>> { >>>>> int tag; >>>> Take the opportunity to change this, the ones in mov_get_codec_tag() and >>>> in validate_codec_tag() to unsigned int, including the return types. >>>> Codec tags in AVCodecTag are unsigned after all. >>>> >>>>> @@ -1596,23 +1616,21 @@ static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) >>>>> if (is_cover_image(track->st)) >>>>> return ff_codec_get_tag(codec_cover_image_tags, track->par->codec_id); >>>>> >>>>> - if (track->mode == MODE_MP4 || track->mode == MODE_PSP) >>>>> - tag = track->par->codec_tag; >>>>> - else if (track->mode == MODE_ISM) >>>>> - tag = track->par->codec_tag; >>>>> - else if (track->mode == MODE_IPOD) { >>>>> + if (track->mode == MODE_IPOD) >>>>> if (!av_match_ext(s->url, "m4a") && >>>>> !av_match_ext(s->url, "m4v") && >>>>> !av_match_ext(s->url, "m4b")) >>>>> av_log(s, AV_LOG_WARNING, "Warning, extension is not .m4a nor .m4v " >>>>> "Quicktime/Ipod might not play the file\n"); >>>>> - tag = track->par->codec_tag; >>>>> - } else if (track->mode & MODE_3GP) >>>>> - tag = track->par->codec_tag; >>>>> - else if (track->mode == MODE_F4V) >>>>> - tag = track->par->codec_tag; >>>>> - else >>>>> + >>>>> + if (track->mode == MODE_MOV) >>>>> tag = mov_get_codec_tag(s, track); >>>>> + else >>>>> + if (!validate_codec_tag(s->oformat->codec_tag, track->par->codec_tag, >>>>> + track->par->codec_id)) >>>>> + tag = 0; >>>>> + else >>>>> + tag = track->par->codec_tag; >>>> And of course make this simply >>>> >>>> tag = validate_codec_tag(...); >>>> >>>> Thanks. >>>> >>>>> >>>>> return tag; >>>>> } >>>>> >>> >>> All good suggestions, thanks. >>> >>> >>> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 09/06/2018 03:40 PM, James Almer wrote: > On 8/27/2018 6:07 PM, John Stebbins wrote: >> On 08/27/2018 02:03 PM, James Almer wrote: >>> On 8/27/2018 5:48 PM, John Stebbins wrote: >>>> On 08/27/2018 01:29 PM, James Almer wrote: >>>>> On 8/27/2018 4:57 PM, John Stebbins wrote: >>>>>> Fixes ticket #6897 >>>>>> --- >>>>>> libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- >>>>>> 1 file changed, 29 insertions(+), 11 deletions(-) >>>>>> >>>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>>>>> index 8a3b651514..dd6281d210 100644 >>>>>> --- a/libavformat/movenc.c >>>>>> +++ b/libavformat/movenc.c >>>>>> @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { >>>>>> { AV_CODEC_ID_NONE, 0 }, >>>>>> }; >>>>>> >>>>>> +static int validate_codec_tag(const AVCodecTag *const *tags, >>>>>> + unsigned int tag, int codec_id) >>>>>> +{ >>>>>> + int i; >>>>>> + >>>>>> + /** >>>>>> + * Check that tag + id is in the table >>>>>> + */ >>>>>> + for (i = 0; tags && tags[i]; i++) { >>>>>> + const AVCodecTag *codec_tags = tags[i]; >>>>>> + while (codec_tags->id != AV_CODEC_ID_NONE) { >>>>>> + if (codec_tags->tag == tag && codec_tags->id == codec_id) { >>>>> Make both tag checks case insensitive using avpriv_toupper4(), then >>>>> return codec_tags->tag instead of 1 if the check succeeds. >>>> I've never seen mismatched case in these tags, but sure, why not... there's plenty I haven't seen. >>> AV1 in IVF is AV01, wheres in mp4 it's av01. That's the case i had in >>> mind when requesting this. >> Hmm, I should probably return codec_tags->tag in this case rather than tag since it is the expected capitalization for >> the container. I'll have to fix that (again). > What's the status of this? With that change i think it should be good to > go in. It's good as far as I'm concerned. I don't have commit privs.
On 9/7/2018 12:09 PM, John Stebbins wrote: > On 09/06/2018 03:40 PM, James Almer wrote: >> On 8/27/2018 6:07 PM, John Stebbins wrote: >>> On 08/27/2018 02:03 PM, James Almer wrote: >>>> On 8/27/2018 5:48 PM, John Stebbins wrote: >>>>> On 08/27/2018 01:29 PM, James Almer wrote: >>>>>> On 8/27/2018 4:57 PM, John Stebbins wrote: >>>>>>> Fixes ticket #6897 >>>>>>> --- >>>>>>> libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- >>>>>>> 1 file changed, 29 insertions(+), 11 deletions(-) >>>>>>> >>>>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>>>>>> index 8a3b651514..dd6281d210 100644 >>>>>>> --- a/libavformat/movenc.c >>>>>>> +++ b/libavformat/movenc.c >>>>>>> @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { >>>>>>> { AV_CODEC_ID_NONE, 0 }, >>>>>>> }; >>>>>>> >>>>>>> +static int validate_codec_tag(const AVCodecTag *const *tags, >>>>>>> + unsigned int tag, int codec_id) >>>>>>> +{ >>>>>>> + int i; >>>>>>> + >>>>>>> + /** >>>>>>> + * Check that tag + id is in the table >>>>>>> + */ >>>>>>> + for (i = 0; tags && tags[i]; i++) { >>>>>>> + const AVCodecTag *codec_tags = tags[i]; >>>>>>> + while (codec_tags->id != AV_CODEC_ID_NONE) { >>>>>>> + if (codec_tags->tag == tag && codec_tags->id == codec_id) { >>>>>> Make both tag checks case insensitive using avpriv_toupper4(), then >>>>>> return codec_tags->tag instead of 1 if the check succeeds. >>>>> I've never seen mismatched case in these tags, but sure, why not... there's plenty I haven't seen. >>>> AV1 in IVF is AV01, wheres in mp4 it's av01. That's the case i had in >>>> mind when requesting this. >>> Hmm, I should probably return codec_tags->tag in this case rather than tag since it is the expected capitalization for >>> the container. I'll have to fix that (again). >> What's the status of this? With that change i think it should be good to >> go in. > > It's good as far as I'm concerned. I don't have commit privs. Did you send the updated version returning codec_tags->tag? I can't find it in my inbox. Otherwise I'd have pushed it.
On 09/07/2018 08:16 AM, James Almer wrote: > On 9/7/2018 12:09 PM, John Stebbins wrote: >> On 09/06/2018 03:40 PM, James Almer wrote: >>> On 8/27/2018 6:07 PM, John Stebbins wrote: >>>> On 08/27/2018 02:03 PM, James Almer wrote: >>>>> On 8/27/2018 5:48 PM, John Stebbins wrote: >>>>>> On 08/27/2018 01:29 PM, James Almer wrote: >>>>>>> On 8/27/2018 4:57 PM, John Stebbins wrote: >>>>>>>> Fixes ticket #6897 >>>>>>>> --- >>>>>>>> libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- >>>>>>>> 1 file changed, 29 insertions(+), 11 deletions(-) >>>>>>>> >>>>>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>>>>>>> index 8a3b651514..dd6281d210 100644 >>>>>>>> --- a/libavformat/movenc.c >>>>>>>> +++ b/libavformat/movenc.c >>>>>>>> @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { >>>>>>>> { AV_CODEC_ID_NONE, 0 }, >>>>>>>> }; >>>>>>>> >>>>>>>> +static int validate_codec_tag(const AVCodecTag *const *tags, >>>>>>>> + unsigned int tag, int codec_id) >>>>>>>> +{ >>>>>>>> + int i; >>>>>>>> + >>>>>>>> + /** >>>>>>>> + * Check that tag + id is in the table >>>>>>>> + */ >>>>>>>> + for (i = 0; tags && tags[i]; i++) { >>>>>>>> + const AVCodecTag *codec_tags = tags[i]; >>>>>>>> + while (codec_tags->id != AV_CODEC_ID_NONE) { >>>>>>>> + if (codec_tags->tag == tag && codec_tags->id == codec_id) { >>>>>>> Make both tag checks case insensitive using avpriv_toupper4(), then >>>>>>> return codec_tags->tag instead of 1 if the check succeeds. >>>>>> I've never seen mismatched case in these tags, but sure, why not... there's plenty I haven't seen. >>>>> AV1 in IVF is AV01, wheres in mp4 it's av01. That's the case i had in >>>>> mind when requesting this. >>>> Hmm, I should probably return codec_tags->tag in this case rather than tag since it is the expected capitalization for >>>> the container. I'll have to fix that (again). >>> What's the status of this? With that change i think it should be good to >>> go in. >> It's good as far as I'm concerned. I don't have commit privs. > Did you send the updated version returning codec_tags->tag? I can't find > it in my inbox. Otherwise I'd have pushed it. > Yup https://ffmpeg.org/pipermail/ffmpeg-devel/2018-August/233739.html
On 9/7/2018 12:27 PM, John Stebbins wrote: > On 09/07/2018 08:16 AM, James Almer wrote: >> On 9/7/2018 12:09 PM, John Stebbins wrote: >>> On 09/06/2018 03:40 PM, James Almer wrote: >>>> On 8/27/2018 6:07 PM, John Stebbins wrote: >>>>> On 08/27/2018 02:03 PM, James Almer wrote: >>>>>> On 8/27/2018 5:48 PM, John Stebbins wrote: >>>>>>> On 08/27/2018 01:29 PM, James Almer wrote: >>>>>>>> On 8/27/2018 4:57 PM, John Stebbins wrote: >>>>>>>>> Fixes ticket #6897 >>>>>>>>> --- >>>>>>>>> libavformat/movenc.c | 40 +++++++++++++++++++++++++++++----------- >>>>>>>>> 1 file changed, 29 insertions(+), 11 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >>>>>>>>> index 8a3b651514..dd6281d210 100644 >>>>>>>>> --- a/libavformat/movenc.c >>>>>>>>> +++ b/libavformat/movenc.c >>>>>>>>> @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { >>>>>>>>> { AV_CODEC_ID_NONE, 0 }, >>>>>>>>> }; >>>>>>>>> >>>>>>>>> +static int validate_codec_tag(const AVCodecTag *const *tags, >>>>>>>>> + unsigned int tag, int codec_id) >>>>>>>>> +{ >>>>>>>>> + int i; >>>>>>>>> + >>>>>>>>> + /** >>>>>>>>> + * Check that tag + id is in the table >>>>>>>>> + */ >>>>>>>>> + for (i = 0; tags && tags[i]; i++) { >>>>>>>>> + const AVCodecTag *codec_tags = tags[i]; >>>>>>>>> + while (codec_tags->id != AV_CODEC_ID_NONE) { >>>>>>>>> + if (codec_tags->tag == tag && codec_tags->id == codec_id) { >>>>>>>> Make both tag checks case insensitive using avpriv_toupper4(), then >>>>>>>> return codec_tags->tag instead of 1 if the check succeeds. >>>>>>> I've never seen mismatched case in these tags, but sure, why not... there's plenty I haven't seen. >>>>>> AV1 in IVF is AV01, wheres in mp4 it's av01. That's the case i had in >>>>>> mind when requesting this. >>>>> Hmm, I should probably return codec_tags->tag in this case rather than tag since it is the expected capitalization for >>>>> the container. I'll have to fix that (again). >>>> What's the status of this? With that change i think it should be good to >>>> go in. >>> It's good as far as I'm concerned. I don't have commit privs. >> Did you send the updated version returning codec_tags->tag? I can't find >> it in my inbox. Otherwise I'd have pushed it. >> > > Yup > https://ffmpeg.org/pipermail/ffmpeg-devel/2018-August/233739.html Ok, pushed the set then. Thanks.
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 8a3b651514..dd6281d210 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1589,6 +1589,26 @@ static const AVCodecTag codec_cover_image_tags[] = { { AV_CODEC_ID_NONE, 0 }, }; +static int validate_codec_tag(const AVCodecTag *const *tags, + unsigned int tag, int codec_id) +{ + int i; + + /** + * Check that tag + id is in the table + */ + for (i = 0; tags && tags[i]; i++) { + const AVCodecTag *codec_tags = tags[i]; + while (codec_tags->id != AV_CODEC_ID_NONE) { + if (codec_tags->tag == tag && codec_tags->id == codec_id) { + return 1; + } + codec_tags++; + } + } + return 0; +} + static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) { int tag; @@ -1596,23 +1616,21 @@ static int mov_find_codec_tag(AVFormatContext *s, MOVTrack *track) if (is_cover_image(track->st)) return ff_codec_get_tag(codec_cover_image_tags, track->par->codec_id); - if (track->mode == MODE_MP4 || track->mode == MODE_PSP) - tag = track->par->codec_tag; - else if (track->mode == MODE_ISM) - tag = track->par->codec_tag; - else if (track->mode == MODE_IPOD) { + if (track->mode == MODE_IPOD) if (!av_match_ext(s->url, "m4a") && !av_match_ext(s->url, "m4v") && !av_match_ext(s->url, "m4b")) av_log(s, AV_LOG_WARNING, "Warning, extension is not .m4a nor .m4v " "Quicktime/Ipod might not play the file\n"); - tag = track->par->codec_tag; - } else if (track->mode & MODE_3GP) - tag = track->par->codec_tag; - else if (track->mode == MODE_F4V) - tag = track->par->codec_tag; - else + + if (track->mode == MODE_MOV) tag = mov_get_codec_tag(s, track); + else + if (!validate_codec_tag(s->oformat->codec_tag, track->par->codec_tag, + track->par->codec_id)) + tag = 0; + else + tag = track->par->codec_tag; return tag; }