This is possible now that the next-API is gone.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Signed-off-by: James Almer <jamrial@gmail.com>
ffio_fill() is used when initially writing unknown length elements;
yet it can happen that the amount of bytes written by it is zero in
which case it is of course unnecessary to ever call it. Whether it is
possible to know this during compiletime depends upon how aggressively
the compiler inlines function calls (i.e. if it inlines calls to
start_ebml_master() where the upper bound for the size of the element
implies that the size will be written on one byte) and this depends upon
optimization settings. It is not the aim of this patch to inline all
calls where it is known that ffio_fill() will be unnecessary, but merely
to make compilers that inline such calls aware of the fact that writing
zero bytes with ffio_fill() is unnecessary. To this end
av_builtin_constant_p() is used to check whether the size is a
compiletime constant.
For GCC 10 this made a difference at -O3 only: The size of .text
decreased from 0x747F (with 29 calls to ffio_fill(), eight of which
use size zero) to 0x7337 (with 21 calls to ffio_fill(), zero of which
use size zero).
For Clang 11 it made a difference at -O2 and -O3: At -O2, the size of
.text decreased from 0x879C to 0x871C (with eight calls to ffio_fill()
eliminated); at -O3 the size of .text decreased from 0xAF2F to 0xAEBF.
Once again, eight calls to ffio_fill() with size zero have been
eliminated.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The value zero for AVPacket.duration means that the duration is unknown,
which in practice means "play this subtitle until overridden by the next
subtitle". Yet for Matroska a BlockGroup with duration zero means
that the subtitle really has a duration zero. "Display until overridden"
is achieved by not setting a duration on the container level at all and
this is achieved by using a SimpleBlock or a BlockGroup without
duration. This commit implements this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
64 bits are needed in order to retain the uid values of Matroska
chapters; the type is kept signed because the semantics of NUT chapters
depend upon whether the id is > 0 or < 0.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, there has been no check that each chapter has a unique id;
there was only a check for whether a chapter id is zero (this happens
often when the chapters originated from a format that lacks the concept
of chapter id and simply counts from zero) which is invalid in Matroska.
In this case the chapter ids are offset by 1 to make them nonnegative.
Yet offsetting won't fix duplicate ids, therefore this is changed to
simply create new chapter uids when the input chapter uids don't conform
to the requirements of Matroska (in which case it can be presumed that
they did not originate from Matroska, so that we don't need to bother
to preserve them).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This is the Matroska equivalent of D_WEBVTT_DESCRIPTIONS and is
therefore only enabled for subtitles.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
According to the new EBML specifications, a string element of length
zero would be read as the default value by a compliant parser.
Reviewed-by: Ridley Combs <rcombs@rcombs.me>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Some legacy applications such as AVI2MVE expect raw RGB bitmaps
to be stored bottom-up, whereas our RIFF BITMAPINFOHEADER assumes
they are always stored top-down and thus write a negative value
for height. This can prevent reading of these files.
Option flipped_raw_rgb added to AVI and Matroska muxers
which will write positive value for height when enabled.
Note that the user has to flip the bitmaps beforehand using other
means such as the vflip filter.
The argument pertaining to a printf %s conversion specifier must not
be NULL, even if the precision (i.e. the number of characters to write)
is zero. If it is NULL, it is undefined behaviour.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
av_stream_get_side_data() tells the caller whether a stream has side
data of a specific type; if present it can also tell the caller the size
of the side data via an optional argument. The Matroska muxer always
used this optional argument, although it doesn't really need the size,
as the relevant side-data are not buffers, but structures. So change
this.
Furthermore, relying on the size also made the code susceptible to
a quirk of av_stream_get_side_data(): It only sets the size argument if
it found side data of the desired type. mkv_write_video_color() checks
for side-data twice with the same variable for the size without resetting
the size in between; if the second type of side-data isn't present, the
size will still be what it was after the first call. This was not
dangerous in practice, as the check for the existence of the second
side-data compared the size with the expected size, so it would only be
problematic if lots of elements were to be added to AVContentLightMetadata.
Reviewed-by: James Almer <jamrial@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska muxer writes the Chapters early when chapters were already
available when writing the header; in this case any tags pertaining to
these chapters get written, too.
Yet if no chapters had been supplied before writing the header, Chapters
can also be written when writing the trailer if any are supplied. Tags
belonging to these chapters were up until now completely ignored.
This commit changes this: Writing the tags belonging to chapters has
been moved to mkv_write_chapters(). If mkv_write_tags() has not been
called yet (i.e. when chapters are written when writing the header),
the AVIOContext for writing the ordinary Tags element is used, but not
output, as this is left to mkv_write_tags() in order to only write one
Tags element. Yet if mkv_write_tags() has already been called,
mkv_write_chapters() will output a Tags element of its own which only
contains the tags for chapters.
When chapters are available initially, the corresponding tags will now
be the first tags in the Tags element; but the ordering of tags in Tags
is irrelevant anyway.
This commit also makes chapter_id_offset local to mkv_write_chapters()
as it is used only there and not reused at all.
Potentially writing a second Tags element means that the maximum number
of SeekHead entries had to be incremented. All the changes to FATE
result from the ensuing increase in the amount of space reserved for the
SeekHead (21 bytes more).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This is needed so that it can access mkv_write_tag() and mkv_check_tag()
without using forward declarations (which are unnecessary here).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, the Matroska muxer writes only one Tags level 1 element
and therefore using a certain place to store the dynamic buffer used for
writing it was hardcoded; yet the Matroska specifications allow an
unlimited amount of Tags elements and we have reason to write a second
one: If chapters are provided after writing the header, they are written
when writing the trailer; yet the corresponding tags are ignored. This
can be fixed by writing them in a second Tags element.
Also use a MatroskaMuxContext * instead of an AVFormatContext * as
parameter in mkv_write_tag() and mkv_write_tag_targets() as that is all
these functions use.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Mostly reindentation after the last commit. Also remove a variable that
is always zero; move another variable to a more local scope and don't
assign a value to a local variable immediately before leaving the function.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
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>
Failures of the allocations that happen under the hood when using dynamic
buffers are usually completely unchecked and the Matroska muxer is no
exception to this.
The API has its part in this, because there is no documented way to
actually check for errors: The return value of both avio_get_dyn_buf()
as well as avio_close_dyn_buf() is only documented as "the length of
the byte buffer", so that using this to return errors would be an API
break.
Therefore this commit uses the only reliable way to check for errors
with avio_get_dyn_buf(): The AVIOContext's error flag. (This is one of
the advantages of avio_get_dyn_buf(): By not destroying the AVIOContext
it is possible to inspect this value.) Checking whether the size or the
pointer vanishes is not enough as it does not check for truncated output
(the dynamic buffer API is int based and so has to truncate the buffer
even when enough memory would be available; it's current actual limit is
even way below INT_MAX).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If one already has the contents of a master elements in a buffer of
known size, then writing a EBML master element is no different from
writing an EBML binary element. It is overtly complicated to use
start/end_ebml_master() as these functions first write an unkown-length
size field of the appropriate length, then write the buffer's contents,
followed by a seek to the length field to overwrite it with the real
size (obtained via avio_tell() although it was already known in
advance), followed by another seek to the previous position. Just use
put_ebml_binary() instead.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
There is a good upper bound for the maximum length of the Colour master
element; it is therefore unnecessary to use a dynamic buffer for it.
A simple buffer on the stack is enough. This commit implements this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Matroska muxer updates several header elements when the output is
seekable; if unseekable, the buffer containing the contents of the element
is immediately freed after writing. Before this commit, there were three
places doing exactly the same: Checking whether the output is seekable
and calling the function that writes and frees or the function that
just writes the EBML master. This has been unified; adding SeekHead
entries for these elements has been unified, too.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, SeekEntries were already added before
start_ebml_master_crc32() was even called and before we were actually
sure that we really write the element the SeekHead references: After
all, we might also error out later; and given that the allocations
implicit in dynamic buffers should be checked, end_ebml_master_crc32()
will eventually have to return errors itself, so that it is the right
place to add SeekHead entries.
The earlier behaviour is of course a remnant of the time in which
start_ebml_master_crc32() really did output something, so that the
position before start_ebml_master_crc32() needed to be recorded.
Erroring out later is also not as dangerous as it seems because in
this case no SeekHead will be written (if it happened when writing
the header, the whole muxing process would abort; if it happened
when writing the trailer (when writing chapters not available initially),
writing the trailer would be aborted and no SeekHead containing the
bogus chapter entry would be written).
This commit does not change the way the SeekEntries are added for those
elements that are output preliminarily; this is so because the SeekHead
is written before those elements are finally output and doing it
otherwise would increase the amount of seeks.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This has previously only been checked if the chapters were initially
available, but not if they were only written in the trailer.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now ff_vorbiscomment_write() used the bytestream API to write
VorbisComments. Therefore the caller had to provide a sufficiently large
buffer to write the output.
Yet two of the three callers (namely the FLAC and the Matroska muxer)
actually want the output to be written via an AVIOContext; therefore
they allocated buffers of the right size just for this purpose (i.e.
they get freed immediately afterwards). Only the Ogg muxer actually
wants a buffer. But given that it is easy to wrap a buffer into an
AVIOContext this commit changes ff_vorbiscomment_write() to use an
AVIOContext for its output.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
ff_vorbiscomment_write() used an AVDictionary ** parameter for a
dictionary whose contents ought to be written; yet this can be replaced
by AVDictionary * since commit 042ca05f0fdc5f4d56a3e9b94bc9cd67bca9a4bc;
and this in turn can be replaced by const AVDictionary * to indicate
that the dictionary isn't modified; the latter also applies to
ff_vorbiscomment_length().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If a FLAC track uses an unconventional channel layout, the Matroska
muxer adds a WAVEFORMATEXTENSIBLE_CHANNEL_MASK VorbisComment to the
CodecPrivate to preserve this information. And given that FLAC uses
24bit length fields, the muxer checks if the length is more than this
and errors out if it is.
Yet this can never happen, because we create the AVDictionary that is
the source for the VorbisComment. It only contains exactly one entry
that can't grow infinitely large (in fact, the length of the
VorbisComment is <= 4 + 33 + 1 + 18 + strlen(LIBAVFORMAT_IDENT)).
So we can simply assert the size to be < (1 << 24) - 4.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Commit 6fd300ac6c added support for WebM
Chunk livestreaming; in this case, both the header as well as each
Cluster is written to a file of its own, so that even if the AVIOContext
seems seekable, the muxer has to behave as if it were not. Yet one of
the added checks makes no sense: It ensures that no SeekHead is written
preliminarily (and hence no SeekHead is written at all) if the option
for livestreaming is set, although one should write the SeekHead in this
case when writing the Header. E.g. the WebM-DASH specification [1]
never forbids writing a SeekHead and in some instances (that don't apply
here) even requires it (if Cues are written after the Clusters).
[1]: https://sites.google.com/a/webmproject.org/wiki/adaptive-streaming/webm-dash-specification
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
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>
