The check for whether we can use the fast path to process
AC coefficients used the qscale value belonging to a different
slice; this worked in practice, because the predicted AC values
are zero in this case, so it does not matter whether we use
the fast or the slow path.
Fix this by checking for first_slice_line instead. This fixes all
the races in the encoding part of the vsynth*-mpeg4-thread tests
(and fixes them if no frame threading is in use for the decoding part).
(The left prediction check may use data from a different slice, too,
but said slice is always processed by the same thread, so that no race
can happen. Given that out-of-slice AC values are zero, it does not
matter whether we use the fast path or the slow path either.)
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Only used by the decoder now that the encoder solves
out-of-slice predictions by setting the out-of-slice
values appropriately.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Now that every slice (and therefore every thread) uses its own
DC array, one can simply set the out-of-slice DC values to its
expected values (this is not possible for the decoders due
to error resilience).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This fixes data races (which are UB) in the MPEG-4 and H.263+
encoder when predicting DC values; these encoders unconditionally
read values from the line above the current line and only check
lateron (via first_slice_line) whether said prediction can be used
at all. It will also allow to remove said checks (by setting the
entries to 1024 upon opening a new slice).
The vsynth{1,2,3,_lena}-mpeg4-thread FATE tests were affected by this:
https://fate.ffmpeg.org/report.cgi?slot=x86_64-archlinux-gcc-tsan-slices&time=20250613002615
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The chroma dc_val pointers are mostly unused (accesses use
dc_val[0] and block_index), so remove them.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Up until now, they are marked as dirty (filled with 1), meaning that
the entries are in need of a reset via ff_clean_intra_table_entries();
but actually, the entries are initialized to the state that
ff_clean_intra_table_entries() produces, so they can be marked
as non-dirty (i.e. filled with 0).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
They are currently allocated separately per slice; allocating them
jointly allows to avoid saving them in ff_update_duplicate_context().
The way it is done also avoids allocating ac_val for encoders that
don't need it (e.g. H.263 or H.263+ with AIC).
This entailed moving setting nb_slices to ff_mpv_init_context_frame()
which is called from ff_mpv_common_frame_size_change(). The resultant
nb_slices will always be one when called from a decoder using
ff_mpv_common_frame_size_change().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This is in preparation for removing removing ac_val[1]
and ac_val[2]. ff_mpeg4_clean_buffers() was the last user of them.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Said block will only be referenced by blocks from the same macroblock,
which will read the new AC values instead of the reset values
from this function.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The underlying element type has always a size of 32B,
so that the pointer is always at least 16B aligned.
Clang uses this to upgrade the (inlined) memset to
aligned stores. GCC doesn't (it does it only when
the alignment hint is provided via __builtin_assume_aligned).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
These are already mostly unused; not using them here is in
preparation for removing them altogether. Use block_index[4..5]
instead for addressing.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
All callers check the corresponding entry of MpegEncContext.mbintra_table
and if set (indicating that the intra tables might have been written to
when decodeing a intra MB, so that they are "dirty"), call
ff_clean_intra_table_entries(), which resets them to default values
and resets the mbintra_table entry. Move resetting to the callers
(via an inline function that also performs the checks). This currently
has the advantage that the additional load of the mbintra_table ptr
can be avoided. It will also allow to simplify
ff_clean_intra_table_entries() (by using block_index[4] and [5]).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Allocate them before the slice contexts, so that they are automatically
copied to the slice contexts. This avoids having to set them in a loop.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
This will allow to perform initializations between ff_mpv_common_init()
and ff_mpv_init_duplicate_contexts() that will be automatically
copied to the slice contexts.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The motion estimation code only uses block_index[0..3],
so only initialize that (just like estimate_motion_thread()
in mpegvideo_enc.c).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
It is not really needed, as none of the buffers allocated
in ff_mpv_common_init() are used lateron; in fact, ff_mpv_common_init()
is called with MpegEncContext.width and height equal to zero
(as well as encoding equal to zero, so that the error resilience
parts are initialized...), so that all the buffers are too small
anyway.
The call to ff_mpv_common_init() has been added in commit
f6774f905f in order to allocate
{current,last,new}_picture.f (an AVFrame). Yet this is unnecessary
since 7814dd77aa.
The only task that ff_mpv_common_init() does that may be used
is initializing the HpelDSPContext embedded in the MpegEncContext.
In fact, it was not initialized before
f6774f905f and not initializing
it passes FATE, yet I can't prove that it is indeed unnecessary.
Therefore it is still initialized and used instead of
SVQ1EncContext.hdsp.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Forgotten in 65015003f5c4b83a8202abfa7420ccf37cde6ce3;
after said commit, freeing scratchbuf on error in svq1_encode_frame()
could lead to segfaults lateron, because the buffer will not
be allocated again.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Given that the MPEG-1/2 decoders unquantize the blocks
on their own, block_last_index is only used to signal
to ff_mpv_reconstruct_mb() whether a block is skipped
or not; but this is only checked for inter macroblocks,
so it is unnecessary to set block_last_index for intra
macroblocks.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Overriding the number of coefficients is only necessary if
ac_pred is in used, not for h263_aic alone (for which only
the DC coefficient is predicted) as it is done here.
And since d50635cd24
the H.263 unquantize-intra functions override the number of
coefficients in case of ac_pred, so we don't have to do this here.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The version ident is printed on stdout for link.exe and redirecting
stdout to /dev/null will cause the output of link.exe to be paged.
This caused configure to hang for some configurations and by
extension some FATE clients. You might want to check if you run
affected configurations automated in FATE clients or similar setups.
Fixes: 45a30e0361
Signed-off-by: Martin Storsjö <martin@martin.st>
