[FFmpeg-devel] [PATCH 1/2] libavcodec/ffv1: Support storing decorrelated LSB raw without rangecoder

2025-03-06 Thread Michael Niedermayer
With 16bit float rawlsb 2 gives 0.66% better compression. This is maybe due
to the quantization tables being tuned to smaller number of bits

rawlsb 4 is about 30% faster than 0 and about 1% worse compression

Above was tested using ACES_OT_VWG_SampleFrames

Signed-off-by: Michael Niedermayer 
---
 libavcodec/ffv1.h |  4 
 libavcodec/ffv1_template.c| 19 ++-
 libavcodec/ffv1dec.c  | 16 ++--
 libavcodec/ffv1dec_template.c | 20 
 libavcodec/ffv1enc.c  | 30 --
 libavcodec/ffv1enc_template.c | 22 +++---
 6 files changed, 91 insertions(+), 20 deletions(-)

diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
index c23d64d54a4..189004f7981 100644
--- a/libavcodec/ffv1.h
+++ b/libavcodec/ffv1.h
@@ -85,10 +85,13 @@ typedef struct FFV1SliceContext {
 int slice_rct_by_coef;
 int slice_rct_ry_coef;
 int remap;
+int rawlsb;
 
 // RefStruct reference, array of MAX_PLANES elements
 PlaneContext *plane;
 PutBitContext pb;
+PutBitContext rawlsb_pb;
+GetBitContext rawlsb_gb;
 RangeCoder c;
 
 int ac_byte_count;   ///< number of bytes used for AC 
coding
@@ -146,6 +149,7 @@ typedef struct FFV1Context {
 int key_frame_ok;
 int context_model;
 int qtable;
+int rawlsb;
 
 int bits_per_raw_sample;
 int packed_at_lsb;
diff --git a/libavcodec/ffv1_template.c b/libavcodec/ffv1_template.c
index abb90a12e49..10206702ee8 100644
--- a/libavcodec/ffv1_template.c
+++ b/libavcodec/ffv1_template.c
@@ -30,24 +30,25 @@ static inline int RENAME(predict)(TYPE *src, TYPE *last)
 }
 
 static inline int RENAME(get_context)(const int16_t 
quant_table[MAX_CONTEXT_INPUTS][MAX_QUANT_TABLE_SIZE],
-  TYPE *src, TYPE *last, TYPE *last2)
+  TYPE *src, TYPE *last, TYPE *last2, int 
rawlsb)
 {
 const int LT = last[-1];
 const int T  = last[0];
 const int RT = last[1];
 const int L  = src[-1];
+const int rawoff = (1<> 1;
 
 if (quant_table[3][127] || quant_table[4][127]) {
 const int TT = last2[0];
 const int LL = src[-2];
-return quant_table[0][(L - LT) & MAX_QUANT_TABLE_MASK] +
-   quant_table[1][(LT - T) & MAX_QUANT_TABLE_MASK] +
-   quant_table[2][(T - RT) & MAX_QUANT_TABLE_MASK] +
-   quant_table[3][(LL - L) & MAX_QUANT_TABLE_MASK] +
-   quant_table[4][(TT - T) & MAX_QUANT_TABLE_MASK];
+return quant_table[0][(L - LT + rawoff >> rawlsb) & 
MAX_QUANT_TABLE_MASK] +
+   quant_table[1][(LT - T + rawoff >> rawlsb) & 
MAX_QUANT_TABLE_MASK] +
+   quant_table[2][(T - RT + rawoff >> rawlsb) & 
MAX_QUANT_TABLE_MASK] +
+   quant_table[3][(LL - L + rawoff >> rawlsb) & 
MAX_QUANT_TABLE_MASK] +
+   quant_table[4][(TT - T + rawoff >> rawlsb) & 
MAX_QUANT_TABLE_MASK];
 } else
-return quant_table[0][(L - LT) & MAX_QUANT_TABLE_MASK] +
-   quant_table[1][(LT - T) & MAX_QUANT_TABLE_MASK] +
-   quant_table[2][(T - RT) & MAX_QUANT_TABLE_MASK];
+return quant_table[0][(L - LT + rawoff >> rawlsb) & 
MAX_QUANT_TABLE_MASK] +
+   quant_table[1][(LT - T + rawoff >> rawlsb) & 
MAX_QUANT_TABLE_MASK] +
+   quant_table[2][(T - RT + rawoff >> rawlsb) & 
MAX_QUANT_TABLE_MASK];
 }
 
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index 3926659ebc9..0f2956eabf7 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -227,6 +227,7 @@ static int decode_slice_header(const FFV1Context *f,
 av_log(f->avctx, AV_LOG_ERROR, "unsupported remap %d\n", 
sc->remap);
 return AVERROR_INVALIDDATA;
 }
+sc->rawlsb = ff_ffv1_get_symbol(c, state, 0);
 }
 }
 
@@ -248,6 +249,7 @@ static int decode_slice(AVCodecContext *c, void *arg)
 FFV1Context *f= c->priv_data;
 FFV1SliceContext *sc = arg;
 int width, height, x, y, ret;
+int chroma_width, chroma_height;
 const int ps  = av_pix_fmt_desc_get(f->pix_fmt)->comp[0].step;
 AVFrame * const p = f->picture.f;
 const int  si = sc - f->slices;
@@ -284,6 +286,8 @@ static int decode_slice(AVCodecContext *c, void *arg)
 height = sc->slice_height;
 x  = sc->slice_x;
 y  = sc->slice_y;
+chroma_width  = AV_CEIL_RSHIFT(width,  f->chroma_h_shift);
+chroma_height = AV_CEIL_RSHIFT(height, f->chroma_v_shift);
 
 if (ac == AC_GOLOMB_RICE) {
 if (f->combined_version >= 0x30002)
@@ -293,11 +297,17 @@ static int decode_slice(AVCodecContext *c, void *arg)
   sc->c.bytestream_start + sc->ac_byte_count,
   (sc->c.bytestream_end - sc->c.bytestream_start - 
sc->ac_byte_count) * 8);
 }
