This code mostly duplicates code in the deinit function; the only
exception is av_opt_free(): The options are freed generically lateron.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This fixes leaks when the trailer is never written.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
seg_init() and seg_write_header() currently contain a few error paths
in which an already opened AVIOContext for the child muxer leaks (namely
if there are unrecognized options for the child muxer or if writing the
header of the child muxer fails); the reason for this is that this
AVIOContext is not closed in the deinit function. If all goes well, it
is closed when writing the trailer. From this it also follows that the
AVIOContext also leaks when the trailer is never written, even when
writing the header succeeds.
But simply freeing said AVIOContext in the deinit function is
complicated by the fact that the AVIOContext may or may not have been
opened via the io_open callback: If options are set to discard header
and trailer, said AVIOContext can also be a null context which must not
be closed via the io_close callback. This may lead to crashes, as
io_close may presume the AVIOContext's opaque to be set. It currently
works with the default io_close callback which simply calls avio_close(),
because avio_close() doesn't care about opaque being NULL since commit
6e8e8431e1. Therefore this commit records
which of the two kinds of AVIOContext is currently in use to use the
right way to close it.
Finally there was one instance (namely if initializing the child muxer
fails with no unrecognized options) where the AVIOContext was always
closed via the io_close callback. The above remark applies to this; it
has been fixed, too.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
A string containing the segment's filename that the segment muxer
allocates got only freed in its write_trailer function. This implies
that it leaks if write_trailer is never called, e.g. if initializing
the child muxer fails. This commit fixes this by freeing the string
in the deinit function instead.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The segment muxer has an option to output a file containing a list of
the segments written. The AVIOContext used for writing this file is
opened via the main AVFormatContext's io_open callback; seg_free()
meanwhile unconditionally closes this AVIOContext by calling
ff_format_io_close() with the child muxer (the one for the actual output
format) as AVFormatContext.
The problem hereby is that the child AVFormatContext need not exist,
even when the AVIOContext does. This leads to a segfault in
ff_format_io_close() when the child muxer's io_close callback is called.
Situations in which the AVFormatContext can be NULL range from an
invalid reference stream parameter to an unavailable/bogus/unsupported
output format to inability to allocate the AVFormatContext.
The solution is to simply close the AVIOContext with the AVFormatContext
that was used to open it: The main AVFormatContext.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If the user has set none of the options specifying the segments'
durations, a default value of 2s is used by duplicating a "2" string and
using av_parse_time() on it. Yet duplicating the string was unchecked
and if the allocation failed, one would get a segfault in
av_parse_time().
This commit solves this by turning said option into an option of type
AV_OPT_TYPE_DURATION (which also uses av_parse_time() internally),
avoiding duplicating the string altogether.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The code to free them is not in the segment muxer's deinit function,
but in its write_trailer function which means that these lists leak if
write_trailer isn't called after their allocation. This happens e.g. if
the given lists are invalid (e.g. consisting only of ',' (which delimit
entries)), so that parsing them fails and so does the muxer's init
function; write_trailer is then never called.
This has been fixed by moving the code to free them to the deinit
function.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The segment muxer copies the user-provided AVCodecParameters to the
newly created child streams in its init function before initializing the
child muxer; and since commit 8e6478b723,
it does this again before calling avformat_write_header() if that is
called from seg_write_header(). The reason for this is complicated:
At that time writing the header was delayed, i.e. it was not triggered
by avformat_write_header() (unless the AVFMT_FLAG_AUTO_BSF was unset),
but instead by writing the very first packet. The rationale behind this
was to allow to run bitstream filters on the packets in the interleavement
queue in order to generate missing extradata from them before the muxer's
write_header function is actually called.
The segment muxer went even further: It initialized the child muxer and
ran the child muxer's check_bitstream functions on the packets in its
own muxing queue and stole any bitstream filters that got inserted. The
reason for this is that the segment muxer has an option to write the
header to a separate file and for this it is needed to write the child
muxer's header without delay, but with correct extradata. Unsetting
AVFMT_FLAG_AUTO_BSF for the child muxer accomplished the first goal and
stealing the bitstream filters the second; and in order for the child
muxer to actually use the updated extradata, the old AVCodecParameters
(set before avformat_init_output()) were overwritten with the new ones.
Updating the extradata proceeded as follows: The bitstream filter itself
simply updated the AVBSFContext's par_out when processing a packet, in
violation of the new BSF API (where par_out may only be set in the init
function); the muxing code then simply forwarded the updated extradata,
overwriting the par_in of the next BSF in the BSF chain with the fresh
par_out of the last one and the AVStream's par with the par_out of the
last BSF. This was an API violation, too, of course, but it made
remuxing ADTS AAC into mp4/matroska work.
But this no longer serves a useful purpose since the aac_adtstoasc BSF
was updated to propagate new extradata via packet side data in commit
f63c3516577d605e51cf16358cbdfa0bc97565d8; the next commit then removed
the code in mux.c passing new extradata along the filter chain. This
alone justifies removing the code for setting the AVCodecParameters a
second time.
But there is even another reason to do so: It is harmful. The ogg muxer
parses the extradata of Theora and Vorbis in its init function and keeps
pointers to parts of it. Said pointers become dangling when the
extradata is overwritten by the segment muxer, leading to
use-after-frees as has happened in ticket #8881 which this commit fixes.
Ticket #8517 is about another issue caused by this: Immediately after
having overwritten the old AVCodecParameters the segment muxer checks
whether the codec_tag is ok (the codec_tag is set generically when
initializing the child muxer based upon muxer-specific lists). The check
used is: If the child output format has such a list and if the codec tag
of the non-child stream does not match the codec id given the list of
codec tags and if there is a match for the codec id in the codec tag
list, then set the codec tag to zero (and not to the existing match),
otherwise set the codec tag of the child stream to the codec tag
of the corresponding stream of the main AVFormatContext (which is btw
redundant given that the child AVCodecParameters have just been
overwritten with the AVCodecParameters of the corresponding stream of
the main AVFormatContext).
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Found via ASAN with the dnn-layer-conv2d FATE-test.
Reviewed-by: Guo, Yejun <yejun.guo@intel.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
buffer_length is a power-of-two and modulo is buffer_length - 1, so that
buffer_length & modulo is zero.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Also unify incrementing the variable containing the pointer
to the currently used HRIR data.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The headphone filter uses an array with as many elements as the
filter has inputs to store some per-input information; yet actually it
only stores information for all inputs except the very first one (which
is special for this filter). Therefore this commit modifies the code to
remove this unused element.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Despite the headphone filter only using one AVFrame at a time, it kept
an array each of whose entries contained a pointer to an AVFrame at all
times; the pointers were mostly NULL. This commit instead replaces them
by using a single pointer to an AVFrame on the stack of the only
function that actually uses them.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The headphone filter allocates a pair of buffers to be used as
intermediate buffers lateron: Before every use they are zeroed, then
some elements of the buffer are set and lateron the complete buffers are
copied into another, bigger buffer. These intermediate buffers are
unnecessary as the data can be directly written into the bigger buffer.
Furthermore, the whole buffer has been zeroed initially and because no
piece of this buffer is set twice (due to the fact that duplicate
channel map entries are skipped), it is unnecessary to rezero the part
of the big buffer that is about to be written to.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Before this commit, the headphone filter called
av_channel_layout_extract_channel() in a loop in order to find out
the index of a channel (given via its AV_CH_* value) in a channel layout.
This commit changes this to av_get_channel_layout_channel_index()
instead.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The documentation of the map argument of the headphone filter states:
"Set mapping of input streams for convolution. The argument is a
’|’-separated list of channel names in order as they are given as
additional stream inputs for filter."
Yet this has not been honoured at all. Instead for the kth given HRIR
channel pair it was checked whether there was a kth mapping and if the
channel position so given was present in the channel layout of the main
input; if so, then the given HRIR channel pair was matched to the kth
channel of the main input. It should actually have been matched to the
channel given by the kth mapping. A consequence of the current algorithm
is that if N additional HRIR channel pairs are given, a permutation of
the first N entries of the mapping does not affect the output at all.
The old code might even set arrays belonging to streams that don't exist
(i.e. whose index is >= the number of channels of the main input
stream); these parts were not read lateron at all. The new code doesn't
do this any longer and therefore the number of elements of some of the
allocated arrays has been reduced (in case the number of mappings was
bigger than the number of channels of the first input stream).
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When the headphone filter is configured to perform its processing in the
frequency domain, it allocates (among other things) two pairs of
buffers, all of the same size. One pair is used to store data in it
during the initialization of the filter; the other pair is only
allocated lateron. It is zero-initialized and yet its data is
immediately overwritten by the content of the other pair of buffers
mentioned above; the latter pair is then freed.
This commit eliminates the pair of intermediate buffers.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The headphone filter has two modes; in one of them (say A), it needs
certain buffers to store data. But it allocated them in both modes.
Furthermore when in mode A it also allocated intermediate buffers of the
same size, initialized them, copied their contents into the permanent
buffers and freed them.
This commit changes this: The permanent buffer is only allocated when
needed; the temporary buffer has been completely avoided.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
They seem to exist for an option that was never implemented.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The delay arrays were never properly initialized, only zero-initialized;
furthermore these arrays duplicate fields in the headphone_inputs
struct. So remove them.
(Btw: The allocations for them have not been checked.)
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The string given by an AVOption that contains the channel assignment
is used only once; ergo it doesn't matter that parsing the string via
av_strtok() is destructive. There is no need to make a copy.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When parsing the channel mapping string (a string containing '|'
delimited tokens each of which is supposed to contain a channel name
like "FR"), the old code would at each step read up to seven uppercase
characters from the input string and give this to
av_get_channel_layout() to parse. The returned layout is then checked
for being a layout with a single channel set by computing its logarithm.
Besides being overtly complicated this also has the drawback of relying
on the assumption that every channel name consists of at most seven
uppercase letters only; but said assumption is wrong: The abbreviation
of the second low frequency channel is LFE2. Furthermore it treats
garbage like "FRfoo" as valid channel.
This commit changes this by using av_get_channel_layout() directly;
furthermore, av_get_channel_layout_nb_channels() (which uses popcount)
is used to find out the number of channels instead of the custom code
to calculate the logarithm.
(As a consequence, certain other formats to specify the channel layouts
are now accepted (like the hex versions of av_get_channel_layout()); but
this is actually not bad at all.)
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The headphone filter has an option for the user to specify an assignment
of inputs to channels (or from pairs of channels of the second input to
channels). Up until now, these channels were stored in an int containing
the logarithm of the channel layout. Yet it is not the logarithm that is
used lateron and so a retransformation was necessary. Therefore this
commit simply stores the uint64_t as is, avoiding the retransformation.
This also has the advantage that unset channels (whose corresponding
entry is zero) can't be mistaken for valid channels any more; the old
code had to initialize the channels to -1 to solve this problem and had
to check for whether a channel is set before the retransformation
(because 1 << -1 is UB).
The only downside of this approach is that the size of the context
increases (by 256 bytes); but this is not exceedingly much.
Finally, the array has been moved to the end of the context; it is only
used a few times during the initialization process and moving it
decreased the offsets of lots of other entries, reducing codesize.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The headphone filter does most of its initialization after its init
function, because it can perform certain tasks only after all but its
first input streams have reached eof. When this happens, it allocates
certain buffers and errors out if an allocation fails.
Yet the filter didn't check whether some of these buffers already exist
(which may happen if an earlier attempt has been interrupted in the
middle (due to an allocation error)) in which case the old buffers leak.
This commit makes sure that initializing the buffers is only attempted
once; if not successfull at the first attempt, future calls to the
filter will error out. Trying to support resuming initialization doesn't
seem worthwhile.
Notice that some allocations were freed before a new allocation was
performed; yet this effort was incomplete. Said code has been removed.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The number of channels can be up to 64, not only 16.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The headphone filter stores the channel position of the ith HRIR stream
in the ith element of an array of 64 elements; but because there is no
check for duplicate channels, it is easy to write beyond the end of the
array by simply repeating channels.
This commit adds a check for duplicate channels to rule this out.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When the headphone filter does its processing in the time domain,
the lengths of the buffers involved are determined by three parameters,
only two of which are relevant here: ir_len and air_len. The former is
the length (in samples) of the longest HRIR input stream and the latter
is the smallest power-of-two bigger than ir_len.
Using optimized functions to calculate the convolution places
restrictions on the alignment of the length of the vectors whose scalar
product is calculated. Therefore said length, namely ir_len, is aligned
on 32; but the number of elements of the buffers used is given by air_len
and for ir_len < 16 a buffer overflow happens.
This commit fixes this by ensuring that air_len is always >= 32 if
processing happens in the time domain.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Not providing any samples makes no sense at all. And if no samples
were provided for one of the HRIR streams, one would either run into
an av_assert1 in ff_inlink_consume_samples() or into a segfault in
take_samples() in avfilter.c.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This buffer was supposed to be initialized by sscanf(input, "%7[A-Z]%n",
buf, &len), yet if the first input character is not in the A-Z range,
buf is not touched (in particular it needn't be zero-terminated if the
failure happened when parsing the first channel and it still contains
the last channel name if the failure happened when one channel name
could be successfully parsed). This is treated as error in which case
buf is used directly in the log message. This commit fixes this by
actually using the string that could not be matched in the log message
instead.
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Use pthread to multithread dnn_execute_layer_conv2d.
Can be tested with command "./ffmpeg_g -i input.png -vf \
format=yuvj420p,dnn_processing=dnn_backend=native:model= \
espcn.model:input=x:output=y:options=conv2d_threads=23 \
-y sr_native.jpg -benchmark"
before patch: utime=11.238s stime=0.005s rtime=11.248s
after patch: utime=20.817s stime=0.047s rtime=1.051s
on my 3900X 12c24t @4.2GHz
About the increase of utime, it's because that CPU HyperThreading
technology makes logical cores twice of physical cores while cpu's
counting performance improves less than double. And utime sums
all cpu's logical cores' runtime. As a result, using threads num
near cpu's logical core's number will double utime, while reduce
rtime less than half for HyperThreading CPUs.
Signed-off-by: Xu Jun <xujunzz@sjtu.edu.cn>
Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
This reverts commit abc884bcc0.
This patch was pushed without actual review.
An actual review would have revealed that the switch to activate
was not done correctly because the logic between request_frame()
and frame_wanted is not as direct with filters with multiple
outputs than with a single output.
Paul B Mahol
Zixing Liu
Limin Wang
Harry Mallon
Andreas Rheinhardt
Mark Reid
Nicolas George
Xu Jun
Gyan Doshi
Zhao Zhili