Mainly reindentation plus some reordering in MatroskaMuxContext;
moreover, use the IS_SEEKABLE() macro troughout the code.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
EBML numbers are variable length numbers: Only seven bits of every byte
are available to encode the number, the other bits encode the length of
the number itself. So an eight byte EBML number can only encode numbers
in the range 0..(2^56 - 1). And when using EBML numbers to encode the
length of an EBML element, the EBML number corresponding to 2^56 - 1 is
actually reserved to mean that the length of the corresponding element
is unknown.
And therefore put_ebml_length() asserted that the length it should
represent is < 2^56 - 1. Yet there was nothing that actually guaranteed
this to be true for the Segment (the main/root EBML element of a
Matroska file that encompasses nearly the whole file). This commit
changes this by checking in advance how big the length is and only
updating the number if it is representable at all; if not, the unknown
length element is not touched.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska muxer has a pair of functions designed to write master
elements whose exact length is not known in advance: start_ebml_master()
and end_ebml_master(). The first one of these would write the EBML ID of
the master element that is about to be started, reserve some bytes for
the length field and record the current position as well as how many
bytes were used for the length field. When writing the master's contents
is finished, end_ebml_master() gets the current position (at the end of
the master element), seeks to the length field using the recorded
position, writes the length field and seeks back to the end of the
master element so that one can continue writing other elements.
But if one wants to modify the content of the master element itself,
then the seek back is superfluous. This is the scenario that presents
itself when writing the trailer: One wants to update several elements
contained in the Segment master element (this is the main/root master
element of a Matroska file) that were already written when writing the
header. The current approach is to seek to the beginning of the file
to update the elements, then seek to the end, call end_ebml_master()
which immediately seeks to the beginning to write the length and seeks
back. The seek to the end (which has only been performed because
end_ebml_master() uses the initial position to determine the length
of the master element) and the seek back are of course superfluous.
This commit avoids these seeks by no longer using start/end_ebml_master()
to write the segment's length field. Instead, it is now written
manually. The new approach is: Seek to the beginning to write the length
field, then update the elements (in the order they appear in the file)
and seek back to the end.
This reduces the ordinary amount of seeks of the Matroska muxer to two
(ordinary excludes scenarios where one has big Chapters or Attachments
or where one writes the Cues at the front).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If the AVIOContext for output was unseekable when writing the header,
no space for Cues would be reserved even if the reserve_index_space
option was used (because it is reasonable to expect that one can't seek
back to the beginning to write the Cues anyway). But if the AVIOContext
was seekable when writing the trailer, it was presumed that space for
the Cues had been reserved when the reserve_index_space option indicated
so even when it was not. As a result, the beginning of the file would be
overwritten.
This commit fixes this: If the reserve_index_space option had been used
and no space has been reserved in advance because of unseekability when
writing the header, then no attempt to write Cues will be performed
when writing the trailer; after all, writing them at the front is
impossible and writing them at the end is probably undesired.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
We won't be able to seek back to write the actual duration anyway.
FATE-tests using the md5pipe command had to be updated due to this change.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska muxer behaves differently in several ways when it thinks
that it is in unseekable/livestreaming mode: It does not add Cue entries
because they won't be written anyway for a livestream and it writes some
elements only preliminarily (with the intention to overwrite them with
an updated version at the end) when non-livestreaming etc.
There are two ways to set the Matroska muxer into livestreaming mode:
Setting an option or by providing an unseekable AVIOContext. Yet the
actual checks were not consistent:
If the AVIOContext was unseekable and no AAC extradata was available
when writing the header, writing the header failed; but if the AVIOContext
was seekable, it didn't, because the muxer expected to get the extradata
via packet side-data. Here the livestreaming option has not been checked,
although one can't use the updated extradata in case it is a livestream.
If the reserve_index_space option was used, space for writing Cues would
be reserved when writing the header unless the AVIOContext was
unseekable. Yet Cues were only written if the livestreaming option was
not set and the AVIOContext was seekable (when writing the trailer), so
if the AVIOContext was seekable and the livestreaming option set, the
reserved space would never be used at all.
If the AVIOContext was unseekable and the livestreaming option was not
set, it would be attempted to update the main length field at the end.
After all, it might be possible that the file is so short that it fits
into the AVIOContext's buffer in which case the seek back would work.
Yet this is dangerous: It might be that we are not dealing with a
simple output file, but that our output gets split into chunks and that
each of these chunks is actually seekable. In this case some part of the
last chunk (namely the eight bytes that have the same offset as the
length field had in the header) will be overwritten with what the muxer
wrongly believes to be the filesize.
(The livestreaming option has been added to deal with this scenario,
yet its documentation ("Write files assuming it is a live stream.")
doesn't make this clear at all. At least the segment muxer does not
set the option for live and given that the chances of successfully
seeking when the output is actually unseekable are slim, it is best to
not attempt to update the length field in the unseekable case at all.)
All these inconsistencies were fixed by treating the output as seekable
if the livestreaming option is not set and if the AVIOContext is
seekable. A macro has been used to enforce consistency and improve code
readability.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If the Matroska muxer's AVIOContext was unseekable when writing the
header, but is seekable when writing the trailer, the code for writing
the trailer presumes that a dynamic buffer exists and tries to update
its content in order to overwrite data that has already been
preliminarily written when writing the header, yet said buffer doesn't
exist as it has been written finally and not preliminarily when writing
the header (because of the unseekability it was presumed that one won't
be able to update the data anyway).
This commit adds a check for this and also for a similar situation
involving updating extradata with new data from packet side-data.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The parsing process of the AVOpt-enabled string controlling the mapping
of input streams to variant streams is roughly as follows: Space and tab
separate variant stream group maps while the entries in each variant
stream group map are separated by ','.
The parsing process of each variant stream group proceeded as follows:
At first the number of occurences of "a:", "v:" and "s:" in each variant
stream group is calculated so that one can can allocate an array of
streams with this number of entries. Then the string is split along ','
and each substring is parsed. If such a substring starts with "a:", "s:"
or "v:" it is treated as stream specifier and (if there is a correct
number after ':') a stream of the variant stream is mapped to one of the
actual input streams.
Nothing actually guarantees that the number of streams allocated initially
equals the number of streams that are mapped to an actual input stream.
These numbers can differ if e.g. the name, the sgroup, agroup or ccgroup
of the variant stream contain "a:", "s:" or "v:".
The problem hereby is that the rest of the code presumes these numbers
to be equal and segfaults if it isn't (because the corresponding input
stream is NULL).
This commit fixes this by modifying the initial counting process to only
count occurences of "a:", "s:" or "v:" that are at the beginning or that
immediately follow a ','.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This avoids accessing an old, no longer valid buffer.
Fixes: out of array access
Fixes: crash_audio-2020
Found-by: le wu <shoulewoba@gmail.com>
Reviewed-by: Marton Balint <cus@passwd.hu>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Call it directly from write_packets_common() instead of indirectly
through prepare_input_packet().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This commit stops using pkt->stream_index as index in an AVFormatContext's
streams array before actually comparing the value with the count of
streams in said array. 96e5e6abb9 used
pkt->stream_index in prepare_input_packet() before checking and
6406351222 did likewise in
write_packets_common().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If you have a file with multiple Metadata Keys, the second time you parse
the keys, you will re-alloc c->meta_keys without freeing the old one.
This change will avoid parsing all the consecutive Metadata keys.
Reviewed-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
A temporary heap array currently stores pids from all streams. It is
used to make sure there are no duplicated pids. However, this array is
not needed because the pids from past streams are stored in the
MpegTSWriteStream structs.
Reviewed-by: Marton Balint <cus@passwd.hu>
Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
Until now, we would have only attempted to utilize already decrypted
data if it was enough to fill the size of buffer requested, that could
very well be up to 32 kilobytes.
With keep-alive connections this would just lead to recv blocking
until rw_timeout had been reached, as the connection would not be
officially closed after each transfer. This would also lead to a
loop, as such timed out I/O request would just be attempted again.
By just returning the available decrypted data, keep-alive based
connectivity such as HLS playback is fixed with schannel.
The dec_buf seems to be properly managed between read calls,
and we have no logic to decrypt before attempting socket I/O.
Thus - until now - such data would not be decrypted in case of
connections such as HTTP keep-alive, as the recv call would
always get executed first, block until rw_timeout, and then get
retried by retry_transfer_wrapper.
Thus - if data is received - decrypt all of it right away. This way
it is available for the following requests in case they can be
satisfied with it.
For every variantstream vs, vs->packets_written is set to one, only to be
set to zero a few lines below. Given that the relevant structure has
been zeroed during the allocation, this commit removes both assignments.
A redundant initialization for vs->init_range_length has been removed as
well a few lines below. Given that the relevant structure has been
zeroed during the allocation, this commit removes both assignments. A
redundant initialization for vs->init_range_length has been removed as
well.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
By the av_strtok() description:
* On the first call to av_strtok(), s should point to the string to
* parse, and the value of saveptr is ignored. In subsequent calls, s
* should be NULL, and saveptr should be unchanged since the previous
* call.
Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
The demuxer code assumes the existence of a video stream
Fixes: assertion failure
Fixes: 21512/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5699660783288320
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
When the Ogg muxer writes a page, it has to do three things: It needs to
write a page header, then it has to actually copy the page data and then
it has to calculate and write a CRC checksum of both header as well as
data at a certain position in the page header.
To do this, the muxer used a dynamic buffer for both writing as well as
calculating the checksum via an AVIOContext's feature to automatically
calculate checksums on the data it writes. This entails an allocation of
an AVIOContext, of the opaque specific to dynamic buffers and of the
buffer itself (which may be reallocated multiple times) as well as
memcopying the data (first into the AVIOContext's small write buffer,
then into the dynamic buffer's big buffer).
This commit changes this: The page header is no longer written into a
dynamic buffer any more; instead the (small) page header is written into
a small buffer on the stack. The CRC is then calculated directly via
av_crc() on both the page header as well as the page data. Then both the
page header and the page data are written.
Finally, ogg_write_page() can now no longer fail, so it has been
modified to return nothing; this also fixed a bug in the only caller of
this function: It didn't check the return value.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Mainly includes reindentation and returning directly (i.e. without
a goto fail when possible).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
hls_init() would at first allocate the vtt_basename string, then
allocate the vtt_m3u8_name string followed by several operations that
may fail and then open the subtitles' output context. Yet upon freeing,
these strings were only freed when the subtitles' output context
existed, ensuring that they leak if something goes wrong between their
allocation and the opening of the subtitles' output context. So drop the
check for whether this output context exists.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This fixes memleaks in instances such as:
a) When an allocation fails at one of the two places in hls_init() where
the error is returned immediately without goto fail first.
b) When an error happens when writing the header.
c) When an allocation fails at one of the three places in
hls_write_trailer() where the error is returned immediately without goto
fail first.
d) When one decides not to write the trailer at all (e.g. because of
errors when writing packets).
Furthermore, it removes code duplication and allows to return
immediately, without goto fail first.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Several variables which are only used when the HLS_SINGLE_FILE flag is
unset have been set even when this flag is set. This has been changed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska specification allows multiple (level 1) Tags elements per
file, yet our demuxer didn't: While it parsed any amount of Tags
elements it found in front of the Clusters (albeit with warnings because
of duplicate elements), it would treat any Tags element only referenced
via a SeekHead entry as already parsed if any Tags element has already
been parsed; therefore this Tags element would not be parsed at all.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
There can be more than one SeekHead in a Matroska file, but most of the
other level 1 elements can only occur once.* Therefore the Matroska
demuxer only allows one entry per ID in its internal list of level 1
elements known to it; the only exception to this are SeekHeads.
The only exception to this are SeekHeads: When one is encountered
(either directly or in the list of entries read from SeekHeads),
a new entry in the list of known level-1 elements is always added,
even when this entry is actually already known.
This leads to lots of seeks in case of circular SeekHeads: Each time a
SeekHead is parsed, a new entry for a SeekHead will be added to the list
of entries read from SeekHeads. The exception for SeekHeads mentioned
above now implies that this SeekHead will always appear new and unparsed
and parsing will be attempted. This continued until the list of known
level-1 elements is full.
Fixing this is pretty simple: Don't add a new entry for a SeekHead if
its position matches the position of an already known SeekHead.
*: Actually, there can be multiple Tags and several other level 1
elements are "identically recurring" which means they may be resent
multiple times, but each instance must be absolutely identical to the
previous.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
A Seek element in a Matroska SeekHead should contain a SeekID and a
SeekPosition element and upon reading, they should be sanitized:
Given that IDs are restricted to 32 bit, longer SeekIDs should be treated
as invalid. Instead currently the lower 32 bits have been used.
For SeekPosition, no checks were performed for the element to be
present and if present, whether it was excessively large (i.e. the
absolute file position described by it exceeding INT64_MAX). The
SeekPosition element had a default value of -1 which means that a check
seems to have been intended; but it was not implemented. This commit adds
a check for overflow to the calculation of the absolute file position of
the referenced level 1 elements.
Using -1 (i.e. UINT64_MAX) as default value for SeekPosition implies that
a Seek element without SeekPosition will run afoul of this check.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Generic retime functionality is replaced by a few lines of code directly in the
muxers which used it, which seems a lot easier to understand and this way the
retiming is not dependant of the input durations.
Also remove retimeinterleave, since it is not used by anything anymore.
Signed-off-by: Marton Balint <cus@passwd.hu>
And rename it to retimeinterleave, use the pcm_rechunk bitstream filter for
rechunking.
By seperating the two functions we hopefully get cleaner code.
Signed-off-by: Marton Balint <cus@passwd.hu>
Previously only 1:1 bitstream filters were supported, the end of the stream was
not signalled to the bitstream filters and time base changes were ignored.
This change also allows muxers to set up bitstream filters regardless of the
autobsf flag during write_header instead of during check_bitstream and those
bitstream filters will always be executed.
Signed-off-by: Marton Balint <cus@passwd.hu>
Andreas Rheinhardt
Michael Niedermayer
Derek Buitenhuis
Thierry Foucu
Dale Curtis
Andriy Gelman
Jan Ekström
Steven Liu
James Almer
Limin Wang
Marton Balint