Fixes: out of array access
Fixes: 69098/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG2VIDEO_fuzzer-6107989688778752
Fixes: 69599/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG4_fuzzer-4848626296225792.fuzz
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
Add check for av_packet_new_side_data() to avoid null pointer
dereference if allocation fails.
Fixes: bdc1220eeb ("h263enc: Add an option for outputting info about MBs as side data")
Signed-off-by: Jiasheng Jiang <jiashengjiangcool@outlook.com>
Signed-off-by: Anton Khirnov <anton@khirnov.net>
These must not be modified (even when they are initialized at runtime
and therefore modifiable).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
It is an MPEG-4-only value; it is always five for the MPEG-4
encoder, so just hardcode this value and move the MpegEncContext
field to Mpeg4DecContext.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This is beneficial for performance: When concatenating
the file from the vsynth1-h261 fate-test 100 times,
performance (measured by timing the codec's decode callback)
improved by 9.6%.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Calling it is the first thing ff_clean_h263_qscales() and
ff_clean_mpeg4_qscales() do anyway.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
A MECmpContext is quite big (792B here) and given
how ff_update_duplicate_context() works, it is (unfortunately)
copied quite frequently when using slice threading.
Therefore keep only what is needed from MECmpContext
and remove MECmpContext from MpegEncContext.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Several of the potential choices of comparison functions
need an initialized MpegEncContext (initialized for encoding,
not only ff_mpv_common_init()) or they crash when called.
Modify ff_set_cmp() to check for this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
MECmpContext.ildct_cmp is an array of function pointers that
are not set by ff_me_cmp_init(), but that are set by users
to one of the other arrays via ff_set_cmp().
Remove these pointers from MECmpContext and add pointers
for the actually used functions to its users. (The DV encoder
already did so.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
MECmpContext has several arrays of function pointers that
are not set by ff_me_cmp_init(), but that are set by users
to one of the other arrays via ff_set_cmp().
One of these other users is mpegvideo_enc; it is the only user
of MECmpContext.frame_skip_cmp and it only uses one of these
function pointers at all.
This commit therefore moves this function pointer to MpegEncContext;
and removes the array from MECmpContext.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The majority of the stuff performed in it needs to be done only
once; so factor it out into a function of its own to be called
in the user's init function.
Also avoid using MpegEncContext in it, to separate the two
a bit more.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Not all compare functions are implemented for all compare function
types. Therefore check for the existence of the used functions.
Fixes issue #10245.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
It has been added in 3a87ac948f,
but there was never an implementation different from the ordinary
dct_quantize of it.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Use the common approach whereby the _c versions are set first
and then (potentially) overwritten by the arch-specific ones
instead of calling the arch-specific code first, followed by
setting the function pointers that have not already been set.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The C version is set in ff_dct_encode_init(), yet the MIPS version
is set in dct_init() (in ff_mpv_common_init() and therefore also
for decoders). This commit fixes this inconsistency.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The H.264 decoder used reference to store its picture_structure
into it; yet it does not use mpegvideo any more since commit
2c54155407. Afterwards commit
629259bdb5 removed the last remnants.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This also rids us of the requirement to preserve the PutBitContext
in ff_update_duplicate_context().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Only set the ScratchpadContext and let the users that need it
(i.e. encoders) set the MotionEstContext stuff themselves.
Also add an explicit pointer to ScratchpadContext to point
to the allocated buffer so that none of the other scratchpad
pointers is singled out as being used for the allocations.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Given that MPVPictures are already directly shared between threads
in case of frame-threaded decoding, one can simply use it to
pass decoding progress information between threads. This allows
to avoid one level of indirection; it also means avoids allocations
(of the ThreadFrameProgress structure) in case of frame-threading
and indeed makes ff_thread_release_ext_buffer() decoder-only
(actually, H.264-decoder-only).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Up until now, an initialized MpegEncContext had an array of
MPVPictures (way more than were ever needed) and the MPVPicture*
contained in the MPVWorkPictures as well as the input_picture
and reordered_input_picture arrays (for the encoder) pointed
into this array. Several of the pointers could point to the
same slot and because there was no reference counting involved,
one had to check for aliasing before unreferencing.
Furthermore, given that these pointers were not ownership pointers
the pointers were often simply reset without unreferencing
the slot (happened e.g. for the RV30 and RV40 decoders) or
there were moved without resetting the src pointer (happened
for the encoders where the entries in the input_picture
and reordered_input_picture arrays were not reset).
Instead actually releasing these pictures was performed by looping
over the whole array and checking which one of the entries needed
to be kept. Given that the array had way too many slots (36),
this meant that more than 30 MPVPictures have been unnecessarily
unreferenced in every ff_mpv_frame_start(); something similar
happened for the encoder.
This commit changes this by making the MPVPictures refcounted
via the RefStruct API. The MPVPictures itself are part of a pool
so that this does not entail constant allocations; instead,
the amount of allocations actually goes down, because the
earlier code used such a large array of MPVPictures (36 entries) and
allocated an AVFrame for every one of these on every
ff_mpv_common_init(). In fact, the pool is only freed when closing
the codec, so that reinitializations don't lead to new allocations
(this avoids having to sync the pool in update_thread_context).
Making MPVPictures refcounted also has another key benefit:
It makes it possible to directly share them across threads
(when using frame-threaded decoding), eliminating ugly code
with underlying av_frame_ref()'s; sharing these pictures
can't fail any more.
The pool is allocated in ff_mpv_decode_init() for decoders,
which therefore can fail now. This and the fact that the pool
is not unreferenced in ff_mpv_common_end() also necessitated
to mark several mpegvideo-decoders with the FF_CODEC_CAP_INIT_CLEANUP
flag.
*: This also means that there is no good reason any more for
ff_mpv_common_frame_size_change() to exist.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
It already avoids a goto and will be useful in the future
to be able to specify each functions tasks and obligations.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
There are two types of MPVPictures: Three (cur_pic, last_pic, next_pic)
that are directly part of MpegEncContext and an array of MPVPictures
that are separately allocated and are mostly accessed via pointers
(cur|last|next)_pic_ptr; they are also used to store AVFrames in the
encoder (necessary due to B-frames). As the name implies, each of the
former is directly associated with one of the _ptr pointers:
They actually share the same underlying buffers, but the ones
that are part of the context can have their data pointers offset
and their linesize doubled for field pictures.
Up until now, each of these had their own references; in particular,
there was an underlying av_frame_ref() to sync cur_pic and cur_pic_ptr
etc. This is wasteful.
This commit changes this relationship: cur_pic, last_pic and next_pic
now become MPVWorkPictures; this structure does not have an AVFrame
at all any more, but only the cached values of data and linesize.
It also contains a pointer to the corresponding MPVPicture, establishing
a more natural relationsship between the two.
This already means that creating the context-pictures from the pointers
can no longer fail.
What has not been changed is the fact that the MPVPicture* pointers
are not ownership pointers and that the MPVPictures are part of an
array of MPVPictures that is owned by a single AVCodecContext.
Doing so will be done in a latter commit.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This way said function sets everything (except for the actual
contents of the frame's data). Also rename it to prepare_picture()
given its new role.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
ff_alloc_picture() currently does two things: It checks the
consistency of the linesize (which should not be necessary, but is)
and it allocates certain buffers. (It does not actually allocate
the picture buffers, so its name is misleading.)
This commit splits it into two separate functions. The rationale
for this is that for the encoders, every picture needs its linesizes
checked, but not every picture needs these extra buffers.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Specifically, add const to the pointed-to-type of pointers
that point to something static or that belong to last_pic
or next_pic (because modifying these might lead to data races).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Use the cached values from MpegEncContext.(cur|last|next)_pic
instead of the corresponding *_pic_ptr.
Also do the same in wmv2dec.c and mpegvideo_enc.c.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This avoids an indirection and is in preparation for removing
the AVFrame from MpegEncContext.(cur|last|next)_pic altogether.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
No mpegvideo based codec supports alpha.
While just at it, also make the define shorter.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
current_picture->cur_pic, last_picture->last_pic, similarly
for new_picture and next_picture.
Also rename the corresponding *_ptr fields.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This avoids constant allocations+frees and will also allow
to simply switch to the RefStruct API, thereby avoiding
the overhead of the AVBuffer API.
It also simplifies the code, because it removes the "needs_realloc"
field: It was added in 435c0b87d2,
before the introduction of the AVBuffer API: given that these buffers
may be used by different threads, they were not freed immediately
and instead were marked as being freed later by setting needs_realloc.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
ff_init_me() was being called after ff_update_duplicate_context(),
which caused the propagation of the initialization to other thread
contexts to be delayed by one frame.
In the case of mpeg4 (or flipflop_rounding), this would make the
hpel_put functions differ between the first thread (which would be
correctly initialized) and the other threads (which would be stale
from the previous frame).
current_picture is not changed after frame_start() at all
and it therefore does not need to be updated (i.e. copied to the
slice thread contexts) a second time.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Michael Niedermayer
Jiasheng Jiang
Andreas Rheinhardt
Ramiro Polla