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>
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.
This reverts commit e134c20374ee3cbc6d04885d306b02c9871683a2.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
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.
The classical ptr = av_realloc(ptr, size).
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Also return 0 after successfully reading a packet.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Also simply return 0 in case a packet has been successfully read.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Increasing it by 2048 entries per realloc is exessive.
Reduces memory usage, especially on long, non fragmented output.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: James Almer <jamrial@gmail.com>
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>
Instead just reuse the filename string that is given via an option
for the child muxer's url field.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
It is no longer needed given that the function pointers of the child
muxer's AVOutputFormat are no longer called directly.
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>
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>
In particular the flags are important so that AVFMT_FLAG_BITEXACT can be
honoured by the child muxer.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
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>
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>
Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
Reviewed-by: Tomas Härdin <tjoppen@acc.umu.se>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
and also add padding to it; moreover, don't use memcpy to write one byte
to extradata.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Do it only if the packet has been successfully allocated in
av_new_packet() -- otherwise on error a completely uninitialized packet
would be unreferenced later.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The classical ptr = av_realloc(ptr, size), just with av_fast_realloc().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
by putting the palette in the demuxer's context. This also allows to
remove this demuxer's read_close-function.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: signed integer overflow: 2 * 1210064928 cannot be represented in type 'int'
Fixes: 20873/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5761116909338624
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: memleaks
Fixes: 21084/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5655975492321280
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>
It doesn't do anything: All allocated blocks have already been freed in
write_header.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
There was no consensus wheter or not to allow unofficial frame rates due to
possible interoperability issues, a compromise is to only allow it if -strict
mode is set to unofficial.
Signed-off-by: Marton Balint <cus@passwd.hu>
