[FFmpeg-devel] [PATCH] libavformat/dashdec: Fix buffer overflow in segment URL resolution
From: xiaohuanshu
Problem:
The max_url_size calculation for DASH segment URLs only considered the base URL
length, leading to buffer overflow when the segment's sourceURL exceeded the
pre-allocated buffer. This triggered the log error:
"DASH request for url 'invalid:truncated'".
Reproduce:
1. A test sample "long-sourceurl-sample.mpd" (deliberately designed with a long
sourceURL) was uploaded to VideoLAN's repository.
2. Reproduce with short base path:
ffmpeg -i /tmp/short_path/long-sourceurl-sample.mpd
-> Triggers "invalid:truncated" error
3. With artificially lengthened base path (e.g. /aaa/../bbb/../...):
ffmpeg -i /long/../path/../with/../many/../segments/long-sourceurl-sample.mpd
-> URL resolves correctly (though HTTP fetch fails due to fake URL)
Fix:
Recalculate max_url_size by considering both base URL and sourceURL lengths,
ensuring sufficient buffer allocation during URL concatenation.
Signed-off-by: xiaohuanshu
---
libavformat/dashdec.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index c3f3d7f3f8..f604d363c4 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -606,7 +606,7 @@ static int parse_manifest_segmenturlnode(AVFormatContext
*s, struct representati
char *initialization_val = NULL;
char *media_val = NULL;
char *range_val = NULL;
-int max_url_size = c ? c->max_url_size: MAX_URL_SIZE;
+int max_url_size = 0;
int err;
if (!av_strcasecmp(fragmenturl_node->name, "Initialization")) {
@@ -620,6 +620,12 @@ static int parse_manifest_segmenturlnode(AVFormatContext
*s, struct representati
xmlFree(initialization_val);
return AVERROR(ENOMEM);
}
+max_url_size = FFMAX(
+c ? c->max_url_size : 0,
+initialization_val ? aligned(strlen(initialization_val) +
+ (rep_id_val ? strlen(rep_id_val)
: 0) +
+ (rep_bandwidth_val ?
strlen(rep_bandwidth_val) : 0)) : 0);
+max_url_size = max_url_size ? max_url_size : MAX_URL_SIZE;
rep->init_section->url = get_content_url(baseurl_nodes, 4,
max_url_size,
rep_id_val,
@@ -641,6 +647,12 @@ static int parse_manifest_segmenturlnode(AVFormatContext
*s, struct representati
xmlFree(media_val);
return AVERROR(ENOMEM);
}
+max_url_size = FFMAX(
+c ? c->max_url_size : 0,
+initialization_val ? aligned(strlen(initialization_val) +
+ (rep_id_val ? strlen(rep_id_val)
: 0) +
+ (rep_bandwidth_val ?
strlen(rep_bandwidth_val) : 0)) : 0);
+max_url_size = max_url_size ? max_url_size : MAX_URL_SIZE;
seg->url = get_content_url(baseurl_nodes, 4,
max_url_size,
rep_id_val,
--
2.39.5 (Apple Git-154)
___
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 00/22] Deprecate av_uninit
From: Zhao Zhili The macro is meant to suppress false uninitialized warnings. However, sometimes these 'false uninitialized warnings' are really undefined behavior, and leading to real issue like crash, e.g., ab792634197e. For false uninitialized warnings, it can be silenced by initialization, and compiler can easily optimize away unnecessary initializations. av_uninit shouldn't be used in any case. Zhao Zhili (22): avfilter/af_aecho: Remove use of av_uninit avfilter/af_compand: Remove use of av_uninit avfilter/vsrc_mandelbrot: Remove use of av_uninit avformat/electronicarts: Remove use of av_uninit swscale/yuv2rgb: Remove use of av_uninit avcodec/ac3enc: Remove use of av_uninit avcodec/bfi: Remove use of av_uninit avcodec/dvdsubenc: Remove use of av_uninit avcodec/eamad: Remove use of av_uninit avcodec/ffv1enc: Remove use of av_uninit avcodec/flacdec: Remove use of av_uninit avcodec/lpc: Remove use of av_uninit avcodec/mpeg4videodec: Remove use av_uninit avcodec/qtrleenc: Remove use of av_uninit avcodec/ra144enc: Remove use av_uninit avcodec/vp8: Remove use of av_uninit avcodec/wmavoice: Remove use of av_uninit avformat/flvdec: Remove use of av_uninit avformat/srtp: Remove use of av_uninit avformat/wavdec: Remove use of av_uninit avformat/tests/seek: Remove use of av_uninit avutil/attributes: Make av_uninit do nothing libavcodec/ac3enc.c | 5 +++-- libavcodec/ac3enc_template.c | 16 libavcodec/bfi.c | 2 +- libavcodec/dvdsubenc.c| 2 +- libavcodec/eamad.c| 2 +- libavcodec/ffv1enc.c | 4 ++-- libavcodec/ffv1enc_template.c | 4 ++-- libavcodec/flacdec.c | 2 +- libavcodec/lpc.c | 2 +- libavcodec/mpeg4videodec.c| 2 +- libavcodec/qtrleenc.c | 2 +- libavcodec/ra144enc.c | 4 ++-- libavcodec/vp8.c | 2 +- libavcodec/wmavoice.c | 4 ++-- libavfilter/af_aecho.c| 2 +- libavfilter/af_compand.c | 2 +- libavfilter/vsrc_mandelbrot.c | 2 +- libavformat/electronicarts.c | 2 +- libavformat/flvdec.c | 4 ++-- libavformat/srtp.c| 4 ++-- libavformat/tests/seek.c | 2 +- libavformat/wavdec.c | 2 +- libavutil/attributes.h| 9 - libswscale/yuv2rgb.c | 2 +- 24 files changed, 42 insertions(+), 42 deletions(-) -- 2.46.0 ___ 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 01/22] avfilter/af_aecho: Remove use of av_uninit
From: Zhao Zhili --- libavfilter/af_aecho.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/af_aecho.c b/libavfilter/af_aecho.c index ff316eaa67..4406e0dbb2 100644 --- a/libavfilter/af_aecho.c +++ b/libavfilter/af_aecho.c @@ -164,7 +164,7 @@ static void echo_samples_## name ##p(AudioEchoContext *ctx, \ const double in_gain = ctx->in_gain;\ const int nb_echoes = ctx->nb_echoes; \ const int max_samples = ctx->max_samples; \ -int i, j, chan, av_uninit(index); \ +int i, j, chan, index = 0; \ \ av_assert1(channels > 0); /* would corrupt delay_index */ \ \ -- 2.46.0 ___ 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 02/22] avfilter/af_compand: Remove use of av_uninit
From: Zhao Zhili --- libavfilter/af_compand.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/af_compand.c b/libavfilter/af_compand.c index 69de1360f1..3b6a83c056 100644 --- a/libavfilter/af_compand.c +++ b/libavfilter/af_compand.c @@ -195,7 +195,7 @@ static int compand_delay(AVFilterContext *ctx, AVFrame *frame) AVFilterLink *inlink = ctx->inputs[0]; const int channels = inlink->ch_layout.nb_channels; const int nb_samples = frame->nb_samples; -int chan, i, av_uninit(dindex), oindex, av_uninit(count); +int chan, i, dindex = 0, oindex, count = 0; AVFrame *out_frame = NULL; int err; -- 2.46.0 ___ 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 03/22] avfilter/vsrc_mandelbrot: Remove use of av_uninit
From: Zhao Zhili
---
libavfilter/vsrc_mandelbrot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavfilter/vsrc_mandelbrot.c b/libavfilter/vsrc_mandelbrot.c
index 3f14f1e7f9..84eeadb703 100644
--- a/libavfilter/vsrc_mandelbrot.c
+++ b/libavfilter/vsrc_mandelbrot.c
@@ -256,7 +256,7 @@ static void draw_mandelbrot(AVFilterContext *ctx, uint32_t
*color, int linesize,
}
for(x=0; xw; x++){
-float av_uninit(epsilon);
+float epsilon = 0.0f;
const double cr=s->start_x+scale*(x-s->w/2);
double zr=cr;
double zi=ci;
--
2.46.0
___
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 06/22] avcodec/ac3enc: Remove use of av_uninit
From: Zhao Zhili
---
libavcodec/ac3enc.c | 5 +++--
libavcodec/ac3enc_template.c | 16
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
index 3649289865..17d45097ac 100644
--- a/libavcodec/ac3enc.c
+++ b/libavcodec/ac3enc.c
@@ -1692,7 +1692,7 @@ static void ac3_output_frame_header(AC3EncodeContext *s,
PutBitContext *pb)
*/
static void output_audio_block(AC3EncodeContext *s, PutBitContext *pb, int blk)
{
-int ch, i, baie, bnd, got_cpl, av_uninit(ch0);
+int ch, i, baie, bnd, got_cpl;
AC3Block *block = &s->blocks[blk];
/* block switching */
@@ -1852,6 +1852,7 @@ static void output_audio_block(AC3EncodeContext *s,
PutBitContext *pb, int blk)
/* mantissas */
got_cpl = !block->cpl_in_use;
for (ch = 1; ch <= s->channels; ch++) {
+int ch0 = 0;
int b, q;
if (!got_cpl && ch > 1 && block->channel_in_cpl[ch-1]) {
@@ -2341,7 +2342,7 @@ static av_cold int validate_options(AC3EncodeContext *s)
*/
static av_cold void set_bandwidth(AC3EncodeContext *s)
{
-int blk, ch, av_uninit(cpl_start);
+int blk, ch, cpl_start = 0;
if (s->cutoff) {
/* calculate bandwidth based on user-specified cutoff frequency */
diff --git a/libavcodec/ac3enc_template.c b/libavcodec/ac3enc_template.c
index 049666fdca..84887c8079 100644
--- a/libavcodec/ac3enc_template.c
+++ b/libavcodec/ac3enc_template.c
@@ -93,7 +93,7 @@ static void apply_channel_coupling(AC3EncodeContext *s)
#else
int32_t (*fixed_cpl_coords)[AC3_MAX_CHANNELS][16] = cpl_coords;
#endif
-int av_uninit(blk), ch, bnd, i, j;
+int ch, bnd, i, j;
CoefSumType energy[AC3_MAX_BLOCKS][AC3_MAX_CHANNELS][16] = {{{0}}};
int cpl_start, num_cpl_coefs;
@@ -109,7 +109,7 @@ static void apply_channel_coupling(AC3EncodeContext *s)
cpl_start = FFMIN(256, cpl_start + num_cpl_coefs) - num_cpl_coefs;
/* calculate coupling channel from fbw channels */
-for (blk = 0; blk < s->num_blocks; blk++) {
+for (int blk = 0; blk < s->num_blocks; blk++) {
AC3Block *block = &s->blocks[blk];
CoefType *cpl_coef = &block->mdct_coef[CPL_CH][cpl_start];
if (!block->cpl_in_use)
@@ -134,7 +134,7 @@ static void apply_channel_coupling(AC3EncodeContext *s)
while (i < s->cpl_end_freq) {
int band_size = s->cpl_band_sizes[bnd];
for (ch = CPL_CH; ch <= s->fbw_channels; ch++) {
-for (blk = 0; blk < s->num_blocks; blk++) {
+for (int blk = 0; blk < s->num_blocks; blk++) {
AC3Block *block = &s->blocks[blk];
if (!block->cpl_in_use || (ch > CPL_CH &&
!block->channel_in_cpl[ch]))
continue;
@@ -149,7 +149,7 @@ static void apply_channel_coupling(AC3EncodeContext *s)
}
/* calculate coupling coordinates for all blocks for all channels */
-for (blk = 0; blk < s->num_blocks; blk++) {
+for (int blk = 0; blk < s->num_blocks; blk++) {
AC3Block *block = &s->blocks[blk];
if (!block->cpl_in_use)
continue;
@@ -164,7 +164,7 @@ static void apply_channel_coupling(AC3EncodeContext *s)
}
/* determine which blocks to send new coupling coordinates for */
-for (blk = 0; blk < s->num_blocks; blk++) {
+for (int blk = 0; blk < s->num_blocks; blk++) {
AC3Block *block = &s->blocks[blk];
AC3Block *block0 = blk ? &s->blocks[blk-1] : NULL;
@@ -205,9 +205,9 @@ static void apply_channel_coupling(AC3EncodeContext *s)
/* calculate final coupling coordinates, taking into account reusing of
coordinates in successive blocks */
for (bnd = 0; bnd < s->num_cpl_bands; bnd++) {
-blk = 0;
+int blk = 0;
while (blk < s->num_blocks) {
-int av_uninit(blk1);
+int blk1 = 0;
AC3Block *block = &s->blocks[blk];
if (!block->cpl_in_use) {
@@ -236,7 +236,7 @@ static void apply_channel_coupling(AC3EncodeContext *s)
}
/* calculate exponents/mantissas for coupling coordinates */
-for (blk = 0; blk < s->num_blocks; blk++) {
+for (int blk = 0; blk < s->num_blocks; blk++) {
AC3Block *block = &s->blocks[blk];
if (!block->cpl_in_use)
continue;
--
2.46.0
___
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 05/22] swscale/yuv2rgb: Remove use of av_uninit
From: Zhao Zhili --- libswscale/yuv2rgb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libswscale/yuv2rgb.c b/libswscale/yuv2rgb.c index ff8e013da4..eebe840ad6 100644 --- a/libswscale/yuv2rgb.c +++ b/libswscale/yuv2rgb.c @@ -733,7 +733,7 @@ av_cold int ff_yuv2rgb_c_init_tables(SwsInternal *c, const int inv_table[4], uint8_t *y_table; uint16_t *y_table16; uint32_t *y_table32; -int i, base, rbase, gbase, bbase, av_uninit(abase), needAlpha; +int i, base, rbase, gbase, bbase, abase = 0, needAlpha; const int yoffs = (fullRange ? 384 : 326) + YUVRGB_TABLE_LUMA_HEADROOM; const int table_plane_size = 1024 + 2*YUVRGB_TABLE_LUMA_HEADROOM; -- 2.46.0 ___ 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 07/22] avcodec/bfi: Remove use of av_uninit
From: Zhao Zhili
---
libavcodec/bfi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/bfi.c b/libavcodec/bfi.c
index 1d2d3a7204..4c47861791 100644
--- a/libavcodec/bfi.c
+++ b/libavcodec/bfi.c
@@ -93,7 +93,7 @@ static int bfi_decode_frame(AVCodecContext *avctx, AVFrame
*frame,
while (dst != frame_end) {
static const uint8_t lentab[4] = { 0, 2, 0, 1 };
-unsigned int byte = bytestream2_get_byte(&g), av_uninit(offset);
+unsigned int byte = bytestream2_get_byte(&g), offset = 0;
unsigned int code = byte >> 6;
unsigned int length = byte & ~0xC0;
--
2.46.0
___
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 08/22] avcodec/dvdsubenc: Remove use of av_uninit
From: Zhao Zhili
---
libavcodec/dvdsubenc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/dvdsubenc.c b/libavcodec/dvdsubenc.c
index 00bab35988..818ac2b0f5 100644
--- a/libavcodec/dvdsubenc.c
+++ b/libavcodec/dvdsubenc.c
@@ -123,7 +123,7 @@ static void count_colors(AVCodecContext *avctx, unsigned
hits[33],
unsigned count[256] = { 0 };
uint32_t *palette = (uint32_t *)r->data[1];
uint32_t color;
-int x, y, i, j, match, d, best_d, av_uninit(best_j);
+int x, y, i, j, match, d, best_d, best_j = 0;
uint8_t *p = r->data[0];
for (y = 0; y < r->h; y++) {
--
2.46.0
___
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 09/22] avcodec/eamad: Remove use of av_uninit
From: Zhao Zhili
---
libavcodec/eamad.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/eamad.c b/libavcodec/eamad.c
index 44dac46083..cf2f5c1e9d 100644
--- a/libavcodec/eamad.c
+++ b/libavcodec/eamad.c
@@ -195,7 +195,7 @@ static int decode_motion(GetBitContext *gb)
static int decode_mb(MadContext *s, AVFrame *frame, int inter, int mb_x, int
mb_y)
{
int mv_map = 0;
-int av_uninit(mv_x), av_uninit(mv_y);
+int mv_x = 0, mv_y = 0;
int j;
if (inter) {
--
2.46.0
___
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 10/22] avcodec/ffv1enc: Remove use of av_uninit
From: Zhao Zhili
---
libavcodec/ffv1enc.c | 4 ++--
libavcodec/ffv1enc_template.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
index 807850a432..b1f4f7bca6 100644
--- a/libavcodec/ffv1enc.c
+++ b/libavcodec/ffv1enc.c
@@ -1231,7 +1231,7 @@ static void load_rgb_float32_frame(FFV1Context *f,
FFV1SliceContext *sc,
for (y = 0; y < h; y++) {
for (x = 0; x < w; x++) {
-int b, g, r, av_uninit(a);
+int b, g, r, a = 0;
g = *((const uint32_t *)(src[0] + x*4 + stride[0]*y));
b = *((const uint32_t *)(src[1] + x*4 + stride[1]*y));
@@ -1510,7 +1510,7 @@ static int encode_float32_rgb_frame(FFV1Context *f,
FFV1SliceContext *sc,
sample[p][i]= RENAME(sc->sample_buffer) + p*ring_size*(w+6) +
((h+i-y)%ring_size)*(w+6) + 3;
for (x = 0; x < w; x++) {
-int b, g, r, av_uninit(a);
+int b, g, r, a = 0;
g = sc->bitmap[0][x + w*y];
b = sc->bitmap[1][x + w*y];
r = sc->bitmap[2][x + w*y];
diff --git a/libavcodec/ffv1enc_template.c b/libavcodec/ffv1enc_template.c
index 64f3c420c5..12d90197e5 100644
--- a/libavcodec/ffv1enc_template.c
+++ b/libavcodec/ffv1enc_template.c
@@ -142,7 +142,7 @@ static void RENAME(load_rgb_frame)(FFV1Context *f,
FFV1SliceContext *sc,
for (y = 0; y < h; y++) {
for (x = 0; x < w; x++) {
-int b, g, r, av_uninit(a);
+int b, g, r, a = 0;
if (sizeof(TYPE) == 4 || transparency) {
g = *((const uint16_t *)(src[0] + x*2 + stride[0]*y));
@@ -192,7 +192,7 @@ static int RENAME(encode_rgb_frame)(FFV1Context *f,
FFV1SliceContext *sc,
sample[p][i]= RENAME(sc->sample_buffer) + p*ring_size*(w+6) +
((h+i-y)%ring_size)*(w+6) + 3;
for (x = 0; x < w; x++) {
-int b, g, r, av_uninit(a);
+int b, g, r, a = 0;
if (lbd) {
unsigned v = *((const uint32_t*)(src[0] + x*4 + stride[0]*y));
b = v& 0xFF;
--
2.46.0
___
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 11/22] avcodec/flacdec: Remove use of av_uninit
From: Zhao Zhili
---
libavcodec/flacdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/flacdec.c b/libavcodec/flacdec.c
index 0c88f577a1..1219f5b20a 100644
--- a/libavcodec/flacdec.c
+++ b/libavcodec/flacdec.c
@@ -303,7 +303,7 @@ static int decode_subframe_fixed(FLACContext *s, int32_t
*decoded,
int pred_order, int bps)
{
const int blocksize = s->blocksize;
-unsigned av_uninit(a), av_uninit(b), av_uninit(c), av_uninit(d);
+unsigned a = 0, b = 0, c = 0, d = 0;
int i;
int ret;
--
2.46.0
___
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 12/22] avcodec/lpc: Remove use of av_uninit
From: Zhao Zhili
---
libavcodec/lpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/lpc.c b/libavcodec/lpc.c
index e793e54038..d1a84d0c5b 100644
--- a/libavcodec/lpc.c
+++ b/libavcodec/lpc.c
@@ -278,7 +278,7 @@ int ff_lpc_calc_coefs(LPCContext *s,
if (lpc_type == FF_LPC_TYPE_CHOLESKY) {
LLSModel *m = s->lls_models;
LOCAL_ALIGNED(32, double, var, [FFALIGN(MAX_LPC_ORDER+1,4)]);
-double av_uninit(weight);
+double weight = 0.0;
memset(var, 0, FFALIGN(MAX_LPC_ORDER+1,4)*sizeof(*var));
/* Avoids initializing with an unused value when lpc_passes == 1 */
--
2.46.0
___
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 13/22] avcodec/mpeg4videodec: Remove use av_uninit
From: Zhao Zhili
---
libavcodec/mpeg4videodec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index e21f1d24a2..d3d68a1e33 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -1336,7 +1336,7 @@ static inline int mpeg4_decode_block(Mpeg4DecContext
*ctx, int16_t *block,
{
MpegEncContext *s = &ctx->m;
int level, i, last, run, qmul, qadd, pred;
-int av_uninit(dc_pred_dir);
+int dc_pred_dir = 0;
const RLTable *rl;
const RL_VLC_ELEM *rl_vlc;
const uint8_t *scan_table;
--
2.46.0
___
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 14/22] avcodec/qtrleenc: Remove use of av_uninit
From: Zhao Zhili --- libavcodec/qtrleenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/qtrleenc.c b/libavcodec/qtrleenc.c index ae341c60b6..91e30ce152 100644 --- a/libavcodec/qtrleenc.c +++ b/libavcodec/qtrleenc.c @@ -145,7 +145,7 @@ static void qtrle_encode_line(QtrleEncContext *s, const AVFrame *p, int line, ui unsigned int skipcount; /* This will be the number of consecutive equal pixels in the current * frame, starting from the ith one also */ -unsigned int av_uninit(repeatcount); +unsigned int repeatcount = 0; /* The cost of the three different possibilities */ int total_skip_cost; -- 2.46.0 ___ 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 15/22] avcodec/ra144enc: Remove use av_uninit
From: Zhao Zhili
---
libavcodec/ra144enc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/ra144enc.c b/libavcodec/ra144enc.c
index d38c39ce14..c671689ab9 100644
--- a/libavcodec/ra144enc.c
+++ b/libavcodec/ra144enc.c
@@ -192,8 +192,8 @@ static void create_adapt_vect(float *vect, const int16_t
*cb, int lag)
static int adaptive_cb_search(const int16_t *adapt_cb, float *work,
const float *coefs, float *data)
{
-int i, av_uninit(best_vect);
-float score, gain, best_score, av_uninit(best_gain);
+int i, best_vect = 0;
+float score, gain, best_score, best_gain = 0;
float exc[BLOCKSIZE];
gain = best_score = 0;
--
2.46.0
___
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 16/22] avcodec/vp8: Remove use of av_uninit
From: Zhao Zhili --- libavcodec/vp8.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c index c42170519f..ea8457c0b1 100644 --- a/libavcodec/vp8.c +++ b/libavcodec/vp8.c @@ -2625,7 +2625,7 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame, VP8Context *s = avctx->priv_data; int ret, i, referenced, num_jobs; enum AVDiscard skip_thresh; -VP8Frame *av_uninit(curframe), *prev_frame; +VP8Frame *curframe = NULL, *prev_frame; if (is_vp7) ret = vp7_decode_frame_header(s, avpkt->data, avpkt->size); -- 2.46.0 ___ 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 17/22] avcodec/wmavoice: Remove use of av_uninit
From: Zhao Zhili
---
libavcodec/wmavoice.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/wmavoice.c b/libavcodec/wmavoice.c
index 7fc735ad1d..7e776eb1c0 100644
--- a/libavcodec/wmavoice.c
+++ b/libavcodec/wmavoice.c
@@ -1497,8 +1497,8 @@ static int synth_frame(AVCodecContext *ctx, GetBitContext
*gb, int frame_idx,
float *excitation, float *synth)
{
WMAVoiceContext *s = ctx->priv_data;
-int n, n_blocks_x2, log_n_blocks_x2, av_uninit(cur_pitch_val);
-int pitch[MAX_BLOCKS], av_uninit(last_block_pitch);
+int n, n_blocks_x2, log_n_blocks_x2, cur_pitch_val = 0;
+int pitch[MAX_BLOCKS], last_block_pitch = 0;
/* Parse frame type ("frame header"), see frame_descs */
int bd_idx = s->vbm_tree[get_vlc2(gb, frame_type_vlc, 6, 3)],
block_nsamples;
--
2.46.0
___
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 18/22] avformat/flvdec: Remove use of av_uninit
From: Zhao Zhili --- libavformat/flvdec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c index b90ed34b1c..5dbe82ca0e 100644 --- a/libavformat/flvdec.c +++ b/libavformat/flvdec.c @@ -1361,8 +1361,8 @@ static int flv_read_packet(AVFormatContext *s, AVPacket *pkt) int stream_type = -1; int64_t next, pos, meta_pos; int64_t dts, pts = AV_NOPTS_VALUE; -int av_uninit(channels); -int av_uninit(sample_rate); +int channels = 0; +int sample_rate = 0; AVStream *st = NULL; int last = -1; int orig_size; -- 2.46.0 ___ 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 19/22] avformat/srtp: Remove use of av_uninit
From: Zhao Zhili
---
libavformat/srtp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavformat/srtp.c b/libavformat/srtp.c
index 7e5a42e327..d1e69c0702 100644
--- a/libavformat/srtp.c
+++ b/libavformat/srtp.c
@@ -128,8 +128,8 @@ int ff_srtp_decrypt(struct SRTPContext *s, uint8_t *buf,
int *lenptr)
{
uint8_t iv[16] = { 0 }, hmac[20];
int len = *lenptr;
-int av_uninit(seq_largest);
-uint32_t ssrc, av_uninit(roc);
+int seq_largest = 0;
+uint32_t ssrc, roc = 0;
uint64_t index;
int rtcp, hmac_size;
--
2.46.0
___
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 20/22] avformat/wavdec: Remove use of av_uninit
From: Zhao Zhili
---
libavformat/wavdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
index ae6ff0e022..1eb129126d 100644
--- a/libavformat/wavdec.c
+++ b/libavformat/wavdec.c
@@ -350,7 +350,7 @@ static const AVMetadataConv wav_metadata_conv[] = {
/* wav input */
static int wav_read_header(AVFormatContext *s)
{
-int64_t size, av_uninit(data_size);
+int64_t size, data_size = 0;
int64_t sample_count = 0;
int rf64 = 0, bw64 = 0;
uint32_t tag;
--
2.46.0
___
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 21/22] avformat/tests/seek: Remove use of av_uninit
From: Zhao Zhili
---
libavformat/tests/seek.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/tests/seek.c b/libavformat/tests/seek.c
index 94a72d9422..eeb220235d 100644
--- a/libavformat/tests/seek.c
+++ b/libavformat/tests/seek.c
@@ -118,7 +118,7 @@ int main(int argc, char **argv)
}
for(i=0; ; i++){
AVPacket pkt = { 0 };
-AVStream *av_uninit(st);
+AVStream *st = NULL;
char ts_buf[60];
if(ret>=0){
--
2.46.0
___
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 22/22] avutil/attributes: Make av_uninit do nothing
From: Zhao Zhili The macro is dangerous since it can hide real issue. --- libavutil/attributes.h | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/libavutil/attributes.h b/libavutil/attributes.h index 04c615c952..a7c64cf8fc 100644 --- a/libavutil/attributes.h +++ b/libavutil/attributes.h @@ -150,11 +150,10 @@ # define av_alias #endif -#if (defined(__GNUC__) || defined(__clang__)) && !defined(__INTEL_COMPILER) -#define av_uninit(x) x=x -#else -#define av_uninit(x) x -#endif +/** + * This macro is deprecated + */ +#define av_uninit(x) x #if defined(__GNUC__) || defined(__clang__) #define av_builtin_constant_p __builtin_constant_p -- 2.46.0 ___ 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 04/22] avformat/electronicarts: Remove use of av_uninit
From: Zhao Zhili
---
libavformat/electronicarts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
index 04acf3a409..fa256c4137 100644
--- a/libavformat/electronicarts.c
+++ b/libavformat/electronicarts.c
@@ -595,7 +595,7 @@ static int ea_read_packet(AVFormatContext *s, AVPacket *pkt)
int hit_end = 0;
unsigned int chunk_type, chunk_size;
int ret = 0, packet_read = 0, key = 0, vp6a;
-int av_uninit(num_samples);
+int num_samples = 0;
while ((!packet_read && !hit_end) || partial_packet) {
chunk_type = avio_rl32(pb);
--
2.46.0
___
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 00/22] Deprecate av_uninit
Zhao Zhili (HE12025-04-11): > From: Zhao Zhili > > The macro is meant to suppress false uninitialized warnings. However, > sometimes these 'false uninitialized warnings' are really undefined > behavior, and leading to real issue like crash, e.g., ab792634197e. > > For false uninitialized warnings, it can be silenced by initialization, > and compiler can easily optimize away unnecessary initializations. > > av_uninit shouldn't be used in any case. NAK, you are hiding the UBs, not fixing the bugs. If the author of the code put av_uninit, that means they believe the value will always have been initialized by the part of the code responsible for it. If that is not true, then it is a bug that can lead to an exploitable security issue or a silent data corruption. With your changes, nothing proves that the = 0 you put there is the right value, the bug is still there: the code expects the value to be correctly set, but instead there is an arbitrary 0. At least, with av_uninit, valgrind and fuzzing can find the bugs. Regards, -- 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 00/22] Deprecate av_uninit
> On Apr 11, 2025, at 16:36, Nicolas George wrote: > > Zhao Zhili (HE12025-04-11): >> From: Zhao Zhili >> >> The macro is meant to suppress false uninitialized warnings. However, >> sometimes these 'false uninitialized warnings' are really undefined >> behavior, and leading to real issue like crash, e.g., ab792634197e. >> >> For false uninitialized warnings, it can be silenced by initialization, >> and compiler can easily optimize away unnecessary initializations. >> >> av_uninit shouldn't be used in any case. > > NAK, you are hiding the UBs, not fixing the bugs. > > If the author of the code put av_uninit, that means they believe the > value will always have been initialized by the part of the code > responsible for it. If that is not true, then it is a bug that can lead > to an exploitable security issue or a silent data corruption. > > With your changes, nothing proves that the = 0 you put there is the > right value, the bug is still there: the code expects the value to be > correctly set, but instead there is an arbitrary 0. > > At least, with av_uninit, valgrind and fuzzing can find the bugs. With UB, the compiler can remove branch check and assign some random value to it, which cannot be detected by valgrind. For ab792634197e, the UB is there for decades and never detected by valgrind, and the warning is silenced by av_uninit. > > Regards, > > -- > 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". ___ 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 00/22] Deprecate av_uninit
> On Apr 11, 2025, at 16:36, Nicolas George wrote: > > Zhao Zhili (HE12025-04-11): >> From: Zhao Zhili >> >> The macro is meant to suppress false uninitialized warnings. However, >> sometimes these 'false uninitialized warnings' are really undefined >> behavior, and leading to real issue like crash, e.g., ab792634197e. >> >> For false uninitialized warnings, it can be silenced by initialization, >> and compiler can easily optimize away unnecessary initializations. >> >> av_uninit shouldn't be used in any case. > > NAK, you are hiding the UBs, not fixing the bugs. By the way, logic bug isn’t equal to UB, so I’m not hiding UB. Who put av_uninit in the code means there is no logic bug. If there is, the patchset fixed UB or replaced UB by deterministic logic error, which can’t be worse. > > If the author of the code put av_uninit, that means they believe the > value will always have been initialized by the part of the code > responsible for it. If that is not true, then it is a bug that can lead > to an exploitable security issue or a silent data corruption. If there is no bug for code with av_uninit, the patchset does nothing really. If there is, the patchset fixed or makes the issue deterministic. We don’t initialized all variables when declaration. But if there is a sometimes-uninitialized warning, there is some reason for compiler. Uninitialized warning isn’t the same as deprecated or unused, it should never be ignored in my opinion. > > With your changes, nothing proves that the = 0 you put there is the > right value, the bug is still there: the code expects the value to be > correctly set, but instead there is an arbitrary 0. > > At least, with av_uninit, valgrind and fuzzing can find the bugs. > > Regards, > > -- > 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". ___ 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 00/22] Deprecate av_uninit
Zhao Zhili (HE12025-04-11): > With UB, the compiler can remove branch check and assign some random > value to it, which cannot be detected by valgrind. > > For ab792634197e, the UB is there for decades and never detected by > valgrind, and the warning is silenced by av_uninit. You make a valid point, but my own point still stands: your change make it harder to find bugs by removing the difference between “I put this initialization there because it is the right value” and “I put this there because the compiler is too stupid to figure it on its own”. I have to ideas to accommodate both issues: - Have a FATE instance with valgrind and optimizations disabled, so that the compiler does not optimize the code away because of the UB and valgrind catches the bug. - Initialize to 0xDEADBEEF and add av_assert2(val != 0xDEADBEEF) at the place where it will be used. The second one might be fragile, but less than an UB. > By the way, logic bug isn’t equal to UB, so I’m not hiding UB. Yes you are. > Who put av_uninit in the code means there is no logic bug. If there is, > the patchset fixed UB or replaced UB by deterministic logic error, which > can’t be worse. The deterministic logic error is not worse, but it is not better either, and it is harder to detect. > If there is, the patchset fixed or makes the issue deterministic. This patches fixes NOTHING, I hope we agree on it. What you did is basically equivalent to removing an assert that fails and hoping the code that comes later will be fine. Making the bug deterministic is not better if it makes it harder to detect. > We don’t initialized all variables when declaration. But if there is a > sometimes-uninitialized warning, there is some reason for compiler. > Uninitialized warning isn’t the same as deprecated or unused, it should > never be ignored in my opinion. You are wrong on this, the compiler is often unable to figure out that the code always sets the variable before reading it when the developer can prove it. Compilers are getting better, but they are still far from perfect, and av_unused is precisely there for that lack of perfection, and needs to stay there. Regards, -- 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 00/22] Deprecate av_uninit
> On Apr 11, 2025, at 17:32, Nicolas George wrote: > > Zhao Zhili (HE12025-04-11): >> With UB, the compiler can remove branch check and assign some random >> value to it, which cannot be detected by valgrind. >> >> For ab792634197e, the UB is there for decades and never detected by >> valgrind, and the warning is silenced by av_uninit. > > You make a valid point, but my own point still stands: your change make > it harder to find bugs by removing the difference between “I put this > initialization there because it is the right value” and “I put this > there because the compiler is too stupid to figure it on its own”. > > I have to ideas to accommodate both issues: > > - Have a FATE instance with valgrind and optimizations disabled, so that > the compiler does not optimize the code away because of the UB and > valgrind catches the bug. > > - Initialize to 0xDEADBEEF and add av_assert2(val != 0xDEADBEEF) at the > place where it will be used. This is only check on particular compiler implementation. It works until someone use a different compiler and UB kicks in. > > The second one might be fragile, but less than an UB. > >> By the way, logic bug isn’t equal to UB, so I’m not hiding UB. > > Yes you are. > >> Who put av_uninit in the code means there is no logic bug. If there is, >> the patchset fixed UB or replaced UB by deterministic logic error, which >> can’t be worse. > > The deterministic logic error is not worse, but it is not better either, > and it is harder to detect. > >> If there is, the patchset fixed or makes the issue deterministic. > > This patches fixes NOTHING, I hope we agree on it. What you did is > basically equivalent to removing an assert that fails and hoping the > code that comes later will be fine. > > Making the bug deterministic is not better if it makes it harder to > detect. > >> We don’t initialized all variables when declaration. But if there is a >> sometimes-uninitialized warning, there is some reason for compiler. >> Uninitialized warning isn’t the same as deprecated or unused, it should >> never be ignored in my opinion. > > You are wrong on this, the compiler is often unable to figure out that > the code always sets the variable before reading it when the developer > can prove it. Compilers are getting better, but they are still far from > perfect, and av_unused is precisely there for that lack of perfection, > and needs to stay there. > > Regards, > > -- > 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". ___ 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 00/22] Deprecate av_uninit
Zhao Zhili (HE12025-04-11): > This is only check on particular compiler implementation. It works until > someone > use a different compiler and UB kicks in. No, with this suggestion there is no UB. -- 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".
[FFmpeg-devel] [RFC] Remove use of av_uninit
Background: 1. There is a av_uninit macro which suppress uninitialized variable warning #if (defined(__GNUC__) || defined(__clang__)) && !defined(__INTEL_COMPILER) #define av_uninit(x) x=x #else #define av_uninit(x) x #endif 2. Declaration after statement coding style is allowed now 3. There is a crash fixed by ab792634197 which is related to av_uninit, and I have confronted it again recently commit ab792634197e364ca1bb194f9abe36836e42f12d Date: Mon Oct 18 12:31:38 2021 +0300 seek: Fix crashes in ff_seek_frame_binary if built with latest Clang 14 What I suggests: 1. Deprecated av_uninit and removed it in the future 2. Remove usage in current code 3. When there is uninitiated variable warning a. Declare variable when need if it’s possible, so it can be initiated at the same time b. Use valid default value if there is c. If there is no valid default value, or you think the variable never being used uninitialized, assign an insane value and add assert before use if av_assert1(value != invalid_value); Note: 1. I’m not suggesting always initialize variables 2. I’m suggesting remove av_uninit, until someone has an idea to make it more useful and less harmful. Let’s ensure we stay on track and maintain focus on the central issue. ___ 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 00/22] Deprecate av_uninit
Hello,
1. I agree that av_uninit should be removed; in fact, I also wanted to
do so and already made some commits to do so where I consider the commit
to be beneficial on its own (even without the av_uninit removal) like
348461e550f88c5e27afeafc85e7ace11fe8fae4 and its parent.
2. You fail to actually cite the spec. Here is the proper part (C11
6.3.2.1 (2)):
"If the lvalue designates an object of automatic storage duration that
could have been declared with the register storage class (never had its
address taken), and that object is uninitialized (not declared with an
initializer and no assignment to it has been performed prior to use),
the behavior is undefined."
According to this, most of the uses of av_uninit are UB themselves, so
removing it is really fixing UB. This makes Nicolas' counterpoint about
hiding UB moot. It is also the reason why I wanted to remove av_uninit.
3. But I disagree that the variables marked with av_uninit should be
initialized. They need not be unless the initial value is really used
for something (even if only passed to a function, as this would be UB
according to the spec cited above; this is the reason behind
ab792634197e364ca1bb194f9abe36836e42f12d, whereas tools like Valgrind
where happy to pass an uninitialized value to a function if said
function would not use it). Just like we do it with stack variables.
Zhao Zhili:
>
>
>> On Apr 11, 2025, at 16:36, Nicolas George wrote:
>>
>> Zhao Zhili (HE12025-04-11):
>>> From: Zhao Zhili
>>>
>>> The macro is meant to suppress false uninitialized warnings. However,
>>> sometimes these 'false uninitialized warnings' are really undefined
>>> behavior, and leading to real issue like crash, e.g., ab792634197e.
>>>
>>> For false uninitialized warnings, it can be silenced by initialization,
>>> and compiler can easily optimize away unnecessary initializations.
>>>
>>> av_uninit shouldn't be used in any case.
>>
>> NAK, you are hiding the UBs, not fixing the bugs.
>
> By the way, logic bug isn’t equal to UB, so I’m not hiding UB.
>
> Who put av_uninit in the code means there is no logic bug. If there is,
> the patchset fixed UB or replaced UB by deterministic logic error, which
> can’t be worse.
>
>>
>> If the author of the code put av_uninit, that means they believe the
>> value will always have been initialized by the part of the code
>> responsible for it. If that is not true, then it is a bug that can lead
>> to an exploitable security issue or a silent data corruption.
>
> If there is no bug for code with av_uninit, the patchset does nothing really.
> If there is, the patchset fixed or makes the issue deterministic.
4. Wrong: You convey to the reader that this initial value will be used.
And you probably also make the compiler emit code to initialize the value.
>
> We don’t initialized all variables when declaration. But if there is a
> sometimes-uninitialized warning, there is some reason for compiler.
> Uninitialized warning isn’t the same as deprecated or unused, it should
> never be ignored in my opinion.
>
5. The point above is completely wrong. GCC produced tons of these
warnings that were silenced by av_uninit. Until the warning has been
disabled in de6061203e2d509579ab110fb1873aade34320f5. GCC devs in
general seem to not care much about producing bogus warnings (see the
-Wstringop-overflow= warnings produced by aacsbr_template.c). (Clang
warnings about uninitialized values are different though.)
Just take a look at the lavf/electronicarts.c code. You can clearly see
that num_samples is used iff it has been initialized (unless chunk_size
is zero or av_get_packet() errors out). It would only be different if
one of these avio function or av_get_packet() were to change the
demuxer's private context. Using a smaller scope for num_samples (even
when I use a restrict qualified pointer to const to access the private
context in the smaller scope (where the private context is indeed not
changed) to indicate to the compiler that nothing changes the private
context in this scope does not inhibit GCC from emitting
-Wmaybe-uninitialized warning, only -Wno-maybe-uninitialized does. I
really don't know what GCC is thinking here.
But given that we now set -Wno-maybe-uninitialized for GCC, simply
removing these av_uninit does not seem to lead to warnings.
>>
>> With your changes, nothing proves that the = 0 you put there is the
>> right value, the bug is still there: the code expects the value to be
>> correctly set, but instead there is an arbitrary 0.
>>
>> At least, with av_uninit, valgrind and fuzzing can find the bugs.
6. Valgrind is not really the right tool for the job here. Consider code
like the following
int foo;
// something
for (int i = 0; i < count; i++) {
if (condition(i))
foo = bar(i);
}
ctx->baz = foo;
foo may be stored in a register and said register may have already been
written to in "something" and valgrind doesn't know that said register
is now supposed to hold a new variable. msan (memory san
Re: [FFmpeg-devel] [RFC] Remove use of av_uninit
Zhao Zhili (HE12025-04-11): > Background: > > 1. There is a av_uninit macro which suppress uninitialized variable warning > > #if (defined(__GNUC__) || defined(__clang__)) && !defined(__INTEL_COMPILER) > #define av_uninit(x) x=x > #else > #define av_uninit(x) x > #endif Yes. > 2. Declaration after statement coding style is allowed now I fail to see the connection. Oh, reading further, I guess you think moving the declaration of the variable will be enough to let the compiler know the variable is not used and silence the warning. I strongly doubt it might have that effect. > 3. There is a crash fixed by ab792634197 which is related to av_uninit, and I > have confronted it again recently So, what does it prove? It proves that 16 years ago (e658657528d) Diego silenced a warning without making sure the logic itself was sound. Which is exactly what you are proposing to do right now in a different way. > 1. Deprecated av_uninit and removed it in the future > > 2. Remove usage in current code Not before or without 3. > 3. When there is uninitiated variable warning > > a. Declare variable when need if it’s possible, so it can be initiated at the > same time > b. Use valid default value if there is > c. If there is no valid default value, or you think the variable never being > used uninitialized, assign an insane value and add assert before use if > > av_assert1(value != invalid_value); Are you volunteering to do that huge work? Running FATE with valgrind and no optimization seems like a quicker way of finding the real bugs. Regards, -- 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".
[FFmpeg-devel] [PATCH] avcodec/decode: Only use ff_progress_frame_get_buffer() with blank input
Patch attached.
- Andreas
From 90d34e5d87a390b6e0058426a1f0b1168eb5ebeb Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt
Date: Fri, 11 Apr 2025 11:01:01 +0200
Subject: [PATCH] avcodec/decode: Only use ff_progress_frame_get_buffer() with
blank input
All users (namely HEVC) that use ff_progress_frame_alloc()
should just use ff_thread_get_buffer(). Using
ff_progress_frame_get_buffer() is not a must; it is merely
a convenience wrapper.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/decode.c| 11 +++
libavcodec/hevc/refs.c | 6 +++---
libavcodec/progressframe.h | 10 --
3 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 14702fb041..c2b2dd6e3b 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1761,14 +1761,9 @@ int ff_progress_frame_alloc(AVCodecContext *avctx, ProgressFrame *f)
int ff_progress_frame_get_buffer(AVCodecContext *avctx, ProgressFrame *f, int flags)
{
-int ret;
-
-check_progress_consistency(f);
-if (!f->f) {
-ret = ff_progress_frame_alloc(avctx, f);
-if (ret < 0)
-return ret;
-}
+int ret = ff_progress_frame_alloc(avctx, f);
+if (ret < 0)
+return ret;
ret = ff_thread_get_buffer(avctx, f->progress->f, flags);
if (ret < 0) {
diff --git a/libavcodec/hevc/refs.c b/libavcodec/hevc/refs.c
index 6f291bbf79..ab2e075af0 100644
--- a/libavcodec/hevc/refs.c
+++ b/libavcodec/hevc/refs.c
@@ -29,6 +29,7 @@
#include "hevc.h"
#include "hevcdec.h"
#include "progressframe.h"
+#include "thread.h"
#include "libavutil/refstruct.h"
void ff_hevc_unref_frame(HEVCFrame *frame, int flags)
@@ -157,10 +158,9 @@ static HEVCFrame *alloc_frame(HEVCContext *s, HEVCLayerContext *l)
}
}
-ret = ff_progress_frame_get_buffer(s->avctx, &frame->tf,
- AV_GET_BUFFER_FLAG_REF);
+ret = ff_thread_get_buffer(s->avctx, frame->f, AV_GET_BUFFER_FLAG_REF);
if (ret < 0)
-return NULL;
+goto fail;
frame->rpl = av_refstruct_allocz(s->pkt.nb_nals * sizeof(*frame->rpl));
if (!frame->rpl)
diff --git a/libavcodec/progressframe.h b/libavcodec/progressframe.h
index 32a345beec..e3cb83c5b4 100644
--- a/libavcodec/progressframe.h
+++ b/libavcodec/progressframe.h
@@ -102,10 +102,9 @@ void ff_progress_frame_report(ProgressFrame *f, int progress);
void ff_progress_frame_await(const ProgressFrame *f, int progress);
/**
- * This function allocates ProgressFrame.f
- * May be called before ff_progress_frame_get_buffer() in the cases where the
- * AVFrame needs to be accessed before the ff_thread_get_buffer() call in
- * ff_progress_frame_alloc().
+ * This function sets up the ProgressFrame, i.e. ProgressFrame.f
+ * and ProgressFrame.progress. ProgressFrame.f will be blank
+ * (as if from av_frame_alloc() or av_frame_unref()) on success.
*
* @note: This must only be called by codecs with the
*FF_CODEC_CAP_USES_PROGRESSFRAMES internal cap.
@@ -113,8 +112,7 @@ void ff_progress_frame_await(const ProgressFrame *f, int progress);
int ff_progress_frame_alloc(struct AVCodecContext *avctx, ProgressFrame *f);
/**
- * This function sets up the ProgressFrame, i.e. allocates ProgressFrame.f
- * if needed, and also calls ff_thread_get_buffer() on the frame.
+ * Wrapper around ff_progress_frame_alloc() and ff_thread_get_buffer().
*
* @note: This must only be called by codecs with the
*FF_CODEC_CAP_USES_PROGRESSFRAMES internal cap.
--
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 1/2] avcodec/h261dec: Export key frame information
Andreas Rheinhardt: > Patches attached. > > - Andreas > Will apply this patchset tomorrow unless there are objections. - Andreas ___ 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] avcodec/avcodec: Remove always-false check
Andreas Rheinhardt: > Patch attached. > > - Andreas > Will apply this patch tomorrow unless there are objections. - Andreas ___ 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 01/10] avcodec/hq{xvlc, _hqadata}: Deduplicate and hardcode cbp, table
Andreas Rheinhardt: > Patches attached. > > - Andreas > Will apply this patchset tomorrow unless there are objections. - Andreas ___ 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] libpostproc: remove AMD 3DNow! define
It was removed / left unreferenced some time ago. --- libpostproc/postprocess.h | 1 - 1 file changed, 1 deletion(-) diff --git a/libpostproc/postprocess.h b/libpostproc/postprocess.h index 5decb7e8a9..cc48c552cc 100644 --- a/libpostproc/postprocess.h +++ b/libpostproc/postprocess.h @@ -87,7 +87,6 @@ void pp_free_context(pp_context *ppContext); #define PP_CPU_CAPS_MMX 0x8000 #define PP_CPU_CAPS_MMX2 0x2000 -#define PP_CPU_CAPS_3DNOW 0x4000 #define PP_CPU_CAPS_ALTIVEC 0x1000 #define PP_CPU_CAPS_AUTO 0x0008 -- 2.39.5 ___ 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] libpostproc: remove AMD 3DNow! define
Sean McGovern: > It was removed / left unreferenced some time ago. > --- > libpostproc/postprocess.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/libpostproc/postprocess.h b/libpostproc/postprocess.h > index 5decb7e8a9..cc48c552cc 100644 > --- a/libpostproc/postprocess.h > +++ b/libpostproc/postprocess.h > @@ -87,7 +87,6 @@ void pp_free_context(pp_context *ppContext); > > #define PP_CPU_CAPS_MMX 0x8000 > #define PP_CPU_CAPS_MMX2 0x2000 > -#define PP_CPU_CAPS_3DNOW 0x4000 > #define PP_CPU_CAPS_ALTIVEC 0x1000 > #define PP_CPU_CAPS_AUTO 0x0008 > Missing deprecation etc. As has been said. - Andreas ___ 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] libpostproc: remove AMD 3DNow! define
On Fri, Apr 11, 2025 at 9:17 AM Andreas Rheinhardt wrote: > > Sean McGovern: > > It was removed / left unreferenced some time ago. > > --- > > libpostproc/postprocess.h | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/libpostproc/postprocess.h b/libpostproc/postprocess.h > > index 5decb7e8a9..cc48c552cc 100644 > > --- a/libpostproc/postprocess.h > > +++ b/libpostproc/postprocess.h > > @@ -87,7 +87,6 @@ void pp_free_context(pp_context *ppContext); > > > > #define PP_CPU_CAPS_MMX 0x8000 > > #define PP_CPU_CAPS_MMX2 0x2000 > > -#define PP_CPU_CAPS_3DNOW 0x4000 > > #define PP_CPU_CAPS_ALTIVEC 0x1000 > > #define PP_CPU_CAPS_AUTO 0x0008 > > > > Missing deprecation etc. As has been said. WHY does this need a deprecation period? It neither enables nor disables anything. > > - Andreas > > ___ > 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". -- Sean McGovern ___ 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] libpostproc: remove AMD 3DNow! define
Sean McGovern: > On Fri, Apr 11, 2025 at 9:17 AM Andreas Rheinhardt > wrote: >> >> Sean McGovern: >>> It was removed / left unreferenced some time ago. >>> --- >>> libpostproc/postprocess.h | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/libpostproc/postprocess.h b/libpostproc/postprocess.h >>> index 5decb7e8a9..cc48c552cc 100644 >>> --- a/libpostproc/postprocess.h >>> +++ b/libpostproc/postprocess.h >>> @@ -87,7 +87,6 @@ void pp_free_context(pp_context *ppContext); >>> >>> #define PP_CPU_CAPS_MMX 0x8000 >>> #define PP_CPU_CAPS_MMX2 0x2000 >>> -#define PP_CPU_CAPS_3DNOW 0x4000 >>> #define PP_CPU_CAPS_ALTIVEC 0x1000 >>> #define PP_CPU_CAPS_AUTO 0x0008 >>> >> >> Missing deprecation etc. As has been said. > > WHY does this need a deprecation period? It neither enables nor > disables anything. > Because it is public API. Users may set still set it (although it does nothing). - Andreas ___ 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 v2] libpostproc: deprecate the AMD 3DNow! define
It was left unreferenced in 1f0948272a0fcd0e4947f629b600983f3338c02f. --- libpostproc/postprocess.h | 2 ++ libpostproc/version.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/libpostproc/postprocess.h b/libpostproc/postprocess.h index 5decb7e8a9..7010c2b51b 100644 --- a/libpostproc/postprocess.h +++ b/libpostproc/postprocess.h @@ -87,7 +87,9 @@ void pp_free_context(pp_context *ppContext); #define PP_CPU_CAPS_MMX 0x8000 #define PP_CPU_CAPS_MMX2 0x2000 +#if PP_AMD_3DNOW #define PP_CPU_CAPS_3DNOW 0x4000 +#endif #define PP_CPU_CAPS_ALTIVEC 0x1000 #define PP_CPU_CAPS_AUTO 0x0008 diff --git a/libpostproc/version.h b/libpostproc/version.h index bcbdd210c4..5a272630bf 100644 --- a/libpostproc/version.h +++ b/libpostproc/version.h @@ -43,4 +43,6 @@ #define LIBPOSTPROC_IDENT "postproc" AV_STRINGIFY(LIBPOSTPROC_VERSION) +#define PP_AMD_3DNOW (LIBPOSTPROC_VERSION_MAJOR < 60) + #endif /* POSTPROC_VERSION_H */ -- 2.39.5 ___ 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] libpostproc: remove AMD 3DNow! define
Sean McGovern (HE12025-04-11): > WHY does this need a deprecation period? It neither enables nor > disables anything. If somebody uses it in their code, the fact that it does nothing will not prevent the compilation to fail. Regards, -- 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] [RFC] FFmpeg Execution Graph Visualization
Michael Niedermayer (HE12025-03-29):
> can you repost the AVWriter patches ?
Sure, but I need to find the latest branch, and it is proving a cause of
procrastination for me. In the meantime, let me explain anew the
principles behind it, after a few years I might be able to express them
better.
(I hope it means you are considering re-asserting your authority to
approve it on principle pending review of the code. I will not invest
more coding into it unless I know nobody has the authority to put it to
waste with “it does not belong”.)
1. Why we need it
We use a lot of text for small things. As a command-line tool, we use
text to communicate with users. Even as a library, when GUIs do not have
the proper widget they will use text, and we need to provide the
necessary support functions.
We should have a rule that for every type we add, we must have a
corresponding to-text API function. Furthermore, these functions should
all have the same type (up to the type of the opaque pointer), so that
we can store them in a structure that describes the type along with
other information (from string, ref/unref, etc.).
We also need text for inter-process communication, like exporting
statistics from filters.
Some of these to-text conversions happen once per frame. That means they
should happen without dynamic allocation. Quite complex code is
warranted to prevent dynamic allocations once per frame: see the frame
and buffer pools too. But some of these to-text conversion have an
unbounded output. That means the API needs to be able to handle long
output.
BPrint meets the criteria: BPrint is as fast as a buffer on the stack
but can handle strings of arbitrary length.
Unfortunately, BPrint is ugly. The name is ugly, and more importantly,
it is inflexible, it works only with flat buffers in memory.
2. How AVWriter does it
AVWriter is an attempt — successful, in my opinion — at keeping what is
good in BPrint and fixing what is ugly, in particular by giving it
features similar to AVIO.
The design principle is the same as AVIO: a structure with callbacks
that perform the low-level operations.
The difference with AVIO is that effort were made to keep the structures
small and allocatable by the caller.
Also, the back-end is allowed to provide only a few methods: printf-like
or write-like or get_buffer-like, and the framework will make sure to
use one that is available. That means quite a bit of code to handle
doing a printf() on a back-end that only has get_buffer, but it is code
isolated within a single file and with ample FATE coverage.
One of the tricks I used to avoid dynamic allocations is to store the
structure of methods and the structure with an instance as a structure
with two pointer elements passed by value.
Another trick I used to avoid dynamic allocations is to store the size
of a structure in it. That way, if a caller is compiled with an old
version of the library, it stores a smaller size than the current one,
and the new version of the library can test and avoid accessing the new
fields. That lets the caller allocate the structure while keeping the
ability to add fields to the structure.
Apart from that, is is rather straightforward. The default AVWriter is a
simple wrapper around BPrint, but there are also quite a few other
built-in ones: wrapper around stdio and av_log (without storing the
whole string), wrapper around a fixed-size buffer. AVwriter can also act
as a filter: get an AVWriter, you can create a new one that will encode
to base64 or HTML entities on the fly.
3. Where it will go
The trick passing a struct of methods and the object as a structure with
two members passed by value is meant to become a very light-weight
object system. Thus if ctx is a pointer to a structure of type T, we can
define serialize_t as a structure with functions and consider
{ serialize_t, ctx } as an object of the class “serializable”. But we
can also make it an object of the class “refcounted” by pairing it with
another structure.
The object system, including casts from one class to another, can be
de-centralized, new classes ca be defined anywhere in the code without a
central registry. That will be useful to enhance several parts of our
code:
- Side data would not need to be added in libavutil. A pair of
demuxer-muxer in libavformat can define a new type of side data by
defining the methods to handle it.
- Filters with unusual parsing needs can define a new type of option and
plug it into the standard options parsing system. It is extremely
useful if the needs of the filter are too specific to add a type in
libavutil but too far from something existing to be able to use it
conveniently.
- Pluging new types into the options system will automatically fix our
“escaping hell” issue where users sometimes need 42 levels of \ in
front of a : to achieve their goals.
- We can implement a full-featured av_printf() function that serializes
on the fly:
av_printf(out, "Stream %d at %d Hz, %@
[FFmpeg-devel] [PATCH 1/2] avcodec/asvenc: Don't use FF_INPUT_BUFFER_MIN_SIZE
Patches attached.
- Andreas
From e2ca8a7268f0d6071d170b1d99e5a264392e1b2e Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt
Date: Fri, 11 Apr 2025 00:36:59 +0200
Subject: [PATCH 1/2] avcodec/asvenc: Don't use FF_INPUT_BUFFER_MIN_SIZE
ASV-1/2 does not really have a header and so using
FF_INPUT_BUFFER_MIN_SIZE is wasteful as well as ugly
(such bounds should be codec-specific).
Signed-off-by: Andreas Rheinhardt
---
libavcodec/asvenc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/asvenc.c b/libavcodec/asvenc.c
index 9218a41021..e7d931cca9 100644
--- a/libavcodec/asvenc.c
+++ b/libavcodec/asvenc.c
@@ -271,8 +271,8 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
return ret;
}
-if ((ret = ff_alloc_packet(avctx, pkt, c->mb_height * c->mb_width * MAX_MB_SIZE +
- FF_INPUT_BUFFER_MIN_SIZE)) < 0)
+ret = ff_alloc_packet(avctx, pkt, c->mb_height * c->mb_width * MAX_MB_SIZE + 3);
+if (ret < 0)
return ret;
init_put_bits(&a->pb, pkt->data, pkt->size);
--
2.45.2
From 653e8d3da311b56914fbe80b56507ee5fe38 Mon Sep 17 00:00:00 2001
From: Andreas Rheinhardt
Date: Fri, 11 Apr 2025 00:38:59 +0200
Subject: [PATCH 2/2] avcodec/asvenc: Use tighter MAX_MB_SIZE constant
Also document the constant.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/asvenc.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/libavcodec/asvenc.c b/libavcodec/asvenc.c
index e7d931cca9..52666ee547 100644
--- a/libavcodec/asvenc.c
+++ b/libavcodec/asvenc.c
@@ -50,6 +50,14 @@ typedef struct ASVEncContext {
int q_intra_matrix[64];
} ASVEncContext;
+enum {
+ASV1_MAX_BLOCK_SIZE = 8 + 10 * FFMAX(2 /* skip */, 5 /* ccp */ + 4 * 11 /* level */) + 5,
+ASV1_MAX_MB_SIZE= 6 * ASV1_MAX_BLOCK_SIZE,
+ASV2_MAX_BLOCK_SIZE = 4 + 8 + 16 * (6 /* ccp */ + 4 * 13 /* level */),
+ASV2_MAX_MB_SIZE= 6 * ASV2_MAX_BLOCK_SIZE,
+MAX_MB_SIZE = (FFMAX(ASV1_MAX_MB_SIZE, ASV2_MAX_MB_SIZE) + 7) / 8
+};
+
static inline void asv1_put_level(PutBitContext *pb, int level)
{
unsigned int index = level + 3;
@@ -177,8 +185,6 @@ static inline void asv2_encode_block(ASVEncContext *a, int16_t block[64])
}
}
-#define MAX_MB_SIZE (30 * 16 * 16 * 3 / 2 / 8)
-
static inline int encode_mb(ASVEncContext *a, int16_t block[6][64])
{
int i;
--
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] [RFC] AVDictionary2
Hi On Tue, Apr 08, 2025 at 09:30:16PM +, softworkz . wrote: [...] > To tell you the truth - at that point I was thinking: "Ah, clever! That's why > the AVDictionary is done like that" 😊 The dictionary implementation is not clever look at copy for example it iterates over av_dict_set() which itself calls av_dict_get() which it itself iterates over the dictionary so av_dict_copy() is O(n^2) for example also a single fate run, calls av_dict_iterate() 4921207 times and fate should mostly be short small files and minimal self contained testcases thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment. 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] [RFC] AVDictionary2
On Tue, Apr 08, 2025 at 12:19:59PM +0200, Michael Niedermayer wrote: > Hi all > > As i have too many things to do already i did the most logic thing and > started thinking about a new and unrelated idea. > > This is a list of problems and ideas, that everyone is welcome to add to and > comment on. > > AVDictionary is just bad. > > * its complicated internally with > unneeded alternative (AV_DICT_DONT_STRDUP_VAL/KEY) these are rarely used > and probably not relevant for performance. > > * all basic operations are as slow as possible. > you want to find, update or remove an entry, search through all entries > > * its heavy on memory allocations > 1 malloc for key, 1 malloc for value, 1 realloc on the AVDictionaryEntry > array > that makes 2+ malloc() for every "foo"="bar" > > Ideas: > 1. put the node struct (AVDictionaryEntry), the key and value in the same >allocated block, 1 malloc() instead of 2. >We can simply concatenate the key and value string, we could even use the >0 terminator instead of the 2nd pointer. Either way the whole >can go to the end of the Node structure for a tree > 1b. Now if we did put the key and value together, we can order in the tree >by this combined entity. Why ? because now we have a unique ordering >and also the key+value could be required to be always unique. Simplifying >things from what we have now and making it more replicatable, no >more changes in output because order changed > 2. We have a simple AVL tree implementation which we could use to make >all operations O(log n) instead of O(n) > 3. We could go with hash tables, splay trees, critbit trees or something >else. hash tables have issues with malicious/odd input which would >require more complexity to workaround. > > Of course we could also go a step further and eliminate the malloc per > node and put it all in a linear array. > As in, insert -> append at the end, > realloc with every power of 2 size increase > complete rebuild once enough elements are removed > not sure this isnt overkill for a metadata string dictionary > > I probably wont have time to implement this in the near future but as i > was thinking about this, it seemed to make sense to write this down and > post here > > git grep av_dict | wc is 1436 > > So its used a bit, justifying looking at improving it > > > git grep AV_DICT_DONT_STRDUP | wc is 87 > git grep AV_DICT_DONT_STRDUP libavutil/ tests doc | wc is 20 > > Seems not too common and one malloc/copy of a string once per metadata entry > which is once per file generally, seems a strange optimization to me If anyone wants a testcase that shows dictionary speed issues, try to do anything with this file: https://samples.ffmpeg.org/nut/meta.nut its a single image 256x256 jpeg with a lot of metadata thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable 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".
[FFmpeg-devel] [PATCH 1/2] avformat/id3v2: Print the unknown encoding
Signed-off-by: Michael Niedermayer --- libavformat/id3v2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c index 3bdd23a2509..90314583a74 100644 --- a/libavformat/id3v2.c +++ b/libavformat/id3v2.c @@ -304,7 +304,7 @@ static int decode_str(AVFormatContext *s, AVIOContext *pb, int encoding, } break; default: -av_log(s, AV_LOG_WARNING, "Unknown encoding\n"); +av_log(s, AV_LOG_WARNING, "Unknown encoding %d\n", encoding); } if (ch) -- 2.49.0 ___ 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 2/2] avformat/id3v2: Check that decode_str() did advance
Fixes infinite loop with unknown encodings
We could alternatively error out from decode_str() or consume all of taglen
this would affect other callers though.
Fixes:
409819224/clusterfuzz-testcase-minimized-ffmpeg_dem_H261_fuzzer-6003527535362048
Signed-off-by: Michael Niedermayer
---
libavformat/id3v2.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
index 90314583a74..e3f7f9e2a90 100644
--- a/libavformat/id3v2.c
+++ b/libavformat/id3v2.c
@@ -341,10 +341,13 @@ static void read_ttag(AVFormatContext *s, AVIOContext
*pb, int taglen,
taglen--; /* account for encoding type byte */
while (taglen > 1) {
+int current_taglen = taglen;
if (decode_str(s, pb, encoding, &dst, &taglen) < 0) {
av_log(s, AV_LOG_ERROR, "Error reading frame %s, skipped\n", key);
return;
}
+if (current_taglen == taglen)
+return;
count++;
--
2.49.0
___
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/2] avcodec/asvenc: Don't use FF_INPUT_BUFFER_MIN_SIZE
On Fri, Apr 11, 2025 at 08:57:09PM +0200, Andreas Rheinhardt wrote:
> Patches attached.
>
> - Andreas
> asvenc.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> f02ccdc5e7ec0835b2af195734590d1e3f294d33
> 0001-avcodec-asvenc-Don-t-use-FF_INPUT_BUFFER_MIN_SIZE.patch
> From e2ca8a7268f0d6071d170b1d99e5a264392e1b2e Mon Sep 17 00:00:00 2001
> From: Andreas Rheinhardt
> Date: Fri, 11 Apr 2025 00:36:59 +0200
> Subject: [PATCH 1/2] avcodec/asvenc: Don't use FF_INPUT_BUFFER_MIN_SIZE
>
> ASV-1/2 does not really have a header and so using
> FF_INPUT_BUFFER_MIN_SIZE is wasteful as well as ugly
ok
> (such bounds should be codec-specific).
If the time this costs cannot be used for more valuable work
[...]
> asvenc.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
> 32d062bcc89be62b21c93f9d4e9005b195206aa8
> 0002-avcodec-asvenc-Use-tighter-MAX_MB_SIZE-constant.patch
> From 653e8d3da311b56914fbe80b56507ee5fe38 Mon Sep 17 00:00:00 2001
> From: Andreas Rheinhardt
> Date: Fri, 11 Apr 2025 00:38:59 +0200
> Subject: [PATCH 2/2] avcodec/asvenc: Use tighter MAX_MB_SIZE constant
>
> Also document the constant.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/asvenc.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/asvenc.c b/libavcodec/asvenc.c
> index e7d931cca9..52666ee547 100644
> --- a/libavcodec/asvenc.c
> +++ b/libavcodec/asvenc.c
> @@ -50,6 +50,14 @@ typedef struct ASVEncContext {
> int q_intra_matrix[64];
> } ASVEncContext;
>
> +enum {
> +ASV1_MAX_BLOCK_SIZE = 8 + 10 * FFMAX(2 /* skip */, 5 /* ccp */ + 4 * 11
> /* level */) + 5,
> +ASV1_MAX_MB_SIZE= 6 * ASV1_MAX_BLOCK_SIZE,
> +ASV2_MAX_BLOCK_SIZE = 4 + 8 + 16 * (6 /* ccp */ + 4 * 13 /* level */),
> +ASV2_MAX_MB_SIZE= 6 * ASV2_MAX_BLOCK_SIZE,
> +MAX_MB_SIZE = (FFMAX(ASV1_MAX_MB_SIZE, ASV2_MAX_MB_SIZE) + 7) / 8
> +};
beautifull
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
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 v2] configure: Clearer documentation for "disable-safe-bitstream-reader"
On Fri, Apr 11, 2025 at 08:32:55AM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Signed-off-by: Michael Niedermayer > > --- > > configure | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/configure b/configure > > index bd4f8723760..f1db8b6f235 100755 > > --- a/configure > > +++ b/configure > > @@ -436,7 +436,8 @@ Advanced options (experts only): > >--enable-hardcoded-tables use hardcoded tables instead of runtime > > generation > >--disable-safe-bitstream-reader > > disable buffer boundary checking in bitreaders > > - (faster, but may crash) > > + (This disables some security checks and can > > cause undefined behavior, > > +it may be faster, but should only be used with > > trusted input) > >--sws-max-filter-size=N the max filter size swscale uses > > [$sws_max_filter_size_default] > > > > Optimization options (experts only): > > I'd like to keep "crash" in the description. Not everyone (not even > people setting "experts only" options) will be familiar with the term > "undefined behavior". what about: (This disables some security checks and can cause undefined behavior and crashes, it may be faster, but should only be used with trusted input) ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I am the wisest man alive, for I know one thing, and that is that I know nothing. -- Socrates 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 v2] configure: Clearer documentation for "disable-safe-bitstream-reader"
On 12.04.2025 02:11, Michael Niedermayer wrote: On Fri, Apr 11, 2025 at 08:32:55AM +0200, Andreas Rheinhardt wrote: Michael Niedermayer: Signed-off-by: Michael Niedermayer --- configure | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/configure b/configure index bd4f8723760..f1db8b6f235 100755 --- a/configure +++ b/configure @@ -436,7 +436,8 @@ Advanced options (experts only): --enable-hardcoded-tables use hardcoded tables instead of runtime generation --disable-safe-bitstream-reader disable buffer boundary checking in bitreaders - (faster, but may crash) + (This disables some security checks and can cause undefined behavior, +it may be faster, but should only be used with trusted input) --sws-max-filter-size=N the max filter size swscale uses [$sws_max_filter_size_default] Optimization options (experts only): I'd like to keep "crash" in the description. Not everyone (not even people setting "experts only" options) will be familiar with the term "undefined behavior". what about: (This disables some security checks and can cause undefined behavior and crashes, it may be faster, but should only be used with trusted input) Given that the undefined behaviour can in theory even include arbitrary code execution, I'd probably make it even harsher and mention that worst-case being a possibility. ___ 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] VP6 Encoder Proposal- Seeking Feedback and Suggestions
Hi On Tue, Apr 08, 2025 at 07:05:16PM +0530, SUBHANGANI JHA wrote: > Hello FFmpeg developers, > > I'm a 2025 Gsoc applicant and have submitted a proposal for implementing a > basic VP6 Encoder in FFmpeg. I understand the importance of community > engagement and am reaching out to introduce myself and seek any guidance, > feedback,or suggestions you might have on the idea or the approach . > While the proposal is already submitted, I'm highly interested in > contributing regardless of the outcome and would love to get more involved > with the FFmpeg development process . > If there are small tasks or bugs I could help with, or if there is any > documentation or suggestions for working with encoding pipeline in FFmpeg, > I would really appreciate being pointed in the right direction. > > > Looking forward to hearing from you and contributing. Iam not sure someone else has already replied but if you (or someone else) is looking for suggestions there are many some ideas that come to my mind as you mention encoder. a current example would be the FFv1 encoder there are for example 3 different ways to code LSBs. These are in 3 patches on this mailing list. These could be compared in terms of speed and compression. Though likely each patch would only apply cleanly to a git checkout from around that time. If you are interrested in this, i can look at and provide exact links to the patches thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who would give up essential Liberty, to purchase a little temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin 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 v2] configure: Clearer documentation for "disable-safe-bitstream-reader"
On Sat, Apr 12, 2025 at 02:19:08AM +0200, Timo Rothenpieler wrote: > On 12.04.2025 02:11, Michael Niedermayer wrote: > > On Fri, Apr 11, 2025 at 08:32:55AM +0200, Andreas Rheinhardt wrote: > > > Michael Niedermayer: > > > > Signed-off-by: Michael Niedermayer > > > > --- > > > > configure | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/configure b/configure > > > > index bd4f8723760..f1db8b6f235 100755 > > > > --- a/configure > > > > +++ b/configure > > > > @@ -436,7 +436,8 @@ Advanced options (experts only): > > > > --enable-hardcoded-tables use hardcoded tables instead of runtime > > > > generation > > > > --disable-safe-bitstream-reader > > > > disable buffer boundary checking in > > > > bitreaders > > > > - (faster, but may crash) > > > > + (This disables some security checks and can > > > > cause undefined behavior, > > > > +it may be faster, but should only be used > > > > with trusted input) > > > > --sws-max-filter-size=N the max filter size swscale uses > > > > [$sws_max_filter_size_default] > > > > Optimization options (experts only): > > > > > > I'd like to keep "crash" in the description. Not everyone (not even > > > people setting "experts only" options) will be familiar with the term > > > "undefined behavior". > > > > what about: > > > > (This disables some security checks and can cause undefined behavior and > > crashes, it may be faster, but should only be used with trusted input) > > Given that the undefined behaviour can in theory even include arbitrary code > execution, I'd probably make it even harsher and mention that worst-case > being a possibility. what about this: (This disables some security checks and can cause undefined behavior, crashes and arbitrary code execution, it may be faster, but should only be used with trusted input) ? thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB He who knows, does not speak. He who speaks, does not know. -- Lao Tsu 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 v11 1/8] libavcodec/decode.c: intercept `AV_PKT_DATA_METADATA_UPDATE` packet extra data, attach them to the next decoded frame with the same PTS.
On Wed, Apr 09, 2025 at 09:25:56PM -0500, Romain Beauxis wrote:
> Le mer. 9 avr. 2025 à 20:12, Michael Niedermayer
> a écrit :
> >
> > On Fri, Apr 04, 2025 at 04:14:44PM -0500, Romain Beauxis wrote:
> > > ---
> > > libavcodec/decode.c | 130
> > > 1 file changed, 130 insertions(+)
> > >
> > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > > index fca0c7ff58..39d054bdea 100644
> > > --- a/libavcodec/decode.c
> > > +++ b/libavcodec/decode.c
> > [...]
> > > @@ -702,6 +809,8 @@ int attribute_align_arg
> > > avcodec_send_packet(AVCodecContext *avctx, const AVPacke
> > > {
> > > AVCodecInternal *avci = avctx->internal;
> > > DecodeContext *dc = decode_ctx(avci);
> > > +const uint8_t *side_metadata;
> > > +size_t size;
> > > int ret;
> > >
> > > if (!avcodec_is_open(avctx) || !av_codec_is_decoder(avctx->codec))
> > > @@ -719,6 +828,14 @@ int attribute_align_arg
> > > avcodec_send_packet(AVCodecContext *avctx, const AVPacke
> > > ret = av_packet_ref(avci->buffer_pkt, avpkt);
> > > if (ret < 0)
> > > return ret;
> > > +
> > > +side_metadata = av_packet_get_side_data(avpkt,
> > > AV_PKT_DATA_METADATA_UPDATE, &size);
> >
> >
> > > +if (avpkt->pts != AV_NOPTS_VALUE && side_metadata) {
> > > +ret = insert_pending_metadata(&dc->pending_metadata,
> > > avpkt->pts,
> > > + side_metadata, size);
> > > +if (ret < 0)
> > > +return ret;
> >
> > Is the tree needed and a FIFO not enough ?
>
> I believe so.
>
> There could be scenarios where the DTS are submitted out of order and
> we'd still want the metadata to be attached to the frame it was
> intended for.
>
> In fact, I did this change after you suggested such a scenario:
>
> >> Can you describe a scenario that you're thinking about?
>
> > The users feeds several packets into a multi threaded decoder
> > and then depending on the threads and luck sooner or later
> > one frame comes out.
> >
> > Passing some data in a way that disregards this, feels wrong
> > Hypothetically there also could be a 2nd AV_PKT_DATA_METADATA_UPDATE
> > going in before the frame corresponding to the first comes out
> > but i may be missing something
>
> Source: https://ffmpeg.org/pipermail/ffmpeg-devel/2025-March/340948.html
What i was thinkig of is the user passing in multiple
AV_PKT_DATA_METADATA_UPDATE in order
internally these could decode in parallel, but what goes
back to the user is in order again. Now there is frame reordering
but i assumed that metadata updates would not be at that level.
And this would rather be concatenated streams. But i dont know ...
Also if you want to keep the tree code, then error handling may need
to be improved.
Consider that a frame with metadata goes in but decoding has an error
and this frame never comes out.
iam not sure how you want to free metadata for damaged frames that are never
output without assumtations on the ordering
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
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] [RFC] FFmpeg Execution Graph Visualization
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Nicolas George
> Sent: Freitag, 11. April 2025 19:09
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [RFC] FFmpeg Execution Graph Visualization
>
> Michael Niedermayer (HE12025-03-29):
> > can you repost the AVWriter patches ?
>
> Sure, but I need to find the latest branch, and it is proving a cause of
> procrastination for me. In the meantime, let me explain anew the
> principles behind it, after a few years I might be able to express them
> better.
>
> (I hope it means you are considering re-asserting your authority to
> approve it on principle pending review of the code. I will not invest
> more coding into it unless I know nobody has the authority to put it to
> waste with “it does not belong”.)
>
>
> 1. Why we need it
>
> We use a lot of text for small things. As a command-line tool, we use
> text to communicate with users. Even as a library, when GUIs do not have
> the proper widget they will use text, and we need to provide the
> necessary support functions.
>
> We should have a rule that for every type we add, we must have a
> corresponding to-text API function. Furthermore, these functions should
> all have the same type (up to the type of the opaque pointer), so that
> we can store them in a structure that describes the type along with
> other information (from string, ref/unref, etc.).
>
> We also need text for inter-process communication, like exporting
> statistics from filters.
>
> Some of these to-text conversions happen once per frame. That means they
> should happen without dynamic allocation. Quite complex code is
> warranted to prevent dynamic allocations once per frame: see the frame
> and buffer pools too. But some of these to-text conversion have an
> unbounded output. That means the API needs to be able to handle long
> output.
>
> BPrint meets the criteria: BPrint is as fast as a buffer on the stack
> but can handle strings of arbitrary length.
>
> Unfortunately, BPrint is ugly. The name is ugly, and more importantly,
> it is inflexible, it works only with flat buffers in memory.
>
>
> 2. How AVWriter does it
>
> AVWriter is an attempt — successful, in my opinion — at keeping what is
> good in BPrint and fixing what is ugly, in particular by giving it
> features similar to AVIO.
>
> The design principle is the same as AVIO: a structure with callbacks
> that perform the low-level operations.
>
> The difference with AVIO is that effort were made to keep the structures
> small and allocatable by the caller.
>
> Also, the back-end is allowed to provide only a few methods: printf-like
> or write-like or get_buffer-like, and the framework will make sure to
> use one that is available. That means quite a bit of code to handle
> doing a printf() on a back-end that only has get_buffer, but it is code
> isolated within a single file and with ample FATE coverage.
>
> One of the tricks I used to avoid dynamic allocations is to store the
> structure of methods and the structure with an instance as a structure
> with two pointer elements passed by value.
>
> Another trick I used to avoid dynamic allocations is to store the size
> of a structure in it. That way, if a caller is compiled with an old
> version of the library, it stores a smaller size than the current one,
> and the new version of the library can test and avoid accessing the new
> fields. That lets the caller allocate the structure while keeping the
> ability to add fields to the structure.
>
> Apart from that, is is rather straightforward. The default AVWriter is a
> simple wrapper around BPrint, but there are also quite a few other
> built-in ones: wrapper around stdio and av_log (without storing the
> whole string), wrapper around a fixed-size buffer. AVwriter can also act
> as a filter: get an AVWriter, you can create a new one that will encode
> to base64 or HTML entities on the fly.
>
>
> 3. Where it will go
>
> The trick passing a struct of methods and the object as a structure with
> two members passed by value is meant to become a very light-weight
> object system. Thus if ctx is a pointer to a structure of type T, we can
> define serialize_t as a structure with functions and consider
> { serialize_t, ctx } as an object of the class “serializable”. But we
> can also make it an object of the class “refcounted” by pairing it with
> another structure.
>
> The object system, including casts from one class to another, can be
> de-centralized, new classes ca be defined anywhere in the code without a
> central registry. That will be useful to enhance several parts of our
> code:
>
> - Side data would not need to be added in libavutil. A pair of
> demuxer-muxer in libavformat can define a new type of side data by
> defining the methods to handle it.
>
> - Filters with unusual parsing needs can define a new type of option and
> plug it into the standard options parsing system. It is extremely
> useful i
Re: [FFmpeg-devel] [RFC] AVDictionary2
> -Original Message- > From: ffmpeg-devel On Behalf Of > Michael Niedermayer > Sent: Freitag, 11. April 2025 21:06 > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [RFC] AVDictionary2 > > Hi > > On Tue, Apr 08, 2025 at 09:30:16PM +, softworkz . wrote: > [...] > > > To tell you the truth - at that point I was thinking: "Ah, clever! > That's why the AVDictionary is done like that" 😊 > > The dictionary implementation is not clever > look at copy for example it iterates over av_dict_set() which itself > calls > av_dict_get() which it itself iterates over the dictionary > so av_dict_copy() is O(n^2) for example I meant "clever" in the sense of "intentionally kept simple" and using linear lookup in order to be fast in cases like FFprobe where there's just a very small number of items. > also a single fate run, calls av_dict_iterate() 4921207 times > and fate should mostly be short small files and minimal self contained > testcases That's 1.2k per test in average.. Now - look at this masterpiece of a sentence: Isn't it an inevitable FATE for an iterator implementation to see such high invocation counts? 😊😊😊 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] SW's Patchsets Overview
On Tue, Apr 08, 2025 at 10:45:38PM +, softworkz . wrote: > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of > > Michael Niedermayer > > Sent: Mittwoch, 9. April 2025 00:25 > > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] SW's Patchsets Overview > > > > Hi > > > > On Sun, Apr 06, 2025 at 09:12:00PM +, softworkz . wrote: > > > > > > > > > > -Original Message- > > > > From: ffmpeg-devel On Behalf Of > > Marton > > > > Balint > > > > Sent: Sonntag, 6. April 2025 23:05 > > > > To: FFmpeg development discussions and patches > de...@ffmpeg.org> > > > > Subject: Re: [FFmpeg-devel] SW's Patchsets Overview > > > > > > > > > > > > > > > > On Wed, 2 Apr 2025, softworkz . wrote: > > > > > > > > > > > > > > > > > > >> -Original Message- > > > > >> From: ffmpeg-devel On Behalf Of > > > > Marton > > > > >> Balint > > > > >> Sent: Mittwoch, 2. April 2025 21:45 > > > > >> To: FFmpeg development discussions and patches > > > de...@ffmpeg.org> > > > > >> Subject: Re: [FFmpeg-devel] SW's Patchsets Overview > > > > >> > > > > >> > > > > >> > > > > >> On Wed, 2 Apr 2025, softworkz . wrote: > > > > >> > > > > >>> Hello everybody, > > > > >>> > > > > >>> with freshly gained push access rights, I want to act > > responsibly > > > > and > > > > >>> carefully, and also avoid unexpected surprises so I'm not going > > to > > > > >> rush > > > > >>> things. Due to that change, I thought it might be good to post > > an > > > > >>> overview of the patchsets I am intending to push in the near > > future: > > > > >> > > > > >> Thanks for the heads up. > > > > >> > > > > >> [...] > > > > >> > > > > >>> avutil/log: Replace addresses in log output with simple ids > > > > >>> > > > > >>> GitHub:https://github.com/ffstaging/FFmpeg/pull/59 > > > > >>> Patchwork: > > > > >> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14094 > > > > >> > > > > > > > > > > Hi Marton, > > > > > > > > > > thanks a lot for looking at the patchset. > > > > > > > > > >> To be honest, I don't like this at all. You duplicate a lot of > > code > > > > from > > > > >> avutil/log, and the implementation has quite a few problems, some > > of > > > > >> them not really fixable. > > > > > > > > > > Originally, this was a patch against avutil/log. Nicolas objected > > that > > > > > it was adding global state and Hendrik (and Nicolas) suggested > > that I > > > > > should to this in fftools only - outside of the libs, in a was > > that > > > > > fftools get their own logging implementation - with the potential > > of > > > > > being able to do other things in the future that wouldn't make > > sense > > > > in > > > > > the lib code. Letting fftools have their own logging > > implementation of > > > > > can of course only start from a copy in order to retain existing > > > > > behavior. On top of that I applied that little change then. > > > > > > > > > > > > > > >> - creating object IDs in the order the objects log something > > (what if > > > > >> they do not? What if it depends on loglevel?) > > > > >> - tracking object IDs based on their address - objects are > > > > >>allocated and removed at runtime, it is possible that an > > address > > > > will be > > > > >>re-used for a different object later on > > > > > > > > > > The Ids are not meant to have much more value than the addresses > > > > > currently shown - with an important difference: They are short and > > > > > remain the same on repeated execution. Plus: they are counted by > > > > > AVClass, that give a little additional value, but since they are > > just > > > > > "indexing" the addresses, they are in fact prone to the same > > > > > shortcomings like the addresses themselves, meaning that a re- > > > > assignment > > > > > might give you the same id for something different and also > > different > > > > > addresses (in consequence the IDs as well) can reference the same > > > > thing > > > > > (e.g. with buffer refs). > > > > > > > > > >> - linear search of addresses. A long ffmpeg process can > > constantly > > > > >> create > > > > >>objects during runtime, eventually completely depleting the > > pool > > > > and > > > > >>causing an extensive search for all future logs. > > > > > > > > > > I have considered that case. There is a hard limit from when on no > > IDs > > > > > are assigned anymore (all zeros). > > > > > > > > > > > > > > >> So overall I don't think it's worth pursuing this, especially > > since > > > > most > > > > >> users won't care neither about the ID, nor about the address... > > > > > > > > > > Let me give two examples of where I find it useful to have those > > IDs: > > > > > > > > > > On startup decoders can be initialized multiple times, like first > > for > > > > > probing and then for transcoding. Or when there are multiple > > streams > > > > of > > > > > the same type (codec), the log messages can be confusing when the > > log > > > > > output from several identical o
Re: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that decode_str() did advance
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Michael Niedermayer
> Sent: Samstag, 12. April 2025 00:27
> To: FFmpeg development discussions and patches
> Subject: [FFmpeg-devel] [PATCH 2/2] avformat/id3v2: Check that
> decode_str() did advance
>
> Fixes infinite loop with unknown encodings
>
> We could alternatively error out from decode_str() or consume all of
> taglen
> this would affect other callers though.
>
> Fixes: 409819224/clusterfuzz-testcase-minimized-ffmpeg_dem_H261_fuzzer-
> 6003527535362048
> Signed-off-by: Michael Niedermayer
> ---
> libavformat/id3v2.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> index 90314583a74..e3f7f9e2a90 100644
> --- a/libavformat/id3v2.c
> +++ b/libavformat/id3v2.c
> @@ -341,10 +341,13 @@ static void read_ttag(AVFormatContext *s,
> AVIOContext *pb, int taglen,
> taglen--; /* account for encoding type byte */
>
> while (taglen > 1) {
> +int current_taglen = taglen;
> if (decode_str(s, pb, encoding, &dst, &taglen) < 0) {
> av_log(s, AV_LOG_ERROR, "Error reading frame %s,
> skipped\n", key);
> return;
> }
> +if (current_taglen == taglen)
> +return;
>
> count++;
>
> --
> 2.49.0
>
> ___
Hi Michael,
this kind of conflicts with this patch that I had submitted recently:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.54.ffstaging.ffmpeg.1740873449247.ffmpegag...@gmail.com/
I wonder whether my patch would still be prone to the issue your patch is
addressing - do you have a test file perhaps?
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".
