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>
(cherry picked from commit cd0771c38c)
Matroska is built around the principle that a reader does not need to
understand everything in a file in order to be able to make use of it;
it just needs to ignore the data it doesn't know about.
Our demuxer typically follows this principle, but there is one important
instance where it does not: A Block belonging to a TrackEntry with no
associated stream is treated as invalid data (i.e. the demuxer will try
to resync to the next level 1 element because it takes this as a sign
that it has lost sync). Given that we do not create streams if we don't
know or don't support the type of the TrackEntry, this impairs this
demuxer's forward compability.
Furthermore, ignoring Blocks belonging to a TrackEntry without
corresponding stream can (in future commits) also be used to ignore
TrackEntries with obviously bogus entries without affecting the other
TrackEntries (by not creating a stream for said TrackEntry).
Finally, given that matroska_find_track_by_num() already emits its own
error message in case there is no TrackEntry with a given TrackNumber,
the error message (with level AV_LOG_INFO) for this can be removed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
(cherry picked from commit b577968cab)
A Block (meaning both a Block in a BlockGroup as well as a SimpleBlock)
must have at least three bytes after the field containing the encoded
TrackNumber. So if there are <= 3 bytes, the Matroska demuxer would
skip this block, believing it to be an empty, but valid Block.
This might discard valid nonempty Blocks, namely if the track uses header
stripping. And certain definitely spec-incompliant Blocks don't raise
errors: Those with two or less bytes left after the encoded TrackNumber
and those with three bytes left, but with flags indicating that the Block
uses lacing as then there has to be further data describing the lacing.
Furthermore, zero-sized packets were still possible because only the
size of the last entry of a lace was checked.
This commit fixes this. All spec-compliant Blocks that contain data
(even if side data only) are now returned to the caller; spec-compliant
Blocks that don't contain anything are not returned.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
(cherry picked from commit e471faf962)
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>
(cherry picked from commit 8aabcf6c11)
ff_read_packet had potential memleaks:
1. If av_packet_make_refcounted fails, it means that the packet is not
refcounted, but it could nevertheless carry side data and therefore
needs to be unreferenced.
2. If putting a packet on a packet list fails, it wasn't unreferenced.
Furthermore, read_frame_internal leaked a packet's (side) data if a
context update was required and failed.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: James Almer <jamrial@gmail.com>
(cherry picked from commit 66d5e43b99)
A demuxer might have allocated memory while reading the header. If
reading the header was successfull and an error happens before returning
(e.g. when queueing the attached pictures), the read_close function
would have never been called, so that all those allocations would leak.
This commit changes this.
Furthermore, there would be even more memleaks if the error level was
set to AV_EF_EXPLODE in case there is both metadata and id3v2 metadata.
This has been fixed, too.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Marton Balint <cus@passwd.hu>
(cherry picked from commit e2307f4ff1)
Even in this scenario, the frame still contains references to data that
won't be freed if the frame isn't unreferenced. And the AVFrame itself
will leak, too.
Fixes Coverity issue #1441422.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 212077eda4)
apply_palette() would free an AVFrame given to it only via an AVFrame *
(and not via AVFrame **) in three of its four exists (namely in the
normal path and in two error paths). So upon error the caller has no way
to know whether the frame has already been freed or not;
load_apply_palette(), the only caller, opted to free the frame in this
scenario.
This commit changes this by making apply_palette not freeing the frame
at all, which is left to load_apply_palette().
Fixes Coverity issue #1452434.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit adea33f465)
Fixes memleak and Coverity issue #1439587.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 3999c4b374)
Freeing this was forgotten in ad899522.
Fixes#8315 and #8316.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 6f2a3958cf)
Affected the ffmpeg-filter_colorkey FATE-test (but only if the C version
of idct8_add is used and not e.g. the x86 SSE2 version).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Fixes: left shift of negative value -107
Fixes: 20398/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CAVS_fuzzer-5725389278412800
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 0f0f2ab0c3)
by replacing it with a multiplication. Said multiplication can't
overflow an int32_t because lpc_coefs is limited to 16 bit precision.
Fixes the FACE-test acodec-ra144 as well as part of #8217.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit e3fb9af6f1)
Replace "((a << shift) + b) >> shift" by "a + (b >> shift)". This avoids
a left shift which also happens to trigger undefined behaviour in case "a"
is negative. This affected the FATE-tests acodec-adpcm-adx and
acodec-adpcm-adx-trellis; it also fixes ticket #8008.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 59a9d65e0d)
Affected the adpcm-afc, adpcm-ea-1, adpcm-ea-2, adpcm-ea-maxis-xa,
adpcm-thp and ea-cdata FATE-tests. Also fixes ticket #8487.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 3ad8af51b7)
This fixes ticket #7997 as well as the vsynth*-prores_# FATE-tests
(where * ranges over { 1, 2, 3, _lena } and # over { , _int, _444,
_444_int }).
(Given that prev_dc is in the range -0xC000..0x3FFF, no overflow can
happen upon multiplication with 2.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 198081efb7)
The documentation of both avio_open() as well as avio_open2() states
that on failure, the pointer to an AVIOContext given to this function
(via a pointer to a pointer to an AVIOContext) will be set to NULL. Yet
it didn't happen upon failure of ffurl_open_whitelist() or when allocating
the internal buffer failed. This commit changes this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 220846f74f)
Fixes: left shift of 1 by 31 places cannot be represented in type 'int'.
Affected the FATE-tests vsynth1-cinepak, vsynth2-cinepak and
vsynth_lena-cinepak. Also fixes ticket #8220.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit e3650dcfc9)
Affected every usage of vf_xbr, e.g. the FATE-tests filter-2xbr,
filter-3xbr, filter-4xbr.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 4294dc3589)
Affected every usage of this filter; in particular, it affected the
FATE-tests filter-2xbr, filter-3xbr and filter-4xbr.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit fa21194326)
Affected the vsynth*-jpeg2000 and the vsynth*-jpeg2000-97 FATE tests
(where * ranges over { 1, 2, 3, _lena }) as well as ticket #7983.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 5cf593adcd)
Affected the FATE-tests vsynth_lena-dv-411, vsynth1-dv-411,
vsynth2-dv-411 and hevc-paramchange-yuv420p.yuv420p10.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit e2646e23be)
This affected many FATE-tests: The number of failing tests went down
from 663 to 344. (Both numbers exclude tests that failed because of
unaligned accesses in code that is inside #if HAVE_FAST_UNALIGNED.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 736c7c20e7)
Affected the FATE-tests exr-rgb-scanline-pxr24-half-uint32-13x9 and
exr-rgb-scanline-pxr24-uint32.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 8b0f949906)
Fixes ticket #8161 and the acodec-wavpack FATE-test.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
(cherry picked from commit 84974c6fb5)
The pointer arguments to memcpy (and several other functions of the
C standard library) are not allowed to be NULL, not even when the number
of bytes to copy is zero. An AVEncryptionInitInfo's data pointer is
explicitly allowed to be NULL and yet av_encryption_init_info_add_side_data
unconditionally used it as a source pointer to copy from. This commit changes
this so that copying is only done if the number of bytes to copy is > 0.
Fixes ticket #8141 as well as a part of ticket #8150.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Tomas Härdin <tjoppen@acc.umu.se>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit e6018fda14)
Fixes the FATE-tests unknown_layout-ac3, ac3-fixed-encode, ac3-encode
and eac3-encode. It furthermore fixes the ac3-encoder bugs mentioned in
tickets #7994, #8144 and #8159.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 2f289ec914)
Fixes the FATE-tests g723_1-dec-1, g723_1-dec-2 and g723_1-dec-4.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 54e1efda6d)
Fixes the tdsc FATE-test.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 86bef10e7a)
ttaenc contained (1 << unary) - 1 as an argument for a function
expecting an unsigned int. unary can be as big as 31 in this case.
The type of the shift and the whole expression is int, because 1 fits
into an integer, so that the behaviour is undefined if unary == 31
as the result of the shift can't be represented in an int §. Subtraction
by 1 (which makes the result of the whole expression representable in
an int) doesn't change that this is undefined (it usually leads to
signed integer overflow which is undefined, too).
The solution is simple: Make 1 unsigned to change the type of the
whole expression to unsigned int (as the function expects anyway).
Fixes ticket #8153.
§: This of course presupposes the common int range of -2^31..2^31-1
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Paul B Mahol <onemda@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 3ab488a540)
The code for GAB2 subtitles predates refcounting AVPackets. So in order
to transfer the ownership of a packet's data pkt->data was simply stored
and the packet zeroed; in the end (i.e. in the read_close-function) this
data was then simply freed with av_freep(). This of course leads to a leak
of an AVBufferRef and an AVBuffer. It has been fixed by keeping and
eventually unreferencing the packet's buf instead.
Additionally, the packet is now reset via av_packet_unref().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
(cherry picked from commit da44bbefaa)
3469cfab added a check for whether the extradata coincided with the
beginning of the packet's data in order not to add extradata to packets
that already have it. But the check used was buggy for packets whose
size is smaller than the extradata's size. This commit fixes this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit a88a3cdb4b)
by changing the type to unsigned.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 8ae026d74f)
Fixes#8314.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: Peter Ross <pross@xvid.org>
Signed-off-by: James Almer <jamrial@gmail.com>
(cherry picked from commit 373c1c9b69)
The Dash muxer uses submuxers and when one such submuxer has been allocated,
it is initially only stored in a temporary variable. Therefore it leaks
if an error happens between the allocation and storing it permanently.
This commit changes this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Reviewed-by: "Jeyapal, Karthick" <kjeyapal@akamai.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit 8aeab0dbc1)
by freeing it a bit earlier.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
(cherry picked from commit bb20f3dd73)
A Seek element in a Matroska SeekHead should contain a SeekID and a
SeekPosition element and upon reading, they should be sanitized:
Given that IDs are restricted to 32 bit, longer SeekIDs should be treated
as invalid. Instead currently the lower 32 bits have been used.
For SeekPosition, no checks were performed for the element to be
present and if present, whether it was excessively large (i.e. the
absolute file position described by it exceeding INT64_MAX). The
SeekPosition element had a default value of -1 which means that a check
seems to have been intended; but it was not implemented. This commit adds
a check for overflow to the calculation of the absolute file position of
the referenced level 1 elements.
Using -1 (i.e. UINT64_MAX) as default value for SeekPosition implies that
a Seek element without SeekPosition will run afoul of this check.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
(cherry picked from commit 7c243eece3)
Matroska (or actually EBML) uses variable-length numbers where only
seven bits of every byte is usable for the length; the other bits encode
the length of the variable-length number. So in order to find out how
many bytes one needs to encode a given number one can use a loop like
while (num >> 7 * bytes) bytes++; the Matroska muxer effectively did this.
Yet it has a disadvantage: It is impossible for the result of a single
right shift of an unsigned number with most significant bit set to be
zero, because one can only shift by 0..(width - 1). On some
architectures like x64 it is not even possible to do it with undefined
right shifts in which case this leads to an infinite loop.
This can be easily avoided by switching to a loop whose condition is
(num >>= 7). The maximum value the so modified function can return
is 10; any value > 8 is invalid and will now lead to an assert in
put_ebml_num() or in start_ebml_master() (or actually in
put_ebml_size_unknown()).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
(cherry picked from commit 9b0f9003df)
For FLAC, Speex, Opus and VP8 the Ogg muxer allocates two buffers
for building the headers: The first for extradata in an Ogg-specific
format and the second contains a Vorbiscomment. These buffers are
reachable via pointers in the corresponding AVStream's priv_data.
If an error happens during building the headers, the AVStream's
priv_data would be freed. This is pointless in general as it would be
freed generically anyway, but here it is actively harmful: If the second
of the aforementioned allocations fails, the first buffer would leak
upon freeing priv_data.
This commit stops freeing priv_data manually, which allows the muxer to
properly clean up in the deinit function.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
(cherry picked from commit 0fcf74f435)
