On 3/24/2025 10:50 PM, Michael Niedermayer wrote:
HiOn Mon, Mar 24, 2025 at 07:20:50PM -0300, James Almer wrote:Continues from commit 702239bc500b, fixing FATE failures on MacOS. Signed-off-by: James Almer <jamr...@gmail.com> --- Confirmed by Martin Storsjö. Float encoding untested. libavcodec/ffv1.h | 16 ++++ libavcodec/ffv1enc.c | 177 +++++++++++++++++-------------------------- 2 files changed, 84 insertions(+), 109 deletions(-) diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h index 09118e0b7d..d1c239f138 100644 --- a/libavcodec/ffv1.h +++ b/libavcodec/ffv1.h @@ -115,6 +115,22 @@ typedef struct FFV1SliceContext { uint32_t val; //this is unneeded if you accept a dereference on each access uint16_t ndx; } unit[4][65536]; + struct RemapEncoderState { + int delta_stack[65536]; //We need to encode the run value before the adjustments, this stores the adjustments until we know the length of the run + int16_t index_stack[65537]; //only needed with multiple segments + uint8_t state[2][3][32]; + int mul[4096+1]; + RangeCoder rc; + int lu; + int run; + int64_t last_val; + int compact_index; + int mul_count; + int i; + int pixel_num; + int p; + int current_mul_index; + } remap_state; } FFV1SliceContext;please provide a link to the failure
Martin will have to do that. I can't seem to find any FATE instance failing, but he said it affected his OSX machines.
This makes the code increasingly ugly. i dont understand why this breaks fate, fate should not use any of the float code as none should be run in fate ATM. its also all under -strict -2 checks
It also surprised me, since these are functions that need to be called, unlike the fix in 702239bc500b which was in a function actually called by existing tests.
this is temporary data not needed outside float32 and not needed outside the remap table writing. we may need more than one such state. (if we dont use a heuristic but actually encode bruteforce / trial and error) t conflicts with all work i did today theres tons of unused memory. We ATM do 2 things in encode_float32_remap_segment() one is encoding the table the other is writing the remaped pixels into sc->bitmap by using unit[s.p][s.i].ndx sc->bitmap is unused before, unit[s.p][s.i].ndx unused afterwards the input image itself is also not used again half of fltmap32 is unused (thats 512kb alone here)
Yeah, ideally all this is allocated only when needed rather than unconditionally in the slice context. But i didn't go that far since i can't even reproduce this issue.
the code can be writen so it doesnt need the stack but just runs twice over the stuff (not sure how clean this would be but if you try _please_ do it on top of the patches i posted today, the code is simpler and less buggy after these patches But i really dont understand why fate fails in relation to code it never executes.
For the issue fixed in 702239bc500b, i guess it did attempt to reserve stack space even if it never used it. For this one? Beats me.
thx [...] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
