The documentation of av_write_frame() explicitly states that the function
doesn't take ownership of the packets sent to it; while av_write_frame()
does not directly unreference the packets after having written them, it
nevertheless modifies the packet in various ways:
1. The timestamps might be modified either by prepare_input_packet() or
compute_muxer_pkt_fields().
2. If a bitstream filter gets applied, it takes ownership of the
reference and the side-data in the packet sent to it.
In case of do_packet_auto_bsf(), the end result is that the returned packet
contains the output of the last bsf in the chain. If an error happens,
a blank packet will be returned; a packet may also simply not lead to
any output (vp9_superframe).
This also implies that side data needs to be really copied and can't be
shared with the input packet.
The method choosen here minimizes copying of data: When the input isn't
refcounted and no bitstream filter is applied, the packet's data will
not be copied.
Notice that packets that contain uncoded frames are exempt from this
because these packets are not owned by and returned to the user. This
also moves unreferencing the packets containing uncoded frames to
av_write_frame() in the noninterleaved codepath; in the interleaved
codepath, these packets are already freed in av_interleaved_write_frame(),
so that unreferencing the packets in write_uncoded_frame_internal() is
no longer needed. It has been removed.
Reviewed-by: Marton Balint <cus@passwd.hu>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Now that ff_interleave_add_packet() always returns blank packets, the
input packet to ff_interleave_packet_per_dts() will always be blank on
return as well (if supplied) and the same goes for interleave_packet()
in mux.c. Document these facts and remove the redundant resetting that
happened in av_interleaved_write_frame().
The last reference to the (long removed) destruct field that AVPackets
once had has been removed as well when updating the documentation of
ff_interleave_packet_per_dts().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
When an error happened in ff_interleave_add_packet() when adding
a packet to the packet queue, said packet would not be unreferenced
in ff_interleave_add_packet(), but would be zeroed in
av_interleaved_write_frame(), which results in a memleak.
This has been fixed: ff_interleave_add_packet() now always unreferences
the input packet on error; as a result, it always returns blank packets
which has been documented. Relying on this a call to av_packet_unref()
in ff_audio_rechunk_interleave() can be removed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If writing uncoded frames in noninterleaved mode fails at the preparatory
steps (i.e. before it reaches write_packet()), the packet would not be
unreferenced and the frame would leak. This is fixed by unreferencing
the packet in write_uncoded_frame_internal() instead.
This also makes it possible to remove the unreferencing in
write_packet() itself: In noninterleaved mode frames are now freed in
write_uncoded_frame_internal(), while they are freed in interleaved
mode when their containing packet gets unreferenced (like normal
packets).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Currently uncoded frames (i.e. packets whose data actually points to an
AVFrame) are not refcounted. As a consequence, calling av_packet_unref()
on them will not free them, but may simply make sure that they leak by
losing the pointer to the frame.
This commit changes this by actually making uncoded frames refcounted.
In order not to rely on sizeof(AVFrame) (which is not part of the public
API and so must not be used here in libavformat) the packet's data is
changed to a (padded) buffer containing just a pointer to an AVFrame.
Said buffer is owned by an AVBuffer with a custom free function that
frees the frame as well as the buffer. Thereby the pointer/the AVBuffer
owns the AVFrame.
Said ownership can actually be transferred by copying and resetting
the pointer, as might happen when actually writing the uncoded frames
in AVOutputFormat.write_uncoded_frame() (although currently no muxer
makes use of this possibility).
This makes packets containing uncoded frames compatible with
av_packet_unref(). This already has three advantages in interleaved mode:
1. If an error happens at the preparatory steps (before the packet is
put into the interleavement queue), the frame is properly freed.
2. If the trailer is never written, the frames still in the
interleavement queue will now be properly freed by
ff_packet_list_free().
3. The custom code for moving the packet to the packet list in
ff_interleave_add_packet() can be removed.
It will also simplify fixing further memleaks in future commits.
Suggested-by: Marton Balint <cus@passwd.hu>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
This generates a potential memory leak, and mixes side data from the last
packet with other properties from the first.
Keep all the properties from the first packet only in the output packet
instead.
Signed-off-by: James Almer <jamrial@gmail.com>
The only difference of the currently used write_packet()-function to
ff_raw_write_packet() is that the former also counts the number of
frames. Yet doing so in the muxer itself is unnecessary as this is
already done generically in write_packet() in libavformat/mux.c.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
These muxers don't depend on the WebM Chunk or the WebM DASH Manifest
muxers.
Furthermore, remove some #if checks in webm_chunk.c and webmdashenc.c.
They are always true now that webm_chunk.c and webmdashenc.c are only
compiled when their corresponding muxers are enabled.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
avio_internal.h has been included in this muxer since the beginning and
was never needed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
libavutil/avstring.h is unnecessary since 8a632b3e. The other
unnecessary headers were never used.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The webm_chunk muxer requires the WebM muxer, yet it does not directly
require anything from libavformat/matroska.c (it does not even include
the corresponding header). So remove the dependency from the Makefile
and add a _select to configure.
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>
The mdcv atom isn't in ISO/IEC 14496-12:2015 but it is expected to be
added soon. See:
http://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/259529.html
The mdcv atom is already parsed in FFmpeg in mov.c.
Signed-off-by: Michael Bradshaw <mjbshaw@google.com>
The switch cases were missing:
- Primaries: bt470m, film, smpte428, and ebu3213.
- TRCs: gamma22, gamma28, linear, log, log_sqrt, iec61966_2_4, bt1361,
iec61966_2_1, bt2020_10bit, and bt2020_12bit.
- Space: rgb, fcc, ycgco, bt2020_cl, smpte2085, chroma-derived-nc,
chroma-derived-c, and ictcp.
They also annoyingly remapped the following (which are functionally
equivalent but can be treated differently by clients):
- smpte240m primaries to smpte170m.
- smpte170m TRC to bt709.
- bt470bg color space to smpte170m.
The enum values in FFmpeg are the same values as ITU-T H.273 and
ISO/IEC 23001-8 so we can just use them directly, which is both simpler
and preserves the user intent.
Signed-off-by: Michael Bradshaw <mjbshaw@google.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>
dts would start over at the beginning of each trun when they should be
computed contiguously for each trun in a traf
Fixes ticket 8070
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
If some but not all moof's are referenced in an sidx, whole fragments
were being skipped.
Fixes tickets 7377, 7389, and 8502
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Regression since 8d3630c540 where keys were changed
to not be touppered but the picture block strcmp was not changed to be case-insensitive.
Fixes ticket #8608.
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>
vobsub_read_packet() didn't check whether an array of AVPackets was
valid and therefore used uninitialized values.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
if the directory name of the segments contains "%v".
This memleak is caused by masking the pointer that will eventually
be freed by a variable of the same name in a smaller scope.
Therefore the pointer that gets freed is always NULL when it is
freed and the allocated data leaks.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
