[FFmpeg-devel] [PATCH 1/2] libavcodec/ffv1: Support storing decorrelated LSB raw without rangecoder
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
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
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
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
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
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
> -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
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
> 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
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
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()
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
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
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
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
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
> -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
> -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
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
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
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
> -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
> -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
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
..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
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
> -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
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
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".