Since commit 4aa0665f39, the dynamic
buffer destined for the contents of the current Cluster is no longer
constantly allocated, reallocated and then freed after writing the
content; instead it is reset and reused when closing a Cluster.
Yet the code in mkv_write_trailer() still checked for whether a Cluster
is open by checking whether the pointer to the dynamic buffer is NULL or
not (instead of checking whether the position of the current Cluster is
-1 or not). If a Cluster was not open, an empty Cluster would be output.
One usually does not run into this issue, because unless there are
errors, there are only three possibilities to not have an opened Cluster
at the end of writing a packet:
The first is if one sent an audio packet to the muxer. It might trigger
closing and outputting the old Cluster, but because the muxer caches
audio packets internally, it would not be output immediately and
therefore no new Cluster would be opened.
The second is an audio packet that does not contain data (such packets
are sometimes sent for side-data only, e.g. by the FLAC encoder). The
only difference to the first scenario is that such packets are not
cached.
The third is if one explicitly flushes the muxer by sending a NULL
packet via av_write_frame().
If one also allows for errors, then there is also another possibility:
Caching the audio packet may fail in the first scenario.
If one calls av_write_trailer() after the first scenario, the cached
audio packet will be output when writing the trailer, for which
a Cluster is opened and everything is fine; because flushing the muxer
does currently not output the cached audio packet (if one is cached),
the issue also does not exist if an audio packet has been cached before
flushing. The issue only exists in one of the other scenarios.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reindentation, removal of { } if they contain only one statement
and moving the return statement to a line of its own in situations
like "if (ret < 0) return ret;". Moreover, several overlong lines
were made shorter and a camelCase variable received a name in line
with our naming conventions.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, the Matroska muxer would mark a track as default if it had
the disposition AV_DISPOSITION_DEFAULT or if there was no track with
AV_DISPOSITION_DEFAULT set; in the latter case even more than one track
of a kind (audio, video, subtitles) was marked as default which is not
sensible.
This commit changes the logic used to mark tracks as default. There are
now three modes for this:
a) In the "infer" mode the first track of every type (audio, video,
subtitles) with default disposition set will be marked as default; if
there is no such track (for a given type), then the first track of this
type (if existing) will be marked as default. This behaviour is inspired
by mkvmerge. It ensures that the default flags will be set in a sensible
way even if the input comes from containers that lack the concept of
default flags. This mode is the default mode.
b) The "infer_no_subs" mode is similar to the "infer" mode; the
difference is that if no subtitle track with default disposition exists,
no subtitle track will be marked as default at all.
c) The "passthrough" mode: Here the track will be marked as default if
and only the corresponding input stream had disposition default.
This fixes ticket #8173 (the passthrough mode is ideal for this) as
well as ticket #8416 (the "infer_no_subs" mode leads to the desired
output).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
At the end of encoding, the FLAC encoder sends a packet whose side data
contains updated extradata (e.g. a correct md5 checksum). The Matroska
muxer uses this to update the CodecPrivate.
In doing so, the stream's codecpar was copied. But given that writing
a FLAC CodecPrivate does not modify the used AVCodecParameters at all,
there is no need to do so and this commit changes this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Several EBML Master elements for which a good upper bound of the final
length was available were nevertheless written without giving an
upper bound of the final length to start_ebml_master(), so that their
length fields were eight bytes long. This has been changed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska muxer does not write every stream as a Matroska track;
some streams are written as AttachedFile. But should no stream be
written as a Matroska track, the Matroska muxer would nevertheless
write a Tracks element without a TrackEntry. This is against the spec.
This commit changes this and only writes a Tracks if there is a Matroska
track.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
As WebM doesn't support Attachments, the Matroska muxer drops them when
in WebM mode. This happened silently until this commit which adds a
warning for this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
In order to determine whether the current Cluster needs to be closed
because of the limits on clustersize and clustertime,
mkv_write_packet() would first get the size of the current Cluster by
applying avio_tell() on the dynamic buffer holding the current Cluster.
It did this without checking whether there is a dynamic buffer for
writing Clusters open right now.
In this case (which happens when writing the first packet)
avio_tell() returned AVERROR(EINVAL); yet it is not good to rely on
avio_tell() (or actually, avio_seek()) to handle the situation
gracefully.
Fixing this is easy: Only check whether a Cluster needs to be closed
if a Cluster is in fact open.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When creating DASH streams, the TrackNumber is externally prescribed
and not derived from the number of streams in the AVFormatContext, so
if the number of tracks for a file using an explicit TrackNumber was
more than one, the resulting file would be broken (it would be impossible
to tell to which track a Block belongs if different tracks share the
same TrackNumber). So disallow this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska muxer currently only adds CuePoints in three cases:
a) For video keyframes. b) For the first audio frame in a new Cluster if
in DASH-mode. c) For subtitles. This means that ordinary Matroska audio
files won't have any Cues which impedes seeking.
This commit changes this. For every track in a file without video track
it is checked and tracked whether a Cue entry has already been added
for said track for the current Cluster. This is used to add a Cue entry
for each first packet of each track in each Cluster.
Implements #3149.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska file format has practically no limit on the number of
tracks (the current limit is 2^56 - 1); yet because they are encoded in
a variable length format in (Simple)Blocks this muxer has simply imposed
a limit on the number of tracks in order to ensure that they can always
be written on one byte in order to simplify the muxing process.
This commit removes said limit.
Also, zero is an invalid TrackNumber, so disallow this value in the
dash_track_number option.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This commit factors the ability to write ordinary EBML numbers out of
the functions for writing EBML lengths. This is in preparation for
future commits.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
EBML uses variable length integers both for the EBML IDs as well as for
the EBML lengths; Matroska also uses them for the TrackNumber in
(Simple)Blocks and for the lengths of laces when EBML lacing is used.
When encoding EBML lengths, certain encodings have a special meaning,
namely that the element has an unknown length. This is not so when
encoding general EBML variable length integers.
Yet the functions called ebml_num_size() and put_ebml_num() had this
special meaning hardcoded, i.e. they are there to write EBML lengths and
not general EBML numbers. So rename them.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Matroska (or actually EBML) uses variable-length numbers where only
seven bits of every byte is usable for the length; the other bits encode
the length of the variable-length number. So in order to find out how
many bytes one needs to encode a given number one can use a loop like
while (num >> 7 * bytes) bytes++; the Matroska muxer effectively did this.
Yet it has a disadvantage: It is impossible for the result of a single
right shift of an unsigned number with most significant bit set to be
zero, because one can only shift by 0..(width - 1). On some
architectures like x64 it is not even possible to do it with undefined
right shifts in which case this leads to an infinite loop.
This can be easily avoided by switching to a loop whose condition is
(num >>= 7). The maximum value the so modified function can return
is 10; any value > 8 is invalid and will now lead to an assert in
put_ebml_num() or in start_ebml_master() (or actually in
put_ebml_size_unknown()).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Use the mime_types of the corresponding AVCodecDescriptor instead of
tables specific to Matroska. The former are generally more encompassing:
They contain every item of the current lists except "text/plain" for
AV_CODEC_ID_TEXT and "binary" for AV_CODEC_ID_BIN_DATA.
The former has been preserved by special-casing it while the latter is
a hack added in c9212abf so that the demuxer (which uses the same tables)
sets the appropriate CodecID for broken files ("binary" is not a correct
mime type at all); using it for the muxer was a mistake. The correct
mime type for AV_CODEC_ID_BIN_DATA is "application/octet-stream" and
this is what one gets from the AVCodecDescriptor.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This has happened when writing chapters: Both editions as well as
chapters are by default not hidden and given that we don't support
writing hidden chapters at all, we don't need to write said elements at
all. The same goes for ChapterFlagEnabled.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, mkv_write_track() received the index of the stream whose
header data it is about to write as parameter; this index has until
recently been explicitly used to generate both TrackNumber and TrackUID.
But this is no longer so and as there is no reason why the function
for writing a single TrackEntry should even know the index of the
TrackEntry it is about to write, said index is replaced in the list of
function parameters by the corresponding AVStream and mkv_track.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
mkv_cuepoint (the structure used to store the index entries in the
Matroska muxer) currently contains fields for both the index of the
packet's stream in the AVFormatContext.streams array and for the
Matroska TrackNumber; correspondingly, mkv_add_cuepoint() has parameters
for both. But these two numbers can't be chosen independently, so get
rid of the TrackNumber.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Attachments are streams in FFmpeg, but they are not tracks in Matroska.
Yet they were counted when checking a limit for the number of tracks that
the Matroska muxer imposes. This is unnecessary and has been changed.
Also use unsigned variables for the variables denoting TrackNumbers as
negative TrackNumbers are impossible.
(The Matroska file format actually has practically no limit on the
number of tracks and this is purely what our muxer supports. But even if
this limit were removed/relaxed in the future, it still makes sense to
use small TrackNumbers as this patch does, because greater numbers need
more bytes to encode.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Using random values for TrackUID and FileUID (as happens when the
AVFMT_FLAG_BITEXACT flag is not set) has the obvious downside of making
the output indeterministic. This commit mitigates this by writing the
potentially random values with a fixed size of eight byte, even if their
actual values would fit into less than eight bytes. This ensures that
even in non-bitexact mode, the differences between two files generated
with the same settings are restricted to a few bytes in the header.
(Namely the SegmentUID, the TrackUIDs (in Tracks as well as when
referencing them via TagTrackUID), the FileUIDs (in Attachments as
well as in TagAttachmentUID) as well as the CRC-32 checksums of the
Info, Tracks, Attachments and Tags level-1-elements.) Without this
patch, there might be an offset/a size difference between two such
files.
The FATE-tests had to be updated because the fixed-sized UIDs are also
used in bitexact mode.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If there are Attachments to write, the Matroska muxer currently
allocates two objects: An array that contains an entry for each
AttachedFile containing just the stream index of the corresponding
stream and the FileUID used for this AttachedFile; and a structure with
a pointer to said array and a counter for said array. These uids are
generated via code special to Attachments: It uses an AVLFG in the
normal and a sha of the attachment data in the bitexact case. (Said sha
requires an allocation, too.)
But now that an uid is generated for each stream in mkv_init(), there is
no need any more to use special code for generating the FileUIDs of
AttachedFiles: One can simply use the uid already generated for the
corresponding stream. And this makes the whole allocations of the
structures for AttachedFiles as well as the structures itself superfluous.
They have been removed.
In case AVFMT_FLAG_BITEXACT is set, the uids will be different from the
old ones which is the reason why the FATE-test lavf-mkv_attachment
needed to be updated. The old method had the drawback that two
AttachedFiles with the same data would have the same FileUID.
The new one doesn't.
Also notice that the dynamic buffer used to write the Attachments leaks
if an error happens when writing the buffer. By removing the
allocations potential sources of errors have been removed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This commit reuses the random seed generated in mkv_init() (to determine
the TrackUIDs) for the SegmentUID in order to avoid a potentially
expensive call to av_get_random_seed().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, the TrackUID of a Matroska track which is supposed to be
random was not random at all: It always coincided with the TrackNumber
which is usually the 1-based index of the corresponding stream in the
array of AVStreams. This has been changed: It is now set via an AVLFG
if AVFMT_FLAG_BITEXACT is not set. Otherwise it is set like it is set
now (the only change happens if an explicit track number has been
chosen via dash_track_number, because the system used in the normal
situation is now used, too). In particular, no FATE tests need to be
updated.
This also fixes a bug in case the dash_track_number option was used:
In this case the TrackUID was set to the provided number, but the tags
were written with a TagTrackUID simply based upon the index, so that
the tags didn't apply to the track they ought to apply to.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Tags in the Matroska file format can be summarized as follows: There is
a level 1-element called Tags containing one or many Tag elements each
of which in turn contain a Targets element and one or many SimpleTags.
Each SimpleTag roughly corresponds to a single key-value pair similar to
an AVDictionaryEntry. The Targets meanwhile contains information to what
the metadata contained in the SimpleTags contained in the containing Tag
applies (i.e. to the file as a whole or to an individual track).
The Matroska muxer writes such metadata. It puts the metadata of every
stream into a Tag whose Targets makes it point to the corresponding
track. And if the output is seekable, then it also adds another Tag for
each track whose Targets corresponds to the track and where it reserves
space in a SimpleTag to write the duration at the end of the muxing
process into.
Yet there is no reason to write two Tag elements for a track and a few
bytes (typically 24 bytes per track) can be saved by adding the duration
SimpleTag to the other Tag of the same track (if it exists).
FATE has been updated because the output files changed. (Tests that
write to unseekable output (pipes) needn't be updated (no duration tag
has ever been written for them) and the same applies to tests without
further metadata.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
AVChapters have an int as id field and therefore this value can appear
<= 0. When remuxing from Matroska, this value actually contains
the lower 32 bits of the original ChapterUID (which can be 64 bits).
In order to ensure that the ChapterUID is always > 0, they were offset
as follows (since 07704c61): First max(0, 1LL - chapter[i].id) was computed
and stored in an uint32_t. And then the IDs were offset using this value.
This has two downsides:
1. It does not ensure that the UID is actually != 0: Namely if there is
a chapter with id == INT_MIN, then the offset will be 2^31 + 1 and a
chapter with id == INT_MAX will become 2^31 - 1 + 2^31 + 1 = 2^32 = 0,
because the actual calculation was performed in 32 bits.
2. As soon as a chapter id appears to be negative, a nontrivial offset
is used, so that not even a ChapterUID that only uses 32 bits is
preserved.
So change this by treating the id as an unsigned value internally and
only offset (by 1) if an id vanishes. The actual offsetting then has to
be performed in 64 bits in order to make sure that no UINT32_MAX wraps
around.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
subtitles.h has been included in order to use ff_subtitles_next_line()
to help parsing srt subtitles which at that time had their timing as
part of the payload and not as part of the AVPacket fields. When this
changed (in 55180b32) it has been forgotten to remove this header.
libavcodec/internal.h meanwhile has been added in bb47aa5850 and has
never been used at all.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
8ffcc826 added support for muxing BlockAdditions with BlockAddID equal
to one. The restriction to BlockAddID == 1 probably resulted from
a limitation to what was needed; yet over time this led to three
occurences of "(side_data_size && additional_id == 1)". This commit
changes this by setting side_data_size to 0 if additional_id != 1.
It also stops hardcoding 1 for the value of BlockAddID to write;
but it still upholds the requirement that it is 1. See below.
Despite BlockAddId actually having a default value of 1, it is still
written, because until very recently (namely dbc50f8a) our demuxer
used a wrong default value of 0.
Furthermore, use put_ebml_binary() to write the BlockAdditional element.
(The Matroska specifications have evolved and now the BlockAddID 1 is
reserved for the codec (as described in the codec's codec mapping),
BlockMore elements with BlockAddID > 1 are now of a more
codec-independent nature and require a BlockAdditionalMapping in the
track's TrackEntry. Given that this muxer does not support writing said
BlockAdditionalMapping yet (actually, none have been defined yet), we
have to uphold the requirement that BlockAddID == 1.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When updating the Tags at the end, the Matroska muxer would twice check
for whether (!mkv->is_live) is true, despite this code being only executed
if it is. Furthermore, a loop iterates over all the streams even when
there is no Tags element to update at all, because the check for whether
there are Tags is only performed later. This commit fixes this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avio_close_dyn_buf() has a bug: When the write pointer does not point to
the end of the written data when calling it (i.e. when one has performed
a seek back to update already written data), it would not add padding to
the end of the buffer, but to the current position, overwriting other
data; furthermore the reported size would be wrong (off by the amount of
data it has overwritten with padding).
In order not to run into this when updating already written elements or
elements for which size has only been reserved, the Matroska muxer would
first record the current position of the dynamic buffer, then seek to
the desired position, perform the update and seek back to the earlier
position.
But now that end_ebml_master_crc32() does not make use of
avio_close_dyn_buf() any more, this is no longer necessary.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska muxer uses a dynamic buffer to buffer the content of
Clusters before eventually writing them. Up until now, each time a
Cluster was written, the dynamic buffer was closed, i.e. freed; now it
is only reset, saving allocations of the AVIOContext itself, its opaque
as well as most of the reallocations of the buffer.
This is advantageous performance-wise, in particular on systems where
reallocations are slow (namely Windows). The following table shows the
decicyles for writing a frame on Linux (Ubuntu 19.10) and Windows (7)
on an x64 Haswell (to /dev/null on Linux, to stdout which is discarded
on Windows (the default values of the size and duration of clusters for
seekable output have been explicitly set in this case); in all tests,
writing CRC-32 values has been disabled in all tests; calls to the muxer's
write_packet function in write_packet() in libavformat/mux.c have been
timed; each of the following tests has been repeated 50 times):
| Windows before | Windows after | Linux before | Linux after
_________________________________________________________________
A | 979437 | 192304 | 259500 | 183320
B | 715936 | 155648 | 152786 | 130879
C | 265115 | 56034 | 78496 | 53243
D | 386224 | 80307 | 128894 | 75354
E | 21732 | 10695 | 11320 | 9801
(A is a 10.2 mb/s file with a GOP length of 2s, amounting to an average
Cluster size of about 2.5 MiB; the average Cluster size of B is 1.1 MiB;
for C it is 2.35 MiB, for D it is 0.46 MiB; for E - a file with just a
single audio track of 158kb/s resulting in a Cluster size of about 100
kB, the relative gains were the smallest, probably because of the small
Cluster size.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Writing the language of WebVTT in WebM proceeded differently than the
language of all other tracks: In case no language was given, it does not
write anything instead of "und" (for undefined). Because the default
value of the Language element in WebM (that inherited it from Matroska)
is "eng" (for English), any such track will actually be flagged as
English.
Doing it this way goes back to commit 509642b4 (the commit adding
support for WebVTT) and no reason for this has been given in the commit
message or in the discussion about this patch on the mailing list; the
best I can think of is this: the WebM wiki contains "The srclang attribute
is stored as the Language sub-element." Someone unfamiliar with default
values in Matroska/WebM could interpret this as meaning that no Language
element should be written if the language is unknown. And this is wrong
and this commit changes it.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
mkv_write_track() currently has three places where it checks for whether
the current codec type is audio: One in a switch and two outside of it.
These checks can be combined by moving the code after the other two checks
inside the audio-related part of the switch.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
Andreas Rheinhardt