[FFmpeg-devel] [PATCH] avformat/mov: make compatible brands more readable

[Zhao Zhili]

> On Sep 9, 2020, at 10:24 PM, James Almer <jamrial at gmail.com> wrote:
> 
> On 9/9/2020 10:07 AM, quinkblack at foxmail.com wrote:
>> From: Zhao Zhili <quinkblack at foxmail.com>
>> 
>> Use comma as separator between multiple compatible brands.
> 
> Wont this potentially break parsing of the output of ffmpeg/ffprobe, or
> even of the compatible_brands key in c->fc->metadata when using the dict
> API?

The format of metadata value is not documented API, it's fail enough
to say the user may depend on the undefined behavior.

After a second thought, is it safe enough to change be output of
ffmpeg/ffprobe but keep c->fc->metadata unmodified?

> 
>> ---
>> libavformat/mov.c | 24 +++++++++++++++---------
>> 1 file changed, 15 insertions(+), 9 deletions(-)
>> 
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index 690beb10ce..8f5341f925 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -1093,7 +1093,7 @@ static int aax_filter(uint8_t *input, int size, MOVContext *c)
>> static int mov_read_ftyp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>> {
>>     uint32_t minor_ver;
>> -    int comp_brand_size;
>> +    int comp_brand_count;
>>     char* comp_brands_str;
>>     uint8_t type[5] = {0};
>>     int ret = ffio_read_size(pb, type, 4);
>> @@ -1107,19 +1107,25 @@ static int mov_read_ftyp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>>     minor_ver = avio_rb32(pb); /* minor version */
>>     av_dict_set_int(&c->fc->metadata, "minor_version", minor_ver, 0);
>> 
>> -    comp_brand_size = atom.size - 8;
>> -    if (comp_brand_size < 0 || comp_brand_size == INT_MAX)
>> +    comp_brand_count = (atom.size - 8) / 4;
>> +    if (!comp_brand_count)
>> +        return 0;
>> +    else if (comp_brand_count < 0 || comp_brand_count > INT_MAX / 5)
>>         return AVERROR_INVALIDDATA;
>> -    comp_brands_str = av_malloc(comp_brand_size + 1); /* Add null terminator */
>> +    /* add separator between multiple brands, add null terminator */
>> +    comp_brands_str = av_malloc(comp_brand_count * 5);
>>     if (!comp_brands_str)
>>         return AVERROR(ENOMEM);
>> 
>> -    ret = ffio_read_size(pb, comp_brands_str, comp_brand_size);
>> -    if (ret < 0) {
>> -        av_freep(&comp_brands_str);
>> -        return ret;
>> +    for (int i = 0; i < comp_brand_count; i++) {
>> +        ret = ffio_read_size(pb, comp_brands_str + i * 5, 4);
>> +        if (ret < 0) {
>> +            av_freep(&comp_brands_str);
>> +            return ret;
>> +        }
>> +        comp_brands_str[i * 5 + 4] = ',';
>>     }
>> -    comp_brands_str[comp_brand_size] = 0;
>> +    comp_brands_str[comp_brand_count * 5 - 1] = '\0';
>>     av_dict_set(&c->fc->metadata, "compatible_brands",
>>                 comp_brands_str, AV_DICT_DONT_STRDUP_VAL);
>> 
>> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".