[FFmpeg-devel] [PATCH 3/4] lavf/movenc: Fail when codec tag is invalid for format

[John Stebbins]

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.

-- 
John      GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01  83F0 49F1 D7B2 60D4 D0F7


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180827/9ecc64af/attachment.sig>