Fixes: runtime error: signed integer overflow: -9166684017437101870 + -2495066639299164439 cannot be represented in type
Fixes: Chromium bug 791349
Reported-by: Matt Wolenetz <wolenetz@google.com>
Reviewed-by: Matt Wolenetz <wolenetz@google.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes seek for files with empty edits and files with negative ctts
(dts_shift > 0). Added fate samples and tests.
Signed-off-by: Sasi Inguva <isasi@isasi.mtv.corp.google.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
In this loop, |i| is the "index". And the memory allocated should be at
least the current "count", which is |i + 1|.
BUG=801821
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Fixes: runtime error: signed integer overflow: 5 * -2147483647 cannot be represented in type 'int'
Fixes: Chromium bug 817338
Reviewed-by: Matt Wolenetz <wolenetz@google.com>
Reported-by: Matt Wolenetz <wolenetz@google.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
mov_read_ctts ignores ctts entries having count <= 0. Generally, the
aggregate of all ctts entries' count fields resulting from mov_read_ctts
can be less than the corresponding sample_count.
mov_build_index attempts to normalize any existing ctts_data counts to
be 1, to make a 1-1 mapping of a ctts_data entry to a sample.
That 1-1 mapping left a tail of uninitialized ctts_data entries when the
aggregate, normalized ctts_count < sample_count.
Even more generally, later usage of ctts_data may depend on the entire
ctts_allocated_size having been initialized.
This change memsets the entire allocation of the normalized ctts_data in
mov_build_index, to prevent use of uninitialized data later.
BUG=816787
Change-Id: I7fd7db255e3aeed076ee32c90cb2df211741c052
Reviewed-on: https://chromium-review.googlesource.com/947110
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The allocated size of sc->ctts_data is
(st->nb_index_entries + entries) * sizeof(*sc->ctts_data).
The size to memset at offset sc->ctts_data + sc->ctts_count should be
(st->nb_index_entries + entries - sc->ctts_count) *
sizeof(*sc->ctts_data))
The current code missed |entries| I believe, which was introduced in
https://patchwork.ffmpeg.org/patch/5541/.
However, after offline discussion, it seems the original code is much
more clear to read (before https://patchwork.ffmpeg.org/patch/5541/).
Hence this CL revert the memset logic to it's previous state by
remembering the |old_ctts_allocated_size|, and only memset the newly
allocated entries.
BUG=812567
Change-Id: Ibe94c7138e5818bfaae76866bfa6619a9b8a2b6b
Reviewed-on: https://chromium-review.googlesource.com/934925
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
An index should never be equal to the count. Hence we must make sure
*ctts_index < ctts_count.
Reviewed-by: Sasi Inguva <isasi@google.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Demuxing only. Muxing is disabled as altref frame handling is not
defined in the spec, and there's no way to know the presence of
such frames during stream initialization.
Based on a patch by Steven Liu.
Fixes ticket #7000
Reviewed-by: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
An invalid file may cause huge alloc. Delay expansion of ctts entries
until the number of samples is known in mov_build_index.
Fixes: 23
Found-by: zhao dongzhuo, AD-lab of Venustech
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
On some failure paths, the error code is not correctly set.
Signed-off-by: Pan Bian <bianpan2016@163.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
In function mov_read_custom(), it returns 0 on the path that av_malloc()
returns a NULL pointer. 0 indicates success. An error code should be
assigned to ret.
Signed-off-by: Pan Bian <bianpan2016@163.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
In the event of ff_mov_read_stsd_entries() failure, sc->stsd_count
is not updated, even if the function allocates extradata memory.
Instead update the sc->stsd_count as entries are parsed so that
mov_read_close() can do the right thing.
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Potentially fixes:
https://bugs.chromium.org/p/chromium/issues/detail?id=786269#c1
In theory, the crash can be triggered by an invalid stream that has
either tfdt or trun outside of the moof
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Should be unconditionally freed at the end of mov_fix_index() in
case it hasn't been used during the fix up.
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Sasi Inguva <isasi-at-google.com@ffmpeg.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
After c2a8f0fcbe this can happen on normal edit lists starting on a B-frame.
Signed-off-by: Sasi Inguva <isasi@google.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
* commit 'defe307fb22beca60a632e976ab97e5edd4aee25':
mov: move stsd finalization to an appropriate place
mov: Do not set stsd_count if mov_read_stsd() fails
mov: log and return early on non-positive stsd entry counts
See 8b43ee4054656feb641d
Merged-by: James Almer <jamrial@gmail.com>
Subtract the calculated dts offset from the requested timestamp before
seeking. This fixes an error "Error while filtering: Operation not
permitted" observed with a short file which contains only one key frame
and starts with negative timestamps.
Then, av_index_search_timestamp() returns a valid negative timestamp,
but mov_seek_stream bails out with AVERROR_INVALIDDATA.
Fixes ticket #6139.
Signed-off-by: Jonas Licht <jonas.licht@fem.tu-ilmenau.de>
Signed-off-by: Peter Große <pegro@friiks.de>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
When keyframe intervals of dash segments are not perfectly aligned,
fragments in the stream can overlap in time. The previous sorting by
timestamp causes packets to be read out of decode order and results
in decode errors.
Insert new "trun" index entries into index_entries in the order that
the trun are referenced by the sidx.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Sets the correct start padding value when an edit list is present.
A new fate test is added, fate-mov-440hz-10ms, to ensure this is
handled correctly.
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Sasi Inguva <isasi-at-google.com@ffmpeg.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
* commit '236577230051ad61ec67fa2d68e817d54232d2a0':
mov: Ignore old spherical metadata when newer version is present
Merged-by: James Almer <jamrial@gmail.com>
* commit 'b446f0e98f85e2e931b476e52b319f1c49244660':
mov: Do not try to parse multiple stsd for the same track
See 8b43ee4054
Merged-by: James Almer <jamrial@gmail.com>
Fixes: Missing EOF check in loop
No testcase
Found-by: Xiaohei and Wangchu from Alibaba Security Team
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
MP4 files with fragments might have the first moof box that is mentioned
in a fragment index before the first mdat box. Since it is then already
parsed by mov_read_header, we have to make sure that mov_switch_root
will not parse it again when seeking by setting the headers_read flag in
the index. Parsing it a second time would cause the ctts_data array to
receive a second copy of the information from the trun box, leading to
wrong PTS values for the second and following fragments in presence of
B-frames.
Fixes ticket 6560.
Signed-off-by: Daniel Glöckner <daniel-gl@gmx.net>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
ctts data in ffmpeg relies on the index entries array to be 1:1
with samples... yet sc->sample_count can be read directly from
the 'stsz' box and index entries are only generated if a chunk
count has been read from 'stco' box.
Ensure that if sc->sample_count > 0, sc->chunk_count is too as
a basic sanity check. Additionally we need to check that after
the index is built we have the right number of entries, so we
also check in mov_read_trun() that sc->sample_count ==
st->nb_index_entries.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
When sidx box support is enabled, the code will skip reading all
trun boxes (each containing ctts entries for samples inthat box).
If seeks are attempted before all ctts values are known, the old
code would dump ctts entries into the wrong location. These are
then used to compute pts values which leads to out of order and
incorrectly timestamped packets.
This patch fixes ctts processing by always using the index returned
by av_add_index_entry() as the ctts_data index. When the index gains
new entries old values are reshuffled as appropriate.
This approach makes sense since the mov demuxer is already relying
on the mapping of AVIndex entries to samples for correct demuxing.
As a result of this all ctts entries are now 1-count. A followup
change will be submitted to remove support for > 1 count entries
which will simplify seeking.
Notes for future improvement:
Probably there are other boxes (stts, stsc, etc) that are impacted
by this issue... this patch only attempts to fix ctts since it
completely breaks packet timestamping.
This patch continues using an array for the ctts data, which is not
the most ideal given the rearrangement that needs to happen (via
memmove as new entries are read in). Ideally AVIndex and the ctts
data would be set-type structures so addition is always worst case
O(lg(n)) instead of the O(n^2) that exists now; this slowdown is
noticeable during seeks.
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Michael Niedermayer
Sasi Inguva
James Almer
Xiaohan Wang
Matt Wolenetz
Tianqiang Liu
Marton Balint
Carl Eugen Hoyos
tiejun.peng
John Stebbins
Pan Bian
Vittorio Giovara
Dale Curtis
Jacob Trimble
Fredrik Hubinette
Jonas Licht
Sasi Inguva
Pablo Montilla
Daniel Glöckner