avformat/matroskaenc: Simplify writing Void elements
Reserving space in Matroska works by writing a Void element. And until
now this worked as follows: The current position was recorded and the
EBML ID as well as the length field written; then the new position was
recorded to know how much more to write. Afterwards the actual writing
has been performed via ffio_fill().
But it is unnecessary to explicitly use the positions (obtained via
avio_tell()) to find out how much still needs to be written, because the
length of the ID and the length field are known. So rewrite the function
to no longer use them.
Also, given that ffio_fill() uses an int parameter and given that no
current caller (and no sane future caller) will want to reserve several
GB of space, make the size parameter of put_ebml_void() itself an int.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/matroskaenc: Avoid seek when writing Cues at the front
When the Cues are written in front of the Cluster, the muxer would seek
to the beginning (to where the Cues ought to be written) and write the
Cues; afterwards it would seek back to the end of the file only to seek
to the beginning once again to update several elements there. This
commit removes the seek to the end.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/matroskaenc: Fix edge case of writing Cues at the beginning
The Matroska muxer has the ability to write the Cues (the index) at the
beginning of the file (in front of the Cluster): The user inputs the
amount of space that should be reserved at the beginning of the file and
if this is sufficient, the Cues will be written there and the part of the
reserved space not used up by the Cues will be filled with a "Void"
element.
There is just one problem with this: One can not fill a single byte this
way, because said Void element is minimally two bytes long (one byte ID,
one byte length field). Up until now, if one reserved one byte more than
needed, one would run into an assert when writing the Void element.
There are two solutions for this: Error out if it happens. Or adjust the
length field of the Cues in order to ensure that the above situation
can't happen (i.e. write the length on one byte more than necessary).
The first solution is very unsatisfactory, as enough space has been
reserved. Therefore this commit implements the second solution.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/matroskaenc: Don't fail if reserved Cues space doesn't suffice
When the user opted to write the Cues at the beginning, the Cues were
simply written without checking in advance whether enough space has been
reserved for them. If it wasn't enough, the data following the space
reserved for the Cues was simply overwritten, corrupting the file.
This commit changes this by checking whether enough space has been
reserved for the Cues before outputting anything. If it isn't enough,
no Cues will be output at all and the file will be finalized normally,
yet writing the trailer will nevertheless return an error to notify
the user that his wish of having Cues at the front of the file hasn't
been fulfilled.
This change opens new usecases for this option: It is now safe to use
this option to e.g. record live streams or to use it when muxing the
output of an expensive encoding, because when the reserved space turns
out to be insufficient, one ends up with a file that just lacks Cues
but is otherwise fine.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/matroskaenc: Update the default version of WavPack
The Matroska muxer currently assumed WavPack version 4.03 in case it was
not explicitly signalled via extradata; but following a recommendation
from David Bryant, the WavPack creator, this is changed to 4.10.
Reviewed-by: David Bryant <david@wavpack.com> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
It might be used by the Matroska muxer. This is also the reason why the
FATE-tests for muxing WavPack into Matroska needed to be updated: They
now write the correct version 4.07 and not 4.03 as before.
Reviewed-by: David Bryant <david@wavpack.com> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/matroskadec: Add a workaround for missing WavPack extradata
mkvmerge versions 6.2 to 40.0 had a bug that made it not propagate the
WavPack extradata (containing the WavPack version) during remuxing from
a Matroska file; currently our demuxer would treat every WavPack block
encountered as invalid data (unless the WavPack stream is to be
discarded (i.e. the streams discard is >= AVDISCARD_ALL)) and try to
resync to the next level 1 element.
Luckily, the WavPack version is currently not really important; so we
fix this problem by assuming a version. David Bryant, the creator of
WavPack, recommended using version 0x410 (the most recent version) for
this. And this is what this commit does.
A FATE-test for this has been added.
Reviewed-by: David Bryant <david@wavpack.com> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
John Rummell [Mon, 30 Mar 2020 21:56:11 +0000 (14:56 -0700)]
libavformat/oggdec.c: Check return value from avio_read()
If the buffer doesn't contain enough bytes when reading a stream,
fail rather than continuing on with unitialized data. Caught by
Chromium fuzzers (crbug.com/1054229).
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
1. When set_parameters was removed from AVOutputFormat in 2fb75019, it
was forgotten to remove the comment pertaining to it. Said comment now
appeared to apply to interleave_packet(); it is of course nonsense and
has been replaced by an accurate description.
2. The description of av_write_uncoded_frame() suggested
av_interleaved_write_frame() as a replacement if the input is not
already correctly interleaved; it also referred to said function for
details. Given that said function can't write AVFrames and that the
specifics of writing uncoded frames are explained in the description
of av_interleaved_write_uncoded_frame(), both references have been fixed.
3. Removed an outdated comment about avformat_seek_file().
Reviewed-by: Marton Balint <cus@passwd.hu> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If ff_interleave_add_packet failed, the content of the newly created
packet new_pkt would leak.
Also, it is unnecessary to zero-initialize a packet that will be put
into av_new_packet lateron as the latter already initializes the packet.
Therefore this has been removed, too.
Reviewed-by: Marton Balint <cus@passwd.hu> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/mux: Only prepare input packet if there is a packet
It is unnecessary to call prepare_input_packet if there is no packet as
it doesn't do anything, except when the currently inactive code guarded
by !FF_API_COMPUTE_PKT_FIELDS2 || !FF_API_LAVF_AVCTX becomes active:
Then attempting to access pkt->stream_index will crash.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avutil/opt: Don't use NULL for %s string in a log message
If one calls av_opt_set() with an incorrect string to set the value of
an option of type AV_OPT_TYPE_VIDEO_RATE, the given string is used in a
log message via %s. This also happens when the string is actually a
nullpointer in which case using it for %s is forbidden.
This commit changes this by erroring out early in case of a nullpointer.
This also fixes a warning from GCC 9.2:
"‘%s’ directive argument is null [-Wformat-overflow=]"
Reviewed-by: Anton Khirnov <anton@khirnov.net> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Fixes: out of array read Fixes: 21286/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HCA_fuzzer-5683183715876864 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Reviewed-by: Paul B Mahol <onemda@gmail.com> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
avformat/matroskaenc: Remove unused function parameter
end_ebml_master_crc32_preliminary() has a MatroskaMuxContext as
parameter that isn't used at all. So remove it.
Furthermore it doesn't close its dynamic buffer; it just uses the
underlying buffer and therefore it only needs a pointer to the
dynamic buffer, not a pointer to a pointer.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/matroskaenc: Write level 1 elements in one go
Up until now, writing level 1 elements proceeded as follows: First, the
element id was written to the ordinary output AVIOContext and a dynamic
buffer was opened for the content of the level 1 element in
start_ebml_master_crc32(). Then this buffer was actually used and after it
was closed (in end_ebml_master_crc32()), the size field corresponding to
the buffer's size was written, after which the actual data was written.
This commit changes this: Nothing is written to the main AVIOContext any
more in start_ebml_master_crc32(). end_ebml_master_crc32() now writes
both the id, the length field as well as the data. This implies that
one can start a level 1 element in memory without outputting anything.
This is done to enable to test whether enough space has been reserved
for the Cues (if space has been reserved for them) before writing them.
A large duration between outputting the header and outputting the rest
could also break certain streaming usecases like the one from #8578
(which this commit fixes).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When the Matroska muxer writes the Cues (the index), it groups index
entries with the same timestamp into the same CuePoint to save space.
But given Matroska's variable-length length fields, it either needs
to have an upper bound of the final size of the CuePoint before writing it
or the CuePoint has to be assembled in a different buffer, so that after
having assembled the CuePoint (when the real size is known), the CuePoint's
header can be written and its data copied after it.
The first of these approaches is the currently used one. This entails
finding out the number of entries in a CuePoint before starting the
CuePoint and therefore means that the list is read at least twice.
Furthermore, a worst-case upper-bound for the length of a single entry
was used, so that sometimes bytes are wasted on length fields.
This commit switches to the second approach. This is no longer more
expensive than the current approach if one only resets the dynamic
buffer used to write the CuePoint's content instead of opening a new
buffer for every CuePoint: Writing the trailer of a file with 540.000
CuePoints improved actually from 219054414 decicycles to 2164379394
decicycles (based upon 50 iterations).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/aviobuf: Add function to reset dynamic buffer
Resetting a dynamic buffer means to keep the AVIOContext and the
internal buffer used by the dynamic buffer. This is done in order to
save (re)allocations when one has a workflow where one opens and closes
dynamic buffers in sequence.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, the Matroska muxer would allocate a structure containing
three members: The segment offset, a pointer to an array containing Cue
(index) entries and a counter for said array. It is unnecessary to
allocate it separately and it is unnecessary to contain the segment
offset in said structure, as it duplicates another field contained in
the MatroskaMuxContext. This commit implements the corresponding
changes.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When writing the SeekHead (a form of index) at the end of the muxing
process, mkv_write_seekhead() would first seek to the position where the
SeekHead ought to be written, then write it there and seek back to the
original position afterwards. Which means: To the end of the file.
Afterwards, a seek to the beginning of the file is performed to update
further values. This of course means that the second seek in
mkv_write_seekhead() was unnecessary.
This has been changed: A new parameter was added to mkv_write_seekhead()
containing the destination for the second seek, effectively eliminating
the seek to the end of the file after writing the SeekHead.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/matroskaenc: Check for failure when writing SeekHead
mkv_write_seekhead() would up until now try to seek to the position where
the SeekHead ought to be written, write the SeekHead and seek back. The
first of these seeks was checked as was writing, yet the seek back was
unchecked. Moreover the return value of mkv_write_seekhead() was unchecked
(the ordinary return value was the position where the SeekHead was written).
This commit changes this: Everything is checked. In the unseekable case
(where the first seek may nevertheless work when it happens in the buffer)
a failure at the first seek is not considered an error. In any case,
failure to seek back is an error.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/matroskaenc: Improve calculating EBML ID size
When the Matroska muxer writes an EBML ID, it calculates the length of
said ID before; and it does this as if this were a number that needs to
be encoded as EBML number: The formula used is (av_log2(id + 1) - 1) / 7
+ 1. But the constants used already contain the VINT_MARKER (the leading
bit indicating the length of the EBML number) and therefore the algorithm
used makes no sense. Instead the position of the most significant byte
set gives the desired length.
The algorithm used until now worked because EBML numbers are subject to
restrictions: If the EBML number takes up k bytes, then the bit 1 << (7
* k) is set and av_log2(id) is 7 * k. So the current algorithm produces
the correct result unless the EBML ID is of the form 7 * k - 1 because
of the "id + 1". But contrary to encoding lengths as EBML number (where
the + 1 exists to avoid the encodings reserved for unknown length),
such EBML numbers are simply forbidden as EBML IDs and as such none of
them were ever written.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This commit updates the documentation of av_read_frame() to match its
actual behaviour in several ways:
1. On success, av_read_frame() always returns refcounted packets.
2. It can handle uninitialized packets.
3. On error, it always returns blank packets.
This will allow callers to not initialize or unref unnecessarily.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/dashdec: Don't allocate and leak strings that are never used
Since commit e134c203 strdups of several elements of a manifest are kept
in the DASHContext; but said commit completely forgot to free these
strings again (with xmlFree()). Given that these strings are never used
at all, this commit closes this leak by reverting said commit.
avcodec/avcodec, avpacket: Return blank packet on av_packet_ref() failure
Up until now, it was completely unspecified what the content of the
destination packet dst was on error. Depending upon where the error
happened calling av_packet_unref() on dst might be dangerous.
This commit changes this by making sure that dst is blank on error, so
unreferencing it again is safe (and still pointless). This behaviour is
documented.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avcodec/avpacket: Always treat dst in av_packet_ref as uninitialized
av_packet_ref() mostly treated the destination packet dst as uninitialized,
i.e. the destination fields were simply overwritten. But if the source
packet was not reference-counted, dst->buf was treated as if it pointed
to an already allocated buffer (if != NULL) to be reallocated to the
desired size.
The documentation did not explicitly state whether the dst will be treated
as uninitialized, but it stated that if the source packet is not refcounted,
a new buffer in dst will be allocated. This and the fact that the side-data
as well as the codepath taken in case src is refcounted always treated the
packet as uninitialized means that dst should always be treated as
uninitialized for the sake of consistency. And this behaviour has been
explicitly documented.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Lynne [Mon, 23 Mar 2020 22:03:24 +0000 (22:03 +0000)]
movenc: mark Opus encapsulation as stable
The specifications are de-facto frozen now as they've already been used in
production for years, the author has indicated reluctance on IRC to change
it further, and the only potential changes would, from what I understand,
be forward-compatible.
Linjie Fu [Mon, 9 Mar 2020 14:54:59 +0000 (22:54 +0800)]
checkasm/hevc_add_res: prepare test data only if the fuction is not tested
check_func will return NULL for functions that have already been tested. If
the func is tested and skipped (which happens several times), there is no
need to prepare data(randomize_buffers and memcpy).
Move relative code in compare_add_res(), prepare data and do check only if
the function is not tested.
Signed-off-by: Linjie Fu <linjie.fu@intel.com> Signed-off-by: Anton Khirnov <anton@khirnov.net>
Linjie Fu [Fri, 27 Dec 2019 08:47:35 +0000 (16:47 +0800)]
lavc/pthread_frame: Update user context in ff_frame_thread_free
Resolution/format changes lead to re-initialization of hardware
accelerations(vaapi/dxva2/..) with new hwaccel_priv_data in
the worker-thread. But hwaccel_priv_data in user context won't
be updated until the resolution changing frame is output.
A termination with "-vframes" just after the reinit will lead to:
1. memory leak in worker-thread.
2. double free in user-thread.
Update user context in ff_frame_thread_free with the last thread
submit_packet() was called on.
avformat/matroskaenc: Avoid allocations for SeekHead
Up until e7ddafd5, the Matroska muxer wrote two SeekHeads: One at the
beginning referencing the main level 1 elements (i.e. not the Clusters)
and one at the end, referencing the Clusters. This second SeekHead was
useless and has therefore been removed. Yet the SeekHead-related
functions and structures are still geared towards this usecase: They
are built around an allocated array of variable size that gets
reallocated every time an element is added to it although the maximum
number of Seek entries is a small compile-time constant, so that one should
rather include the array in the SeekHead structure itself; and said
structure should be contained in the MatroskaMuxContext instead of being
allocated separately.
The earlier code reserved space for a SeekHead with 10 entries, although
we currently write at most 6. Reducing said number implied that every
Matroska/Webm file will be 84 bytes smaller and required to adapt
several FATE tests; furthermore, the reserved amount overestimated the
amount needed for for the SeekHead's length field and how many bytes
need to be reserved to write a EBML Void element, bringing the total
reduction to 89 bytes.
This also fixes a potential segfault: If !mkv->is_live and if the
AVIOContext is initially unseekable when writing the header, the
SeekHead is already written when writing the header and this used to
free the SeekHead-related structures that have been allocated. But if
the AVIOContext happens to be seekable when writing the trailer, it will
be attempted to write the SeekHead again which will lead to segfaults
because the corresponding structures have already been freed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This fixes memleaks if an error happens after one of the allocations
in init; or if the trailer isn't written (e.g. because there was an
error when writing a packet).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
chunk_start_index (which was set via an option) was only used to
initialize chunk_index and otherwise unused. So initialize chunk_index
directly via the option and remove chunk_start_index.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The webm_chunk muxer caches its output to a dynamic buffer and when it
outputs anything, it explicitly flushes it. So set the flags indicating
that flushing after each packet should not be done automatically
(basically avoiding avio_write_marker() to be called by flush_if_needed()
in libavformat/mux.c).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/webm_chunk: Use API functions for child muxer
instead of calling the write_header/packet/trailer functions directly
via the function pointers. Also, use distinct AVStreams for the child
AVFormatContext (up until now the two AVFormatContexts shared their
AVStreams because allocating their own was deemed too onerous).
Using the function pointers directly meant that the Matroska muxer's
init-function was never called, because init-functions were only
introduced a few months after webm_chunk has been added and no one
thought of/bothered to adapt webm_chunk for this (when the init-function
was added in b287d7ea, the code setting the timebase was moved to it,
so that the timebases were no longer set to ms-precision when using
the webm_chunk muxer; this has been fixed after some time in 42a635dd
by setting the timebases direcly (instead of calling the init-function)).
And when 982a98a0 added a deinit-function for the Matroska muxer, it
introduced memleaks in webm_chunk, because the child muxer's internal
structures were no longer freed when calling write_trailer directly.
(Given that the init function has never ever been called, the child
muxer has never ever been properly initialized, so that the
deinit-function was not called when freeing the child context.)
This commit stops calling the function pointers directly and instead
uses the standard API functions for muxers. This fixes the above
mentioned memleaks. (Memleaks are still possible on error. This will be
fixed in a future commit that adds a deinit-function to webm_chunk
itself.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avformat/webmdashenc: Don't use custom option for bitexactness
The WebM DASH Manifest muxer can write manifests for live streams and
these contain an entry that depends on the time the manifest is written;
an AVOption to make the output reproducible has been added for tests.
But this is unnecessary, as there already is a method for reproducible
output: The AVFMT_FLAG_BITEXACT-flag of the AVFormatContext. Therefore
this commit removes the custom option.
Given that the description of said option contained "private option -
users should never set this" and that it was not documented in
muxers.texi, no deprecation period for this option seemed necessary.
The commands of the FATE-tests for this muxer have been changed to no
longer use this option.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avcodec/hevc, h2645_parse: Fix HEVC NAL unit names and constants
This commit fixes the names and constants of the reserved NAL units
with nal_unit_type 22 resp. 23. They were "IRAP_IRAP_VLC2x", but are
actually "RSV_IRAP_VLC2x".
This also required a change to cbs_h265_syntax_template.c.
Reviewed-by: Anton Khirnov <anton@khirnov.net> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If an URI indicated that the data protocol was in use, it would be
copied into a temporary buffer via strncpy(dst, src, strlen(src)),
thereby ensuring that the trailing \0 would not be copied, despite dst
being uninitialized. dst would then be av_strdup'ed, leading to
potential segfaults.
The solution to this is simple: Don't copy the URI in the temporary
buffer at all, instead av_strdup it directly.
This fixes a -Wstringop-truncation warning emitted by GCC 9.2.
Reviewed-by: Steven Liu <lq@chinaffmpeg.org> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
projects / ffmpeg / log