Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avcodec/ffv1enc: further reduce stack usage
Date: Tue, 25 Mar 2025 02:50:53 +0100
Message-ID: <20250325015053.GU4991@pb2> (raw)
In-Reply-To: <20250324222051.19096-1-jamrial@gmail.com>


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

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

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

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)

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.

thx

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

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data

[-- 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".

  reply	other threads:[~2025-03-25  1:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-24 22:20 James Almer
2025-03-25  1:50 ` Michael Niedermayer [this message]
2025-03-25  1:57   ` James Almer
2025-03-25  2:26     ` Michael Niedermayer
2025-03-25  9:20     ` Martin Storsjö
2025-03-25  9:50       ` Michael Niedermayer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250325015053.GU4991@pb2 \
    --to=michael@niedermayer.cc \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git