+if (sc->rawlsb) {
+int lsb_size = sc->rawlsb * (width * height * (1 + f->transparency) + 
chrom

Re: [FFmpeg-devel] CC statement on alleged insults against the GSoC student et al

2025-03-06 Thread compn
On Tue, 4 Mar 2025 09:56:20 -0600, Kieran Kunhya via ffmpeg-devel wrote:

> >
> > Nevertheless, the CC does issue a warning regarding unnecessarily
> > offensive recent comments by Kieran on other topics, as well as for
> > initially failing to provide background and context regarding Paul’s
> > action on IRC.
> >  
> 
> Hi CC,
> 
> What comments are these? This is very vague as to what I've done.
> 
> Kieran

i agree.

if the cc is going to make decisions on developer behavior they
should at least tell the developer what their wrong behavior was.

otherwise said developer can never learn. i'm not saying this has to be
public, it can be private.

-compn
___
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".


Re: [FFmpeg-devel] [RFC] FFV1 float support

2025-03-06 Thread Michael Niedermayer
Hi Jerome

On Thu, Mar 06, 2025 at 10:20:58PM +0100, Jerome Martinez wrote:
> Le 06/03/2025 à 17:37, Michael Niedermayer a écrit :
> > On Thu, Mar 06, 2025 at 03:14:49AM +0100, Lynne wrote:
> > > On 06/03/2025 01:15, Michael Niedermayer wrote:
> > > > Hi everyone
> > > > 
> > > > Current FFv1 code with my patchset supports 16bit floats. The 
> > > > implementation
> > > > is quite simple. Which is good
> > > > 
> > > > I have tried improving compression, the gains are incremental, but its 
> > > > not
> > > > large overall. For example 44% space gain on the remapping table is just
> > > > 0.1% overall.
> > > > 
> > > > I have few float samples. Its mainly one high quality slideshow of 
> > > > unrelated
> > > > 16bit float images. These excercise a wide range fo things including 
> > > > negative
> > > > color values.
> > > > I think I have only one single (non slideshow) video of float 16 
> > > > samples,
> > > > 
> > > > It turns out the most efficient way to store floats that i found, is to 
> > > > remap
> > > > them to integers. Not to store tham as sign, exponent, mantisse, i 
> > > > tried that
> > > > for many hours.
> 
> I also tried some stuff but IMO the remap to integer is what is currently
> the best for keeping the spec simple & with a good compression, and a more
> complex way is not needed until we find something which really makes a
> difference in term of compression ratio.
> So let's keep it simple, just a remap to int, and we already get all the
> advantage of e.g. the incoming GPU implementation.

yes, i fully agree


> 
> 
> > > > [...]
> > > > What about the mapping itself, it uses a rather simple rle coder. ive 
> > > > spend
> > > > most of the day today tuning that and failing to really beat that.
> > > > Using context from the previous image didnt work with the slideshow 
> > > > material
> > > > i have nor that one video i have. I tried using sign and exponent as 
> > > > context,
> > > > tried previous run, relations of runs, low bits and many more, but no or
> > > > insignificant improvments of yesterdays implementation was achieved.
> > > I did mention this once before, but you should look into the Daala/Opus 
> > > way
> > > of storing rawbits into a separate chunk at the end of the bitstream. It
> > > avoids polluting the range context with equiprobable bits.
> > This may fit into my LSB work but that is not float specific and its not
> > needed for float support.
> > 
> > I originally thought we would do floats through RAW storing LSB, and special
> > handling the "always" 0 and 1 bits. It makes sense but just remapping only
> > the used values to a compact list eliminates all the constant bits for free
> > and we never have more than 16bits per sample
> > 
> > We still can implement storing LSB raw, i have written 2 implementations
> > the first without range coder was this:
> > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2024-October/334718.html
> > 
> > I didnt like it, but it seems more popular
> 
> I am still in favor of this simple and independent way to store the LSB
> bits, the other tentatives seem to only add complexity and performance
> issues while not providing enough gain.
> 
> 
> > 
> > Also given the raw bits are a known and constant length. It could make
> > sense to put them first if they are stored non interleaved.
> 
> Putting them last is better IMO, we can compute the byte offset with the
> constant length while storing some config in the slice header.
> Actually, there are many possibilities, I suggest:
> - permitting both MSB & LSB, because we may have MSB 0 and we would no store
> them (see below)
> - after the count of bits not compressed, we add a flag indicating if we
> store them or not (meaning they are 0)
> - permitting a configuration per stream (Configuration Record) or per slice
> (Slice Header); if a decision is to keep only one place for keeping it
> simple, it would be in the slice header.
> 
> The rational is that we may have:
> - 0 padding for the LSB is when we compress integer from DCP, we have input
> with 16-bit TIFF but only 12 relevant bits, and we need to keep the bit
> depth of 16-bit if we want to say that we do it lossless and that we can
> reverse, especially because 99.999% of frames are with 4 bits of padding but
> you may have a frame for whatever reason (often a bug, but if we do lossless
> we need to store buggy frames) which is not 0, so a performant storage of
> 0-filled LSB while permitting to store the actual bits if not 0.
> - 0 padding for the MSB is when we compress float mapped to integer and
> float range is from 0.0 to 1.0 only, sign & most of mantissa are always 0 so
> we can store them as a flag, but we prepare the stream to have values out of
> range (<0 or > 1).
> - and we don't know that before we start to compress each frame/slice

We have a generic remap table with the float code.
1. This is not really float specific, it could be used with integers
2. Its per slice, so each slice can have a differ

Re: [FFmpeg-devel] [RFC] FFV1 float support

2025-03-06 Thread Michael Niedermayer
Hi Traneptora

On Thu, Mar 06, 2025 at 01:17:30PM -0500, Leo Izen wrote:
> On 3/6/25 11:37 AM, Michael Niedermayer wrote:
> > Hi
> > > 
> > > I did mention this once before, but you should look into the Daala/Opus 
> > > way
> > > of storing rawbits into a separate chunk at the end of the bitstream. It
> > > avoids polluting the range context with equiprobable bits.
> > 
> > 
> > I didnt like it, but it seems more popular
> > 
> 
> May also be worth considering the JPEG XL way, where each entropy-coded
> symbol is a "hybrid integer" of a token read from the entropy stream (ANS)
> followed by zero or more "raw bits" that are spliced in.

Adding a 3rd entropy coder is outside the scope of implementing float support

If such a encoder makes sense, that should be a seperate thing.

And if you belive it performs better than what we have, submit a
patch and benchmarks. Please make sure only to use material free of
patent issues, aka things that predate patents in a way that invalidates
the patents

thx

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


signature.asc
Description: PGP 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".


Re: [FFmpeg-devel] [PATCH] Mark C globals with small code model

2025-03-06 Thread Pranav Kant via ffmpeg-devel
I think you were looking at an older version of the patch. Newer version
didn't have this. Anyhow, there's a new version I uploaded (v3).

On Thu, Feb 27, 2025 at 6:31 PM Lynne  wrote:

> On 25/02/2025 22:37, Pranav Kant via ffmpeg-devel wrote:
> > By default, all globals in C/C++ compiled by clang are allocated
> > in non-large data sections. See [1] for background on code models.
> > For PIC (Position independent code), this is fine as long as binary is
> > small but as binary size increases, users maybe want to use medium/large
> > code models (-mcmodel=medium) which moves data in to large sections.
> > As data in these large sections cannot be accessed using PIC code
> > anymore (as it may be too far away), compiler ends up using a different
> > instruction sequence when building C/C++ code -- using GOT to access
> > these globals (which can be relaxed by linker at link time if binary
> > ends up being smaller). However, assembly files continue to access these
> > globals defined in C/C++ files using older (and invalid instruction
> > sequence). So, we mark all such globals with an attribute that forces
> > them to be allocated in small sections allowing them to validly be
> > accessed from the assembly code.
> >
> > This patch should not have any affect on builds that use small code
> > model, which is the default mode.
> >
> > [1]
> https://eli.thegreenplace.net/2012/01/03/understanding-the-x64-code-models
> > ---
> >   libavcodec/ac3dsp.c| 2 +-
> >   libavcodec/cabac.c | 3 ++-
> >   libavcodec/x86/constants.c | 8 
> >   libavutil/attributes.h | 6 ++
> >   4 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/ac3dsp.c b/libavcodec/ac3dsp.c
> > index 730fa70fff..43b4fcbda9 100644
> > --- a/libavcodec/ac3dsp.c
> > +++ b/libavcodec/ac3dsp.c
> > @@ -104,7 +104,7 @@ static void ac3_update_bap_counts_c(uint16_t
> mant_cnt[16], uint8_t *bap,
> >   mant_cnt[bap[len]]++;
> >   }
> >
> > -DECLARE_ALIGNED(16, const uint16_t, ff_ac3_bap_bits)[16] = {
> > +av_mcmodel_small DECLARE_ALIGNED(16, const uint16_t,
> ff_ac3_bap_bits)[16] = {
> >   0,  0,  0,  3,  0,  4,  5,  6,  7,  8,  9, 10, 11, 12, 14, 16
> >   };
> >
> > diff --git a/libavcodec/cabac.c b/libavcodec/cabac.c
> > index 7d41cd2ae6..dfc3ba135a 100644
> > --- a/libavcodec/cabac.c
> > +++ b/libavcodec/cabac.c
> > @@ -24,12 +24,13 @@
> >* Context Adaptive Binary Arithmetic Coder.
> >*/
> >
> > +#include "libavutil/attributes.h"
> >   #include "libavutil/error.h"
> >   #include "libavutil/mem_internal.h"
> >
> >   #include "cabac.h"
> >
> > -DECLARE_ASM_ALIGNED(1, const uint8_t, ff_h264_cabac_tables)[512 +
> 4*2*64 + 4*64 + 63] = {
> > +av_mcmodel_small DECLARE_ASM_ALIGNED(1, const uint8_t,
> ff_h264_cabac_tables)[512 + 4*2*64 + 4*64 + 63] = {
> >   9,8,7,7,6,6,6,6,5,5,5,5,5,5,5,5,
> >   4,4,4,4,4,4,4,4,4,4,4,4,4,4,4,4,
> >   3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,
> > diff --git a/libavcodec/x86/constants.c b/libavcodec/x86/constants.c
> > index bc7f2b17b8..9a5af2871c 100644
> > --- a/libavcodec/x86/constants.c
> > +++ b/libavcodec/x86/constants.c
> > @@ -18,17 +18,21 @@
> >* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> >*/
> >
> > +#include "libavutil/attributes.h"
> >   #include "libavutil/mem_internal.h"
> >   #include "libavutil/x86/asm.h" // for xmm_reg
> >   #include "constants.h"
> >
> > +av_mcmodel_small
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_1)= {
> 0x0001000100010001ULL, 0x0001000100010001ULL,
> >
>  0x0001000100010001ULL, 0x0001000100010001ULL };
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_2)= {
> 0x0002000200020002ULL, 0x0002000200020002ULL,
> >
>  0x0002000200020002ULL, 0x0002000200020002ULL };
> >   DECLARE_ASM_ALIGNED(16, const xmm_reg,  ff_pw_3)= {
> 0x0003000300030003ULL, 0x0003000300030003ULL };
> > +av_mcmodel_small
> >   DECLARE_ASM_ALIGNED(32, const ymm_reg,  ff_pw_4)= {
> 0x0004000400040004ULL, 0x0004000400040004ULL,
> >
>  0x0004000400040004ULL, 0x0004000400040004ULL };
> > +av_mcmodel_small
> >   DECLARE_ASM_ALIGNED(16, const xmm_reg,  ff_pw_5)= {
> 0x0005000500050005ULL, 0x0005000500050005ULL };
> >   DECLARE_ALIGNED(16, const xmm_reg,  ff_pw_8)= {
> 0x0008000800080008ULL, 0x0008000800080008ULL };
> >   DECLARE_ASM_ALIGNED(16, const xmm_reg,  ff_pw_9)= {
> 0x0009000900090009ULL, 0x0009000900090009ULL };
> > @@ -49,6 +53,7 @@ DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_256)  = {
> 0x0100010001000100ULL, 0x010
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_512)  = {
> 0x0200020002000200ULL, 0x0200020002000200ULL,
> >
>  0x0200020002000200ULL, 0x0200020002000200ULL };
> >   DECLARE_ALIGNED(16, const xmm_reg,  ff_pw_1019) = {
> 0x03FB03FB03FB03FBULL, 0x03FB03FB03FB03FBULL };
> > +av_mcmodel_small
> >   DECLARE_ALIGNED(32, const ymm_reg,  ff_pw_1023) = {
> 0x03ff03ff03ff03ffULL, 0x03ff03ff03ff03ffULL,
> >
>  0x03ff03ff03ff03ffULL, 0x03ff03ff03ff03ffULL};
> >   DECLARE_ALI

Re: [FFmpeg-devel] [PATCH] Mark C globals with small code model

2025-03-06 Thread Pranav Kant via ffmpeg-devel
I uploaded a new patch (v3) that addresses these concerns.

On Thu, Feb 27, 2025 at 5:14 PM Michael Niedermayer 
wrote:

> On Wed, Feb 26, 2025 at 07:44:37PM +, Pranav Kant via ffmpeg-devel
> wrote:
> > By default, all globals in C/C++ compiled by clang are allocated
> > in non-large data sections. See [1] for background on code models.
> > For PIC (Position independent code), this is fine as long as binary is
> > small but as binary size increases, users maybe want to use medium/large
> > code models (-mcmodel=medium) which moves data in to large sections.
> > As data in these large sections cannot be accessed using PIC code
> > anymore (as it may be too far away), compiler ends up using a different
> > instruction sequence when building C/C++ code -- using GOT to access
> > these globals (which can be relaxed by linker at link time if binary
> > ends up being smaller). However, assembly files continue to access these
> > globals defined in C/C++ files using older (and invalid instruction
> > sequence). So, we mark all such globals with an attribute that forces
> > them to be allocated in small sections allowing them to validly be
> > accessed from the assembly code.
> >
> > This patch should not have any affect on builds that use small code
> > model, which is the default mode.
> >
> > [1]
> https://eli.thegreenplace.net/2012/01/03/understanding-the-x64-code-models
> >
> > Signed-off-by: Pranav Kant 
> > ---
> >  libavcodec/ac3dsp.c |  2 ++
> >  libavcodec/cabac.c  |  2 ++
> >  libavcodec/x86/constants.c  |  8 
> >  libavutil/attributes_internal.h | 15 +++
> >  4 files changed, 27 insertions(+)
>
> This produces many warnings:
>
> CC  libavcodec/svq1.o
> In file included from libavcodec/svq1.h:40,
>  from libavcodec/svq1.c:35:
> ./libavutil/attributes_internal.h:43:5: warning: "ARCH_X86_64" is not
> defined, evaluates to 0 [-Wundef]
>43 | #if ARCH_X86_64 && defined(__ELF__) && __has_attribute(model)
>   | ^~~
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If you fake or manipulate statistics in a paper in physics you will never
> get a job again.
> If you fake or manipulate statistics in a paper in medicin you will get
> a job for life at the pharma industry.
> ___
> 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".
>
___
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".


Re: [FFmpeg-devel] [PATCH v2 0/3] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Soft Works



> -Original Message-
> From: ffmpeg-devel  On Behalf Of Soft
> Works
> Sent: Donnerstag, 6. März 2025 18:05
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH v2 0/3] avutil/log: Replace addresses
> in log output with simple ids
> 
> 
> 
> > -Original Message-
> > From: ffmpeg-devel  On Behalf Of
> > Nicolas George
> > Sent: Donnerstag, 6. März 2025 17:43
> > To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH v2 0/3] avutil/log: Replace
> addresses
> > in log output with simple ids
> >
> > Soft Works (HE12025-03-06):
> > > It is the GitGitGadget system which does this this automatically.
> >
> > Your choice to use that thing, your responsibility to not let it
> > misbehave.
> >
> > > But I have removed you now and try to remove you again each time
> when
> > you reply.
> >
> > Do not Cc other people either if they did not ask for it.
> 
> Ok, I'll try to disable that behavior altogether.

