Fixes out of array access Fixes: 24d05e8b84676799c735c9e27d97895e/asan_heap-oob_1b70f6a_2955_7c3652a7f370f9f3ef40642bc2c99bb2.bit Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
avcodec/truemotion1: Initialize mb_change_byte only when needed
Fixes out of array read Fixes: d92114d8c2a019b8a6e50cd2a7301b54/asan_heap-oob_26bf563_60_1d3420277533de9dbf8aba3f93af346f.avi Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
This speeds up build_filter by ~ 50%. This gain should be pretty
consistent across all architectures and platforms.
Essentially, this relies on a observation that the filters have some
even/odd symmetry that may be exploited during the construction of the
polyphase filter bank. In particular, phases (scaled to [0, 1]) in [0.5, 1] are
easily derived from [0, 0.5] and expensive reevaluation of function
points are unnecessary. This requires some rather annoying even/odd
bookkeeping as can be seen from the patch.
I vaguely recall from signal processing theory more general symmetries allowing even greater
optimization of the construction. At a high level, "even functions"
correspond to 2, and one can imagine variations. Nevertheless, for the sake
of some generality and because of existing filters, this is all that is
being exploited.
Currently, this patch relies on phase_count being even or (trivially) 1,
though this is not an inherent limitation to the approach. This
assumption is safe as phase_count is 1 << phase_bits, and is hence a
power of two. There is no way for user API to set it to a nontrivial odd
number. This assumption has been placed as an assert in the code.
To repeat, this assumes even symmetry of the filters, which is the most common
way to get generalized linear phase anyway and is true of all currently
supported filters.
As a side note, accuracy should be identical or perhaps slightly better
due to this "forcing" filter symmetries leading to a better phase
characteristic. As before, I can't test this claim easily, though it may
be of interest.
Patch tested with FATE.
Sample benchmark (x86-64, Haswell, GNU/Linux):
test: swr-resample-dblp-44100-2626
new: 527376779 decicycles in build_filter(loop 1000), 256 runs, 0 skips 524361765 decicycles in build_filter(loop 1000), 512 runs, 0 skips 516552574 decicycles in build_filter(loop 1000), 1024 runs, 0 skips
old: 974178658 decicycles in build_filter(loop 1000), 256 runs, 0 skips 972794408 decicycles in build_filter(loop 1000), 512 runs, 0 skips 954350046 decicycles in build_filter(loop 1000), 1024 runs, 0 skips
Note that lower level optimizations are entirely possible, I focussed on
getting the high level semantics correct. In any case, this should
provide a good foundation.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
avcodec/aacsbr_template: replace qsort with AV_QSORT
When sbr->reset is set in encode_frame, a bunch of qsort calls might get made.
Thus, there is the potential of calling qsort whenever the spectral
contents change.
AV_QSORT is substantially faster due to the inlining of the comparison callback.
Thus, the increase in performance should be worth the increase in binary size.
avcodec/rawenc: Cast argument for av_image_copy_to_buffer() to const
Fixes: libavcodec/rawenc.c:64:40: warning: passing argument 3 of av_image_copy_to_buffer from incompatible pointer type [enabled by default] Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Bryan Huh [Mon, 2 Nov 2015 18:20:39 +0000 (10:20 -0800)]
avformat/cache: Use int64_t to avoid int overflow in cache_read
Fixes an issue where an int64_t ffurl_seek return-value was being stored
in an int (32-bit) "r" variable, leading to integer overflow when seeking
into a large file (>2GB), and ultimately a "Failed to perform internal
seek" error mesage.
To test, try running `ffprobe 'cache:http://<something>'` on a file that
is ~3GB large, whose moov atom is at the end of the file
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
all: use FFDIFFSIGN to resolve possible undefined behavior in comparators
FFDIFFSIGN was created explicitly for this purpose, since the common
return a - b idiom is unsafe regarding overflow on signed integers. It
optimizes to branchless code on common compilers.
FFDIFFSIGN also has the subjective benefit of being easier to read due
to lack of ternary operators.
Tested with FATE.
Things not covered by this are unsigned integers, for which overflows
are well defined, and also places where overflow is clearly impossible,
e.g an instance where the a - b was being done on 24 bit values.
This is of use for defining comparator callbacks. Common approaches like
return x-y are not safe due to the risks of overflow.
Furthermore, the (x > y) - (x < y) trick is optimized to branchless
code.
This also documents this macro accordingly.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
Tobias Rapp [Thu, 29 Oct 2015 08:11:37 +0000 (09:11 +0100)]
avutil/file_open: avoid file handle inheritance on Windows
Avoids inheritance of file handles on Windows systems similar to the
O_CLOEXEC/FD_CLOEXEC flag on Linux.
Fixes file lock issues in Windows applications when a child process
is started with handle inheritance enabled (standard input/output
redirection) while a FFmpeg transcoding is running in the parent
process.
Describes handle inheritance when creating new processes. Handle
inheritance must be enabled (bInheritHandles = TRUE) e.g. when you want
to pass handles for stdin/stdout via lpStartupInfo.
Signed-off-by: Tobias Rapp <t.rapp@noa-audio.com> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
clSetKernelArg can return an error due to lack of memory (for instance):
https://www.khronos.org/registry/cl/sdk/1.1/docs/man/xhtml/clSetKernelArg.html.
Thus this error must be propagated.
Currently should not trigger warnings, but adds robustness.
Untested.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
qsort is called indirectly in filter_frame, suggesting its performance
criticality. AV_QSORT is substantially faster due to the inlining of the
comparison callback. Thus, the increase in performance should be worth
the increase in binary size.
This optimization is just a low hanging fruit. The trac ticket 1430 is
a request for an improved deshake filter.
Sample benchmark (x86-64, Haswell, GNU/Linux):
File: original from https://trac.ffmpeg.org/ticket/1430
command: ffmpeg -stream_loop 8 -i file.webm -vf deshake=rx=64:ry=64 -f null -
Timer truncated at 1024 runs.
new:
28260 decicycles in qsort, 1 runs, 0 skips
35570 decicycles in qsort, 2 runs, 0 skips
39010 decicycles in qsort, 4 runs, 0 skips
46897 decicycles in qsort, 8 runs, 0 skips
40442 decicycles in qsort, 16 runs, 0 skips
41611 decicycles in qsort, 32 runs, 0 skips
40345 decicycles in qsort, 64 runs, 0 skips
38967 decicycles in qsort, 128 runs, 0 skips
38647 decicycles in qsort, 256 runs, 0 skips
40238 decicycles in qsort, 512 runs, 0 skips
39676 decicycles in qsort, 1024 runs, 0 skips
old: 1740280 decicycles in qsort, 1 runs, 0 skips
923560 decicycles in qsort, 2 runs, 0 skips
511330 decicycles in qsort, 4 runs, 0 skips
309720 decicycles in qsort, 8 runs, 0 skips
194900 decicycles in qsort, 16 runs, 0 skips
142686 decicycles in qsort, 32 runs, 0 skips
112516 decicycles in qsort, 64 runs, 0 skips
98166 decicycles in qsort, 128 runs, 0 skips
88147 decicycles in qsort, 256 runs, 0 skips
88706 decicycles in qsort, 512 runs, 0 skips
86783 decicycles in qsort, 1024 runs, 0 skips
Reviewed-by: Nicolas George <george@nsup.org> Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
ISO C restricts enumerator values to the range of int. Thus (for instance) 0x80000000
unfortunately does not work, and throws a warning with -Wpedantic on
clang 3.7.
This fixes it by using alternative expressions that result in identical
values but do not have this issue.
Tested with FATE.
Reviewed-by: Ronald S. Bultje <rsbultje@gmail.com> Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
avutil/mathematics: correct documentation for av_gcd
av_gcd is now always defined regardless of input. This documents this
change in the "documented API". Two benefits (closely related):
1. The function is robust, and there is no need to worry about INT64_MIN, etc.
2. Clients of av_gcd, like av_reduce, can now be made fully correct. Currently,
av_reduce can trigger undefined behavior if e.g num is INT64_MIN due to
integer overflow in the FFABS. Furthermore, this undefined behavior is
completely undocumented, and could be a fuzzer's paradise. The FFABS was needed in the past as
av_gcd was undefined for negative inputs. In order to make av_reduce
robust, it is essential to guarantee that av_gcd works for all int64_t.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
Luca Barbato [Thu, 29 Oct 2015 13:01:07 +0000 (14:01 +0100)]
configure: Add a SONAME entry for the android target
In order to load libraries in Android they need to be unversioned.
The android target section was derived from the BSD ones, and they
that simply drop the SONAME
Android M requires to have a SONAME entry but unversioned.
Vittorio Giovara [Thu, 22 Oct 2015 12:13:28 +0000 (14:13 +0200)]
h264: Print user data SEI under normal debug verbosity
Drop the need of setting -debug bugs since it's not a bug, and the
message is already under a AV_LOG_DEBUG log level. Instead only print
it when there is an actual string in it.
Martin Storsjö [Wed, 28 Oct 2015 20:19:12 +0000 (22:19 +0200)]
os_support: Don't try to return the service name as a string in getnameinfo
Some systems may be lacking getservbyport; the previous ifdef wasn't
quite enough since it still assumed that struct servent was defined,
as pointed out by Clément Gregoire.
Simply remove the possibility to return non-numeric services in
getnameinfo; no caller of getnameinfo within libavformat
currently try to use getnameinfo for retrieving the port number without
NI_NUMERICSERV, and falling back on getservbyport may be non-threadsafe.
Steven Robertson [Sun, 11 Oct 2015 01:27:17 +0000 (18:27 -0700)]
vf_lut: Add support for RGB48 and RGBA64.
Signed-off-by: Steven Robertson <steven@strobe.cc> Reviewed-by: Paul B Mahol <onemda@gmail.com> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
This ensures that no undefined behavior is invoked, while retaining
identical return values in all cases and at no loss of performance
(identical asm on clang and gcc).
Essentially, this patch exchanges undefined behavior with implementation
defined behavior, a strict improvement.
Rationale:
1. The ideal solution is to have the return type a uint64_t. This
unfortunately requires an API change.
2. The only pathological behavior happens if both arguments are
INT64_MIN, to the best of my knowledge. In such a case, the
implementation defined behavior is invoked in the sense that UINT64_MAX
is interpreted as INT64_MIN, which any reasonable implementation will
do. In any case, any usage where both arguments are INT64_MIN is a
fuzzer anyway.
3. Alternatives of checking, etc require branching and lose performance
for no concrete gain - no client cares about av_gcd's actual value when
both args are INT64_MIN. Even if it did, on sane platforms (e.g all the
ones FFmpeg cares about), it produces a correct gcd, namely INT64_MIN.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
projects / ffmpeg / log