Re: [FFmpeg-devel] [PATCH] avcodec/ffv1enc: further reduce stack usage

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2674 bytes --]

Hi Martin

On Tue, Mar 25, 2025 at 11:20:52AM +0200, Martin Storsjö wrote:
> On Mon, 24 Mar 2025, James Almer wrote:
> 
> > On 3/24/2025 10:50 PM, Michael Niedermayer wrote:
> > > Hi
> > > 
> > > On 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 <jamrial@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.
> 
> I set up a couple of macOS FATE instances now, both which show the failure:
> https://fate.ffmpeg.org/history.cgi?slot=aarch64-apple-darwin-macos11
> https://fate.ffmpeg.org/history.cgi?slot=x86_64-apple-darwin-macos11

I wonder if strictly taken, this is a compiler bug, given the code is not
executed or reachable

Ill apply a fix that eliminates these 2 arrays entirely

thx


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".