About 60 patchsets have been submitted via this path already (not counting 
versions) and you are the first one complaining, that's why I haven't paid much 
attention on that part before.

The auto-cc feature is disabled now. Thanks for the feedback and apologies for 
the inconvenience.

sw





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


[FFmpeg-devel] The Concept for the CC Installment is broken by Design

2025-03-06 Thread Soft Works
Hello everybody,

this is not the kind of e-mail you might be expecting from the title. I think 
we had enough of those. Nobody will be blamed; nobody will be criticized. This 
is meant to address everybody equally, specifically not meant to be supportive 
of those who had brought up doubts about the CC before and it’s also not 
against those who believe the CC is a reasonable idea. Even less is it about 
any past or current members of the CC. The error is not on their side. 


The Community Committee Concept is broken by Design
===

A BUG REPORT



1. Principles of Operation

The core procedure in the CC operations is designed as follows:

- A community member X can file a complaint to the CC about 
  inappropriate behavior of another user Y
- The CC looks at the case and decides about whether to take certain actions
- The decision and the applied actions are made public

There is hardly any precedent for this type of procedure in democratic 
countries. It is a fundamental element of justice that a defendant knows who is 
suing him and that the trial is public. Anonymous tips only have a place when 
it comes to crimes, but not in civil law.
In companies and other organizations, there are sometimes procedures in place 
where hints and complaints can be made privately, but in those cases, these are 
managed and resolved privately, not publicly.

Private accusations followed by public punishment are more common in 
totalitarian systems.
Experience has shown that such systems lead to high distrust among people. 
Everybody needs to be careful and watching out continuously for making no 
mistakes, as anybody could denunciate you for something.


Evaluation in our context:

- Community Members:   => loses
 (causes bad atmosphere and public appearance)
- Complainer:  Neutral
  Privacy may sometimes protect the complainer, but in most cases it's   
obvious anyway, and playing/acting as if it wouldn't be the case, creates an 
  atmosphere of dishonesty on top of the trouble.



2. Expectations


So, you are upset about another community member that is very unfriendly to you 
or making accusations and the whole range of bad behavior (and maybe you did 
similar but think you did right and the other one did wrong).

You heard about the CC and you think it's really time that some action is being 
taken, so you file a complaint to the CC right away. 
But nothing happens. The CC is not like police that you can call. It's rather 
made up to work as a kind of court, working on one case after another. And when 
it gets to your case, you might either have forgotten about it already - or 
waiting really desperately for it to look into the case.
But why did you actually complain at the CC in the first place?
What do you want to achieve? That the other person changes mind? Unlikely to 
happen. The other person been given a formal warning? Sounds not much exciting 
neither resolving anything.

What you really want (almost always) is that someone officially says you were 
right and the other one wrong - which is unlikely to happen as the CC shall aim 
for equalization, not dividing.


Evaluation:   Case 1: CC is in favor of yours

- Defendant:  Gets a formal warning issued, 
  shrugs with his shoulders,
  will care about CC even less in the future
- Community:  => loses  
- Complainer: The CC didn't say you were right, 
  the defendant doesn't care about the warning
  You're frustrated, even though you won
  => loses


Evaluation:   Case 2: CC rejects your complaint 

- Complainer: Is more frustrated than ever about everything
  => loses



3. Blame


The fact that the CC is set up like a kind of court is one of the primary 
flaws. You cannot install a kind of court which doesn't have appropriate powers 
like a court has. Without such powers, nobody will ever respect it in the way 
that would be needed.

Also, a court cannot have judges elected by the community. Would you want the 
politicians that you elect be your judge on court? Or some of your friends 
suddenly being your judge?
Judges need to be neutral - ideally unknown and independent persons. Nobody in 
the ffmpeg community would qualify for such a position.
But that's what we have: elected community members which were keen and crazy 
enough to volunteer for such a position.

Evaluation

As a result of the CCs verdict on an issue between two members, there's usually 
a winning (even if it's a nuance, one would think to have "won").

Now, simple Math: 

- Winning Member: Might be happy for a short time
  Then realizing that the relationship with the
  losing member might have received irreparable damage
=> lose
- Losing Member:  Starts hating the Winning member
  Starts hating the CC and its members
  

Re: [FFmpeg-devel] The Concept for the CC Installment is broken by Design

2025-03-06 Thread Soft Works


> The core procedure in the CC operations is designed as follows:
> 
> - A community member X can file a complaint to the CC about
>   inappropriate behavior of another user Y

Erratum: the word "privately" got lost: "..can privately file.."
___
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".


Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Nicolas George
Soft Works (HE12025-03-05):
> Sorry. So - seriously: what would be your recipe then?

I see not just a little of non-trivial code for a very minor feature,
that might be a hint that it would be best to let it go.

Also, if somebody is debugging a program using the libraries, the
pointers are relevant for that program. For that reason, I think the
change is a bad idea in the library.

On the other hand, you could do that change in the fftools. The point
about pointers being relevant does not apply for them, and they can have
as much global state as they want.

-- 
  Nicolas George
___
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".


Re: [FFmpeg-devel] [PATCH v2 0/3] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Nicolas George
ffmpegagent (HE12025-03-05):
> Cc: softworkz , Soft Works
>  , Nicolas George ,
>  Gyan Doshi 

Please do not Cc people who did not ask for it. Especially when headers
say not to.

> Date: Wed, 05 Mar 2025 18:19:40 +
> From: ffmpegagent 
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v2 0/3] avutil/log: Replace addresses in log
>  output with simple ids

Sending a new version so soon when comments are still pending is a waste
of everybody's time.

-- 
  Nicolas George
___
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".


Re: [FFmpeg-devel] [PATCH 04/11] avutil/frame: av_frame_side_data_new_struct()

2025-03-06 Thread James Almer

On 3/6/2025 11:50 AM, Andreas Rheinhardt wrote:

Can't you add a new parameter nb_elems to also handle all the side data
that only has a single array? This would then also cover
AV_FRAME_DATA_REGIONS_OF_INTEREST, AV_FRAME_DATA_VIDEO_ENC_PARAMS,
AV_FRAME_DATA_DETECTION_BBOXES, AV_FRAME_DATA_VIDEO_HINT.
AV_FRAME_DATA_A53_CC, AV_FRAME_DATA_ICC_PROFILE,
AV_FRAME_DATA_SEI_UNREGISTERED, AV_FRAME_DATA_DOVI_RPU_BUFFER and
AV_FRAME_DATA_LCEVC.


I considered it, but then i noticed that VIDEO_ENC_PARAMS also has a 
type argument on its allocator, which even if can be worked around by 
setting the field in question manually after a 
av_frame_side_data_new_struct() call, in some more complex types in the 
future such an extra argument could affect what's allocated.


It's the reason i didn't ping or push this part of the set.



If you want to, I can write the patch for this.


If you can come up with an idea to work around the above (maybe 
type-specific structs with init params?), or consider the scenario is 
not worth the extra complexity and just ignore it, then sure.




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


[FFmpeg-devel] [PATCH v3 3/3] avcodec/exr: use luma+alpha float pixel formats

