The query_formats function of the amix filter tries to allocate a list
of channel layouts which are attached to more permanent objects
(an AVFilter's links) for storage afterwards on success. If attaching
a list to a link succeeds, the link becomes one of the common owners
of the list. Yet if a list has been successfully attached to links (or if
there were no links to attach it to in which case
ff_set_common_channel_layouts() already frees the list) and an error
happens lateron, the list was manually freed, which is wrong, because
the list has either already been freed or it is owned by its links in
which case these links' pointers to their list will become dangling and
there will be double-frees/uses-after-free when these links are cleaned
up automatically.
This commit fixes this by removing the custom freeing code; this is made
possible by using the list in ff_set_common_channel_layouts() directly
after its allocation (without anything that can fail in between).
Notice that ff_set_common_channel_layouts() is buggy itself which can
lead to double-frees on error. This is not fixed in this commit.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Replace using ff_add_format() repeatedly by a single call to
ff_make_format_list(). (Right now this also fixes a memleak: If the
first ff_add_format() succeeds and a subsequent call fails, the list
leaks.)
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The vpp_qsv's query_formats function allocated two AVFilterFormats,
before storing them permanently. If storing the first of them fails,
the function simply returns and the second leaks. This has been fixed by
only allocating the second AVFilterFormats structure after the first one
has been successfully stored.
Fixes Coverity issue #1422231.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The paletteuse's query_formats function allocated three AVFilterFormats
before storing them permanently. If allocating one of them failed, the
three AVFilterFormats structures would be freed with av_freep() which
does not free separately allocated subelements (namely the formats
array) which leak.
Furthermore, if storing one of the first two fails, the function simply
returns and the ones not yet stored leak.
These leaks have been fixed by only creating a new AVFilterFormats after
the last one has already been permanently stored. Furthermore, it is
enough to check whether the elements have been properly stored as
ff_formats_ref() by design returns AVERROR(ENOMEM) if it is provided a
NULL AVFilterFormats *.
Fixes Coverity issues #1270818 and #1270819.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Parsing labeled outputs involves a check for an already known match
(a labeled input with the same name) to pair them together. If yes,
it is attempted to create a link between the two filters; in this case
the AVFilterInOuts have fulfilled their purpose and are freed. Yet if
creating the link fails, these AVFilterInOuts have up until now not been
freed, although they had already been removed from their respective lists
(which means that they are not freed automatically). In other words:
They leak. This commit fixes this.
This fixes ticket #7084. Said ticket contains an example program to
reproduce a leak. It can also be reproduced with ffmpeg alone, e.g. with
the complex filters "[0]null[1],[2]anull[0]" or with "[0]abitscope[0]".
All of these three examples involve media type mismatches which make it
impossible to create the links. The bug could also be triggered by other
means, e.g. failure to allocate the necessary AVFilterLink.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
parse_filter() did not check the return value of av_get_token() for
success; in case name (the name of a filter) was NULL, one got a
segfault in av_strlcpy() (called from create_filter()).
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This happened in parse_link_name() if there was a '[' without matching
']'. While this is not undefined behaviour (pointer arithmetic one
beyond the end of an array works fine as long as there are no accesses),
it is potentially dangerous. It currently isn't (all callers of
parse_link_name() treat this as an error and don't access the string any
more), but making sure that this will never cause trouble in the future
seems nevertheless worthwhile.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
parse_inputs() uses a temporary linked list to parse the labeled inputs
of a filter; said linked list owns its elements (and their names). On
success, the list of unlabeled inputs is appened to the end of the list
of labeled inputs and the new list is returned; yet on failures, nothing
frees the already existing elements of the temporary linked list, leading
to a leak.
This can be triggered by e.g. using '-vf [v][' in the FFmpeg
command-line tool.
This leak seems to exist since 4e781c25b7b1955d1a9a0b0771c3ce1acb0957bd.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This commit removes ff_parse_sample_format(), ff_parse_time_base() and
ff_query_formats_all_layouts() from libavfilter/formats.c. All of these
functions were completely unused. ff_parse_time_base() has not been used
at all since it had been added in 3448404a707b6e236a2ffa7b0453b3300de41b7b;
the last caller of ff_parse_sample_format has been removed in commit
d1c49bcae9b7fd41df5c6804ac7f6a5c271a7c2e. And the one and only caller of
ff_query_formats_all_layouts() (the asyncts filter) has been removed in
commit a8fe8d6b4a35c95aa94fccde5f001041278d197c.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
ff_planar_sample_fmts_array is unused (and was unused since it was added
in 4d4098da009c8340997b8d1abedbf2062e4aa991) and therefore this commit
removes it; ff_packed_sample_fmts_array meanwhile is used only once (in
the amerge filter) and therefore it has been moved to this place.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
dnn_execute_layer_avg_pool() contains the following line:
assert(avgpool_params->padding_method = VALID);
This statement contains an assignment where obviously a comparison was
intended. Furthermore, *avgpool_params is const, so that the attempted
assignment leads to a compilation failure if asserts are enabled
(i.e. if DEBUG is defined which leads libavutil/internal.h to not define
NDEBUG). Moreover, the enumeration constant VALID actually has the value 0,
so that the assert would be triggered if a compiler compiles this with
asserts enabled. Finally, the statement uses assert() directly instead
of av_assert*().
All these errors have been fixed.
Thanks to ubitux for providing a FATE-box [1] where DEBUG is defined.
[1]: http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-ddebug
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Guo, Yejun <yejun.guo@intel.com>
In a function body, a redundant ; is just a null statement that does
nothing. Yet outside a function body, a superfluous ';' like one that
exists if one adds a ';' immediately after a function body's closing
brace is actually invalid C that compilers happen to accept. Yet when
compiled in -pedantic mode, both GCC as well as Clang emit warnings for
this like "ISO C does not allow extra ‘;’ outside of a function
[-Wpedantic]".
The scenario described above existed in vf_overlay.c as a result of
macro expansion. This commit fixes it.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The functions were forgotten in 03c8fe49ea3f2a2444607e541dff15a1ccd7f0c2;
removing them also means that the avassert.h and samplefmt.h headers are
no longer used any more, so they have been removed, too.
Moreover, video.h is unused since b077d8d9082d057d4c7abd9e0b1a98f9651cfaa8
and channel_layout.h is since fdd9663781e3ebc8ebed0704607abd174095a905.
Both headers have therefore been removed, too.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The callers of the ff_merge_*() functions fall into two categories with
quite different needs:
One caller is can_merge_formats() which only wants to test for mergeability
without it merging anything. In order to do so, it duplicates the lists
it intends to test and resets their owners so that they are not modified
by ff_merge_*(). It also means that it needs to receive the merged list
(and not only an int containing whether the lists are mergeable) to
properly free it.
The other callers want the lists to be actually merged. But given the
fact that ff_merge_*() automatically updates the owners of the lists,
they only want the information whether the merge succeeded or not; they
don't want a link to the new list.
Therefore this commit splits these functions in two: ff_merge_*() for
the latter callers and ff_can_merge_*() for the former.
ff_merge_*() doesn't need to return a pointer to the combined list at all
and hence these functions have been modified to return an int, which
allows to distinguish between incompability and memory allocation failures.
ff_can_merge_*() meanwhile doesn't modify its arguments at all obviating
the need for copies. This in turn implies that there is no reason to
return a pointer to the new list, as nothing needs to be freed. These
functions therefore return an int as well. This allowed to completely
remove can_merge_formats() in avfiltergraph.c.
Notice that no ff_can_merge_channel_layouts() has been created, because
there is currently no caller for this. It could be added if needed.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Right now, ff_merge_samplerates() contains three instances of the
MERGE_REF() macro, a macro which reallocates an array, updates some
pointers in a loop and frees several buffers. This commit makes it
possible to contain only one instance of said macro in the function,
thereby reducing codesize.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This is the analogue of cfc65520324ae640299bd321ef88ae76dcee6f78 for
formats and samplerates; in contrast to said commit, one can avoid
allocating a new array for formats as well (the complications of the
generic channel layouts made this impossible for channel layouts).
This commit also starts to move the line continuation '\' chars to the
left to keep them in line with MERGE_REF() as well as with the 80 lines
limit.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
and remove the redundant check.
This check for whether the allocated buffer is sufficient has been added
in commit 1cbf7fb4345a3e5b7791d483241bf4759bde4ece (merging commit
5775a1832c4165e6acc1d307004b38701bb463f4). It is not sufficient to
detect invalid input lists (namely lists with duplicates); its only use
is to avoid buffer overflows. And this can be achieved by simpler means:
Make sure that one allocates space for so many elements as the outer loop
ranges over and break out of the inner loop if a match has been found.
For valid input without duplicates, no further match will be found anyway.
This change will temporarily make the allocated formats array larger
than before and larger than necessary; this will be fixed in a later
commit that avoids the allocation altogether.
If a check for duplicates in the lists is deemed necessary, it should be
done properly somewhere else.
Finally, the error message that is removed in this commit used
__FUNCTION__, which is a GCC extension (C99 added __func__ for this).
So this commit removes a warning when compiling in -pedantic mode.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If an error happens between the allocation of an AVFilterChannelLayout
and its usage (which involves attaching said object to a more permanent
object), the channel layout array leaks. This can simply be fixed by
making sure that nothing is between the allocation and the
aforementioned usage.
Fixes Coverity issue #1250334.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When one merges two AVFilterChannelLayouts structs, there is no need to
allocate a new one. Instead one can reuse one of the two given ones.
If one does this, one also doesn't need to update the references of the
AVFilterChannelLayouts that is reused. Therefore this commit reuses the
structure with the higher refcount.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The channel layouts accepted by ff_merge_channel_layouts() are of two
types: Ordinary channel layouts and generic channel layouts. These are
layouts that match all layouts with a certain number of channels.
Therefore parsing these channel layouts is not done in one go; instead
first the intersection of the ordinary layouts of the first input
list of channel layouts with the ordinary layouts of the second list is
determined, then the intersection of the ordinary layouts of the first
one and the generic layouts of the second one etc. In order to mark the
ordinary channel layouts that have already been matched as used they are
zeroed. The inner loop that does this is as follows:
for (j = 0; j < b->nb_channel_layouts; j++) {
if (a->channel_layouts[i] == b->channel_layouts[j]) {
ret->channel_layouts[ret_nb++] = a->channel_layouts[i];
a->channel_layouts[i] = b->channel_layouts[j] = 0;
}
}
(Here ret->channel_layouts is the array containing the intersection of
the two input arrays.)
Yet the problem with this code is that after a match has been found, the
loop continues the search with the new value a->channel_layouts[i].
The intention of zeroing these elements was to make sure that elements
already paired at this stage are ignored later. And while they are indeed
ignored when pairing ordinary and generic channel layouts later, it has
the exact opposite effect when pairing ordinary channel layouts.
To see this consider the channel layouts A B C D E and E D C B A. In the
first round, A and A will be paired and added to ret->channel_layouts.
In the second round, the input arrays are 0 B C D E and E D C B 0.
At first B and B will be matched and zeroed, but after doing so matching
continues, but this time it will search for 0, which will match with the
last entry of the second array. ret->channel_layouts now contains A B 0.
In the third round, C 0 0 will be added to ret->channel_layouts etc.
This gives a quadratic amount of elements, yet the amount of elements
allocated for said array is only the sum of the sizes of a and b.
This issue can e.g. be reproduced by
ffmpeg -f lavfi -i anullsrc=cl=7.1 \
-af 'aformat=cl=mono|stereo|2.1|3.0|4.0,aformat=cl=4.0|3.0|2.1|stereo|mono' \
-f null -
The fix is easy: break out of the inner loop after having found a match.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This reverts commit f156f4ab2317f22bfef33c7eaead0d5d5f162903.
The checks added by said commit are nonsense because they did not help
in case ff_merge_samplerates() or ff_merge_formats() returned NULL
while freeing one of its arguments: Said freeing does not change
the local variables of can_merge_formats().
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Now that the output's refs-array is only allocated once, it is NULL in
any error case and therefore needn't be freed at all; Instead an
av_assert1() has been added to guarantee it to be NULL.
Furthermore, it is unnecessary to av_freep(&ptr) when ptr == NULL.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
ff_merge_formats(), ff_merge_samplerates() and ff_merge_channel_layouts()
share common semantics: If merging succeeds, a non-NULL pointer is
returned and both input lists (of type AVFilterFormats resp.
AVFilterChannelLayouts) are to be treated as if they had been freed;
the owners of the input parameters (if any) become owners of the
returned list. If merging does not succeed, NULL is returned and both
input lists are supposed to be unchanged.
The problem is that the functions did not abide by these semantics:
In case of reallocation failure, it is possible for these functions
to return NULL after having already freed one of the two input list.
This happens because sometimes the refs-array of the destined output
gets reallocated twice to its final size and if the second of these
reallocations fails, the first of the two inputs has already been freed
and its refs updated to point to the destined output which in this case
will be freed immediately so that all of the already updated pointers
are now dangling. This leads to use-after-frees and memory corruptions
lateron (when these owners get cleaned up, the lists they own get
unreferenced). Should the input lists don't have owners at all, the
caller (namely can_merge_formats() in avfiltergraph.c) thinks that both
the input lists are unchanged and need to be freed, leading to a double
free.
The solution to this is simple: Don't reallocate twice; do it just once.
This also saves a reallocation.
This commit fixes the issue behind Coverity issue #1452636. It might
also make Coverity realize that the issue has been fixed.
Reviewed-by: Nicolas George <george@nsup.org>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Despite its name, this function is not part of the public API, as
formats.h, the header containing its declaration, is a private header.
The formats API was once public API, but that changed long ago
(b74a1da49db5ebed51aceae6cacc2329288a92c1, the commit scheduling it to
become private, is from 2012). That avfilter_make_format64_list() was
forgotten is probably a result of the confusion resulting from the
libav-ffmpeg split.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
It is unused since 8cbb055760c725d0fb99fb759caabb5f4e37e340 and it
actually coincides with avfilter_make_format64_list().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
different backend might need different options for a better performance,
so, add the parameter into dnn interface, as a preparation.
Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
flush_put_bits() already fills the bitstream with zeroes, so it is
unnecessary to align the bitstream before.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
