Certain types of OBUs are stripped away before muxing into Matroska and
ISOBMFF; there are two functions to do this: One that outputs by
directly writing in an AVIOContext and one that returns a freshly
allocated buffer with the units not stripped away copied into it.
The latter option is bad for performance, especially when the input
does already not contain any of the units intended to be stripped away
(this covers typical remuxing scenarios). Therefore this commit changes
this by avoiding allocating and copying when possible; it is possible if
the OBUs to be retained are consecutively in the input buffer (without
an OBU to be discarded between them). In this case, the caller receives
the offset as well as the length of the part of the buffer that contains
the units to be kept. This also avoids copying when e.g. the only unit
to be discarded is a temporal delimiter at the front.
For a 22.7mb/s file with average framesize 113 kB this improved the time
for the calls to ff_av1_filter_obus_buf() when writing Matroska from
313319 decicycles to 2368 decicycles; for another file with 1.5mb/s
(average framesize 7.3 kB) it improved from 34539 decicycles to 1922
decicyles. For these files the only units that needed to be stripped
away were temporal unit delimiters at the front.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Add {, } in situations like
if ()
...
else if ()
/* Comment */
...
else ...
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
This is needed especially for AV1: If a reformatting error happens (e.g.
if the length field of an OBU contained in the current packet indicates
that said OBU extends beyond the current packet), the data pointer is
still NULL, yet the size is unchanged, so that writing the data leads
to a segmentation fault.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
In order to indicate that the frames in a BlockGroup are not keyframes,
one has to add a ReferenceBlock element containing the timestamp of a
referenced Block that has already been written. The timestamp ought to be
relative to the timestamp of the Block it is attached to. Yet the
Matroska muxer used the relative timestamp of the preceding Block of the
track, i.e. the timestamp of the preceding block relative to the
timestamp of the Cluster containing said block (that need not be the
Cluster containing the current Block). This has been fixed.
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>
Converting explicit avio_flush() calls helps us to buffer more data and avoid
flushing the IO context too often which causes reduced IO throughput for
non-streamed file output.
The user can control FLUSH_POINT flushing behaviour using the -flush_packets
option, the default typically means to flush unless a non-streamed file output
is used, so this change should have no adverse effect on streaming even if it
is assumed that after an avio_flush() the output buffer is clean so small
seekbacks within the output buffer will work even when the IO context is not
seekable.
Signed-off-by: Marton Balint <cus@passwd.hu>
To make it consistent with other muxers.
The user can still control the generic flushing behaviour after write_header
(same way as after packets) using the -flush_packets option, the default
typically means to flush unless a non-streamed file output is used.
Therefore this change should have no adverse effect on streaming, even if it is
assumed that the first packet has a clean buffer, so small seekbacks within the
output buffer work even when the IO context is not seekable.
Signed-off-by: Marton Balint <cus@passwd.hu>
The Matroska muxer currently does not check the return value of
ff_isom_write_hvcc(), the function used to write mp4-style
HEVC-extradata as Matroska also uses it. This was intentionally done in
7a5356c72 to allow remuxing from mpeg-ts.
But if ff_isom_write_hvcc() fails, it has not output anything and the
file ends up without CodecPrivate and, if the input was Annex B, with
Annex B data, which is against the spec. So check the return value
again.
The underlying issue of not having extradata seems to have been fixed by
the introduction of the extract_extradata bitstream filter.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: "mypopy@gmail.com" <mypopy@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
to its actual behaviour: That it uses the least amount of bytes unless
overridden.
The current documentation leaves it undefined how many bytes will be used
when no number to use has been given explicitly. But several estimates
(used to write EBML Master elements with a small length field) require
this number to be the least amount of bytes to work. Therefore change
the documentation; and remove a comment about writing length fields
indicating "unkown length". It has been outdated since 0580a122.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
These functions already free it themselves before they allocate the new
extradata.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
By using avio_get_dyn_buf() + ffio_free_dyn_buf() instead of
avio_close_dyn_buf() + av_free() one can avoid an allocation + copy for
small dynamic buffers (i.e. small master elements).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This comment does not account for the fact that the limits on cluster
size and duration are configurable by the user since 98308bd4.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
If mkv_write_trailer() is not called, the cached audio packet might
leak; so unref it in mkv_deinit().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Contains renaming of variables (e.g. mkv_write_cues() contained
variables called tracknum that actually contain the index of a track in
s->streams and not the track number (which can differ in case an
explicit dash track number is set)).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
mkv_start_new_cluster() actually didn't start a new cluster, but ended
the old one instead and emitted a debug message that it had started a
new cluster. This has been changed: The debug message has been moved to
the place that really starts a new cluster and the function has been
renamed to mkv_end_cluster().
Furthermore, without this debug message the function can be used for
flushing.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
The Matroska muxer groups index entries with the same pts together in
order to save a few bytes. Because of Matroska's variable-length length
fields, mkv_write_cues() does this by first finding out how many index
entries will be grouped together before actually writing them.
Currently, it is asserted at both of these stages that the stream index
of the list of designated index entries is valid. But the second assert
is redundant, because the very same index entries have already been
checked.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
The Matroska muxer up until now leaked memory in two scenarios:
1. If an error happened during writing the trailer, as
mkv_write_trailer() returned early without cleaning up.
2. If mkv_write_header() indicated success despite an error in the
underlying AVIOContext. In this case avformat_write_header() returned
the IO error and according to the API the caller is not allowed to call
av_write_trailer(), so that no cleanup happened for the allocations made
in mkv_write_header().
This has been fixed by using a dedicated deinit function.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
After the last few commits, the functions for writing master elements
with CRC-32 elements didn't really make use of the ebml_master
structure any more, so remove these parameters from the functions.
The only things that still need to be kept are the positions of the
level 1 elements that are written preliminarily and updated later.
These positions are stored in the MatroskaMuxContext and
replace the corresponding ebml_master structures.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Up until now, a block's relative offset has been reported as the offset
in the log messages output when writing blocks; given that it is
impossible to know the real offset from the beginning of the file at
this point due to the fact that it is not yet known how many bytes will
be used for the containing cluster's length field both the relative
offset in the cluster as well as the offset of the containing cluster
will be reported from now on.
Furthermore, the TrackNumber of the written block has been added to the
log output.
Also, the log message for writing vtt blocks has been brought in line
with the message for normal blocks.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Up until now, the length field of most level 1 elements has been written
using eight bytes, although it is known in advance how much space the
content of said elements will take up so that it would be possible to
determine the minimal amount of bytes for the length field. This
commit changes this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Fixes intendation, whitespace, a typo and renames a variable
(dyn_bc->cluster_bc) to make its meaning clearer and to bring
it more in line with the naming of similar variables.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Given that in both the seekable as well as the non-seekable mode dynamic
buffers are used to write level 1 elements and that now no seeks are
used in the seekable case any more, the two modes can be combined; as a
consequence, the non-seekable mode automatically inherits the ability to
write CRC-32 elements.
There are no differences in case the output is seekable; when it is not
and writing CRC-32 elements is disabled, there can still be minor
differences because before this commit, the EBML ID and length field
were counted towards the cluster size limit; now they no longer are.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Up until now, the writing process for level 1 elements (those elements
for which CRC-32 elements are written by default) was this in case the
output was seekable: Write the EBML ID, write an "unkown length" EBML
number of the desired length, then write the element into a dynamic
buffer, then write the dynamic buffer (after possible calculation and
writing of the CRC-element), then seek back to the size element and
overwrite the unknown-size element with the real size. The seeking and
overwriting part has been eliminated by not writing the size initially.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
A Matroska EBML ID can only be maximally four bytes long, so make the
variables denoting EBML IDs uint32_t instead of unsigned int to
better reflect this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
All places where end_ebml_master_crc32_preliminary are used already
check for whether the output is seekable, so the check in the function
is redundant.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Since 4e3bdf729a there is no reason any
more to treat the seekable and non-seekable cases separate with regards
to the log message for a new cluster. This effectively reverts
d41aeea8a6.
Also improved the log message: "pts 80dts 0" -> "pts 80, dts 0".
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Up until now, the check for whether to write CRC32 elements was always
mkv->write_crc && mkv->mode != MODE_WEBM. This is equivalent to simply
set write_crc to zero in WebM-mode. And this is what this commit does.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Up until e7ddafd515 the Matroska muxer
wrote a secondary seek head referencing all the clusters. When this
was changed, a (now completely wrong) comment remained and the unique
remaining seek head was still called main_seekhead. This has been
changed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Up until now the EBML Header length field has been written with eight
bytes, although the EBML Header is always so small that only one byte
is needed for it. This patch saves seven bytes for every Matroska/Webm
file.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
The upper bounds currently used for determining the size of a CuePoint's
length field can be improved somewhat; as a result, a CuePoint
containing three CueTrackPositions will now only need a size field
with one byte length.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
The earlier code included the size of the BlockGroup's length field and
the EBML ID in the calculation of the size for the payload and ignored
the size of the duration's length field. This meant that Blockgroups
corresponding to packets with size 2^(7n) - 17 - n - i, i = 0,..., n - 1,
n = 1,..., 8 (i.e. 110, 16364, 16365, 2097130..2097132, ...) were written
with length fields that are unnecessarily long.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
At this point, ts already includes the ts_offset so that the relative
time written with the cluster is already given by ts - mkv->cluster_pts.
It is this number that needs to fit into an int16_t.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
Fix memory leak after write trailer for #7827, only store a audio
packet whose buffer has size greater than zero in cur_audio_pkt.
Audio packets with size zero, but with side-data currently lead to
memleaks, in the Matroska muxer, because they are not properly freed:
They are currently put into an AVPacket in the MatroskaMuxContext to
ensure that the necessary audio is always available for a new cluster,
but are only written and freed when their size is > 0.
As the only use we have for such packets consists in updating the
CodecPrivate it makes no sense to store these packets at all and this
is how this commit solves the memleak.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@googlemail.com>
Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
Nothing prevents it to work except this check. AV1 is already supported
by Matroska muxer and aomenc produces WebM/AV1 files as well.
Signed-off-by: Kagami Hiiragi <kagami@genshiken.org>
Signed-off-by: James Almer <jamrial@gmail.com>
This is a temporary workaround for transcoding scenarious using libaom-av1
encoder, which currently can't propagate extradata during initialization.
Signed-off-by: James Almer <jamrial@gmail.com>
Make sure to not write forbidden OBUs to CodecPrivate, and do the same with
unnecessary OBUs for packets.
Signed-off-by: James Almer <jamrial@gmail.com>
If the API user doesn't set avg_frame_rate, matroskaenc will write the
current timebase as "default duration" for the video track. This makes
no sense, because the "default duration" implies the framerate of the
video. Since the timebase is forced to 1/1000, this will make the
resulting file claim 1000fps.
Drop it and don't write the element. It's optional, so it's better not
to write it if the framerate is unknown.
Strangely does not require FATE changes.