2025-03-06 Thread James Almer
Signed-off-by: James Almer 
---
 libavcodec/exr.c | 70 +++-
 tests/ref/fate/exr-ya-scanline-zip-half-12x8 |  2 +-
 2 files changed, 25 insertions(+), 47 deletions(-)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index b25e9ef397..4482f104d0 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1352,77 +1352,61 @@ static int decode_block(AVCodecContext *avctx, void 
*tdata,
 data_yoffset = FFABS(FFMIN(0, line));
 data_window_offset = (data_yoffset * td->channel_line_size) + data_xoffset;
 
+if (s->channel_offsets[3] >= 0)
+channel_buffer[3] = src + (td->xsize * s->channel_offsets[3]) + 
data_window_offset;
 if (!s->is_luma) {
 channel_buffer[0] = src + (td->xsize * s->channel_offsets[0]) + 
data_window_offset;
 channel_buffer[1] = src + (td->xsize * s->channel_offsets[1]) + 
data_window_offset;
 channel_buffer[2] = src + (td->xsize * s->channel_offsets[2]) + 
data_window_offset;
 rgb_channel_count = 3;
-} else { /* put y data in the first channel_buffer */
+} else { /* put y data in the first channel_buffer and if needed, alpha in 
the second */
 channel_buffer[0] = src + (td->xsize * s->channel_offsets[1]) + 
data_window_offset;
+if (!(s->desc->flags & AV_PIX_FMT_FLAG_PLANAR))
+channel_buffer[1] = channel_buffer[3];
 rgb_channel_count = 1;
 }
- if (s->channel_offsets[3] >= 0)
-channel_buffer[3] = src + (td->xsize * s->channel_offsets[3]) + 
data_window_offset;
 
-if (s->desc->flags & AV_PIX_FMT_FLAG_PLANAR || s->desc->nb_components == 1 
) {
-/* todo: change this when a floating point pixel format with luma with 
alpha is implemented */
-int channel_count = s->channel_offsets[3] >= 0 ? 4 : rgb_channel_count;
-if (s->is_luma) {
-channel_buffer[1] = channel_buffer[0];
-channel_buffer[2] = channel_buffer[0];
-}
-
-for (c = 0; c < channel_count; c++) {
+if (s->desc->flags & AV_PIX_FMT_FLAG_FLOAT) {
+for (c = 0; c < s->desc->nb_components; c++) {
 int plane = s->desc->comp[c].plane;
-ptr = p->data[plane] + window_ymin * p->linesize[plane] + 
(window_xmin * step);
+ptr = p->data[plane] + window_ymin * p->linesize[plane] + 
(window_xmin * step) + s->desc->comp[c].offset;
 
 for (i = 0; i < ysize; i++, ptr += p->linesize[plane]) {
-const uint8_t *src;
+const uint8_t *src = channel_buffer[c];
+uint8_t *ptr_x = ptr + window_xoffset * step;
+
+// Zero out the start if xmin is not 0
+if (s->desc->flags & AV_PIX_FMT_FLAG_PLANAR || !c)
+memset(ptr, 0, bxmin);
 
 if (s->pixel_type == EXR_FLOAT ||
 s->compression == EXR_DWAA ||
 s->compression == EXR_DWAB) {
 // 32-bit
-uint8_t *ptr_x = ptr;
-
-src = channel_buffer[c];
-
-// Zero out the start if xmin is not 0
-memset(ptr_x, 0, bxmin);
-ptr_x += 4 * window_xoffset;
-
-if (trc_func && c < 3) {
-for (int x = 0; x < xsize; x++, ptr_x += 4) {
+if (trc_func && (!c || (c < 3 && s->desc->flags & 
AV_PIX_FMT_FLAG_PLANAR))) {
+for (int x = 0; x < xsize; x++, ptr_x += step) {
 float f = av_int2float(bytestream_get_le32(&src));
 AV_WN32A(ptr_x, av_float2int(trc_func(f)));
 }
 } else if (one_gamma != 1.f) {
-for (int x = 0; x < xsize; x++, ptr_x += 4) {
+for (int x = 0; x < xsize; x++, ptr_x += step) {
 float f = av_int2float(bytestream_get_le32(&src));
 if (f > 0.0f && c < 3)  /* avoid negative values */
 f = powf(f, one_gamma);
 AV_WN32A(ptr_x, av_float2int(f));
 }
 } else {
-for (int x = 0; x < xsize; x++, ptr_x += 4)
+for (int x = 0; x < xsize; x++, ptr_x += step)
 AV_WN32A(ptr_x, bytestream_get_le32(&src));
 }
-memset(ptr_x, 0, axmax);
 } else if (s->pixel_type == EXR_HALF) {
-src = channel_buffer[c];
-
-// Zero out the start if xmin is not 0
-memset(ptr, 0, bxmin);
-
 // 16-bit
-for (x = window_xoffset; x < xsize + window_xoffset; x++) {
-int v = bytestream_get_le16(&src);
-AV_WN16(ptr + x * sizeof(uint16_t), v);
-}
-  

Re: [FFmpeg-devel] Misc mpegvideo patches

2025-03-06 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Ramiro Polla:
>> On Tue, Mar 4, 2025 at 6:05 PM Andreas Rheinhardt
>>  wrote:
>>> Ramiro Polla:

 On 3/4/25 14:42, Andreas Rheinhardt wrote:
> (Mostly trivial) patches attached. A branch is at
> https://github.com/mkver/FFmpeg/tree/mpegvideo_misc


 [PATCH 10/40] avcodec/mpegvideo_enc: Move default_mv_penalty to h261enc.c

> diff --git a/libavcodec/h261enc.c b/libavcodec/h261enc.c
> index dabab9d80a..e33bf35a8a 100644
> --- a/libavcodec/h261enc.c
> +++ b/libavcodec/h261enc.c
> @@ -46,6 +46,7 @@ static struct VLCLUT {
>  uint16_t code;
>  } vlc_lut[H261_MAX_RUN + 1][32 /* 0..2 * H261_MAX_LEN are used */];
>
> +static uint8_t mv_penalty[MAX_FCODE + 1][MAX_DMV * 2 + 1];
>  static uint8_t uni_h261_rl_len [64 * 128];
>  static uint8_t uni_h261_rl_len_last[64 * 128];
>  static uint8_t h261_mv_codes[64][2];
> @@ -370,6 +371,8 @@ av_cold int ff_h261_encode_init(MpegEncContext *s)
>  s->max_qcoeff   = 127;
>  s->ac_esc_length= H261_ESC_LEN;
>
> +s->me.mv_penalty = mv_penalty;
> +
>  s->intra_ac_vlc_length  = s->inter_ac_vlc_length  =
> uni_h261_rl_len;
>  s->intra_ac_vlc_last_length = s->inter_ac_vlc_last_length =
> uni_h261_rl_len_last;
>  ff_thread_once(&init_static_once, h261_encode_init_static);

 This global mv_penalty doesn't seem to be ever initialized; it could be
 declared const.
>>>
>>> But then it would no longer be placed in .bss, but instead in .rodata
>>> and increase binary size.
>>
>> Wow, that's a huge array.
>>
 But it also makes me think that whatever code is using this mv_penalty,
 which is always set to zero, might be calculating things wrong.

>>>
>>> It is obviously done to avoid branches for the codecs that matter. H.261
>>> does not matter much. Apart from that, it is a very cheap workaround
>>> given that this table is .bss.
>>
>> Could you add some comments (either next to the declaration or the
>> commit message) to reflect this? (save space from .rodata, and this
>> being a noop for h.261, which doesn't matter that much)
>>
 [PATCH 15/40] avcodec/ituh263enc: Make SVQ1+Snowenc stop calling
 ff_h263_encode_init()

> diff --git a/libavcodec/ituh263enc.c b/libavcodec/ituh263enc.c
> index 02da090ba4..8313b2c2c1 100644
> --- a/libavcodec/ituh263enc.c
> +++ b/libavcodec/ituh263enc.c
> @@ -65,6 +65,127 @@ static uint8_t  uni_h263_inter_rl_len [64*64*2*2];
 [...]
> +static av_cold void h263_encode_init_static(void)
> +{
> +static uint8_t rl_intra_table[2][2 * MAX_RUN + MAX_LEVEL + 3];
> +
> +ff_rl_init(&ff_rl_intra_aic, rl_intra_table);
> +ff_h263_init_rl_inter();
> +
> +init_uni_h263_rl_tab(&ff_rl_intra_aic,  uni_h263_intra_aic_rl_len);
> +init_uni_h263_rl_tab(&ff_h263_rl_inter, uni_h263_inter_rl_len);
> +
> +init_mv_penalty_and_fcode();
> +}
> +
> +av_cold const uint8_t (*ff_h263_get_mv_penalty(void))[MAX_DMV*2+1]
> +{
> +static AVOnce init_static_once = AV_ONCE_INIT;
> +
> +ff_thread_once(&init_static_once, h263_encode_init_static);
> +
> +return mv_penalty;
> +}
> +

 This approach kind of hides the rest of h263_encode_init_static() inside
 ff_h263_get_mv_penalty(), so the name is a bit misleading. I'd expect
 h263 to still call some init function and ff_h263_get_mv_penalty(), and
 SVQ1 and Snow to only call ff_h263_get_mv_penalty(), which would only
 take care of the mv_penalty table.

>>>
>>> This would entail using another AVOnce etc. and this level of
>>> granularity is just not worth it.
>>
>> Ok.
>>
>>> The name is chosen for what it does for an outsider (i.e. from the
>>> perspective of svq1enc or snowenc, not the actual H.263 based encoders).
>>
>> I'm still not quite happy with the name and how it's used, but it's
>> good enough so I won't insist.
>>
 [PATCH 20/40] avcodec/mpeg4video: Split ff_mpeg4_pred_dc()

> diff --git a/libavcodec/mpeg4videoenc.c b/libavcodec/mpeg4videoenc.c
> index 64fb96a0cf..26f9b40ff7 100644
> --- a/libavcodec/mpeg4videoenc.c
> +++ b/libavcodec/mpeg4videoenc.c
> @@ -806,8 +806,14 @@ void ff_mpeg4_encode_mb(MpegEncContext *s,
> int16_t block[6][64],
>  const uint8_t *scan_table[6];
>  int i;
>
> -for (i = 0; i < 6; i++)
> -dc_diff[i] = ff_mpeg4_pred_dc(s, i, block[i][0], &dir[i],
> 1);
> +for (int i = 0; i < 6; i++) {

 Redeclaring i inside for.
>>>
>>> There are other loops which use this i as loop variable. The shadowing
>>> is IMO less bad than keeping loops in their current form (with iterators
>>> that don't have loop-scope).
>>
>> Agreed. I also prefer scoped iterators.
> 
> I added a comment to #10 and modified #18 as described. I also changed

[FFmpeg-devel] [PATCH 1/2] avcodec/sbcenc: Mark sbc_encode_init() as av_cold

2025-03-06 Thread Andreas Rheinhardt
Patches attached

- Andreas
From 7453c8ca973d886914cba7535a1a0835e9ae4f81 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt 
Date: Thu, 6 Mar 2025 16:00:12 +0100
Subject: [PATCH 1/2] avcodec/sbcenc: Mark sbc_encode_init() as av_cold

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/sbcenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/sbcenc.c b/libavcodec/sbcenc.c
index f2c4fbe329..e2f84f7dfc 100644
--- a/libavcodec/sbcenc.c
+++ b/libavcodec/sbcenc.c
@@ -194,7 +194,7 @@ static size_t sbc_pack_frame(AVPacket *avpkt, struct sbc_frame *frame,
 return put_bytes_output(&pb);
 }
 
-static int sbc_encode_init(AVCodecContext *avctx)
+static av_cold int sbc_encode_init(AVCodecContext *avctx)
 {
 SBCEncContext *sbc = avctx->priv_data;
 struct sbc_frame *frame = &sbc->frame;
-- 
2.45.2

From a200651a7e28e17673ea3638435eefb5e004c7cb Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt 
Date: Thu, 6 Mar 2025 16:02:16 +0100
Subject: [PATCH 2/2] avcodec/sbcenc: Don't use deprecated
 AVCodec.supported_samplerates

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/sbcenc.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libavcodec/sbcenc.c b/libavcodec/sbcenc.c
index e2f84f7dfc..6d23f19f62 100644
--- a/libavcodec/sbcenc.c
+++ b/libavcodec/sbcenc.c
@@ -49,6 +49,8 @@ typedef struct SBCEncContext {
 DECLARE_ALIGNED(SBC_ALIGN, SBCDSPContext, dsp);
 } SBCEncContext;
 
+static const int sbc_samplerates[] = { 16000, 32000, 44100, 48000, 0 };
+
 static int sbc_analyze_audio(SBCDSPContext *s, struct sbc_frame *frame)
 {
 int ch, blk;
@@ -260,8 +262,8 @@ static av_cold int sbc_encode_init(AVCodecContext *avctx)
 avctx->frame_size = 4*((frame->subbands >> 3) + 1) * 4*(frame->blocks >> 2);
 }
 
-for (int i = 0; avctx->codec->supported_samplerates[i]; i++)
-if (avctx->sample_rate == avctx->codec->supported_samplerates[i])
+for (int i = 0; sbc_samplerates[i]; i++)
+if (avctx->sample_rate == sbc_samplerates[i])
 frame->frequency = i;
 
 frame->channels = avctx->ch_layout.nb_channels;
@@ -359,7 +361,7 @@ const FFCodec ff_sbc_encoder = {
  { 0 } },
 .p.sample_fmts = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16,
  AV_SAMPLE_FMT_NONE },
-.p.supported_samplerates = (const int[]) { 16000, 32000, 44100, 48000, 0 },
+.p.supported_samplerates = sbc_samplerates,
 .p.priv_class  = &sbc_class,
 .p.profiles= NULL_IF_CONFIG_SMALL(ff_sbc_profiles),
 };
-- 
2.45.2

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


Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Marvin Scholz


On 6 Mar 2025, at 20:27, Soft Works wrote:

>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of Marvin
>> Scholz
>> Sent: Donnerstag, 6. März 2025 19:59
>> To: FFmpeg development discussions and patches 
>> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log
>> output with simple ids
>>
>>
>>
>> On 6 Mar 2025, at 19:16, Soft Works wrote:
>>
 -Original Message-
 From: ffmpeg-devel  On Behalf Of
>> Marvin
 Scholz
 Sent: Donnerstag, 6. März 2025 18:49
 To: FFmpeg development discussions and patches > de...@ffmpeg.org>
 Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in
>> log
 output with simple ids



 On 6 Mar 2025, at 18:44, Soft Works wrote:

>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of
 Marvin
>> Scholz
>> Sent: Donnerstag, 6. März 2025 18:38
>> To: FFmpeg development discussions and patches >>> de...@ffmpeg.org>
>> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses
>> in
 log
>> output with simple ids
>>
>>
>>
>> On 6 Mar 2025, at 18:02, Soft Works wrote:
>>
 -Original Message-
 From: ffmpeg-devel  On Behalf Of
 Nicolas George
 Sent: Donnerstag, 6. März 2025 11:09
 To: FFmpeg development discussions and patches > de...@ffmpeg.org>
 Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses
>
> [..]
>
>>
>> First of all I want to say I like the idea of having cleaner logs,
>> but...
>>
>> IMHO "complex" logging formatting should be handled by fftools
>> especially if
>> they need global state. Even though thats not the case right now,
>> but
>> just
>> like Nicolas I also would prefer to not add even more global state
 for
>> logging
>> to the library...
>>
>> All the fancy log formatting should be done in a log callback in
>> the
>> fftools and the default library logging callback should just be a
 very
>> basic
>> one, is my opinion on this.
>
> That's all fine and probably reasonable. But is it fair to block a
 small change because some major rework would be desired at some
>> point?
>
> When that change will be made, it will of course move out this
>> little
 change as well.
>
> But are you really saying that this small change cannot be made
 because you don't like the general way of the current implementation?
>

 Just to be clear, I am not blocking this, just wanted to give my
 perspective on the topic.
 So if others think its fine and want it, lets go for it.
>>>
>>> Thanks - and sorry for my retardation, I just realized why it's bad to
>> do it this way with regards to library usage. I'll add a callback so
>> that fftools can do the prefix formatting.
>>>
>>> For the idea of moving all the formatting to fftools, wouldn't this be
>> a major breakage for library consumers who are used to getting the log
>> output formatted like now?
>>>
>>
>> Yeah, one of the reasons why I did not really do any work on this yet as
>> I am really
>> not sure whats the best way to go about this to not be too surprising
>> for existing
>> library users...
>
> I think, people _do_ want the formatting even when using the libs directly, 
> so while separating formatting from the plain logging might make sense, it 
> would probably still need to be in avutil - otherwise most consumers would 
> probably be annoyed and have to copy the formatting code from fftools to 
> their own code base (or similar) - which wouldn't be a win for anybody.

Right, I do think it might be useful to have helpers to make log formatting 
easier for library users (especially because as the final
consumer you can then decide where to put some eventual state needed), and 
maybe also some to make it easier to obtain logging details from
AVClass as this is right now somewhat hard to get right and I had to check the 
source code last time to figure out how its done.

> I see why the global state is bad with regards to this patchset's feature 
> (and V3 will remedy), but avoidance of global state would be much easier and 
> also make more sense if there was some kind of "local state" as a 
> replacement, so that people could for example have separate log outputs when 
> performing separate and independent operations.
> I suppose that's not easy to achieve with the current architecture?

Yeah, I think the end goal should be to have a way to not have a global log 
callback, however that would require quite a bit of
redesign. (This is especially useful if you run independent FFmpeg tasks in 
different threads and want to get separate logs for
each of them.)

A sort of hacky way right now to do this is use of thread local storage and 
then "demultiplex" the messages in the log callback
based on that. However that only works as long as the message does not come 
fro

Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of Marvin
> Scholz
> Sent: Donnerstag, 6. März 2025 21:02
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log
> output with simple ids
> 
> 
> 
> On 6 Mar 2025, at 20:27, Soft Works wrote:
> 
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> Marvin
> >> Scholz
> >> Sent: Donnerstag, 6. März 2025 19:59
> >> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in
> log
> >> output with simple ids
> >>
> >>
> >>
> >> On 6 Mar 2025, at 19:16, Soft Works wrote:
> >>
>  -Original Message-
>  From: ffmpeg-devel  On Behalf Of
> >> Marvin
>  Scholz
>  Sent: Donnerstag, 6. März 2025 18:49
>  To: FFmpeg development discussions and patches  >> de...@ffmpeg.org>
>  Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses
> in
> >> log
>  output with simple ids
> 
> 
> 
>  On 6 Mar 2025, at 18:44, Soft Works wrote:
> 
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
>  Marvin
> >> Scholz
> >> Sent: Donnerstag, 6. März 2025 18:38
> >> To: FFmpeg development discussions and patches   de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses
> >> in
>  log
> >> output with simple ids
> >>
> >>
> >>
> >> On 6 Mar 2025, at 18:02, Soft Works wrote:
> >>
>  -Original Message-
>  From: ffmpeg-devel  On Behalf
> Of
>  Nicolas George
>  Sent: Donnerstag, 6. März 2025 11:09
>  To: FFmpeg development discussions and patches  >> de...@ffmpeg.org>
>  Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace
> addresses
> >
> > [..]
> >
> >>
> >> First of all I want to say I like the idea of having cleaner
> logs,
> >> but...
> >>
> >> IMHO "complex" logging formatting should be handled by fftools
> >> especially if
> >> they need global state. Even though thats not the case right now,
> >> but
> >> just
> >> like Nicolas I also would prefer to not add even more global
> state
>  for
> >> logging
> >> to the library...
> >>
> >> All the fancy log formatting should be done in a log callback in
> >> the
> >> fftools and the default library logging callback should just be a
>  very
> >> basic
> >> one, is my opinion on this.
> >
> > That's all fine and probably reasonable. But is it fair to block a
>  small change because some major rework would be desired at some
> >> point?
> >
> > When that change will be made, it will of course move out this
> >> little
>  change as well.
> >
> > But are you really saying that this small change cannot be made
>  because you don't like the general way of the current
> implementation?
> >
> 
>  Just to be clear, I am not blocking this, just wanted to give my
>  perspective on the topic.
>  So if others think its fine and want it, lets go for it.
> >>>
> >>> Thanks - and sorry for my retardation, I just realized why it's bad
> to
> >> do it this way with regards to library usage. I'll add a callback so
> >> that fftools can do the prefix formatting.
> >>>
> >>> For the idea of moving all the formatting to fftools, wouldn't this
> be
> >> a major breakage for library consumers who are used to getting the
> log
> >> output formatted like now?
> >>>
> >>
> >> Yeah, one of the reasons why I did not really do any work on this yet
> as
> >> I am really
> >> not sure whats the best way to go about this to not be too surprising
> >> for existing
> >> library users...
> >
> > I think, people _do_ want the formatting even when using the libs
> directly, so while separating formatting from the plain logging might
> make sense, it would probably still need to be in avutil - otherwise
> most consumers would probably be annoyed and have to copy the formatting
> code from fftools to their own code base (or similar) - which wouldn't
> be a win for anybody.
> 
> Right, I do think it might be useful to have helpers to make log
> formatting easier for library users (especially because as the final
> consumer you can then decide where to put some eventual state needed),
> and maybe also some to make it easier to obtain logging details from
> AVClass as this is right now somewhat hard to get right and I had to
> check the source code last time to figure out how its done.
> 
> > I see why the global state is bad with regards to this patchset's
> feature (and V3 will remedy), but avoidance of global state would be
> much easier and also make more sense if there was some kind of "local
> state" as a replacement, so that people could for example have separate
> log outputs when performing separate and independent operations.
> > I sup

Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Soft Works



> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Nicolas George
> Sent: Donnerstag, 6. März 2025 11:09
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log
> output with simple ids
> 
> Soft Works (HE12025-03-05):
> > Sorry. So - seriously: what would be your recipe then?
> 
> I see not just a little of non-trivial code for a very minor feature,
> that might be a hint that it would be best to let it go.
> 
> Also, if somebody is debugging a program using the libraries, the
> pointers are relevant for that program. For that reason, I think the
> change is a bad idea in the library.
> 
> On the other hand, you could do that change in the fftools. The point
> about pointers being relevant does not apply for them, and they can have
> as much global state as they want.

HI Nicolas,

initially I failed to see the impact of the array being global, I tend to 
forget about direct usages of the libs, sorry about that.


V3 of the patchset goes the route you are suggesting by introducing a callback 
for formatting of the context prefixes, so the global state lives in fftools 
only.

Thanks
sw
___
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".


[FFmpeg-devel] [PATCH v3 4/4] doc/fftools-common-opts: document memaddresses log flag

2025-03-06 Thread softworkz
From: softworkz 

---
 doc/fftools-common-opts.texi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/fftools-common-opts.texi b/doc/fftools-common-opts.texi
index f6d452c40e..756c843c02 100644
--- a/doc/fftools-common-opts.texi
+++ b/doc/fftools-common-opts.texi
@@ -230,6 +230,8 @@ log to file.
 Indicates that log lines should be prefixed with time information.
 @item datetime
 Indicates that log lines should be prefixed with date and time information.
+@item memaddresses
+Indicates that context prefixes should be printed with memory addresses rather 
than logical ids.
 @end table
 Flags can also be used alone by adding a '+'/'-' prefix to set/reset a single
 flag without affecting other @var{flags} or changing @var{loglevel}. When
-- 
ffmpeg-codebot
___
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".


Re: [FFmpeg-devel] [RFC] FFV1 float support

2025-03-06 Thread Jerome Martinez

Le 06/03/2025 à 19:17, Leo Izen a écrit :
May also be worth considering the JPEG XL way, where each 
entropy-coded symbol is a "hybrid integer" of a token read from the 
entropy stream (ANS) followed by zero or more "raw bits" that are 
spliced in.


I would keep FFV1 away from ANS in the next 15 years due to
https://en.wikipedia.org/wiki/Asymmetric_numeral_systems#Patent_controversy
___
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".


Re: [FFmpeg-devel] [PATCH 1/1] Add vpx range encoder support

2025-03-06 Thread Peter Ross
On Wed, Mar 05, 2025 at 09:59:34PM +0530, MihirGore wrote:
> From: MihirGore23 
>

Hi,

comments below.
 
> ---
>  libavcodec/vpx_rac.h | 75 ++--
>  1 file changed, 59 insertions(+), 16 deletions(-)
> 
> diff --git a/libavcodec/vpx_rac.h b/libavcodec/vpx_rac.h
> index b158cc0754..f01358f71f 100644
> --- a/libavcodec/vpx_rac.h
> +++ b/libavcodec/vpx_rac.h
> @@ -20,7 +20,7 @@
>  
>  /**
>   * @file
> - * Common VP5-VP9 range decoder stuff
> + * Common VP5-VP9 range encoder and decoder functions
>   */
>  
>  #ifndef AVCODEC_VPX_RAC_H
> @@ -34,24 +34,25 @@
>  
>  typedef struct VPXRangeCoder {
>  int high;
> -int bits; /* stored negated (i.e. negative "bits" is a positive number of
> - bits left) in order to eliminate a negate in cache 
> refilling */
> +int bits; 
^
there is some some unnecessary whitespace after 'bits;'
also why did you delete the original comment?

>  const uint8_t *buffer;
>  const uint8_t *end;
>  unsigned int code_word;
>  int end_reached;
> +uint8_t *output_buffer; // Added for encoding
> +uint8_t *output_end;// Added for encoding
>  } VPXRangeCoder;
>  
>  extern const uint8_t ff_vpx_norm_shift[256];
> +
> +/*Decoder Functions */
> +
>  int ff_vpx_init_range_decoder(VPXRangeCoder *c, const uint8_t *buf, int 
> buf_size);
>  
> -/**
> - * returns 1 if the end of the stream has been reached, 0 otherwise.
> - */
>  static av_always_inline int vpx_rac_is_end(VPXRangeCoder *c)
>  {
>  if (c->end <= c->buffer && c->bits >= 0)
> -c->end_reached ++;
> +c->end_reached++;
>  return c->end_reached > 10;
>  }
>  
> @@ -64,7 +65,7 @@ static av_always_inline unsigned int 
> vpx_rac_renorm(VPXRangeCoder *c)
>  c->high   <<= shift;
>  code_word <<= shift;
>  bits   += shift;
> -if(bits >= 0 && c->buffer < c->end) {
> +if (bits >= 0 && c->buffer < c->end) {
>  code_word |= bytestream_get_be16(&c->buffer) << bits;
>  bits -= 16;
>  }
> @@ -72,12 +73,6 @@ static av_always_inline unsigned int 
> vpx_rac_renorm(VPXRangeCoder *c)
>  return code_word;
>  }
>  
> -#if   ARCH_ARM
> -#include "arm/vpx_arith.h"
> -#elif ARCH_X86
> -#include "x86/vpx_arith.h"
> -#endif
> -
>  #ifndef vpx_rac_get_prob
>  #define vpx_rac_get_prob vpx_rac_get_prob
>  static av_always_inline int vpx_rac_get_prob(VPXRangeCoder *c, uint8_t prob)
> @@ -95,7 +90,6 @@ static av_always_inline int vpx_rac_get_prob(VPXRangeCoder 
> *c, uint8_t prob)
>  #endif
>  
>  #ifndef vpx_rac_get_prob_branchy
> -// branchy variant, to be used where there's a branch based on the bit 
> decoded
>  static av_always_inline int vpx_rac_get_prob_branchy(VPXRangeCoder *c, int 
> prob)
>  {
>  unsigned long code_word = vpx_rac_renorm(c);
> @@ -117,7 +111,6 @@ static av_always_inline int 
> vpx_rac_get_prob_branchy(VPXRangeCoder *c, int prob)
>  static av_always_inline int vpx_rac_get(VPXRangeCoder *c)
>  {
>  unsigned int code_word = vpx_rac_renorm(c);
> -/* equiprobable */
>  int low = (c->high + 1) >> 1;
>  unsigned int low_shift = low << 16;
>  int bit = code_word >= low_shift;
> @@ -132,4 +125,54 @@ static av_always_inline int vpx_rac_get(VPXRangeCoder *c)
>  return bit;
>  }
>  
> +//Encoder Functions
> +
> +int ff_vpx_init_range_encoder(VPXRangeCoder *c, uint8_t *buf, int buf_size);
> +

ff_vpx_init_range_encoder function is not defined anywhere.
did you forget to commit vpx_rac.c changes?

> +static av_always_inline void vpx_rac_flush(VPXRangeCoder *c)
> +{
> +for (int i = 0; i < 4; i++) {
> +if (c->output_buffer < c->output_end) {
> +*c->output_buffer++ = c->code_word >> 24;
> +}
> +c->code_word <<= 8;
> +}
> +}

after calling vpx_rac_flush() how does the caller determine the final size
of the output buffer?

finally, does the output of your encoder work with the existing decoder?
simple test case:

```
VPXRangeCoder e;
uint8_t buf[1024] = {0};
ff_vpx_init_range_encoder(&e, buf, sizeof(buf));
vpx_rac_put(&e, 1);
vpx_rac_put(&e, 0);
vpx_rac_put(&e, 1);
vpx_rac_flush(&e);

VPXRangeCoder d;
ff_vpx_init_range_decoder(&d, buf, e.output_buffer - buf);
printf("%d\n", vpx_rac_get(&d));
printf("%d\n", vpx_rac_get(&d));
printf("%d\n", vpx_rac_get(&d));
```

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)


signature.asc
Description: PGP 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".


Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of Marvin
> Scholz
> Sent: Donnerstag, 6. März 2025 18:49
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log
> output with simple ids
> 
> 
> 
> On 6 Mar 2025, at 18:44, Soft Works wrote:
> 
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> Marvin
> >> Scholz
> >> Sent: Donnerstag, 6. März 2025 18:38
> >> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in
> log
> >> output with simple ids
> >>
> >>
> >>
> >> On 6 Mar 2025, at 18:02, Soft Works wrote:
> >>
>  -Original Message-
>  From: ffmpeg-devel  On Behalf Of
>  Nicolas George
>  Sent: Donnerstag, 6. März 2025 11:09
>  To: FFmpeg development discussions and patches  >> de...@ffmpeg.org>
>  Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses
> in
> >> log
>  output with simple ids
> 
>  Soft Works (HE12025-03-05):
> > Sorry. So - seriously: what would be your recipe then?
> 
>  I see not just a little of non-trivial code for a very minor
> feature,
> >>>
> >>> Whether trivial or non-trivial, it's definitely just very little
> code.
> >>>
>  that might be a hint that it would be best to let it go.
> >>>
> >>> This is not a helpful comment. I'm trying hard to be friendly and
> >> productive and I think it's not asked too much to at least try doing
> as
> >> well.
> >>>
> >>>
>  Also, if somebody is debugging a program using the libraries, the
>  pointers are relevant for that program. For that reason, I think
> the
>  change is a bad idea in the library.
> >>>
> >>> It's a valid point, I have acknowledged that already and added a log
> >> flag in V2 which allows to control it.
> >>>
> >>> As a further compromise, we could also enable it by default in case
> >> when DEBUG is defined, how about that?
> >>>
> >>> Generally, debugging is important without doubt, but it doesn't mean
> >> that Millions of users need to see something in the output which is
> only
> >> ever relevant to developers - that's the premise of this patchset.
> >>>
> >>> And even as a developer, those addresses are interesting only in a
> >> very narrow range of cases.
> >>> These addresses have been a major pain point for myself and many
> >> others over years when comparing logfiles. Even the best diffing
> >> algorithms are getting confused by these addresses and I think this
> >> patchset provides a huge benefit for both, users and developers in
> the
> >> future, making their work a lot easier.
> >>>
> >>>
>  On the other hand, you could do that change in the fftools. The
> point
>  about pointers being relevant does not apply for them, and they can
> >> have
>  as much global state as they want.
> >>>
> >>> You know that it's not easily possible to do it from within fftools
> >> because all libs are logging directly to avutil, so it's not quite
> clear
> >> to me what you are up to.
> >>> Do you mean something like a int(* av_log_format_prefix)(...)
> callback
> >> that fftools could register to?
> >>>
> >>
> >> First of all I want to say I like the idea of having cleaner logs,
> >> but...
> >>
> >> IMHO "complex" logging formatting should be handled by fftools
> >> especially if
> >> they need global state. Even though thats not the case right now, but
> >> just
> >> like Nicolas I also would prefer to not add even more global state
> for
> >> logging
> >> to the library...
> >>
> >> All the fancy log formatting should be done in a log callback in the
> >> fftools and the default library logging callback should just be a
> very
> >> basic
> >> one, is my opinion on this.
> >
> > That's all fine and probably reasonable. But is it fair to block a
> small change because some major rework would be desired at some point?
> >
> > When that change will be made, it will of course move out this little
> change as well.
> >
> > But are you really saying that this small change cannot be made
> because you don't like the general way of the current implementation?
> >
> 
> Just to be clear, I am not blocking this, just wanted to give my
> perspective on the topic.
> So if others think its fine and want it, lets go for it.

Thanks - and sorry for my retardation, I just realized why it's bad to do it 
this way with regards to library usage. I'll add a callback so that fftools can 
do the prefix formatting.

For the idea of moving all the formatting to fftools, wouldn't this be a major 
breakage for library consumers who are used to getting the log output formatted 
like now?


Thanks,
sw

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


Re: [FFmpeg-devel] [PATCH v2 0/3] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Soft Works



> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Nicolas George
> Sent: Donnerstag, 6. März 2025 17:43
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH v2 0/3] avutil/log: Replace addresses
> in log output with simple ids
> 
> Soft Works (HE12025-03-06):
> > It is the GitGitGadget system which does this this automatically.
> 
> Your choice to use that thing, your responsibility to not let it
> misbehave.
> 
> > But I have removed you now and try to remove you again each time when
> you reply.
> 
> Do not Cc other people either if they did not ask for it.

Ok, I'll try to disable that behavior altogether.


> When you already know a V3 is coming, yes, of course it is wasting
> people's time.

We're not talking about V3. Your criticism was about V2. I submitted it because 
I knew it was coming, so thanks for acknowledging that my submission of V2 was 
not wasting people's time.

sw

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


[FFmpeg-devel] [PATCH v3 1/4] avutil/log: Add callback for context prefix formatting

2025-03-06 Thread softworkz
From: softworkz 

also adds a log flag AV_LOG_PRINT_MEMADDRESSES, which is meant to
control prefix formatting. The actual formatting has to be performed
by the consuming application which needs to provide a formatting
callback via av_log_set_formatprefix_callback.

Signed-off-by: softworkz 
---
 doc/APIchanges  |  4 
 libavutil/log.c | 24 ++--
 libavutil/log.h | 29 +
 libavutil/version.h |  2 +-
 4 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 5a64836e25..92e4a2e055 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -1,5 +1,9 @@
 The last version increases of all libraries were on 2024-03-07
 
+2025-03-xx - xx - lavu 59.59.100 - log.h
+  Add flag AV_LOG_PRINT_MEMADDRESSES, av_log_set_formatprefix_callback,
+  av_log_formatprefix_default_callback
+
 API changes, most recent first:
 
 2025-03-01 - xx - lavu 59.58.100 - pixfmt.h
diff --git a/libavutil/log.c b/libavutil/log.c
index c5ee876a88..6697bdba70 100644
--- a/libavutil/log.c
+++ b/libavutil/log.c
@@ -315,6 +315,14 @@ static void format_date_now(AVBPrint* bp_time, int 
include_date)
 }
 }
 
+void av_log_formatprefix_default_callback(AVBPrint* buffer, AVClass** avcl, 
int log_flags)
+{
+av_bprintf(buffer+0, "[%s @ %p] ", item_name(avcl, *avcl), avcl);
+}
+
+static void (*av_log_formatprefix_callback)(AVBPrint* part, AVClass** avcl, 
int log_flags) =
+av_log_formatprefix_default_callback;
+
 static void format_line(void *avcl, int level, const char *fmt, va_list vl,
 AVBPrint part[5], int *print_prefix, int type[2])
 {
@@ -327,17 +335,16 @@ static void format_line(void *avcl, int level, const char 
*fmt, va_list vl,
 
 if(type) type[0] = type[1] = AV_CLASS_CATEGORY_NA + 16;
 if (*print_prefix && avc) {
+
 if (avc->parent_log_context_offset) {
-AVClass** parent = *(AVClass ***) (((uint8_t *) avcl) +
-   avc->parent_log_context_offset);
+AVClass** parent = *(AVClass ***) ((uint8_t *)avcl + 
avc->parent_log_context_offset);
 if (parent && *parent) {
-av_bprintf(part+0, "[%s @ %p] ",
-   item_name(parent, *parent), parent);
+av_log_formatprefix_callback(part, parent, flags);
 if(type) type[0] = get_category(parent);
 }
 }
-av_bprintf(part+1, "[%s @ %p] ",
-   item_name(avcl, avc), avcl);
+av_log_formatprefix_callback(part, avcl, flags);
+
 if(type) type[1] = get_category(avcl);
 }
 
@@ -485,6 +492,11 @@ int av_log_get_flags(void)
 return flags;
 }
 
+void av_log_set_formatprefix_callback(void (*callback)(AVBPrint* buffer, 
AVClass** avcl, int log_flags))
+{
+av_log_formatprefix_callback = callback;
+}
+
 void av_log_set_callback(void (*callback)(void*, int, const char*, va_list))
 {
 av_log_callback = callback;
diff --git a/libavutil/log.h b/libavutil/log.h
index dd094307ce..a69888c1ad 100644
--- a/libavutil/log.h
+++ b/libavutil/log.h
@@ -24,6 +24,7 @@
 #include 
 #include "attributes.h"
 #include "version.h"
+#include "bprint.h"
 
 typedef enum {
 AV_CLASS_CATEGORY_NA = 0,
@@ -323,6 +324,29 @@ int av_log_get_level(void);
  */
 void av_log_set_level(int level);
 
+/**
+ * Set the prefix formatting callback
+ *
+ * @note The callback must be thread safe, even if the application does not use
+ *   threads itself as some codecs are multithreaded.
+ *
+ * @see av_log_formatprefix_default_callback
+ *
+ * @param callback A formatting function with a compatible signature.
+ */
+void av_log_set_formatprefix_callback(void (*callback)(AVBPrint* buffer, 
AVClass** avcl, int log_flags));
+
+/**
+ * Default prefix formatting callback
+ *
+ * It prints the message to stderr, optionally colorizing it.
+ *
+ * @param buffer A pointer to the print buffer.
+ * @param avcl The AVClass reference for which to format the prefix.
+ * @param log_flags The enabled logging flags
+ */
+void av_log_formatprefix_default_callback(AVBPrint* buffer, AVClass** avcl, 
int log_flags);
+
 /**
  * Set the logging callback
  *
@@ -416,6 +440,11 @@ int av_log_format_line2(void *ptr, int level, const char 
*fmt, va_list vl,
  */
 #define AV_LOG_PRINT_DATETIME 8
 
+/**
+ * Print memory addresses instead of logical ids in the AVClass prefix.
+ */
+#define AV_LOG_PRINT_MEMADDRESSES 16
+
 void av_log_set_flags(int arg);
 int av_log_get_flags(void);
 
diff --git a/libavutil/version.h b/libavutil/version.h
index 4b584fd569..b6467e2a6d 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  59
-#define LIBAVUTIL_VERSION_MINOR  58
+#define LIBAVUTIL_VERSION_MINOR  59
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
ffmpeg-codebot

__

[FFmpeg-devel] [PATCH v3 0/4] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread ffmpegagent
..and individual numbering. The benefits are:

 * Smaller log file sizes
 * The disambiguation is much easier to recognize and to follow
 * It eventually allows comparing and viewing log file diffs without almost
   every line being different due to those addresses


Before
==

[hevc @ 018e72a89cc0] nal_unit_type:
[hevc @ 018e72a89cc0] Decoding PPS
[hevc @ 018e72a89cc0] nal_unit_type: 39(SEI_P.. [hevc @
018e72a89cc0] Decoding SEI
[mp4 @ 018e72a8e240] All [mp4 @ 018e72a8e240] Afte [hevc @
018e742f6b40] Decoded frame with POC .. detected 16 logical cores
[Parsed_scale_0 @ 018e74382f40] Setting 'w' t.. [Parsed_scale_0 @
018e74382f40] Setting 'h' t.. [Parsed_scale_1 @ 018e74382440]
Setting 'w' t.. [mjpeg @ 018e743210c0] Forcing thread count t.. [mjpeg @
018e743210c0] intra_quant_bias = 96


After
=

[hevc #0] nal_unit_type: [hevc #0] Decoding PPS
[hevc #0] nal_unit_type: 39(SEI_P.. [hevc #0] Decoding SEI
[mp4 #0] All info found
[mp4 #0] After avformat_find_ [hevc #1] Decoded frame with POC 2.
[Parsed_scale_0 #0] Setting 'w' t.. [Parsed_scale_0 #0] Setting 'h' t..
[Parsed_scale_1 #1] Setting 'w' t.. [mjpeg #2] Forcing thread count t..
[mjpeg #2] intra_quant_bias = 96


Versions



V2
==

 * Added log flag for optionally restoring the previous behavior (as
   requested by Gyan)

softworkz (4):
  avutil/log: Add callback for context prefix formatting
  fftools/opt_common: add memaddresses log flag
  fftools: Provide a log formatting callback for context prefixes
  doc/fftools-common-opts: document memaddresses log flag

 doc/APIchanges   |  4 +++
 doc/fftools-common-opts.texi |  2 ++
 fftools/cmdutils.c   | 69 
 fftools/cmdutils.h   |  5 +++
 fftools/ffmpeg.c |  1 +
 fftools/ffplay.c |  1 +
 fftools/ffprobe.c|  1 +
 fftools/opt_common.c |  6 
 libavutil/log.c  | 24 +
 libavutil/log.h  | 29 +++
 libavutil/version.h  |  2 +-
 11 files changed, 137 insertions(+), 7 deletions(-)


base-commit: 5c5be37daff4f4ecbe0c20d6a9f0fdad6eadc9c8
Published-As: 
https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-59%2Fsoftworkz%2Fsubmit_logaddresses-v3
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg 
pr-ffstaging-59/softworkz/submit_logaddresses-v3
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/59

Range-diff vs v2:

 -:  -- > 1:  b8702de13b avutil/log: Add callback for context prefix 
formatting
 2:  858e2cca9c = 2:  84c0848afc fftools/opt_common: add memaddresses log flag
 1:  3a289533a7 ! 3:  105adac4ba avutil/log: Replace addresses in log output 
with simple ids
 @@ Metadata
  Author: softworkz 
  
   ## Commit message ##
 -avutil/log: Replace addresses in log output with simple ids
 +fftools: Provide a log formatting callback for context prefixes
  
 -..and individual numbering. The benefits are:
 +This allows to print logical ids instead of memory addresses.
 +The benefits are:
  
  - Smaller log file sizes
  - The disambiguation is much easier to recognize and to follow
  - It eventually allows comparing and viewing log file diffs
without almost every line being different due to those addresses
  
 -Signed-off-by: softworkz 
 -
 - ## doc/APIchanges ##
 -@@
 - The last version increases of all libraries were on 2024-03-07
 - 
 -+2025-03-xx - xx - lavu 59.59.100 - log.h
 -+  Add flag AV_LOG_PRINT_MEMADDRESSES.
 -+
 - API changes, most recent first:
 + ## fftools/cmdutils.c ##
 +@@ fftools/cmdutils.c: AVDictionary *format_opts, *codec_opts;
   
 - 2025-03-01 - xx - lavu 59.58.100 - pixfmt.h
 -
 - ## libavutil/log.c ##
 -@@ libavutil/log.c: static AVMutex mutex = AV_MUTEX_INITIALIZER;
 + int hide_banner = 0;
   
 - static int av_log_level = AV_LOG_INFO;
 - static int flags;
  +static int nb_class_ids;
  +
  +#define NB_CLASS_IDS 1000
 @@ libavutil/log.c: static AVMutex mutex = AV_MUTEX_INITIALIZER;
  +// exceeded NB_CLASS_IDS entries in class_ids[]
  +return 0;
  +}
 ++
 + void uninit_opts(void)
 + {
 + av_dict_free(&swr_opts);
 +@@ fftools/cmdutils.c: static void check_options(const OptionDef *po)
 + }
 + }
   
 - #define NB_LEVELS 8
 - #if defined(_WIN32) && HAVE_SETCONSOLETEXTATTRIBUTE && HAVE_GETSTDHANDLE
 -@@ libavutil/log.c: static void format_line(void *avcl, int level, const 
char *fmt, va_list vl,
 - 
 - if(type) type[0] = type[1] = AV_CLASS_CATEGORY_NA + 16;
 - if (*print_prefix && avc) {
 -+const int print_mem = flags & AV_LOG_PRINT_MEMADDRESSES;
 ++static const char *item_name(void *obj, const AVClass *cls)
 ++{
 ++  

[FFmpeg-devel] [PATCH v3 2/4] fftools/opt_common: add memaddresses log flag

2025-03-06 Thread softworkz
From: softworkz 

This commit adds the memaddresses log flag.
When specifying this flag at the command line, context prefixes will
be printed with memory addresses like in earlier ffmpeg versions.

Example with memaddresses flag:

[hevc @ 018e72a89cc0] .

without (new behavior):

[hevc #0] .

Signed-off-by: softworkz 
---
 fftools/opt_common.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fftools/opt_common.c b/fftools/opt_common.c
index 2ac3fd4fb3..b71ecc4b31 100644
--- a/fftools/opt_common.c
+++ b/fftools/opt_common.c
@@ -1304,6 +1304,12 @@ int opt_loglevel(void *optctx, const char *opt, const 
char *arg)
 } else {
 flags |= AV_LOG_PRINT_DATETIME;
 }
+} else if (av_strstart(token, "memaddresses", &arg)) {
+if (cmd == '-') {
+flags &= ~AV_LOG_PRINT_MEMADDRESSES;
+} else {
+flags |= AV_LOG_PRINT_MEMADDRESSES;
+}
 } else {
 break;
 }
-- 
ffmpeg-codebot

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


Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids

2025-03-06 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of Marvin
> Scholz
> Sent: Donnerstag, 6. März 2025 18:38
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log
> output with simple ids
> 
> 
> 
> On 6 Mar 2025, at 18:02, Soft Works wrote:
> 
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Nicolas George
> >> Sent: Donnerstag, 6. März 2025 11:09
> >> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in
> log
> >> output with simple ids
> >>
> >> Soft Works (HE12025-03-05):
> >>> Sorry. So - seriously: what would be your recipe then?
> >>
> >> I see not just a little of non-trivial code for a very minor feature,
> >
> > Whether trivial or non-trivial, it's definitely just very little code.
> >
> >> that might be a hint that it would be best to let it go.
> >
> > This is not a helpful comment. I'm trying hard to be friendly and
> productive and I think it's not asked too much to at least try doing as
> well.
> >
> >
> >> Also, if somebody is debugging a program using the libraries, the
> >> pointers are relevant for that program. For that reason, I think the
> >> change is a bad idea in the library.
> >
> > It's a valid point, I have acknowledged that already and added a log
> flag in V2 which allows to control it.
> >
> > As a further compromise, we could also enable it by default in case
> when DEBUG is defined, how about that?
> >
> > Generally, debugging is important without doubt, but it doesn't mean
> that Millions of users need to see something in the output which is only
> ever relevant to developers - that's the premise of this patchset.
> >
> > And even as a developer, those addresses are interesting only in a
> very narrow range of cases.
> > These addresses have been a major pain point for myself and many
> others over years when comparing logfiles. Even the best diffing
> algorithms are getting confused by these addresses and I think this
> patchset provides a huge benefit for both, users and developers in the
> future, making their work a lot easier.
> >
> >
> >> On the other hand, you could do that change in the fftools. The point
> >> about pointers being relevant does not apply for them, and they can
> have
> >> as much global state as they want.
> >
> > You know that it's not easily possible to do it from within fftools
> because all libs are logging directly to avutil, so it's not quite clear
> to me what you are up to.
> > Do you mean something like a int(* av_log_format_prefix)(...) callback
> that fftools could register to?
> >
> 
> First of all I want to say I like the idea of having cleaner logs,
> but...
> 
> IMHO "complex" logging formatting should be handled by fftools
> especially if
> they need global state. Even though thats not the case right now, but
> just
> like Nicolas I also would prefer to not add even more global state for
> logging
> to the library...
> 
> All the fancy log formatting should be done in a log callback in the
> fftools and the default library logging callback should just be a very
> basic
> one, is my opinion on this.

That's all fine and probably reasonable. But is it fair to block a small change 
because some major rework would be desired at some point?

When that change will be made, it will of course move out this little change as 
well.

But are you really saying that this small change cannot be made because you 
don't like the general way of the current implementation?

Thanks
sw


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


[FFmpeg-devel] [PATCH] swscale/input: add support for UYYVYY411

2025-03-06 Thread James Almer
Signed-off-by: James Almer 
---
Untested as i don't have a camera that outputs this, and no decoder or filter
seems to generate it either, but it seemed simple enough to write.

Anyone that can use the libdc1394 device and test would be welcome.

 libswscale/input.c | 23 +++
 libswscale/utils.c |  2 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/libswscale/input.c b/libswscale/input.c
index dfa8ed15ab..6d600385a1 100644
--- a/libswscale/input.c
+++ b/libswscale/input.c
@@ -904,6 +904,23 @@ static void uyvyToUV_c(uint8_t *dstU, uint8_t *dstV, const 
uint8_t *unused0, con
 av_assert1(src1 == src2);
 }
 
+static void uyyvyyToY_c(uint8_t *dst, const uint8_t *src, const uint8_t 
*unused1, const uint8_t *unused2,
+int width, uint32_t *unused, void *opq)
+{
+for (int i = 0; i < width; i++)
+dst[i]  = src[3 * (i >> 1) + 1 + (i & 1)];
+}
+
+static void uyyvyyToUV_c(uint8_t *dstU, uint8_t *dstV, const uint8_t *unused0, 
const uint8_t *src1,
+ const uint8_t *src2, int width, uint32_t *unused, 
void *opq)
+{
+for (int i = 0; i < width; i++) {
+dstU[i] = src1[6 * i + 0];
+dstV[i] = src1[6 * i + 3];
+}
+av_assert1(src1 == src2);
+}
+
 static av_always_inline void nvXXtoUV_c(uint8_t *dst1, uint8_t *dst2,
 const uint8_t *src, int width)
 {
@@ -1714,6 +1731,9 @@ av_cold void ff_sws_init_input_funcs(SwsInternal *c,
 case AV_PIX_FMT_UYVY422:
 *chrToYV12 = uyvyToUV_c;
 break;
+case AV_PIX_FMT_UYYVYY411:
+*chrToYV12 = uyyvyyToUV_c;
+break;
 case AV_PIX_FMT_VYU444:
 *chrToYV12 = vyuToUV_c;
 break;
@@ -2351,6 +2371,9 @@ av_cold void ff_sws_init_input_funcs(SwsInternal *c,
 case AV_PIX_FMT_UYVY422:
 *lumToYV12 = uyvyToY_c;
 break;
+case AV_PIX_FMT_UYYVYY411:
+*lumToYV12 = uyyvyyToY_c;
+break;
 case AV_PIX_FMT_VYU444:
 *lumToYV12 = vyuToY_c;
 break;
diff --git a/libswscale/utils.c b/libswscale/utils.c
index 953bf015e4..389efd7c68 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -91,7 +91,7 @@ static const FormatEntry format_entries[] = {
 [AV_PIX_FMT_YUVJ444P]= { 1, 1 },
 [AV_PIX_FMT_YVYU422] = { 1, 1 },
 [AV_PIX_FMT_UYVY422] = { 1, 1 },
-[AV_PIX_FMT_UYYVYY411]   = { 0, 0 },
+[AV_PIX_FMT_UYYVYY411]   = { 1, 0 },
 [AV_PIX_FMT_BGR8]= { 1, 1 },
 [AV_PIX_FMT_BGR4]= { 0, 1 },
 [AV_PIX_FMT_BGR4_BYTE]   = { 1, 1 },
-- 
2.48.1

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


Re: [FFmpeg-devel] [RFC] FFV1 float support

2025-03-06 Thread Leo Izen

On 3/6/25 11:37 AM, Michael Niedermayer wrote:

Hi


I did mention this once before, but you should look into the Daala/Opus way
of storing rawbits into a separate chunk at the end of the bitstream. It
avoids polluting the range context with equiprobable bits.



I didnt like it, but it seems more popular



May also be worth considering the JPEG XL way, where each entropy-coded 
symbol is a "hybrid integer" of a token read from the entropy stream 
(ANS) followed by zero or more "raw bits" that are spliced in.


- Leo Izen (Traneptora)

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