7546ac2fee made it so that the start_time for mp3 files is
adjusted for skip_samples. However, this appears incorrect because
subsequent packet timestamps are not adjusted and skip_samples are
applied by deleting data from a packet without changing the timestamp.
E.g., we are told the start_time is ~25ms and we get a packet with a
timestamp of 0 that has had the skip_samples discarded from it. As such
rendering engines may incorrectly discard everything prior to the
25ms thinking that is where playback should officially start. Since the
samples were deleted without adjusting timestamps though, the true
start_time is still 0.
Other formats like MP4 with edit lists will adjust both the start
time and the timestamps of subsequent packets to avoid this issue.
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Anton Khirnov <anton@khirnov.net>
It avoids leaving dangling pointers behind in memory.
Also remove redundant checks for whether the URLContext to be closed is
already NULL.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Some flac muxers write truncated metadata picture size if the picture
data do not fit in 24 bits. Detect this by truncting the size found inside
the picture block and if it matches the block size use it and read rest
of picture data.
This workaround is only for flac files and not ogg files with flac
METADATA_BLOCK_PICTURE comments and it can be disabled with strict level
above normal. Currently there is a 500MB limit on truncate size to protect
from large memory allocations.
The truncation bug in lavf flacenc was fixed in e447a4d112
but based on existing broken files other unknown flac muxers seems to truncate also.
Before the fix a broken flac file for reproduction could be generated with:
ffmpeg -f lavfi -i sine -f lavfi -i color=red:size=2400x2400 -map 0:0 -map 1:0 -c✌️0 bmp -disposition:1 attached_pic -t 1 test.flac
Fixes ticket 6333
Signed-off-by: Anton Khirnov <anton@khirnov.net>
ff_id3v2_parse_apic/chapters/priv/priv_dict all had a parameter
extra_meta of type ID3v2ExtraMeta ** as if the functions wanted to make
*extra_meta point to something else. But they don't, so just use an
ID3v2ExtraMeta *.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Up until now, the ID3v2ExtraMeta structure (which is used when parsing
ID3v2 tags containing attached pictures, chapters etc.) contained a
pointer to separately allocated data that depended on the type of the
tag. Yet the difference of the sizes of the largest and the smallest of
these structures is fairly small, so that it is better to simply include
a union of all the possible types of tag-dependent structures in
ID3v2ExtraMeta. This commit implements this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
If the write_id3v2 option is set, the aiff muxer would write id3v2 tags
if there is global metadata or if there are attached pics to write.
Chapters are ignored in this check that precedes writing id3v2 tags.
Yet 47ac344970 added support for writing
chapters as id3v2 tags, so one should check for the existence of chapters,
too; otherwise the chapters would only be written in case there is
global metadata or an attached pic.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The Xiph foundation never standardized either Daala nor its mapping in Ogg,
and all files that were created are undecodable without knowledge of the
git hash.
The description of AVOutputFormat.init contains the statement that "this
method must not write output". Due to this, the webm_chunk muxer defers
opening the AVIOContext for the child muxer until avformat_write_header(),
i.e. there is no AVIOContext when the sub-muxer's avformat_init_output()
is called. But this violates the documentation of said function which
requires the AVFormatContext to have an already opened AVIOContext.
This commit fixes this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Don't use the functions for searching substrings when all one is
looking for is a char anyway. Given that there is already a standard
library function for "find last occurence of a char in a string" also
allows one to remove a custom loop.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The current parsing process for adaptation_sets does not guarantee
every adaptation set to contain at least one stream, because the loop
exits immediately as soon as the end of the string has been reached,
without checking whether the currently active adaptation set group is
lacking a stream. This would lead to segfaults lateron as the rest of
the code presumed that every adaptation set contains a stream. This
commit fixes this by erroring out when the last adaptation set group
is incomplete.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The WebM DASH manifest muxer uses a loop to parse the adaptation_sets
string (which is given by the user and governs which AVStreams are
mapped to what adaptation set) and the very beginning of this loop is
"if (*p == ' ') continue;". This of course leads to an infinite loop if
the condition is true. It is true if e.g. the string begins with ' ' or
if there are more than one ' ' between different adaptation set groups.
To fix this, the parsing process has been modified to consume the space
if it is at a place where it can legitimately occur, i.e. when a new
adaptation set group is expected. The latter restriction implies that an
error is returned if a space exists where none is allowed to exist.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
The syntax of the adaptation_sets string by which the user determines
the mapping of AVStreams to adaptation sets is
"id=x,streams=a,b,c id=y,streams=d,e" (means: the streams with the
indices a, b and c belong to the adaptation set with id x). Yet there
was no check for whether these indices were actual numbers and if there
is a number whether it really extends to the next ',', ' ' or to the
end of the string or not. This commit adds a check for this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
In order to parse a number from a string, the WebM DASH manifest muxer
would duplicate (via heap-allocation) the part of the string that
contains the number, then read the number via atoi() and then free the
duplicate again. This has been replaced by simply using strtoll() (which
in contrast to atoi() has defined behaviour when the number is not
representable).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Since commit c5324d92c5 all custom
interleave_packet() functions always return clean packets (even on
error), so that unreferencing manually can be removed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
AVStream.request_probe as well as AVStream.mux_ts_offset are below the
separator of public and private fields, so that a further "NOT PART OF
PUBLIC API" is redundant.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Move the copying of the frame to vos_data further up in the function,
so that when writing the actual frame data for the first frame, it's
clear that the stream really is in annex b format, for the cases where
we create extradata from the first frame.
Alternatively - we could invert the checks for bitstream format. If
extradata is missing, we can't pretend that the bitstream is in
mp4 form, because we can't even know the NAL unit length prefix size
in that case.
Also avoid creating extradata for AVC intra. If the track tag is
an AVC intra tag, don't copy the frame into vos_data - this matches
other existing cases of how vos_data and TAG_IS_AVCI interact in
other places.
Signed-off-by: Martin Storsjö <martin@martin.st>
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>
Some real-world sites use an authorization header with a bearer token; when
combined with lengthy request parameters to identify the video segment,
it's rather trivial these days to have a request body of more than 4k bytes.
MAX_URL_SIZE is hard-coded to 4k bytes in libavformat/internal.h, and
HTTP_HEADERS_SIZE is 4k as well in libavformat/http.h, so this patch increases
the buffer size to 8k, as that is the default request body limit in Apache, and
most other httpds seem to support at least as much, if not more.
Signed-off-by: Marton Balint <cus@passwd.hu>
Fixes: signed integer overflow: -9223372036854775808 - 45000 cannot be represented in type 'long'
Fixes: ticket8187
Found-by: Suhwan
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: signed integer overflow: 30000299 * 256 cannot be represented in type 'int'
Fixes: ticket8184
Found-by: Suhwan
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: out of array access
Fixes: stack-buffer-overflow-READ-0x0831fff1
Found-by: GalyCannon <galycannon@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The TTA muxer writes a seektable in a dynamic buffer as it receives
packets and when writing the trailer, closes the dynamic buffer using
avio_close_dyn_buf(), writes the seektable and frees the buffer. But
the TTA muxer already has a deinit function which unconditionally
calls ffio_free_dyn_buf() on the dynamic buffer, so switching to
avio_get_dyn_buf() means that one can remove the code to free the
buffer; furthermore, it also might save an allocation if the seektable
is so small that it fits into the dynamic buffer's write buffer or if
adding the padding that avio_close_dyn_buf() adds necessitated
reallocating of the underlying buffer.
Reviewed-by: James Almer <jamrial@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
ff_id3v2_free_extra_meta() takes a ID3V2ExtraMeta ** so that it can
already reset the pointer.
Reviewed-by: Jun Zhao <mypopy@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Sticking a full frame in the extradata works, as the code for writing
the avcC/hvcC extracts the relevant parameter set NAL units - provided
that they actually exist in the frame.
Some encoders don't provide split out extradata directly on init (or
at all). In particular, the MediaFoundation encoder wrapper doesn't
always (depending on the actual encoder device) - this is the case for
Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
This only works for cases where the moov hasn't already been written
(e.g. when not writing fragmented mp4 with empty_moov, unless using
the delay_moov option).
Signed-off-by: Martin Storsjö <martin@martin.st>
2d8d554f15 added a new error condition
to mov_read_stsz() but forgot to free a temporary buffer when it
occurs.
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
because it need be check for success, is should not
change the old way if it failure.
fix ticket: 8674
Signed-off-by: Steven Liu <liuqi05@kuaishou.com>
Each AttachedFile in Matroska can have a FileDescription element that
contains a human-friendly name for the attached file; yet this element
has been ignored up until now. This commit changes this and exports it
as title tag instead (the Matroska muxer mapped the title tag to the
AttachedFile element since support for Attachments was added).
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>
Dale Curtis
Andreas Rheinhardt
Michael Niedermayer
Mattias Wadman
Anton Khirnov
Lynne
Martin Storsjö
Joey Smith
Paul B Mahol
Steven Liu