[FFmpeg-devel] [PATCH major bump 1/6] libavutil/hdr_dynamic_vivid_metadata: fix AVHDRVividColorToneMappingParams

["zhilizhao(赵志立)"]

> On Feb 2, 2023, at 16:16, Anton Khirnov <anton at khirnov.net> wrote:
> 
> Quoting Zhao Zhili (2023-02-02 08:02:03)
>> From: Zhao Zhili <zhilizhao at tencent.com>
>> 
>> There are two group of three_Spline params. Fix the struct
>> definition and usecases inside libavcodec, libavfilter and ffprobe.
>> 
>> Co-Author: Houxiang ZHU <zhuhouxiang at theuwa.com>
>> Signed-off-by: Zhao Zhili <zhilizhao at tencent.com>
>> ---
>> diff --git a/libavutil/hdr_dynamic_vivid_metadata.h b/libavutil/hdr_dynamic_vivid_metadata.h
>> index a34f83072c..4ceddc539d 100644
>> --- a/libavutil/hdr_dynamic_vivid_metadata.h
>> +++ b/libavutil/hdr_dynamic_vivid_metadata.h
>> @@ -126,42 +126,42 @@ typedef struct AVHDRVividColorToneMappingParams {
>>      * The mode of three Spline. the value shall be in the range
>>      * of 0 to 3, inclusive.
>>      */
>> -    int three_Spline_TH_mode;
>> +    int three_Spline_TH_mode[2];
>> 
>>     /**
>>      * three_Spline_TH_enable_MB is in the range of 0.0 to 1.0, inclusive
>>      * and in multiples of 1.0/255.
>>      *
>>      */
>> -    AVRational three_Spline_TH_enable_MB;
>> +    AVRational three_Spline_TH_enable_MB[2];
>> 
>>     /**
>>      * 3Spline_TH_enable of three Spline.
>>      * The value shall be in the range of 0.0 to 1.0, inclusive.
>>      * and in multiples of 1.0/4095.
>>      */
>> -    AVRational three_Spline_TH_enable;
>> +    AVRational three_Spline_TH_enable[2];
>> 
>>     /**
>>      * 3Spline_TH_Delta1 of three Spline.
>>      * The value shall be in the range of 0.0 to 0.25, inclusive,
>>      * and in multiples of 0.25/1023.
>>      */
>> -    AVRational three_Spline_TH_Delta1;
>> +    AVRational three_Spline_TH_Delta1[2];
>> 
>>     /**
>>      * 3Spline_TH_Delta2 of three Spline.
>>      * The value shall be in the range of 0.0 to 0.25, inclusive,
>>      * and in multiples of 0.25/1023.
>>      */
>> -    AVRational three_Spline_TH_Delta2;
>> +    AVRational three_Spline_TH_Delta2[2];
>> 
>>     /**
>>      * 3Spline_enable_Strength of three Spline.
>>      * The value shall be in the range of 0.0 to 1.0, inclusive,
>>      * and in multiples of 1.0/255.
>>      */
>> -    AVRational three_Spline_enable_Strength;
>> +    AVRational three_Spline_enable_Strength[2];
>> } AVHDRVividColorToneMappingParams;
> 
> A major bump is not for breaking APIs however you like, only for things
> that were scheduled in advance that our callers could have prepared for.
> You should add new fields, not change existing ones.
> Also, the documentation and doc/APIchanges need to be updated.

Adding new fields works, but very ugly.

The code never work if three_Spline_num > 1. Breaking API to let user
notice that may not be a bad thing in this specific situation.

> 
> -- 
> Anton Khirnov
> _______________________________________________
> 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".