[FFmpeg-devel] [PATCHv3 2/3] lavfi/loudnorm: add an internal libebur128 library

[Marton Balint]

On Sat, 21 Jan 2017, Michael Niedermayer wrote:

> On Tue, Nov 01, 2016 at 09:08:16PM +0100, Marton Balint wrote:
>> Also contains the following changes to the library:
>> - add ff_ prefix to functions
>> - remove cplusplus defines.
>> - add FF_ prefix to contants and some structs
>> - remove true peak calculation feature, since it uses its own resampler, and
>>   af_loudnorm does not need it.
>> - remove version info and some fprintf(stderr) functions
>> - convert to use av_malloc
>> - always use histogram mode for LRA calculation, otherwise LRA data is slowly
>>   consuming memory making af_loudnorm unfit for 24/7 operation. It also uses a
>>   BSD style linked list implementation which is probably not available on all
>>   platforms. So let's just remove the classic mode which not uses histogram.
>> - add ff_thread_once for calculating static histogram tables
>> - convert some functions to void which cannot fail
>> - remove intrinsics and some unused headers
>> - add support for planar audio
>> - remove channel / sample rate changer function, in ffmpeg usually we simply
>>   alloc a new context
>> - convert some static variables to defines
>> - declare static histogram variables as aligned
>> - convert some initalizations to mallocz
>> - add window size parameter to init function and remove window size setter
>>   function
>> - convert return codes to AVERROR
>> - fix indentation
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  Changelog                 |   1 +
>>  configure                 |   5 -
>>  doc/filters.texi          |   3 -
>>  libavfilter/Makefile      |   2 +-
>>  libavfilter/af_loudnorm.c |  60 ++--
>>  libavfilter/ebur128.c     | 782 ++++++++++++++++++++++++++++++++++++++++++++++
>>  libavfilter/ebur128.h     | 296 ++++++++++++++++++
>>  libavfilter/version.h     |   2 +-
>>  8 files changed, 1111 insertions(+), 40 deletions(-)
>>  create mode 100644 libavfilter/ebur128.c
>>  create mode 100644 libavfilter/ebur128.h
> [...]
>
>> +int ff_ebur128_relative_threshold(FFEBUR128State * st, double *out)
>> +{
>> +    double relative_threshold;
>> +    size_t above_thresh_counter;
>> +
>> +    if (st && (st->mode & FF_EBUR128_MODE_I) != FF_EBUR128_MODE_I)
>> +        return AVERROR(EINVAL);
>> +
>> +    ebur128_calc_relative_threshold(st, &above_thresh_counter,
>> +                                    &relative_threshold);
>
> Coverity dislikes this
> See: 1396246 Dereference after null check
>

Yeah, the current code is not very consistent across the functions, 
sometimes it returns an error if it gets a NULL FFEBUR128State *, 
sometimes it simply dereferences it, making it the callers responsibility 
to not use these functions with a NULL context.

I prefer to simply remove the checks for NULL, making it the callers 
responsibility the check the success of an FFEBUR128State context 
creation. Objection?

Regards,
Marton