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

[Michael Niedermayer]

On Sun, Jan 22, 2017 at 04:30:15PM +0100, Marton Balint wrote:
> 
> 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?

anything thats reasoable consistent and makes the
Coverity warning disppear so it doesnt "hide" more serious issues
LGTM

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170122/e8b7c857/attachment.sig>