[FFmpeg-devel] [PATCH v v2] libavcodec/riscv:add RVV optimized for idct_32x32_8:
From: daichengrong riscv/hevcdsp_idct_rvv: Optimize idct_32x32_8 On Banana PI F3: hevc_idct_32x32_8_c:118945.0 ( 1.00x) hevc_idct_32x32_8_rvv_i64: 28503.7 ( 4.17x) Signed-off-by: daichengrong --- libavcodec/riscv/Makefile | 1 + libavcodec/riscv/hevcdsp_idct_rvv.S | 973 libavcodec/riscv/hevcdsp_init.c | 52 +- 3 files changed, 1006 insertions(+), 20 deletions(-) create mode 100644 libavcodec/riscv/hevcdsp_idct_rvv.S diff --git a/libavcodec/riscv/Makefile b/libavcodec/riscv/Makefile index a80d2fa2e7..dfc33afbee 100644 --- a/libavcodec/riscv/Makefile +++ b/libavcodec/riscv/Makefile @@ -36,6 +36,7 @@ RVV-OBJS-$(CONFIG_H264DSP) += riscv/h264addpx_rvv.o riscv/h264dsp_rvv.o \ OBJS-$(CONFIG_H264QPEL) += riscv/h264qpel_init.o RVV-OBJS-$(CONFIG_H264QPEL) += riscv/h264qpel_rvv.o OBJS-$(CONFIG_HEVC_DECODER) += riscv/hevcdsp_init.o +OBJS-$(CONFIG_HEVC_DECODER) += riscv/hevcdsp_idct_rvv.o RVV-OBJS-$(CONFIG_HEVC_DECODER) += riscv/h26x/h2656_inter_rvv.o OBJS-$(CONFIG_HUFFYUV_DECODER) += riscv/huffyuvdsp_init.o RVV-OBJS-$(CONFIG_HUFFYUV_DECODER) += riscv/huffyuvdsp_rvv.o diff --git a/libavcodec/riscv/hevcdsp_idct_rvv.S b/libavcodec/riscv/hevcdsp_idct_rvv.S new file mode 100644 index 00..561b8ada47 --- /dev/null +++ b/libavcodec/riscv/hevcdsp_idct_rvv.S @@ -0,0 +1,973 @@ +/* + * Copyright (c) 2025 Institue of Software Chinese Academy of Sciences (ISCAS). + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "libavutil/riscv/asm.S" + +const trans, align=4 +.2byte 64, 83, 64, 36 +.2byte 89, 75, 50, 18 +.2byte 90, 87, 80, 70 +.2byte 57, 43, 25, 9 +.2byte 90, 90, 88, 85 +.2byte 82, 78, 73, 67 +.2byte 61, 54, 46, 38 +.2byte 31, 22, 13, 4 +endconst + +.macro sum_sub out, in, c, op, p +mv t0, \c +.ifc \op, - +negt0, t0 +.endif +vsetivlizero, 4, e16, mf2, tu, ma +.ifc \p, 2 +vslidedown.viv8, \in, 4 +vwmacc.vx\out, t0, v8 +.else +vwmacc.vx\out, t0, \in +.endif +.endm + +.macro add_member32 in, t0, t1, t2, t3, op0, op1, op2, op3, p +sum_sub v24, \in, \t0, \op0, \p +sum_sub v25, \in, \t1, \op1, \p +sum_sub v26, \in, \t2, \op2, \p +sum_sub v27, \in, \t3, \op3, \p +.endm + +.macro butterfly e, o, tmp_p, tmp_m +vsetivlizero, 4, e32, m1, tu, ma +vadd.vv \tmp_p, \e, \o +vsub.vv \tmp_m, \e, \o +.endm + +.macro butterfly16 in0, in1, in2, in3, in4, in5, in6, in7 +vsetivlizero, 4, e32, m1, tu, ma +vadd.vv v20, \in0, \in1 +vsub.vv \in0, \in0, \in1 +vadd.vv \in1, \in2, \in3 +vsub.vv \in2, \in2, \in3 +vadd.vv \in3, \in4, \in5 +vsub.vv \in4, \in4, \in5 +vadd.vv \in5, \in6, \in7 +vsub.vv \in6, \in6, \in7 +.endm + +.macro multiply in +vsetivlizero, 4, e16, m1, tu, ma +vse16.v \in, (s0) +ld s2, 0*2(s0) +ld s3, 1*2(s0) +ld s4, 2*2(s0) +ld s5, 3*2(s0) + +vsetivlizero, 4, e16, mf2, tu, ma +vwmul.vxv24, v4, s2 +vwmul.vxv25, v4, s3 +vwmul.vxv26, v4, s4 +vwmul.vxv27, v4, s5 +.endm + +func tr_block1, zve64x +multiplyv0 + +addisp,sp,-8*16 + +.irp i, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, +sdx\i,8*(\i - 10)(sp) +.endr +vsetivlizero, 4, e16, m1, tu, ma +vse16.v v0, (s0) +ld x10, 0*2(s0) +ld x11, 1*2(s0) +ld x12, 2*2(s0) +ld x13, 3*2(s0) +vse16.v v1, (s0) +ld x14, 0*2(s0) +ld x15, 1*2(s0) +ld x16, 2*2(s0) +ld x17, 3*2(s0) +vse16.v v2, (s0)
[FFmpeg-devel] [PATCH 1/5] avformat/apvdec: Use ffio_read_size()
Patches attached. - Andreas From de945d797738c78c3435da1cb64201d00256f702 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Sun, 27 Apr 2025 20:14:35 +0200 Subject: [PATCH 1/5] avformat/apvdec: Use ffio_read_size() Fixes potential use of uninitialized data. Signed-off-by: Andreas Rheinhardt --- libavformat/apvdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c index e1ac34b003..9f94a901ec 100644 --- a/libavformat/apvdec.c +++ b/libavformat/apvdec.c @@ -164,7 +164,7 @@ static int apv_read_header(AVFormatContext *s) err = ffio_ensure_seekback(s->pb, sizeof(buffer)); if (err < 0) return err; -size = avio_read(s->pb, buffer, sizeof(buffer)); +size = ffio_read_size(s->pb, buffer, sizeof(buffer)); if (size < 0) return size; -- 2.45.2 From 3e8f9107090d8bef97b389e8d28ccbe03d3f45f2 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Mon, 28 Apr 2025 11:25:26 +0200 Subject: [PATCH 2/5] avformat/apvdec: Check before access The signature check would segfault in case the packet could not be allocated or if nothing could be read. Furthermore, read_packet callbacks are supposed to return zero on success, yet the current code returned the size of the packet. Signed-off-by: Andreas Rheinhardt --- libavformat/apvdec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c index 9f94a901ec..6a972c6d9a 100644 --- a/libavformat/apvdec.c +++ b/libavformat/apvdec.c @@ -225,6 +225,8 @@ static int apv_read_packet(AVFormatContext *s, AVPacket *pkt) } ret = av_get_packet(s->pb, pkt, au_size); +if (ret < 0) +return ret; pkt->flags= AV_PKT_FLAG_KEY; signature = AV_RB32(pkt->data); @@ -233,7 +235,7 @@ static int apv_read_packet(AVFormatContext *s, AVPacket *pkt) return AVERROR_INVALIDDATA; } -return ret; +return 0; } const FFInputFormat ff_apv_demuxer = { -- 2.45.2 From 87b90d0b6f60d2cd005bd9417f2ecd2f7a781bcd Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Mon, 28 Apr 2025 11:31:49 +0200 Subject: [PATCH 3/5] avformat/apvdec: Fix seeking pkt->pos pointed to the actual packet data, not to the start of the access unit. Signed-off-by: Andreas Rheinhardt --- libavformat/apvdec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c index 6a972c6d9a..a0a2b7681e 100644 --- a/libavformat/apvdec.c +++ b/libavformat/apvdec.c @@ -227,6 +227,7 @@ static int apv_read_packet(AVFormatContext *s, AVPacket *pkt) ret = av_get_packet(s->pb, pkt, au_size); if (ret < 0) return ret; +pkt->pos -= 4; pkt->flags= AV_PKT_FLAG_KEY; signature = AV_RB32(pkt->data); -- 2.45.2 From 5de3c95d8858cc5c133c806e6b45c97103316637 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Sun, 27 Apr 2025 20:20:02 +0200 Subject: [PATCH 4/5] avformat/apvdec: Remove inappropriate INIT_CLEANUP flag The init-cleanup flag makes no sense for a demuxer without a read_close() function. Signed-off-by: Andreas Rheinhardt --- libavformat/apvdec.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libavformat/apvdec.c b/libavformat/apvdec.c index a0a2b7681e..28948766fc 100644 --- a/libavformat/apvdec.c +++ b/libavformat/apvdec.c @@ -244,7 +244,6 @@ const FFInputFormat ff_apv_demuxer = { .p.long_name= NULL_IF_CONFIG_SMALL("APV raw bitstream"), .p.extensions = "apv", .p.flags= AVFMT_GENERIC_INDEX | AVFMT_NOTIMESTAMPS, -.flags_internal = FF_INFMT_FLAG_INIT_CLEANUP, .read_probe = apv_probe, .read_header= apv_read_header, .read_packet= apv_read_packet, -- 2.45.2 From 8701b4e95e040a072e009a21afc3c05883f87c64 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Mon, 28 Apr 2025 11:34:33 +0200 Subject: [PATCH 5/5] avcodec/apv_entropy: Remove ff_apv_read_vlc() There is no need for testing-only code to exist in release builds, developers can add testing/debug code just fine locally if they need it. Signed-off-by: Andreas Rheinhardt --- libavcodec/apv_decode.h | 9 - libavcodec/apv_entropy.c | 6 -- 2 files changed, 15 deletions(-) diff --git a/libavcodec/apv_decode.h b/libavcodec/apv_decode.h index 34c6176ea0..4749116e6b 100644 --- a/libavcodec/apv_decode.h +++ b/libavcodec/apv_decode.h @@ -68,13 +68,4 @@ int ff_apv_entropy_decode_block(int16_t *coeff, GetBitContext *gbc, APVEntropyState *state); -/** - * Read a single APV VLC code. - * - * This entrypoint is exposed for testing. - */ -unsigned int ff_apv_read_vlc(GetBitContext *gbc, int k_param, - const APVVLCLUT *lut); - - #endif /* AVCODEC_APV_DECODE_H */ diff --git a/libavcodec/apv_entropy.c b/libavcodec/apv_entropy.c index 00e0b4fbdf..0cce6b0847 100644 --- a/libavcodec/apv_entropy.c +++ b/libavcodec/
Re: [FFmpeg-devel] [PATCH v2] lavc/vvc: Detect subpic overlaps at CTU level
Hi Frank, Thank you for the v2. Could we remove all asserts? Asserts can cause the application to crash at runtime. On Sun, Apr 27, 2025 at 4:48 PM Frank Plowman wrote: > In d5dbcc00d889fb17948b025a468b00ddbea9e058, it was hoped that detection > of subpicture overlaps could be performed at the tile level, so as to > avoid introducing per-CTU checks. Unfortunately since that patch, > fuzzing has indicated there are some structures involving > pps_subpic_one_or_more_tiles_slice where tile-level checking is not > sufficient. Performing the check at the CTU level should (touch wood) > be the be-all and and-all of this, as CTUs are the lowest common > denominator of the picture partitioning. > > Signed-off-by: Frank Plowman > --- > Changes since v1: > * Merge pps_add_ctus and pps_add_ctus_check > * Change if/else for early-exit where possible > > --- > libavcodec/vvc/ps.c | 71 - > 1 file changed, 31 insertions(+), 40 deletions(-) > > diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c > index e8c312d8ac..ed96268bae 100644 > --- a/libavcodec/vvc/ps.c > +++ b/libavcodec/vvc/ps.c > @@ -408,6 +408,8 @@ static int pps_add_ctus(VVCPPS *pps, int *off, const > int rx, const int ry, > int start = *off; > for (int y = 0; y < h; y++) { > for (int x = 0; x < w; x++) { > +if (*off >= pps->ctb_count) > +return AVERROR_INVALIDDATA; > pps->ctb_addr_in_slice[*off] = ctu_rs(rx + x, ry + y, pps); > (*off)++; > } > @@ -420,9 +422,11 @@ static void pps_single_slice_picture(VVCPPS *pps, int > *off) > pps->num_ctus_in_slice[0] = 0; > for (int j = 0; j < pps->r->num_tile_rows; j++) { > for (int i = 0; i < pps->r->num_tile_columns; i++) { > -pps->num_ctus_in_slice[0] += pps_add_ctus(pps, off, > +const int ret = pps_add_ctus(pps, off, > pps->col_bd[i], pps->row_bd[j], > pps->r->col_width_val[i], pps->r->row_height_val[j]); > +av_assert2(ret >= 0); > +pps->num_ctus_in_slice[0] += ret; > } > } > } > @@ -451,50 +455,36 @@ static void subpic_tiles(int *tile_x, int *tile_y, > int *tile_x_end, int *tile_y_ > (*tile_y_end)++; > } > > -static bool mark_tile_as_used(bool *tile_in_subpic, const int tx, const > int ty, const int tile_columns) > -{ > -const size_t tile_idx = ty * tile_columns + tx; > -if (tile_in_subpic[tile_idx]) { > -/* the tile is covered by other subpictures */ > -return false; > -} > -tile_in_subpic[tile_idx] = true; > -return true; > -} > - > -static int pps_subpic_less_than_one_tile_slice(VVCPPS *pps, const VVCSPS > *sps, const int i, const int tx, const int ty, int *off, bool > *tile_in_subpic) > +static int pps_subpic_less_than_one_tile_slice(VVCPPS *pps, const VVCSPS > *sps, const int i, const int tx, const int ty, int *off) > { > -const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] + > sps->r->sps_subpic_height_minus1[i]; > -const int tile_bottom = pps->row_bd[ty] + pps->r->row_height_val[ty] > - 1; > -const bool is_final_subpic_in_tile = subpic_bottom == tile_bottom; > - > -if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic, tx, > ty, pps->r->num_tile_columns)) > -return AVERROR_INVALIDDATA; > - > -pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, > +const int ret = pps_add_ctus(pps, off, > sps->r->sps_subpic_ctu_top_left_x[i], > sps->r->sps_subpic_ctu_top_left_y[i], > sps->r->sps_subpic_width_minus1[i] + 1, > sps->r->sps_subpic_height_minus1[i] + 1); > +if (ret < 0) > +return ret; > > +pps->num_ctus_in_slice[i] = ret; > return 0; > } > > static int pps_subpic_one_or_more_tiles_slice(VVCPPS *pps, const int > tile_x, const int tile_y, const int x_end, const int y_end, > -const int i, int *off, bool *tile_in_subpic) > +const int i, int *off) > { > for (int ty = tile_y; ty < y_end; ty++) { > for (int tx = tile_x; tx < x_end; tx++) { > -if (!mark_tile_as_used(tile_in_subpic, tx, ty, > pps->r->num_tile_columns)) > -return AVERROR_INVALIDDATA; > - > -pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off, > +const int ret = pps_add_ctus(pps, off, > pps->col_bd[tx], pps->row_bd[ty], > pps->r->col_width_val[tx], pps->r->row_height_val[ty]); > +if (ret < 0) > +return ret; > + > +pps->num_ctus_in_slice[i] += ret; > } > } > return 0; > } > > -static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, > int *off, bool *tile_in_subpic) > +static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, > int *off) > { > int tx, ty, x_end, y_end; > > @@ -503,9 +493,9 @@ static int pps_subpic_slice(VVCPPS *pps, const VVCSPS > *sps, const int i, int *of >
Re: [FFmpeg-devel] [PATCH] avfilter/vf_lut3d_opencl Initial support for OpenCL implementation of vf_lut3d.
Added an OpenCL lut3d video filter implementation for the current lut3d video filter. - This OpenCL implementation currently supports only .cube LUT format and 3 interpolations - nearest,trilinear,tetrahedral (default). - Outputs match CPU version up to floating point errors. Regards, Jan Studeny On Apr 28, 2025 at 13:39 +0300, Jan Studený via ffmpeg-devel , wrote: > --- > libavfilter/Makefile | 1 + > libavfilter/allfilters.c | 1 + > libavfilter/opencl/lut3d.cl | 177 ++ > libavfilter/opencl_source.h | 2 + > libavfilter/vf_lut3d_opencl.c | 444 ++ > 5 files changed, 625 insertions(+) > create mode 100644 libavfilter/opencl/lut3d.cl > create mode 100644 libavfilter/vf_lut3d_opencl.c > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile > index 7c0d879ec9..6524d0f91a 100644 > --- a/libavfilter/Makefile > +++ b/libavfilter/Makefile > @@ -378,6 +378,7 @@ OBJS-$(CONFIG_LUT1D_FILTER) += vf_lut3d.o > OBJS-$(CONFIG_LUT_FILTER) += vf_lut.o > OBJS-$(CONFIG_LUT2_FILTER) += vf_lut2.o framesync.o > OBJS-$(CONFIG_LUT3D_FILTER) += vf_lut3d.o framesync.o > +OBJS-$(CONFIG_LUT3D_OPENCL_FILTER) += vf_lut3d_opencl.o opencl.o > opencl/lut3d.o > OBJS-$(CONFIG_LUTRGB_FILTER) += vf_lut.o > OBJS-$(CONFIG_LUTYUV_FILTER) += vf_lut.o > OBJS-$(CONFIG_MASKEDCLAMP_FILTER) += vf_maskedclamp.o framesync.o > diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c > index 740d9ab265..72c2f48ac4 100644 > --- a/libavfilter/allfilters.c > +++ b/libavfilter/allfilters.c > @@ -353,6 +353,7 @@ extern const FFFilter ff_vf_lut; > extern const FFFilter ff_vf_lut1d; > extern const FFFilter ff_vf_lut2; > extern const FFFilter ff_vf_lut3d; > +extern const FFFilter ff_vf_lut3d_opencl; > extern const FFFilter ff_vf_lutrgb; > extern const FFFilter ff_vf_lutyuv; > extern const FFFilter ff_vf_maskedclamp; > diff --git a/libavfilter/opencl/lut3d.cl b/libavfilter/opencl/lut3d.cl > new file mode 100644 > index 00..16dfecdc4e > --- /dev/null > +++ b/libavfilter/opencl/lut3d.cl > @@ -0,0 +1,177 @@ > +/* > + * Copyright (c) 2025 Jan Studeny > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + */ > + > +typedef struct rgbvec { > + float r, g, b, a; > +} rgbvec; > + > +#define MIN(X, Y) (((X) < (Y)) ? (X) : (Y)) > + > +#define NEAR(x) ((int)((x) + .5)) > +#define PREV(x) ((int)(x)) > +#define NEXT(x) (MIN((int)(x) + 1, lut_edge_size - 1)) > + > +/** > + * Get the nearest defined point > + */ > +static rgbvec interp_nearest(float4 px, __global const rgbvec *lut, int > lut_edge_size) > +{ > + int r = NEAR(px[0]); > + int g = NEAR(px[1]); > + int b = NEAR(px[2]); > + int index = r * lut_edge_size * lut_edge_size + g * lut_edge_size + b; > + return lut[index]; > +} > + > +static float lerpf(float v0, float v1, float f) > +{ > + return v0 + (v1 - v0) * f; > +} > + > +static rgbvec lerp(const rgbvec *v0, const rgbvec *v1, float f) > +{ > + rgbvec v = { > + lerpf(v0->r, v1->r, f), lerpf(v0->g, v1->g, f), lerpf(v0->b, v1->b, f) > + }; > + return v; > +} > +/** > + * Interpolate using the 8 vertices of a cube > + * @see https://en.wikipedia.org/wiki/Trilinear_interpolation > + */ > +static rgbvec interp_trilinear(float4 px, __global const rgbvec *lut, int > lut_edge_size) > +{ > + const int lutsize2 = lut_edge_size * lut_edge_size; > + const int lutsize = lut_edge_size; > + > + const int prev[] = { PREV(px[0]), PREV(px[1]), PREV(px[2]) }; > + const int next[] = { NEXT(px[0]), NEXT(px[1]), NEXT(px[2]) }; > + > + const rgbvec d = { > + px[0] - prev[0], > + px[1] - prev[1], > + px[2] - prev[2] > + }; > + > + const rgbvec c000 = lut[prev[0] * lutsize2 + prev[1] * lutsize + prev[2]]; > + const rgbvec c001 = lut[prev[0] * lutsize2 + prev[1] * lutsize + next[2]]; > + const rgbvec c010 = lut[prev[0] * lutsize2 + next[1] * lutsize + prev[2]]; > + const rgbvec c011 = lut[prev[0] * lutsize2 + next[1] * lutsize + next[2]]; > + const rgbvec c100 = lut[next[0] * lutsize2 + prev[1] * lutsize + prev[2]]; > + const rgbvec c101 = lut[next[0] * lutsize2 + prev[1] * lutsize + next[2]]; > + const rgbvec c110 = lut[next[0] * lutsize2 + next[1] * lutsize + prev[2]]; > + const rgbvec c111 = lut[next[0] * lutsize2 + next[1] * lutsize + next[2]]; > + > + const rgbvec c00 = lerp(&
[FFmpeg-devel] [PATCH] postproc/tests: Add test tools to .gitignore
Patch attached. - Andreas From d1961c101dfd095c219c4ff56d5f5523574976ce Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Mon, 28 Apr 2025 16:01:13 +0200 Subject: [PATCH] postproc/tests: Add test tools to .gitignore Signed-off-by: Andreas Rheinhardt --- libpostproc/tests/.gitignore | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 libpostproc/tests/.gitignore diff --git a/libpostproc/tests/.gitignore b/libpostproc/tests/.gitignore new file mode 100644 index 00..d77aa7e3dd --- /dev/null +++ b/libpostproc/tests/.gitignore @@ -0,0 +1,2 @@ +/blocktest +/stripetest -- 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".
[FFmpeg-devel] [PATCH] avformat/av1dec: fix setting AVPacket->pos in Annex-B demuxer
This demuxers reads encapsulation bytes before reading codec data into the output packets, so take such offset into consideration. Signed-off-by: James Almer --- libavformat/av1dec.c | 8 1 file changed, 8 insertions(+) diff --git a/libavformat/av1dec.c b/libavformat/av1dec.c index 8c0b8fe975..38001b124f 100644 --- a/libavformat/av1dec.c +++ b/libavformat/av1dec.c @@ -36,6 +36,7 @@ typedef struct AV1DemuxContext { AVRational framerate; uint32_t temporal_unit_size; uint32_t frame_unit_size; +int64_t pos; } AV1DemuxContext; //return < 0 if we need more data @@ -96,6 +97,8 @@ static int av1_read_header(AVFormatContext *s) if (ret < 0) return ret; +c->pos = avio_tell(s->pb); + return 0; } @@ -224,6 +227,7 @@ static int annexb_read_packet(AVFormatContext *s, AVPacket *pkt) { AV1DemuxContext *const c = s->priv_data; uint32_t obu_unit_size; +int64_t pos = c->pos; int ret, len; retry: @@ -234,6 +238,7 @@ retry: } if (!c->temporal_unit_size) { +c->pos = avio_tell(s->pb); len = leb(s->pb, &c->temporal_unit_size, 1); if (len == AVERROR_EOF) goto end; else if (len < 0) return len; @@ -279,6 +284,9 @@ end: if (ret == AVERROR(EAGAIN)) goto retry; +if (!ret) +pkt->pos = pos; + return ret; } -- 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] [RFC] Shaping the AVTextFormat API Surface
On date Sunday 2025-04-27 17:54:21 +, softworkz . wrote: > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of > > Stefano Sabatini > > Sent: Sonntag, 27. April 2025 12:42 > > To: FFmpeg development discussions and patches > de...@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface > > > > On date Friday 2025-04-25 13:16:59 +, softworkz . wrote: > > [...] > > > Tell me what I should check for and what not in those four groups of > > > functions and for those things which should be checked, tell me > > which > > > way (return error, return silently, allow segfault or use an > > assertion). > > > > > > Then I'll apply that to all those functions in a uniform and > > consistent > > > way without even arguing and the case is closed. > > > > > > I just don't want to leave it alone like now without clear patterns, > > > that's all 😊 > > > > I don't really have an answer. > > ...still by far the best one. > > > > Probably it's good to start from the > > docs, so that we have a definition of the semantics in advance, for > > example stating that a pointer should not be NULL and so on so that at > > least we know what is to be considered undefined behavior. As noted by > > Nicolas, the pattern is dependant on the function behavior and on > > practical ergonomy considerations. > > > > It also would be nice to have a good set of guidelines. > > Exactly. That's one of the things I would like to work out here. > > > [..] > > > This might fail in several ways: bikeshed might be NULL or invalid > > (e.g. a pointer to an unrelated structure), level might be invalid > > (e.g. negative or >MAX_SLICE_LEVEL) or the bikeshed might contain > > already too many slices. > > > > The level might be checked by the programmer, so we might decide to > > have an assert. About the count check it is validated from within the > > function (since we need to access the bikeshed context) so we want to > > provide feedback and fail. > > > > For both of these two examples, doing nothing does not seem a good > > idea. That's probably only good if we want to enable idem-potency or > > when one of the parameter can be interpreted as a "none" argument. > > > > For example: > >if (color == NULL) { > >return 0; > >} > > > > In this case we should specify the behavior in the documentation, > > since that defines what is the undefined behavior and the input > > expectactions. > > This all makes sense and the practical part is now to apply that kind > of considerations to the individual APIs we have. > > Probably it's best when I start by making a suggestion as a starting > point, then we can refine it from there: > > > 1. AVTextFormatter Implementations > == > > print_section_header(AVTextFormatContext *tctx, const void *data); > print_section_footer(AVTextFormatContext *tctx); > print_integer(AVTextFormatContext *tctx, const char * key, int64_t); > print_string(AVTextFormatContext *tctx, const char *key, const char *value); > > Rules > > - assert tctx and key > - data and value can be null Also: should we return en error in case of invalid nesting level? This is context dependent so maybe this should be a recoverable error - my guess is yes although this means complicating usage. > 2. AVTextWriter Implementations > === > > writer_w8(AVTextWriterContext *wctx, int b); > writer_put_str(AVTextWriterContext *wctx, const char *str); > writer_vprintf(AVTextWriterContext *wctx, const char *fmt, va_list vl); assuming this is directly used by a programmer, the variadic variant might also make sense > > > Rules > > - assert wctx > - str, fmt, vl - ? Can the operation fail? Should we return an error code? > > 3. TextFormat API > = > > > avtext_print_section_header(*tctx, const void *data, int section_id) > avtext_print_section_footer(*tctx) > avtext_print_integer(*tctx, const char *key, int64_t val) > avtext_print_integer_flags(*tctx, const char *key, int64_t val, int flags) a single variant might do (as we have a single print_string) > avtext_print_unit_int(*tctx, const char *key, int value, const char *unit) > avtext_print_rational(*tctx, const char *key, AVRational q, char sep) > avtext_print_time(*tctx, const char *key, int64_t ts, const AVRational > *time_base, int is_duration) > avtext_print_ts(*tctx, const char *key, int64_t ts, int is_duration) > avtext_print_string(*tctx, const char *key, const char *val, int flags) > avtext_print_data(*tctx, const char *key, const uint8_t *data, int size) > avtext_print_data_hash(*tctx, const char *key, const uint8_t *data, int size) > avtext_print_integers(*tctx, const char *key, uint8_t *data, int size, > const char *format, int columns, int bytes, int > offset_add) is this really needed? also this seems a complication as it implies tabular format > > > Rules > > - assert tctx and key > - how
Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> -Original Message- > From: ffmpeg-devel On Behalf Of Stefano > Sabatini > Sent: Dienstag, 29. April 2025 00:27 > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface > > On date Sunday 2025-04-27 17:54:21 +, softworkz . wrote: > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > > > Stefano Sabatini > > > Sent: Sonntag, 27. April 2025 12:42 > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface > > > [..] > > > > Probably it's best when I start by making a suggestion as a starting > > point, then we can refine it from there: > > > > > > 1. AVTextFormatter Implementations > > == > > > > print_section_header(AVTextFormatContext *tctx, const void *data); > > print_section_footer(AVTextFormatContext *tctx); > > print_integer(AVTextFormatContext *tctx, const char * key, int64_t); > > print_string(AVTextFormatContext *tctx, const char *key, const char *value); > > > > > Rules > > > > - assert tctx and key > > > - data and value can be null > > Also: should we return en error in case of invalid nesting level? > This is context dependent so maybe this should be a recoverable error > - my guess is yes although this means complicating usage. This is handled by doing nothing in that case. The functions are all void. Changing that and checking the return values would probably blow-up the printing code in ffprobe.c to 200% or 300% (when for all) or still a large amount when only for section header and footer. "Do nothing" has always been a behavior for that case, but it wasn't implemented consistently. > > > 2. AVTextWriter Implementations > > === > > > > writer_w8(AVTextWriterContext *wctx, int b); > > writer_put_str(AVTextWriterContext *wctx, const char *str); > > > writer_vprintf(AVTextWriterContext *wctx, const char *fmt, va_list vl); > > assuming this is directly used by a programmer, the variadic variant > might also make sense That's one of the big questions, whether this should be public. I'd rather say no - not for consumption. It's rather public in the sense that a lib consumer could create their own implementations and supply it to av_textformat_open(), then use the other textformat apis to write output - but not by calling the implementation functions of AVTextWriter directly. > > > > Rules > > > > - assert wctx > > - str, fmt, vl - ? > > Can the operation fail? Should we return an error code? Our implementation functions return void and so do most of the upstream functions that the writers are calling. Same as above, probably not economic to add checks for all invocations. > > 3. TextFormat API > > = > > > > > > avtext_print_section_header(*tctx, const void *data, int section_id) > > avtext_print_section_footer(*tctx) > > > avtext_print_integer(*tctx, const char *key, int64_t val) > > avtext_print_integer_flags(*tctx, const char *key, int64_t val, int flags) > > a single variant might do (as we have a single print_string) Yea, I'll try. > > avtext_print_unit_int(*tctx, const char *key, int value, const char *unit) > > avtext_print_rational(*tctx, const char *key, AVRational q, char sep) > > avtext_print_time(*tctx, const char *key, int64_t ts, const AVRational > *time_base, int is_duration) > > avtext_print_ts(*tctx, const char *key, int64_t ts, int is_duration) > > avtext_print_string(*tctx, const char *key, const char *val, int flags) > > avtext_print_data(*tctx, const char *key, const uint8_t *data, int size) > > avtext_print_data_hash(*tctx, const char *key, const uint8_t *data, int > size) > > > avtext_print_integers(*tctx, const char *key, uint8_t *data, int size, > > const char *format, int columns, int bytes, int > offset_add) > > is this really needed? also this seems a complication as it implies > tabular format That's not my addition. It is used in ffprobe.c for DisplayMatrix. > > Rules > > > > - assert tctx and key > > > - how about uint8_t *data, unit and val in ..print_string? > > what are the current use cases? Can we have empty data/unit/val? Do we > need to support null semantics? I seem to remember we do, let's check. Ok > > > 4. TextWriter API > > = > > > > avtextwriter_context_open(AVTextWriterContext **pwctx, const AVTextWriter > *writer) > > avtextwriter_context_close(AVTextWriterContext **pwctx) > > avtextwriter_create_stdout(AVTextWriterContext **pwctx) > > avtextwriter_create_avio(AVTextWriterContext **pwctx, AVIOContext *avio_ctx, > int close_on_uninit) > > avtextwriter_create_file(AVTextWriterContext **pwctx, const char > *output_filename) > > avtextwriter_create_buffer(AVTextWriterContext **pwctx, AVBPrint *buffer) > > > > > > Rules > > > > - **pwctx: leave unchecked > > - writer: return AVERROR(EINVAL) > > - avio_ctx:
[FFmpeg-devel] [PATCH v2 1/2] tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis, opus, flac} streams.
--- tests/Makefile | 4 + tests/api/Makefile | 2 +- tests/api/api-dump-stream-meta-test.c | 182 + tests/fate/ogg-flac.mak| 11 ++ tests/fate/ogg-opus.mak| 11 ++ tests/fate/ogg-vorbis.mak | 11 ++ tests/ref/fate/ogg-flac-chained-meta.txt | 12 ++ tests/ref/fate/ogg-opus-chained-meta.txt | 27 +++ tests/ref/fate/ogg-vorbis-chained-meta.txt | 17 ++ 9 files changed, 276 insertions(+), 1 deletion(-) create mode 100644 tests/api/api-dump-stream-meta-test.c create mode 100644 tests/fate/ogg-flac.mak create mode 100644 tests/fate/ogg-opus.mak create mode 100644 tests/fate/ogg-vorbis.mak create mode 100644 tests/ref/fate/ogg-flac-chained-meta.txt create mode 100644 tests/ref/fate/ogg-opus-chained-meta.txt create mode 100644 tests/ref/fate/ogg-vorbis-chained-meta.txt diff --git a/tests/Makefile b/tests/Makefile index 0c08f68713..10871f28f8 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -220,6 +220,9 @@ include $(SRC_PATH)/tests/fate/mpeg4.mak include $(SRC_PATH)/tests/fate/mpegps.mak include $(SRC_PATH)/tests/fate/mpegts.mak include $(SRC_PATH)/tests/fate/mxf.mak +include $(SRC_PATH)/tests/fate/ogg-vorbis.mak +include $(SRC_PATH)/tests/fate/ogg-flac.mak +include $(SRC_PATH)/tests/fate/ogg-opus.mak include $(SRC_PATH)/tests/fate/oma.mak include $(SRC_PATH)/tests/fate/opus.mak include $(SRC_PATH)/tests/fate/pcm.mak @@ -278,6 +281,7 @@ $(FATE_FFPROBE) $(FATE_FFMPEG_FFPROBE) $(FATE_SAMPLES_FFPROBE) $(FATE_SAMPLES_FF $(FATE_SAMPLES_FASTSTART): tools/qt-faststart$(EXESUF) $(FATE_SAMPLES_DUMP_DATA) $(FATE_SAMPLES_DUMP_DATA-yes): tools/venc_data_dump$(EXESUF) $(FATE_SAMPLES_SCALE_SLICE): tools/scale_slice_test$(EXESUF) +$(FATE_SAMPLES_DUMP_STREAM_META): tests/api/api-dump-stream-meta-test$(EXESUF) ifdef SAMPLES FATE += $(FATE_EXTERN) diff --git a/tests/api/Makefile b/tests/api/Makefile index c96e636756..a2cb06a729 100644 --- a/tests/api/Makefile +++ b/tests/api/Makefile @@ -1,7 +1,7 @@ APITESTPROGS-$(call ENCDEC, FLAC, FLAC) += api-flac APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264 APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264-slice -APITESTPROGS-yes += api-seek +APITESTPROGS-yes += api-seek api-dump-stream-meta APITESTPROGS-$(call DEMDEC, H263, H263) += api-band APITESTPROGS-$(HAVE_THREADS) += api-threadmessage APITESTPROGS += $(APITESTPROGS-yes) diff --git a/tests/api/api-dump-stream-meta-test.c b/tests/api/api-dump-stream-meta-test.c new file mode 100644 index 00..629b3a576a --- /dev/null +++ b/tests/api/api-dump-stream-meta-test.c @@ -0,0 +1,182 @@ +/* + * Copyright (c) 2025 Romain Beauxis + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +/** + * Dump stream metadata + */ + +#include "libavcodec/avcodec.h" +#include "libavformat/avformat.h" +#include "libavutil/timestamp.h" + +static int dump_stream_meta(const char *input_filename) { +const AVCodec *codec = NULL; +AVPacket *pkt = NULL; +AVFrame *fr = NULL; +AVFormatContext *fmt_ctx = NULL; +AVCodecContext *ctx = NULL; +AVCodecParameters *origin_par = NULL; +AVStream *st; +int stream_idx = 0; +int result; +char *metadata; + +result = avformat_open_input(&fmt_ctx, input_filename, NULL, NULL); +if (result < 0) { +av_log(NULL, AV_LOG_ERROR, "Can't open file\n"); +return result; +} + +result = avformat_find_stream_info(fmt_ctx, NULL); +if (result < 0) { +av_log(NULL, AV_LOG_ERROR, "Can't get stream info\n"); +goto end; +} + +if (fmt_ctx->nb_streams > 1) { +av_log(NULL, AV_LOG_ERROR, "More than one stream found in input!\n"); +goto end; +} + +origin_par = fmt_ctx->streams[stream_idx]->codecpar; +st = fmt_ctx->streams[stream_idx]; + +result = av_dict_get_string(st->metadata, &metadata, '=', ':'); +if (resul
[FFmpeg-devel] [PATCH v2 0/2] Remove chained ogg stream header packets from demuxer
These patches remove the ogg header packets from secondary chainged ogg streams from the demuxer. First, a test utility is added to track what is currently happening with chained streams. Then the changes are introduced: the packet demuxing function is used to explicitely tell the demuxer to skip header packets. Also, the packet demuxing functions are adapted to properly copy extra data from the new chained streams so that decoding can keep happening. The diff from the test output makes it possible to follow what the changes do to the extracted streams. Test samples are available at: https://www.dropbox.com/scl/fo/xrtrna2rxr1j354hrtymq/AGwemlxHYecBLNmQ8Fsy--4?rlkey=lzilr4m9w4gfdqygoe172vvy8&dl=0 # Changes since last version: * Fixed api-dump-stream-meta-test.c frame decoding logic * Fixed oggparsevorbis.c packet skipping logic. Romain Beauxis (2): tests: Add stream dump test API util, use it to dump stream data for chained ogg/{vorbis,opus,flac} streams. ogg/{vorbis,flac,opus}: Remove header packets from subsequent ogg streams from the demuxer output. libavformat/oggdec.c | 26 +-- libavformat/oggdec.h | 6 + libavformat/oggparseflac.c | 28 +++- libavformat/oggparseopus.c | 12 ++ libavformat/oggparsevorbis.c | 12 +- tests/Makefile | 4 + tests/api/Makefile | 2 +- tests/api/api-dump-stream-meta-test.c | 182 + tests/fate/ogg-flac.mak| 11 ++ tests/fate/ogg-opus.mak| 11 ++ tests/fate/ogg-vorbis.mak | 11 ++ tests/ref/fate/ogg-flac-chained-meta.txt | 10 ++ tests/ref/fate/ogg-opus-chained-meta.txt | 26 +++ tests/ref/fate/ogg-vorbis-chained-meta.txt | 14 ++ 14 files changed, 338 insertions(+), 17 deletions(-) create mode 100644 tests/api/api-dump-stream-meta-test.c create mode 100644 tests/fate/ogg-flac.mak create mode 100644 tests/fate/ogg-opus.mak create mode 100644 tests/fate/ogg-vorbis.mak create mode 100644 tests/ref/fate/ogg-flac-chained-meta.txt create mode 100644 tests/ref/fate/ogg-opus-chained-meta.txt create mode 100644 tests/ref/fate/ogg-vorbis-chained-meta.txt -- 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 v2 2/2] ogg/{vorbis, flac, opus}: Remove header packets from subsequent ogg streams from the demuxer output.
--- libavformat/oggdec.c | 26 ++-- libavformat/oggdec.h | 6 + libavformat/oggparseflac.c | 28 -- libavformat/oggparseopus.c | 12 ++ libavformat/oggparsevorbis.c | 12 -- tests/ref/fate/ogg-flac-chained-meta.txt | 2 -- tests/ref/fate/ogg-opus-chained-meta.txt | 1 - tests/ref/fate/ogg-vorbis-chained-meta.txt | 3 --- 8 files changed, 68 insertions(+), 22 deletions(-) diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index 5339fdd32c..5557eb4a14 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -239,10 +239,6 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, os->start_trimming = 0; os->end_trimming = 0; -/* Chained files have extradata as a new packet */ -if (codec == &ff_opus_codec) -os->header = -1; - return i; } @@ -605,20 +601,26 @@ static int ogg_packet(AVFormatContext *s, int *sid, int *dstart, int *dsize, } else { os->pflags= 0; os->pduration = 0; + +ret = 0; if (os->codec && os->codec->packet) { if ((ret = os->codec->packet(s, idx)) < 0) { av_log(s, AV_LOG_ERROR, "Packet processing failed: %s\n", av_err2str(ret)); return ret; } } -if (sid) -*sid = idx; -if (dstart) -*dstart = os->pstart; -if (dsize) -*dsize = os->psize; -if (fpos) -*fpos = os->sync_pos; + +if (!ret) { +if (sid) +*sid = idx; +if (dstart) +*dstart = os->pstart; +if (dsize) +*dsize = os->psize; +if (fpos) +*fpos = os->sync_pos; +} + os->pstart += os->psize; os->psize= 0; if(os->pstart == os->bufpos) diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h index 43df23f4cb..09f698f99a 100644 --- a/libavformat/oggdec.h +++ b/libavformat/oggdec.h @@ -38,6 +38,12 @@ struct ogg_codec { * -1 if an error occurred or for unsupported stream */ int (*header)(AVFormatContext *, int); +/** + * Attempt to process a packet as a data packet + * @return 1 if the packet was a header from a chained bitstream. + * 0 if the packet was a regular data packet. + * -1 if an error occurred or for unsupported stream + */ int (*packet)(AVFormatContext *, int); /** * Translate a granule into a timestamp. diff --git a/libavformat/oggparseflac.c b/libavformat/oggparseflac.c index f25ed9cc15..d66b85b09e 100644 --- a/libavformat/oggparseflac.c +++ b/libavformat/oggparseflac.c @@ -27,6 +27,8 @@ #include "oggdec.h" #define OGG_FLAC_METADATA_TYPE_STREAMINFO 0x7F +#define OGG_FLAC_MAGIC "\177FLAC" +#define OGG_FLAC_MAGIC_SIZE sizeof(OGG_FLAC_MAGIC)-1 static int flac_header (AVFormatContext * s, int idx) @@ -78,6 +80,27 @@ flac_header (AVFormatContext * s, int idx) return 1; } +static int +flac_packet (AVFormatContext * s, int idx) +{ +struct ogg *ogg = s->priv_data; +struct ogg_stream *os = ogg->streams + idx; + +if (os->psize > OGG_FLAC_MAGIC_SIZE && +!memcmp( +os->buf + os->pstart, +OGG_FLAC_MAGIC, +OGG_FLAC_MAGIC_SIZE)) +return 1; + +if (os->psize > 0 && +((os->buf[os->pstart] & 0x7F) == FLAC_METADATA_TYPE_VORBIS_COMMENT)) { +return 1; +} + +return 0; +} + static int old_flac_header (AVFormatContext * s, int idx) { @@ -127,10 +150,11 @@ fail: } const struct ogg_codec ff_flac_codec = { -.magic = "\177FLAC", -.magicsize = 5, +.magic = OGG_FLAC_MAGIC, +.magicsize = OGG_FLAC_MAGIC_SIZE, .header = flac_header, .nb_header = 2, +.packet = flac_packet, }; const struct ogg_codec ff_old_flac_codec = { diff --git a/libavformat/oggparseopus.c b/libavformat/oggparseopus.c index 218e9df581..07ddba8c82 100644 --- a/libavformat/oggparseopus.c +++ b/libavformat/oggparseopus.c @@ -81,6 +81,7 @@ static int opus_header(AVFormatContext *avf, int idx) if (priv->need_comments) { if (os->psize < 8 || memcmp(packet, "OpusTags", 8)) return AVERROR_INVALIDDATA; + ff_vorbis_stream_comment(avf, st, packet + 8, os->psize - 8); priv->need_comments--; return 1; @@ -125,6 +126,17 @@ static int opus_packet(AVFormatContext *avf, int idx) return AVERROR_INVALIDDATA; } + if (os->psize > 8 && !memcmp(packet, "OpusHead", 8)) { +if ((ret = ff_alloc_extradata(st->codecpar, os->psize)) < 0) +return ret; + +memcpy(st->codecpar->extradata, packet, os->psize); +return 1; +} + +if (os->psize > 8 && !memcmp(packet, "OpusTags", 8)) +return 1; + if ((!os->lastpts || o
Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply quality improvements
On date Monday 2025-04-28 20:40:37 +, softworkz . wrote: [...] > > > > > /* validate replace string */ > > > > > { > > > > > -const uint8_t *p = tctx->string_validation_replacement; > > > > > -const uint8_t *endp = p + strlen(p); > > > > > +const uint8_t *p = (uint8_t *)tctx- > > >string_validation_replacement; > > > > > +const uint8_t *endp = p + strlen((const char *)p); > > > > > while (*p) { > > > > > const uint8_t *p0 = p; > > > > > int32_t code; > > > > > ret = av_utf8_decode(&code, &p, endp, tctx- > > > > >string_validation_utf8_flags); > > > > > if (ret < 0) { > > > > > > > > > AVBPrint bp; > > > > > -av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); > > > > > +av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); > > > > > bprint_bytes(&bp, p0, p - p0), > > > > > av_log(tctx, AV_LOG_ERROR, > > > > > "Invalid UTF8 sequence %s found in string > > > > validation replace '%s'\n", > > > > > bp.str, tctx- > > >string_validation_replacement); > > > > > -return ret; > > > > > +av_bprint_finalize(&bp, NULL); > > > > > > > > > > I see no advantage in this, since it's adding dynamic allocation and > > > > the need to free which was previously not needed > > > > > > The dynamic allocation does not happen until the buffer content size > > > does not fit anymore into the stack-alloced memory. > > > > Yes, but is this ever happening? > > Are we sure that it's not? Because unless we are 100% sure, we need > to check the return value and when that check indicates an overflow > of the buffer - what then? How to remedy? > > The check for the return value alone - even without any remediation - > is more complicated code (more lines) than a single line with > av_bprint_finalize(). > > That's my general rationale behind favoring _UNLIMITED. > > There are more cases where I have that, and where it's really relevant > but for this one, I'll just remove the change, it's not worth the > discussion. So I checked the code, a valid UTF8 sequence should be no longer than six bytes, and reading the implementation of av_utf8_decode I could not quickly find out the maximum possible size of an invalid sequence, so I guess your argument has a point in case of crafted sequences - although possibly we might compute the maximum size and it might be small (assuming there are no mistakes in the implementation and no regressions are introduced). Even in that case, the worst it might happen would be that the sequence is truncated, so this should have no security implications anyway - while allocation might fail (in case of a very looong crafted string), leading again to truncation. That said, I'm not against this change but it should be moved to a dedicated change. ___ 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 v7 02/13] fftools/textformat: Apply quality improvements
> -Original Message- > From: Stefano Sabatini > Sent: Montag, 28. April 2025 23:47 > To: softworkz . > Cc: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply quality > improvements > > On date Monday 2025-04-28 20:40:37 +, softworkz . wrote: > [...] > > > > > > /* validate replace string */ > > > > > > { > > > > > > -const uint8_t *p = tctx->string_validation_replacement; > > > > > > -const uint8_t *endp = p + strlen(p); > > > > > > +const uint8_t *p = (uint8_t *)tctx- > > > >string_validation_replacement; > > > > > > +const uint8_t *endp = p + strlen((const char *)p); > > > > > > while (*p) { > > > > > > const uint8_t *p0 = p; > > > > > > int32_t code; > > > > > > ret = av_utf8_decode(&code, &p, endp, tctx- > > > > > >string_validation_utf8_flags); > > > > > > if (ret < 0) { > > > > > > > > > > > AVBPrint bp; > > > > > > -av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); > > > > > > +av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); > > > > > > bprint_bytes(&bp, p0, p - p0), > > > > > > av_log(tctx, AV_LOG_ERROR, > > > > > > "Invalid UTF8 sequence %s found in > string > > > > > validation replace '%s'\n", > > > > > > bp.str, tctx- > > > >string_validation_replacement); > > > > > > -return ret; > > > > > > +av_bprint_finalize(&bp, NULL); > > > > > > > > > > > > > I see no advantage in this, since it's adding dynamic allocation and > > > > > the need to free which was previously not needed > > > > > > > > The dynamic allocation does not happen until the buffer content size > > > > does not fit anymore into the stack-alloced memory. > > > > > > Yes, but is this ever happening? > > > > > Are we sure that it's not? Because unless we are 100% sure, we need > > to check the return value and when that check indicates an overflow > > of the buffer - what then? How to remedy? > > > > The check for the return value alone - even without any remediation - > > is more complicated code (more lines) than a single line with > > av_bprint_finalize(). > > > > That's my general rationale behind favoring _UNLIMITED. > > > > There are more cases where I have that, and where it's really relevant > > but for this one, I'll just remove the change, it's not worth the > > discussion. > > So I checked the code, a valid UTF8 sequence should be no longer than > six bytes, and reading the implementation of av_utf8_decode I could > not quickly find out the maximum possible size of an invalid sequence, > so I guess your argument has a point in case of crafted sequences - > although possibly we might compute the maximum size and it might be > small (assuming there are no mistakes in the implementation and no > regressions are introduced). Even in that case, the worst it might > happen would be that the sequence is truncated, so this should have no > security implications anyway - while allocation might fail (in case of > a very looong crafted string), leading again to truncation. Yea, that aligns with my view. When I added the mermaid output I was really glad to find out that it fits fairly well into the sections logic, but the data that is in play is quite different in some cases, especially when it comes to the size of certain strings, and right away I ran into a bug due to a truncated string from an AVBprint with SIZE_AUTOMATIC. Which brought me to say: "Stop making assumptions". Making assumptions about potential max string lengths (like by looking at all call sites) is not a robust approach. Nobody else knows which assumptions you were making, or even more typical: at some point you might even forget about it and run into it yourself. 😃 Then I looked through all BPrints and there wasn't a single one where I could have said I'm 100% sure that it won't overflow (such cases exist of course - elsewhere). Having _UNLIMITED everywhere (in the AVTextFormat scope) also has advantages in maintenance: - It will always just work - No need to check return values of BPrint APIs - Every BPrint needs a finalize() call - always, without condition this is much easier to work with, and to maintain - less error-prone - The finalize() call is generally negligible unless it's a very special case What is even remaining on the pro-side for _SIZE_AUTOMATIC? - You save the finalize() call And that's it. Might sometimes be relevant or more elegant - yet often not. At the moment, all BPrint inits are _UNLIMITED already (in the scope of these APIs) and it's a good and safe thing IMO. > That said, I'm not against this change but it should be moved to a > dedicated change. Roger! Thanks sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.or
Re: [FFmpeg-devel] [PATCH] postproc/tests: Add test tools to .gitignore
On Mon, Apr 28, 2025 at 04:02:14PM +0200, Andreas Rheinhardt wrote: > Patch attached. > > - Andreas > .gitignore |2 ++ > 1 file changed, 2 insertions(+) > a3523219c532b9afd7011d59964fa5f210b9f2d2 > 0001-postproc-tests-Add-test-tools-to-.gitignore.patch > From d1961c101dfd095c219c4ff56d5f5523574976ce Mon Sep 17 00:00:00 2001 > From: Andreas Rheinhardt > Date: Mon, 28 Apr 2025 16:01:13 +0200 > Subject: [PATCH] postproc/tests: Add test tools to .gitignore > > Signed-off-by: Andreas Rheinhardt > --- > libpostproc/tests/.gitignore | 2 ++ > 1 file changed, 2 insertions(+) > create mode 100644 libpostproc/tests/.gitignore LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB z(9) = an object that transcends all computable functions describable in finite terms. - ChatGPT in 2024 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 v8 01/15] fftools/textformat: Formatting and whitespace changes
From: softworkz Reviewed-by: Stefano Sabatini Signed-off-by: softworkz --- fftools/textformat/avtextformat.c | 92 +++--- fftools/textformat/avtextformat.h | 20 +++ fftools/textformat/avtextwriters.h | 11 ++-- fftools/textformat/tf_compact.c| 91 - fftools/textformat/tf_default.c| 20 +++ fftools/textformat/tf_flat.c | 26 + fftools/textformat/tf_ini.c| 36 ++-- fftools/textformat/tf_json.c | 10 ++-- fftools/textformat/tf_xml.c| 30 +- 9 files changed, 177 insertions(+), 159 deletions(-) diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c index 9200b9b1ad..128ed243dd 100644 --- a/fftools/textformat/avtextformat.c +++ b/fftools/textformat/avtextformat.c @@ -34,9 +34,9 @@ #include "libavutil/opt.h" #include "avtextformat.h" -#define SECTION_ID_NONE -1 +#define SECTION_ID_NONE (-1) -#define SHOW_OPTIONAL_FIELDS_AUTO -1 +#define SHOW_OPTIONAL_FIELDS_AUTO (-1) #define SHOW_OPTIONAL_FIELDS_NEVER 0 #define SHOW_OPTIONAL_FIELDS_ALWAYS 1 @@ -64,14 +64,14 @@ static const char *textcontext_get_formatter_name(void *p) static const AVOption textcontext_options[] = { { "string_validation", "set string validation mode", - OFFSET(string_validation), AV_OPT_TYPE_INT, {.i64=AV_TEXTFORMAT_STRING_VALIDATION_REPLACE}, 0, AV_TEXTFORMAT_STRING_VALIDATION_NB-1, .unit = "sv" }, + OFFSET(string_validation), AV_OPT_TYPE_INT, { .i64 = AV_TEXTFORMAT_STRING_VALIDATION_REPLACE }, 0, AV_TEXTFORMAT_STRING_VALIDATION_NB - 1, .unit = "sv" }, { "sv", "set string validation mode", - OFFSET(string_validation), AV_OPT_TYPE_INT, {.i64=AV_TEXTFORMAT_STRING_VALIDATION_REPLACE}, 0, AV_TEXTFORMAT_STRING_VALIDATION_NB-1, .unit = "sv" }, -{ "ignore", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AV_TEXTFORMAT_STRING_VALIDATION_IGNORE}, .unit = "sv" }, -{ "replace", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AV_TEXTFORMAT_STRING_VALIDATION_REPLACE}, .unit = "sv" }, -{ "fail",NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AV_TEXTFORMAT_STRING_VALIDATION_FAIL},.unit = "sv" }, -{ "string_validation_replacement", "set string validation replacement string", OFFSET(string_validation_replacement), AV_OPT_TYPE_STRING, {.str=""}}, -{ "svr", "set string validation replacement string", OFFSET(string_validation_replacement), AV_OPT_TYPE_STRING, {.str="\xEF\xBF\xBD"}}, + OFFSET(string_validation), AV_OPT_TYPE_INT, { .i64 = AV_TEXTFORMAT_STRING_VALIDATION_REPLACE }, 0, AV_TEXTFORMAT_STRING_VALIDATION_NB - 1, .unit = "sv" }, +{ "ignore", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AV_TEXTFORMAT_STRING_VALIDATION_IGNORE }, .unit = "sv" }, +{ "replace", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AV_TEXTFORMAT_STRING_VALIDATION_REPLACE }, .unit = "sv" }, +{ "fail",NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AV_TEXTFORMAT_STRING_VALIDATION_FAIL },.unit = "sv" }, +{ "string_validation_replacement", "set string validation replacement string", OFFSET(string_validation_replacement), AV_OPT_TYPE_STRING, { .str = "" } }, +{ "svr", "set string validation replacement string", OFFSET(string_validation_replacement), AV_OPT_TYPE_STRING, { .str = "\xEF\xBF\xBD" } }, { NULL } }; @@ -126,7 +126,7 @@ void avtext_context_close(AVTextFormatContext **ptctx) int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *formatter, AVTextWriterContext *writer_context, const char *args, -const struct AVTextFormatSection *sections, int nb_sections, +const AVTextFormatSection *sections, int nb_sections, int show_value_unit, int use_value_prefix, int use_byte_value_binary_prefix, @@ -200,7 +200,7 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form av_dict_free(&opts); } -if (show_data_hash) { +if (show_data_hash) if ((ret = av_hash_alloc(&tctx->hash, show_data_hash)) < 0) { if (ret == AVERROR(EINVAL)) { const char *n; @@ -211,7 +211,6 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form } return ret; } -} /* validate replace string */ { @@ -224,10 +223,10 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form if (ret < 0) { AVBPrint bp; av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); -bprint_bytes(&bp, p0, p-p0), -av_log(tctx, AV_LOG_ERROR, - "Invalid UTF8 sequence %s found in string validation replace '%s'\n", - bp.str, tctx->string_validation_replacement); +bprint_bytes(&bp, p0, p - p0); +a
[FFmpeg-devel] [PATCH v8 02/15] fftools/textformat: Apply quality improvements
From: softworkz Perform multiple improvements to increase code robustness. In particular: - favor unsigned counters for loops - add missing checks - avoid possible leaks - move variable declarations to inner scopes when feasible - provide explicit type-casting when needed Signed-off-by: softworkz --- fftools/textformat/avtextformat.c | 84 --- fftools/textformat/avtextformat.h | 6 +-- fftools/textformat/tf_default.c | 8 ++- fftools/textformat/tf_ini.c | 2 +- fftools/textformat/tf_json.c | 17 --- fftools/textformat/tf_xml.c | 3 -- fftools/textformat/tw_avio.c | 9 +++- 7 files changed, 83 insertions(+), 46 deletions(-) diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c index 128ed243dd..32cec9106f 100644 --- a/fftools/textformat/avtextformat.c +++ b/fftools/textformat/avtextformat.c @@ -93,9 +93,8 @@ static const AVClass textcontext_class = { static void bprint_bytes(AVBPrint *bp, const uint8_t *ubuf, size_t ubuf_size) { -int i; av_bprintf(bp, "0X"); -for (i = 0; i < ubuf_size; i++) +for (unsigned i = 0; i < ubuf_size; i++) av_bprintf(bp, "%02X", ubuf[i]); } @@ -137,6 +136,8 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form AVTextFormatContext *tctx; int i, ret = 0; +av_assert0(ptctx && formatter); + if (!(tctx = av_mallocz(sizeof(AVTextFormatContext { ret = AVERROR(ENOMEM); goto fail; @@ -209,13 +210,13 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form av_log(NULL, AV_LOG_ERROR, " %s", n); av_log(NULL, AV_LOG_ERROR, "\n"); } -return ret; +goto fail; } /* validate replace string */ { -const uint8_t *p = tctx->string_validation_replacement; -const uint8_t *endp = p + strlen(p); +const uint8_t *p = (uint8_t *)tctx->string_validation_replacement; +const uint8_t *endp = p + strlen((const char *)p); while (*p) { const uint8_t *p0 = p; int32_t code; @@ -228,6 +229,7 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form "Invalid UTF8 sequence %s found in string validation replace '%s'\n", bp.str, tctx->string_validation_replacement); return ret; +goto fail; } } } @@ -255,6 +257,11 @@ static const char unit_bit_per_second_str[] = "bit/s"; void avtext_print_section_header(AVTextFormatContext *tctx, const void *data, int section_id) { +if (section_id < 0 || section_id >= tctx->nb_sections) { +av_log(tctx, AV_LOG_ERROR, "Invalid section_id for section_header: %d\n", section_id); +return; +} + tctx->level++; av_assert0(tctx->level < SECTION_MAX_NB_LEVELS); @@ -268,6 +275,11 @@ void avtext_print_section_header(AVTextFormatContext *tctx, const void *data, in void avtext_print_section_footer(AVTextFormatContext *tctx) { +if (tctx->level < 0 || tctx->level >= SECTION_MAX_NB_LEVELS) { +av_log(tctx, AV_LOG_ERROR, "Invalid level for section_footer: %d\n", tctx->level); +return; +} + int section_id = tctx->section[tctx->level]->id; int parent_section_id = tctx->level ? tctx->section[tctx->level - 1]->id @@ -285,7 +297,11 @@ void avtext_print_section_footer(AVTextFormatContext *tctx) void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t val) { -const struct AVTextFormatSection *section = tctx->section[tctx->level]; +const AVTextFormatSection *section; + +av_assert0(key && tctx->level >= 0 && tctx->level < SECTION_MAX_NB_LEVELS); + +section = tctx->section[tctx->level]; if (section->show_all_entries || av_dict_get(section->entries_to_show, key, NULL, 0)) { tctx->formatter->print_integer(tctx, key, val); @@ -354,17 +370,18 @@ struct unit_value { const char *unit; }; -static char *value_string(AVTextFormatContext *tctx, char *buf, int buf_size, struct unit_value uv) +static char *value_string(const AVTextFormatContext *tctx, char *buf, int buf_size, struct unit_value uv) { double vald; -int64_t vali; +int64_t vali = 0; int show_float = 0; if (uv.unit == unit_second_str) { vald = uv.val.d; show_float = 1; } else { -vald = vali = uv.val.i; +vald = (double)uv.val.i; +vali = uv.val.i; } if (uv.unit == unit_second_str && tctx->use_value_sexagesimal_format) { @@ -383,17 +400,17 @@ static char *value_string(AVTextFormatContext *tctx, char *buf, int buf_size, st int64_t index; if (uv.unit == unit_byte_str && tctx->use_byte_value_binary_prefix) { -index = (int64_t) (log2(vald)) / 10; -index =
[FFmpeg-devel] [PATCH v8 05/15] fftools/avtextformat: Re-use BPrint in loop
From: softworkz Instead of initializing a new BPrint in each iteration of the loop, re-use the same BPrint struct and just clear it for each iteration. Signed-off-by: softworkz --- fftools/textformat/avtextformat.c | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c index 1cd9555ca8..b2c3aa3fc7 100644 --- a/fftools/textformat/avtextformat.c +++ b/fftools/textformat/avtextformat.c @@ -311,24 +311,28 @@ void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t va static inline int validate_string(AVTextFormatContext *tctx, char **dstp, const char *src) { -const uint8_t *p, *endp; +const uint8_t *p, *endp, *srcp = (const uint8_t *)src; AVBPrint dstbuf; +AVBPrint bp_invalid_seq; int invalid_chars_nb = 0, ret = 0; +*dstp = NULL; av_bprint_init(&dstbuf, 0, AV_BPRINT_SIZE_UNLIMITED); +av_bprint_init(&bp_invalid_seq, 0, AV_BPRINT_SIZE_UNLIMITED); -endp = src + strlen(src); -for (p = src; *p;) { -uint32_t code; +endp = srcp + strlen(src); +for (p = srcp; *p;) { +int32_t code; int invalid = 0; const uint8_t *p0 = p; if (av_utf8_decode(&code, &p, endp, tctx->string_validation_utf8_flags) < 0) { -AVBPrint bp; -av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); -bprint_bytes(&bp, p0, p-p0); -av_log(tctx, AV_LOG_DEBUG, - "Invalid UTF-8 sequence %s found in string '%s'\n", bp.str, src); + +av_bprint_clear(&bp_invalid_seq); + +bprint_bytes(&bp_invalid_seq, p0, p - p0); + +av_log(tctx, AV_LOG_DEBUG, "Invalid UTF-8 sequence %s found in string '%s'\n", bp_invalid_seq.str, src); invalid = 1; } @@ -358,6 +362,7 @@ static inline int validate_string(AVTextFormatContext *tctx, char **dstp, const end: av_bprint_finalize(&dstbuf, dstp); +av_bprint_finalize(&bp_invalid_seq, NULL); return ret; } -- ffmpeg-codebot ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v8 04/15] fftools/textformat: Rename name param to key for API consistency
From: softworkz Signed-off-by: softworkz --- fftools/textformat/avtextformat.c | 14 +++--- fftools/textformat/avtextformat.h | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c index 32cec9106f..1cd9555ca8 100644 --- a/fftools/textformat/avtextformat.c +++ b/fftools/textformat/avtextformat.c @@ -507,7 +507,7 @@ void avtext_print_ts(AVTextFormatContext *tctx, const char *key, int64_t ts, int avtext_print_integer(tctx, key, ts); } -void avtext_print_data(AVTextFormatContext *tctx, const char *name, +void avtext_print_data(AVTextFormatContext *tctx, const char *key, const uint8_t *data, int size) { AVBPrint bp; @@ -532,11 +532,11 @@ void avtext_print_data(AVTextFormatContext *tctx, const char *name, data += l; size -= l; } -avtext_print_string(tctx, name, bp.str, 0); +avtext_print_string(tctx, key, bp.str, 0); av_bprint_finalize(&bp, NULL); } -void avtext_print_data_hash(AVTextFormatContext *tctx, const char *name, +void avtext_print_data_hash(AVTextFormatContext *tctx, const char *key, const uint8_t *data, int size) { char buf[AV_HASH_MAX_SIZE * 2 + 64] = { 0 }; @@ -549,10 +549,10 @@ void avtext_print_data_hash(AVTextFormatContext *tctx, const char *name, av_hash_update(tctx->hash, data, size); len = snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx->hash)); av_hash_final_hex(tctx->hash, (uint8_t *)&buf[len], (int)sizeof(buf) - len); -avtext_print_string(tctx, name, buf, 0); +avtext_print_string(tctx, key, buf, 0); } -void avtext_print_integers(AVTextFormatContext *tctx, const char *name, +void avtext_print_integers(AVTextFormatContext *tctx, const char *key, uint8_t *data, int size, const char *format, int columns, int bytes, int offset_add) { @@ -560,7 +560,7 @@ void avtext_print_integers(AVTextFormatContext *tctx, const char *name, unsigned offset = 0; int l, i; -if (!name || !data || !format || columns <= 0 || bytes <= 0) +if (!key || !data || !format || columns <= 0 || bytes <= 0) return; av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); @@ -578,7 +578,7 @@ void avtext_print_integers(AVTextFormatContext *tctx, const char *name, av_bprintf(&bp, "\n"); offset += offset_add; } -avtext_print_string(tctx, name, bp.str, 0); +avtext_print_string(tctx, key, bp.str, 0); av_bprint_finalize(&bp, NULL); } diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h index c61dca159f..8ff503401a 100644 --- a/fftools/textformat/avtextformat.h +++ b/fftools/textformat/avtextformat.h @@ -148,11 +148,11 @@ void avtext_print_time(AVTextFormatContext *tctx, const char *key, int64_t ts, c void avtext_print_ts(AVTextFormatContext *tctx, const char *key, int64_t ts, int is_duration); -void avtext_print_data(AVTextFormatContext *tctx, const char *name, const uint8_t *data, int size); +void avtext_print_data(AVTextFormatContext *tctx, const char *key, const uint8_t *data, int size); -void avtext_print_data_hash(AVTextFormatContext *tctx, const char *name, const uint8_t *data, int size); +void avtext_print_data_hash(AVTextFormatContext *tctx, const char *key, const uint8_t *data, int size); -void avtext_print_integers(AVTextFormatContext *tctx, const char *name, uint8_t *data, int size, +void avtext_print_integers(AVTextFormatContext *tctx, const char *key, uint8_t *data, int size, const char *format, int columns, int bytes, int offset_add); const AVTextFormatter *avtext_get_formatter_by_name(const char *name); -- ffmpeg-codebot ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v8 03/15] fftools/textformat: Remove unused print_rational() pointer from AVTextFormatter
From: softworkz Signed-off-by: softworkz --- fftools/textformat/avtextformat.h | 1 - fftools/textformat/avtextwriters.h | 5 - 2 files changed, 6 deletions(-) diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h index 86feb9d09d..c61dca159f 100644 --- a/fftools/textformat/avtextformat.h +++ b/fftools/textformat/avtextformat.h @@ -75,7 +75,6 @@ typedef struct AVTextFormatter { void (*print_section_header)(AVTextFormatContext *tctx, const void *data); void (*print_section_footer)(AVTextFormatContext *tctx); void (*print_integer) (AVTextFormatContext *tctx, const char *, int64_t); -void (*print_rational) (AVTextFormatContext *tctx, AVRational *q, char *sep); void (*print_string)(AVTextFormatContext *tctx, const char *, const char *); int flags; ///< a combination or AV_TEXTFORMAT__FLAG_* } AVTextFormatter; diff --git a/fftools/textformat/avtextwriters.h b/fftools/textformat/avtextwriters.h index 34db3f1832..fd5cda04ef 100644 --- a/fftools/textformat/avtextwriters.h +++ b/fftools/textformat/avtextwriters.h @@ -21,14 +21,9 @@ #ifndef FFTOOLS_TEXTFORMAT_AVTEXTWRITERS_H #define FFTOOLS_TEXTFORMAT_AVTEXTWRITERS_H -#include #include -#include "libavutil/attributes.h" -#include "libavutil/dict.h" #include "libavformat/avio.h" #include "libavutil/bprint.h" -#include "libavutil/rational.h" -#include "libavutil/hash.h" typedef struct AVTextWriterContext AVTextWriterContext; -- ffmpeg-codebot ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v8 14/15] fftools/graphprint: Add execution graph printing
From: softworkz The key benefits are: - Different to other graph printing methods, this is outputting: - all graphs with runtime state (including auto-inserted filters) - each graph with its inputs and outputs - all filters with their in- and output pads - all connections between all input- and output pads - for each connection: - the runtime-negotiated format and media type - the hw context - if video hw context, both: hw pixfmt + sw pixfmt - Output can either be printed to stdout or written to specified file - Output is machine-readable - Use the same output implementation as ffprobe, supporting multiple formats Signed-off-by: softworkz --- doc/ffmpeg.texi | 10 + fftools/Makefile | 20 +- fftools/ffmpeg.c |4 + fftools/ffmpeg.h |3 + fftools/ffmpeg_filter.c |5 + fftools/ffmpeg_opt.c | 13 + fftools/graph/graphprint.c| 1102 + fftools/graph/graphprint.h| 30 + fftools/textformat/avtextformat.c |4 +- fftools/textformat/avtextformat.h | 29 + fftools/textformat/tf_mermaid.c | 658 + fftools/textformat/tf_mermaid.h | 41 ++ 12 files changed, 1917 insertions(+), 2 deletions(-) create mode 100644 fftools/graph/graphprint.c create mode 100644 fftools/graph/graphprint.h create mode 100644 fftools/textformat/tf_mermaid.c create mode 100644 fftools/textformat/tf_mermaid.h diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index 17ba876ea3..35675b5309 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -1394,6 +1394,16 @@ It is on by default, to explicitly disable it you need to specify @code{-nostats @item -stats_period @var{time} (@emph{global}) Set period at which encoding progress/statistics are updated. Default is 0.5 seconds. +@item -print_graphs (@emph{global}) +Prints execution graph details to stderr in the format set via -print_graphs_format. + +@item -print_graphs_file @var{filename} (@emph{global}) +Writes execution graph details to the specified file in the format set via -print_graphs_format. + +@item -print_graphs_format @var{format} (@emph{global}) +Sets the output format (available formats are: default, compact, csv, flat, ini, json, xml, mermaid, mermaidhtml) +The default format is json. + @item -progress @var{url} (@emph{global}) Send program-friendly progress information to @var{url}. diff --git a/fftools/Makefile b/fftools/Makefile index a30bec889e..361a4fd574 100644 --- a/fftools/Makefile +++ b/fftools/Makefile @@ -9,6 +9,8 @@ AVBASENAMES = ffmpeg ffplay ffprobe ALLAVPROGS = $(AVBASENAMES:%=%$(PROGSSUF)$(EXESUF)) ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF)) +include $(SRC_PATH)/fftools/resources/Makefile + OBJS-ffmpeg += \ fftools/ffmpeg_dec.o\ fftools/ffmpeg_demux.o \ @@ -19,8 +21,21 @@ OBJS-ffmpeg += \ fftools/ffmpeg_mux_init.o \ fftools/ffmpeg_opt.o\ fftools/ffmpeg_sched.o \ +fftools/graph/graphprint.o\ fftools/sync_queue.o\ fftools/thread_queue.o \ +fftools/textformat/avtextformat.o \ +fftools/textformat/tf_compact.o \ +fftools/textformat/tf_default.o \ +fftools/textformat/tf_flat.o \ +fftools/textformat/tf_ini.o \ +fftools/textformat/tf_json.o \ +fftools/textformat/tf_mermaid.o \ +fftools/textformat/tf_xml.o \ +fftools/textformat/tw_avio.o \ +fftools/textformat/tw_buffer.o\ +fftools/textformat/tw_stdout.o\ +$(OBJS-resman)\ OBJS-ffprobe += \ fftools/textformat/avtextformat.o \ @@ -29,10 +44,12 @@ OBJS-ffprobe += \ fftools/textformat/tf_flat.o \ fftools/textformat/tf_ini.o \ fftools/textformat/tf_json.o \ +fftools/textformat/tf_mermaid.o \ fftools/textformat/tf_xml.o \ fftools/textformat/tw_avio.o \ fftools/textformat/tw_buffer.o\ fftools/textformat/tw_stdout.o\ +$(OBJS-resman)\ OBJS-ffplay += fftools/ffplay_renderer.o @@ -42,7 +59,7 @@ ifdef HAVE_GNU_WINDRES OBJS-$(1) += fftools/fftoolsres.o endif $(1)$(PROGSSUF)_g$(EXESUF): $$(OBJS-$(1)) -$$(OBJS-$(1)): | fftools fftools/textformat fftools/resources +$$(OBJS-$(1)): | fftools fftools/textformat fftools/resources fftools/graph $$(OBJS-$(1)): CFLAGS += $(CFLAGS-$(1)) $(1)$(PROGSSUF)_g$(EXESUF): LDFLAGS += $(LDFLAGS-$(1)) $(1)$(PROGSSUF)_g$(EXESUF): FF_EXTRALIBS += $(EXTRALIBS-$(1)) @@ -57,6 +74,7 @@ fftools/ffprobe.o fftools/cmdutils.o: libavutil/ffversion.h | fftools OUTDIRS += fftools OUTDIRS += fftools/textformat OUTDIRS += fftools/resources +OUTDIRS += fftools/graph ifdef AVPROGS install: install-progs install-data diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index dc321fb4a2..6766ec209
[FFmpeg-devel] [PATCH v8 12/15] fftools/resources: Add resource manager files
From: softworkz Signed-off-by: softworkz --- ffbuild/common.mak | 28 ++- fftools/Makefile | 3 +- fftools/resources/.gitignore | 4 + fftools/resources/Makefile | 27 +++ fftools/resources/graph.css | 353 +++ fftools/resources/graph.html | 86 + fftools/resources/resman.c | 213 + fftools/resources/resman.h | 50 + 8 files changed, 762 insertions(+), 2 deletions(-) create mode 100644 fftools/resources/.gitignore create mode 100644 fftools/resources/Makefile create mode 100644 fftools/resources/graph.css create mode 100644 fftools/resources/graph.html create mode 100644 fftools/resources/resman.c create mode 100644 fftools/resources/resman.h diff --git a/ffbuild/common.mak b/ffbuild/common.mak index ca45a0f368..6717092d44 100644 --- a/ffbuild/common.mak +++ b/ffbuild/common.mak @@ -139,6 +139,32 @@ else $(BIN2C) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) $@ $(subst .,_,$(basename $(notdir $@))) endif +# 1) Preprocess CSS to a minified version +%.css.min: %.css + # Must start with a tab in the real Makefile + sed 's!/\\*.*\\*/!!g' $< \ + | tr '\n' ' ' \ + | tr -s ' ' \ + | sed 's/^ //; s/ $$//' \ + > $@ + +# 2) Gzip the minified CSS +%.css.min.gz: %.css.min + $(M)gzip -nc9 $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) >$@ + +# 3) Convert the gzipped CSS to a .c array +%.css.c: %.css.min.gz $(BIN2CEXE) + $(BIN2C) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) $@ $(subst .,_,$(basename $(notdir $@))) + +# 4) Gzip the HTML file (no minification needed) +%.html.gz: TAG = GZIP +%.html.gz: %.html + $(M)gzip -nc9 $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) > $@ + +# 5) Convert the gzipped HTML to a .c array +%.html.c: %.html.gz $(BIN2CEXE) + $(BIN2C) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) $@ $(subst .,_,$(basename $(notdir $@))) + clean:: $(RM) $(BIN2CEXE) $(CLEANSUFFIXES:%=ffbuild/%) @@ -214,7 +240,7 @@ $(TOOLOBJS): | tools OUTDIRS := $(OUTDIRS) $(dir $(OBJS) $(HOBJS) $(HOSTOBJS) $(SLIBOBJS) $(SHLIBOBJS) $(STLIBOBJS) $(TESTOBJS)) -CLEANSUFFIXES = *.d *.gcda *.gcno *.h.c *.ho *.map *.o *.objs *.pc *.ptx *.ptx.gz *.ptx.c *.ver *.version *$(DEFAULT_X86ASMD).asm *~ *.ilk *.pdb +CLEANSUFFIXES = *.d *.gcda *.gcno *.h.c *.ho *.map *.o *.objs *.pc *.ptx *.ptx.gz *.ptx.c *.ver *.version *.html.gz *.html.c *.css.gz *.css.c *$(DEFAULT_X86ASMD).asm *~ *.ilk *.pdb LIBSUFFIXES = *.a *.lib *.so *.so.* *.dylib *.dll *.def *.dll.a define RULES diff --git a/fftools/Makefile b/fftools/Makefile index e9c9891c34..a30bec889e 100644 --- a/fftools/Makefile +++ b/fftools/Makefile @@ -42,7 +42,7 @@ ifdef HAVE_GNU_WINDRES OBJS-$(1) += fftools/fftoolsres.o endif $(1)$(PROGSSUF)_g$(EXESUF): $$(OBJS-$(1)) -$$(OBJS-$(1)): | fftools fftools/textformat +$$(OBJS-$(1)): | fftools fftools/textformat fftools/resources $$(OBJS-$(1)): CFLAGS += $(CFLAGS-$(1)) $(1)$(PROGSSUF)_g$(EXESUF): LDFLAGS += $(LDFLAGS-$(1)) $(1)$(PROGSSUF)_g$(EXESUF): FF_EXTRALIBS += $(EXTRALIBS-$(1)) @@ -56,6 +56,7 @@ all: $(AVPROGS) fftools/ffprobe.o fftools/cmdutils.o: libavutil/ffversion.h | fftools OUTDIRS += fftools OUTDIRS += fftools/textformat +OUTDIRS += fftools/resources ifdef AVPROGS install: install-progs install-data diff --git a/fftools/resources/.gitignore b/fftools/resources/.gitignore new file mode 100644 index 00..5f496535a6 --- /dev/null +++ b/fftools/resources/.gitignore @@ -0,0 +1,4 @@ +*.html.c +*.css.c +*.html.gz +*.css.gz diff --git a/fftools/resources/Makefile b/fftools/resources/Makefile new file mode 100644 index 00..2487b1ca52 --- /dev/null +++ b/fftools/resources/Makefile @@ -0,0 +1,27 @@ +clean:: + $(RM) $(CLEANSUFFIXES:%=fftools/resources/%) + + +HTML_RESOURCES := $(SRC_PATH)/fftools/resources/graph.html \ + +# .html => (gzip) .html.gz => (bin2c) .html.c => (cc) .o +HTML_RESOURCES_GZ := $(HTML_RESOURCES:.html=.html.gz) +HTML_RESOURCES_C := $(HTML_RESOURCES_GZ:.html.gz=.html.c) +HTML_RESOURCES_OBJS := $(HTML_RESOURCES_C:.c=.o) + +CSS_RESOURCES := $(SRC_PATH)/fftools/resources/graph.css \ + +# .css => (sh) .css.min => (gzip) .css.min.gz => (bin2c) .css.c => (cc) .o +CSS_RESOURCES_MIN := $(CSS_RESOURCES:.css=.css.min) +CSS_RESOURCES_GZ := $(CSS_RESOURCES_MIN:.css.min=.css.min.gz) +CSS_RESOURCES_C := $(CSS_RESOURCES_GZ:.css.min.gz=.css.c) +CSS_RESOURCES_OBJS := $(CSS_RESOURCES_C:.c=.o) + +# Uncomment to prevent deletion +#.PRECIOUS: %.css.c %.css.min %.css.gz %.css.min.gz %.html.gz %.html.c + +OBJS-resman += \ +fftools/resources/resman.o \ +$(HTML_RESOURCES_OBJS) \ +$(CSS_RESOURCES_OBJS) \ + diff --git a/fftools/resources/graph.css b/fftools/resources/graph.css new file mode 100644 index 00..ab480673ab --- /dev/null +++ b/fftools/resources/graph.css @@ -0,0 +1,353 @@ +/* Variables */ +.root { +--ff-colvideo: #6eaa7b; +--ff-colaudio:
[FFmpeg-devel] [PATCH v8 15/15] fftools/graphprint: Now, make it a Killer-Feature!
From: softworkz remember this: -sg <= show-graph Signed-off-by: softworkz --- doc/ffmpeg.texi | 4 + fftools/Makefile | 1 + fftools/ffmpeg.c | 2 +- fftools/ffmpeg.h | 1 + fftools/ffmpeg_filter.c | 2 +- fftools/ffmpeg_opt.c | 4 + fftools/graph/filelauncher.c | 205 +++ fftools/graph/graphprint.c | 48 +++- fftools/graph/graphprint.h | 32 ++ 9 files changed, 296 insertions(+), 3 deletions(-) create mode 100644 fftools/graph/filelauncher.c diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index 35675b5309..6e9e7aed0e 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -1404,6 +1404,10 @@ Writes execution graph details to the specified file in the format set via -prin Sets the output format (available formats are: default, compact, csv, flat, ini, json, xml, mermaid, mermaidhtml) The default format is json. +@item -sg (@emph{global}) +Writes the execution graph to a temporary html file (mermaidhtml format) and +tries to launch it in the default browser. + @item -progress @var{url} (@emph{global}) Send program-friendly progress information to @var{url}. diff --git a/fftools/Makefile b/fftools/Makefile index 361a4fd574..56a2910212 100644 --- a/fftools/Makefile +++ b/fftools/Makefile @@ -22,6 +22,7 @@ OBJS-ffmpeg += \ fftools/ffmpeg_opt.o\ fftools/ffmpeg_sched.o \ fftools/graph/graphprint.o\ +fftools/graph/filelauncher.o \ fftools/sync_queue.o\ fftools/thread_queue.o \ fftools/textformat/avtextformat.o \ diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 6766ec209c..9875a1f7fd 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -309,7 +309,7 @@ const AVIOInterruptCB int_cb = { decode_interrupt_cb, NULL }; static void ffmpeg_cleanup(int ret) { -if (print_graphs || print_graphs_file) +if (print_graphs || print_graphs_file || show_graph) print_filtergraphs(filtergraphs, nb_filtergraphs, input_files, nb_input_files, output_files, nb_output_files); if (do_benchmark) { diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index 7fbf0ad532..49fea0307d 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -721,6 +721,7 @@ extern int print_graphs; extern char *print_graphs_file; extern char *print_graphs_format; extern int auto_conversion_filters; +extern int show_graph; extern const AVIOInterruptCB int_cb; diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index b774606562..e82e333b7f 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -2985,7 +2985,7 @@ read_frames: finish: -if (print_graphs || print_graphs_file) +if (print_graphs || print_graphs_file || show_graph) print_filtergraph(fg, fgt.graph); // EOF is normal termination diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index 3d1efe32f9..24713d640f 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -79,6 +79,7 @@ int vstats_version = 2; int print_graphs = 0; char *print_graphs_file = NULL; char *print_graphs_format = NULL; +int show_graph = 0; int auto_conversion_filters = 1; int64_t stats_period = 50; @@ -1748,6 +1749,9 @@ const OptionDef options[] = { { "print_graphs_format", OPT_TYPE_STRING, 0, { &print_graphs_format }, "set the output printing format (available formats are: default, compact, csv, flat, ini, json, xml, mermaid, mermaidhtml)", "format" }, +{ "sg", OPT_TYPE_BOOL, 0, +{ &show_graph }, +"create execution graph as temporary html file and try to launch it in the default browser" }, { "auto_conversion_filters", OPT_TYPE_BOOL, OPT_EXPERT, { &auto_conversion_filters }, "enable automatic conversion filters globally" }, diff --git a/fftools/graph/filelauncher.c b/fftools/graph/filelauncher.c new file mode 100644 index 00..0cf5f15cf1 --- /dev/null +++ b/fftools/graph/filelauncher.c @@ -0,0 +1,205 @@ +/* + * Copyright (c) 2025 - softworkz + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include +#include +#include + +#if defined(_WIN32) +# include +# include +#else +# include +# include +#
[FFmpeg-devel] [PATCH v8 11/15] avfilter/avfilter: Add avfilter_link_get_hw_frames_ctx()
From: softworkz Signed-off-by: softworkz --- doc/APIchanges | 3 +++ libavfilter/avfilter.c | 9 + libavfilter/avfilter.h | 12 3 files changed, 24 insertions(+) diff --git a/doc/APIchanges b/doc/APIchanges index 75d66f87f3..d0869561f3 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2025-03-28 API changes, most recent first: +2025-02-xx - xx - lavfi 10.10.100 - avfilter.h + Add avfilter_link_get_hw_frames_ctx(). + 2025-04-21 - xx - lavu 60.2.100 - log.h Add AV_CLASS_CATEGORY_HWDEVICE. diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c index 64c1075c40..c76d43a215 100644 --- a/libavfilter/avfilter.c +++ b/libavfilter/avfilter.c @@ -989,6 +989,15 @@ enum AVMediaType avfilter_pad_get_type(const AVFilterPad *pads, int pad_idx) return pads[pad_idx].type; } +AVBufferRef *avfilter_link_get_hw_frames_ctx(AVFilterLink *link) +{ +FilterLink *plink = ff_filter_link(link); +if (plink->hw_frames_ctx) +return av_buffer_ref(plink->hw_frames_ctx); + +return NULL; +} + static int default_filter_frame(AVFilterLink *link, AVFrame *frame) { return ff_filter_frame(link->dst->outputs[0], frame); diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h index a89d3cf658..f85929dc5c 100644 --- a/libavfilter/avfilter.h +++ b/libavfilter/avfilter.h @@ -96,6 +96,18 @@ const char *avfilter_pad_get_name(const AVFilterPad *pads, int pad_idx); */ enum AVMediaType avfilter_pad_get_type(const AVFilterPad *pads, int pad_idx); +/** + * Get the hardware frames context of a filter link. + * + * @param link an AVFilterLink + * + * @return a ref-counted copy of the link's hw_frames_ctx field if there is + * a hardware frames context associated with the link or NULL otherwise. + * The returned AVBufferRef needs to be released with av_buffer_unref() + * when it is no longer used. + */ +AVBufferRef* avfilter_link_get_hw_frames_ctx(AVFilterLink *link); + /** * Lists of formats / etc. supported by an end of a link. * -- ffmpeg-codebot ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v8 13/15] fftools/ffmpeg_mux: Make ms_from_ost() inline
From: softworkz Signed-off-by: softworkz --- fftools/ffmpeg_mux.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fftools/ffmpeg_mux.h b/fftools/ffmpeg_mux.h index f41f2c18fa..4ca8ab73a4 100644 --- a/fftools/ffmpeg_mux.h +++ b/fftools/ffmpeg_mux.h @@ -123,7 +123,7 @@ typedef struct Muxer { int mux_check_init(void *arg); -static MuxStream *ms_from_ost(OutputStream *ost) +static inline MuxStream *ms_from_ost(OutputStream *ost) { return (MuxStream*)ost; } -- ffmpeg-codebot ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v8 00/15] Execution Graph Printing
Shortest cover letter for my longest-running FFmpeg patchset: * Apply * Build * Add the "-sg" switch to any FFmpeg command line * Press 'q' when you don't want to wait SG = Show Graph Documentation and examples can be found here: https://github.com/softworkz/ffmpeg_output_apis/wiki Version Updates === V2 == * Rebased on top of Andreas' improvements * Applied changes from review (thanks, Andreas) V3 == * Fixed all "new warnings" * Fixed out-of-tree building (thanks, Michael) V4 == * Resolved merge conflict * Fixed build on MinGW (missing include due to WIN32_LEAN_AND_MEAN being defined) (thanks, Michael) V5 == * Applied changes as per review from Stefano (thanks!) * Introduced AVTextFormatOptions struct for options in avtext_context_open() V6 == * Fix "new warning" in 2nd last commit * Squash patches 04 and 05 (they weren't truely independent) * Applied changes as per review from Stefano (thanks!) V7 == * Bitten by OOT builds once again (thanks, Michael) V8 == * New commit Remove void (*print_rational) from AVTextFormatter (unused) * New commit fftools/textformat: Rename name param to key for API consistency * print_int Extend existing function instead of adding print_int_flags * Fix registered_formatters[] array size * avtextwriters.h: Remove unused includes * graphprint.c: Make BPrint inits consistent * tf_json: Check nesting level for value printing * And other review suggestions by Stefano (thanks!) . softworkz (15): fftools/textformat: Formatting and whitespace changes fftools/textformat: Apply quality improvements fftools/textformat: Remove unused print_rational() pointer from AVTextFormatter fftools/textformat: Rename name param to key for API consistency fftools/avtextformat: Re-use BPrint in loop fftools/textformat: Introduce AVTextFormatOptions for avtext_context_open() fftools/textformat: Introduce common header and deduplicate code fftools/tf_internal: Use av_default_item_name fftools/textformat: Add flags param to function avtext_print_integer() fftools/ffmpeg_filter: Move some declaration to new header file avfilter/avfilter: Add avfilter_link_get_hw_frames_ctx() fftools/resources: Add resource manager files fftools/ffmpeg_mux: Make ms_from_ost() inline fftools/graphprint: Add execution graph printing fftools/graphprint: Now, make it a Killer-Feature! doc/APIchanges |3 + doc/ffmpeg.texi| 14 + ffbuild/common.mak | 28 +- fftools/Makefile | 22 +- fftools/ffmpeg.c |4 + fftools/ffmpeg.h |4 + fftools/ffmpeg_filter.c| 195 + fftools/ffmpeg_filter.h| 234 ++ fftools/ffmpeg_mux.h |2 +- fftools/ffmpeg_opt.c | 17 + fftools/ffprobe.c | 15 +- fftools/graph/filelauncher.c | 205 + fftools/graph/graphprint.c | 1148 fftools/graph/graphprint.h | 62 ++ fftools/resources/.gitignore |4 + fftools/resources/Makefile | 27 + fftools/resources/graph.css| 353 + fftools/resources/graph.html | 86 +++ fftools/resources/resman.c | 213 ++ fftools/resources/resman.h | 50 ++ fftools/textformat/avtextformat.c | 242 +++--- fftools/textformat/avtextformat.h | 78 +- fftools/textformat/avtextwriters.h | 16 +- fftools/textformat/tf_compact.c| 121 +-- fftools/textformat/tf_default.c| 55 +- fftools/textformat/tf_flat.c | 51 +- fftools/textformat/tf_ini.c| 62 +- fftools/textformat/tf_internal.h | 81 ++ fftools/textformat/tf_json.c | 64 +- fftools/textformat/tf_mermaid.c| 658 fftools/textformat/tf_mermaid.h| 41 + fftools/textformat/tf_xml.c| 68 +- fftools/textformat/tw_avio.c | 18 +- fftools/textformat/tw_buffer.c |9 +- fftools/textformat/tw_stdout.c | 10 +- libavfilter/avfilter.c |9 + libavfilter/avfilter.h | 12 + 37 files changed, 3701 insertions(+), 580 deletions(-) create mode 100644 fftools/ffmpeg_filter.h create mode 100644 fftools/graph/filelauncher.c create mode 100644 fftools/graph/graphprint.c create mode 100644 fftools/graph/graphprint.h create mode 100644 fftools/resources/.gitignore create mode 100644 fftools/resources/Makefile create mode 100644 fftools/resources/graph.css create mode 100644 fftools/resources/graph.html create mode 100644 fftools/resources/resman.c create mode 100644 fftools/resources/resman.h create mode 100644 fftools/textformat/tf_internal.h create mode 100644 fftools/textformat/tf_mermaid.c create mode 100644 fftools/textformat/tf_mermaid.h base-commit: 25b0a8e295749a60a238ba0d6fe7a3742937b6bb Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-66
[FFmpeg-devel] [PATCH v8 07/15] fftools/textformat: Introduce common header and deduplicate code
From: softworkz Also change writer_printf signature in AVTextWriter to use va_list, so that it can be called by the new function writer_printf() in tf_internal.h. Reviewed-by: Stefano Sabatini Signed-off-by: softworkz --- fftools/textformat/avtextwriters.h | 2 +- fftools/textformat/tf_compact.c| 32 --- fftools/textformat/tf_default.c| 27 +++--- fftools/textformat/tf_flat.c | 25 +++-- fftools/textformat/tf_ini.c| 24 +++-- fftools/textformat/tf_internal.h | 85 ++ fftools/textformat/tf_json.c | 43 +++ fftools/textformat/tf_xml.c| 35 +--- fftools/textformat/tw_avio.c | 9 ++-- fftools/textformat/tw_buffer.c | 9 ++-- fftools/textformat/tw_stdout.c | 10 ++-- 11 files changed, 160 insertions(+), 141 deletions(-) create mode 100644 fftools/textformat/tf_internal.h diff --git a/fftools/textformat/avtextwriters.h b/fftools/textformat/avtextwriters.h index fd5cda04ef..9297babeb1 100644 --- a/fftools/textformat/avtextwriters.h +++ b/fftools/textformat/avtextwriters.h @@ -36,7 +36,7 @@ typedef struct AVTextWriter { void (*uninit)(AVTextWriterContext *wctx); void (*writer_w8)(AVTextWriterContext *wctx, int b); void (*writer_put_str)(AVTextWriterContext *wctx, const char *str); -void (*writer_printf)(AVTextWriterContext *wctx, const char *fmt, ...); +void (*writer_vprintf)(AVTextWriterContext *wctx, const char *fmt, va_list vl); } AVTextWriter; typedef struct AVTextWriterContext { diff --git a/fftools/textformat/tf_compact.c b/fftools/textformat/tf_compact.c index d4ac296a42..e52888239e 100644 --- a/fftools/textformat/tf_compact.c +++ b/fftools/textformat/tf_compact.c @@ -28,23 +28,7 @@ #include "libavutil/bprint.h" #include "libavutil/error.h" #include "libavutil/opt.h" - - -#define writer_w8(wctx_, b_) (wctx_)->writer->writer->writer_w8((wctx_)->writer, b_) -#define writer_put_str(wctx_, str_) (wctx_)->writer->writer->writer_put_str((wctx_)->writer, str_) -#define writer_printf(wctx_, fmt_, ...) (wctx_)->writer->writer->writer_printf((wctx_)->writer, fmt_, __VA_ARGS__) - - -#define DEFINE_FORMATTER_CLASS(name) \ -static const char *name##_get_name(void *ctx) \ -{ \ -return #name ; \ -} \ -static const AVClass name##_class = { \ -.class_name = #name,\ -.item_name = name##_get_name, \ -.option = name##_options\ -} +#include "tf_internal.h" /* Compact output */ @@ -157,9 +141,12 @@ static av_cold int compact_init(AVTextFormatContext *wctx) static void compact_print_section_header(AVTextFormatContext *wctx, const void *data) { CompactContext *compact = wctx->priv; -const struct AVTextFormatSection *section = wctx->section[wctx->level]; -const struct AVTextFormatSection *parent_section = wctx->level ? -wctx->section[wctx->level-1] : NULL; +const AVTextFormatSection *section = tf_get_section(wctx, wctx->level); +const AVTextFormatSection *parent_section = tf_get_parent_section(wctx, wctx->level); + +if (!section) +return; + compact->terminate_line[wctx->level] = 1; compact->has_nested_elems[wctx->level] = 0; @@ -210,8 +197,11 @@ static void compact_print_section_header(AVTextFormatContext *wctx, const void * static void compact_print_section_footer(AVTextFormatContext *wctx) { -const struct AVTextFormatSection *section = wctx->section[wctx->level]; CompactContext *compact = wctx->priv; +const AVTextFormatSection *section = tf_get_section(wctx, wctx->level); + +if (!section) +return; if (!compact->nested_section[wctx->level] && compact->terminate_line[wctx->level] && diff --git a/fftools/textformat/tf_default.c b/fftools/textformat/tf_default.c index ad97173b0b..019bda9d44 100644 --- a/fftools/textformat/tf_default.c +++ b/fftools/textformat/tf_default.c @@ -27,21 +27,7 @@ #include "avtextformat.h" #include "libavutil/bprint.h" #include "libavutil/opt.h" - -#define writer_w8(wctx_, b_) (wctx_)->writer->writer->writer_w8((wctx_)->writer, b_) -#define writer_put_str(wctx_, str_) (wctx_)->writer->writer->writer_put_str((wctx_)->writer, str_) -#define writer_printf(wctx_, fmt_, ...) (wctx_)->writer->writer->writer_printf((wctx_)->writer, fmt_, __VA_ARGS__) - -#define DEFINE_FORMATTER_CLASS(name) \ -static const char *name##_get_name(void *ctx) \ -{ \ -return #name ; \ -} \ -static const AVClass name##_class = { \ -.class_name = #name,\ -.item_name = name##_get_name, \
[FFmpeg-devel] [PATCH v8 10/15] fftools/ffmpeg_filter: Move some declaration to new header file
From: softworkz to allow filtergraph printing to access the information. Signed-off-by: softworkz --- fftools/ffmpeg_filter.c | 190 +--- fftools/ffmpeg_filter.h | 234 2 files changed, 235 insertions(+), 189 deletions(-) create mode 100644 fftools/ffmpeg_filter.h diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index d314aec206..eab9487f97 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -21,6 +21,7 @@ #include #include "ffmpeg.h" +#include "ffmpeg_filter.h" #include "libavfilter/avfilter.h" #include "libavfilter/buffersink.h" @@ -42,44 +43,6 @@ // FIXME private header, used for mid_pred() #include "libavcodec/mathops.h" -typedef struct FilterGraphPriv { -FilterGraph fg; - -// name used for logging -char log_name[32]; - -int is_simple; -// true when the filtergraph contains only meta filters -// that do not modify the frame data -int is_meta; -// source filters are present in the graph -int have_sources; -int disable_conversions; - -unsigned nb_outputs_done; - -const char *graph_desc; - -int nb_threads; - -// frame for temporarily holding output from the filtergraph -AVFrame *frame; -// frame for sending output to the encoder -AVFrame *frame_enc; - -Scheduler *sch; -unsigned sch_idx; -} FilterGraphPriv; - -static FilterGraphPriv *fgp_from_fg(FilterGraph *fg) -{ -return (FilterGraphPriv*)fg; -} - -static const FilterGraphPriv *cfgp_from_cfg(const FilterGraph *fg) -{ -return (const FilterGraphPriv*)fg; -} // data that is local to the filter thread and not visible outside of it typedef struct FilterGraphThread { @@ -102,157 +65,6 @@ typedef struct FilterGraphThread { uint8_t *eof_out; } FilterGraphThread; -typedef struct InputFilterPriv { -InputFilter ifilter; - -InputFilterOptions opts; - -int index; - -AVFilterContext*filter; - -// used to hold submitted input -AVFrame*frame; - -/* for filters that are not yet bound to an input stream, - * this stores the input linklabel, if any */ -uint8_t*linklabel; - -// filter data type -enum AVMediaTypetype; -// source data type: AVMEDIA_TYPE_SUBTITLE for sub2video, -// same as type otherwise -enum AVMediaTypetype_src; - -int eof; -int bound; -int drop_warned; -uint64_tnb_dropped; - -// parameters configured for this input -int format; - -int width, height; -AVRational sample_aspect_ratio; -enum AVColorSpace color_space; -enum AVColorRange color_range; - -int sample_rate; -AVChannelLayout ch_layout; - -AVRational time_base; - -AVFrameSideData **side_data; -int nb_side_data; - -AVFifo *frame_queue; - -AVBufferRef*hw_frames_ctx; - -int displaymatrix_present; -int displaymatrix_applied; -int32_t displaymatrix[9]; - -int downmixinfo_present; -AVDownmixInfo downmixinfo; - -struct { -AVFrame *frame; - -int64_t last_pts; -int64_t end_pts; - -/// marks if sub2video_update should force an initialization -unsigned int initialize; -} sub2video; -} InputFilterPriv; - -static InputFilterPriv *ifp_from_ifilter(InputFilter *ifilter) -{ -return (InputFilterPriv*)ifilter; -} - -typedef struct FPSConvContext { -AVFrame *last_frame; -/* number of frames emitted by the video-encoding sync code */ -int64_t frame_number; -/* history of nb_frames_prev, i.e. the number of times the - * previous frame was duplicated by vsync code in recent - * do_video_out() calls */ -int64_t frames_prev_hist[3]; - -uint64_t dup_warning; - -int last_dropped; -int dropped_keyframe; - -enum VideoSyncMethod vsync_method; - -AVRationalframerate; -AVRationalframerate_max; -const AVRational *framerate_supported; -int framerate_clip; -} FPSConvContext; - -typedef struct OutputFilterPriv { -OutputFilterofilter; - -int index; - -void *log_parent; -charlog_name[32]; - -char *name; - -AVFilterContext*filter; - -/* desired output stream properties */ -int format; -int width, height; -int sample_rate; -AVChannelLayout ch_layout; -
[FFmpeg-devel] [PATCH v8 09/15] fftools/textformat: Add flags param to function avtext_print_integer()
From: softworkz Make this function work analog to avtext_print_string() which already has a flags parameter. Signed-off-by: softworkz --- fftools/ffprobe.c | 2 +- fftools/textformat/avtextformat.c | 24 ++-- fftools/textformat/avtextformat.h | 2 +- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index 1277b1e4f9..444810cf43 100644 --- a/fftools/ffprobe.c +++ b/fftools/ffprobe.c @@ -422,7 +422,7 @@ static void log_callback(void *ptr, int level, const char *fmt, va_list vl) avtext_print_string(tfc, k, pbuf.str, 0); \ } while (0) -#define print_int(k, v) avtext_print_integer(tfc, k, v) +#define print_int(k, v) avtext_print_integer(tfc, k, v, 0) #define print_q(k, v, s)avtext_print_rational(tfc, k, v, s) #define print_str(k, v) avtext_print_string(tfc, k, v, 0) #define print_str_opt(k, v) avtext_print_string(tfc, k, v, AV_TEXTFORMAT_PRINT_STRING_OPTIONAL) diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c index 91469ef576..5b0f92d5cb 100644 --- a/fftools/textformat/avtextformat.c +++ b/fftools/textformat/avtextformat.c @@ -290,10 +290,20 @@ void avtext_print_section_footer(AVTextFormatContext *tctx) tctx->level--; } -void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t val) +void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t val, int flags) { const AVTextFormatSection *section; +av_assert0(tctx); + +if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER) +return; + +if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO +&& (flags & AV_TEXTFORMAT_PRINT_STRING_OPTIONAL) +&& !(tctx->formatter->flags & AV_TEXTFORMAT_FLAG_SUPPORTS_OPTIONAL_FIELDS)) +return; + av_assert0(key && tctx->level >= 0 && tctx->level < SECTION_MAX_NB_LEVELS); section = tctx->section[tctx->level]; @@ -445,10 +455,12 @@ int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char * section = tctx->section[tctx->level]; -if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER || -(tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO -&& (flags & AV_TEXTFORMAT_PRINT_STRING_OPTIONAL) -&& !(tctx->formatter->flags & AV_TEXTFORMAT_FLAG_SUPPORTS_OPTIONAL_FIELDS))) +if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER) +return 0; + +if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO +&& (flags & AV_TEXTFORMAT_PRINT_STRING_OPTIONAL) +&& !(tctx->formatter->flags & AV_TEXTFORMAT_FLAG_SUPPORTS_OPTIONAL_FIELDS)) return 0; if (section->show_all_entries || av_dict_get(section->entries_to_show, key, NULL, 0)) { @@ -504,7 +516,7 @@ void avtext_print_ts(AVTextFormatContext *tctx, const char *key, int64_t ts, int if ((!is_duration && ts == AV_NOPTS_VALUE) || (is_duration && ts == 0)) avtext_print_string(tctx, key, "N/A", AV_TEXTFORMAT_PRINT_STRING_OPTIONAL); else -avtext_print_integer(tctx, key, ts); +avtext_print_integer(tctx, key, ts, 0); } void avtext_print_data(AVTextFormatContext *tctx, const char *key, diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h index 87f57d8c24..8f406de322 100644 --- a/fftools/textformat/avtextformat.h +++ b/fftools/textformat/avtextformat.h @@ -138,7 +138,7 @@ void avtext_print_section_header(AVTextFormatContext *tctx, const void *data, in void avtext_print_section_footer(AVTextFormatContext *tctx); -void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t val); +void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t val, int flags); int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char *val, int flags); -- ffmpeg-codebot ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v8 06/15] fftools/textformat: Introduce AVTextFormatOptions for avtext_context_open()
From: softworkz This allows future addition of options without changes to the signature of avtext_context_open(). Reviewed-by: Stefano Sabatini Signed-off-by: softworkz --- fftools/ffprobe.c | 13 + fftools/textformat/avtextformat.c | 21 - fftools/textformat/avtextformat.h | 16 +--- 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index f5c83925b9..1277b1e4f9 100644 --- a/fftools/ffprobe.c +++ b/fftools/ffprobe.c @@ -3168,10 +3168,15 @@ int main(int argc, char **argv) if (ret < 0) goto end; -if ((ret = avtext_context_open(&tctx, f, wctx, f_args, - sections, FF_ARRAY_ELEMS(sections), show_value_unit, -use_value_prefix, use_byte_value_binary_prefix, use_value_sexagesimal_format, -show_optional_fields, show_data_hash)) >= 0) { +AVTextFormatOptions tf_options = { +.show_optional_fields = show_optional_fields, +.show_value_unit = show_value_unit, +.use_value_prefix = use_value_prefix, +.use_byte_value_binary_prefix = use_byte_value_binary_prefix, +.use_value_sexagesimal_format = use_value_sexagesimal_format, +}; + +if ((ret = avtext_context_open(&tctx, f, wctx, f_args, sections, FF_ARRAY_ELEMS(sections), tf_options, show_data_hash)) >= 0) { if (f == &avtextformatter_xml) tctx->string_validation_utf8_flags |= AV_UTF8_FLAG_EXCLUDE_XML_INVALID_CONTROL_CODES; diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c index b2c3aa3fc7..91469ef576 100644 --- a/fftools/textformat/avtextformat.c +++ b/fftools/textformat/avtextformat.c @@ -125,13 +125,7 @@ void avtext_context_close(AVTextFormatContext **ptctx) int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *formatter, AVTextWriterContext *writer_context, const char *args, -const AVTextFormatSection *sections, int nb_sections, -int show_value_unit, -int use_value_prefix, -int use_byte_value_binary_prefix, -int use_value_sexagesimal_format, -int show_optional_fields, -char *show_data_hash) +const AVTextFormatSection *sections, int nb_sections, AVTextFormatOptions options, char *show_data_hash) { AVTextFormatContext *tctx; int i, ret = 0; @@ -154,11 +148,11 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form goto fail; } -tctx->show_value_unit = show_value_unit; -tctx->use_value_prefix = use_value_prefix; -tctx->use_byte_value_binary_prefix = use_byte_value_binary_prefix; -tctx->use_value_sexagesimal_format = use_value_sexagesimal_format; -tctx->show_optional_fields = show_optional_fields; +tctx->show_value_unit = options.show_value_unit; +tctx->use_value_prefix = options.use_value_prefix; +tctx->use_byte_value_binary_prefix = options.use_byte_value_binary_prefix; +tctx->use_value_sexagesimal_format = options.use_value_sexagesimal_format; +tctx->show_optional_fields = options.show_optional_fields; if (nb_sections > SECTION_MAX_NB_SECTIONS) { av_log(tctx, AV_LOG_ERROR, "The number of section definitions (%d) is larger than the maximum allowed (%d)\n", nb_sections, SECTION_MAX_NB_SECTIONS); @@ -201,7 +195,7 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form av_dict_free(&opts); } -if (show_data_hash) +if (show_data_hash) { if ((ret = av_hash_alloc(&tctx->hash, show_data_hash)) < 0) { if (ret == AVERROR(EINVAL)) { const char *n; @@ -212,6 +206,7 @@ int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *form } goto fail; } +} /* validate replace string */ { diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h index 8ff503401a..87f57d8c24 100644 --- a/fftools/textformat/avtextformat.h +++ b/fftools/textformat/avtextformat.h @@ -117,17 +117,19 @@ struct AVTextFormatContext { unsigned int string_validation_utf8_flags; }; +typedef struct AVTextFormatOptions { +int show_optional_fields; +int show_value_unit; +int use_value_prefix; +int use_byte_value_binary_prefix; +int use_value_sexagesimal_format; +} AVTextFormatOptions; + #define AV_TEXTFORMAT_PRINT_STRING_OPTIONAL 1 #define AV_TEXTFORMAT_PRINT_STRING_VALIDATE 2 int avtext_context_open(AVTextFormatContext **ptctx, const AVTextFormatter *formatter, AVTextWriterContext *writer_context, const char *args, -const AVTextFormatSection *sections, int nb_sections, -int show_value_unit, -
[FFmpeg-devel] [PATCH v8 08/15] fftools/tf_internal: Use av_default_item_name
From: softworkz Reviewed-by: Stefano Sabatini Signed-off-by: softworkz --- fftools/textformat/tf_internal.h | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/fftools/textformat/tf_internal.h b/fftools/textformat/tf_internal.h index 362a4cbc38..484886b7a7 100644 --- a/fftools/textformat/tf_internal.h +++ b/fftools/textformat/tf_internal.h @@ -29,13 +29,9 @@ #include "avtextformat.h" #define DEFINE_FORMATTER_CLASS(name)\ -static const char *name##_get_name(void *ctx) \ -{ \ -return #name ; \ -} \ static const AVClass name##_class = { \ .class_name = #name,\ -.item_name = name##_get_name, \ +.item_name = av_default_item_name, \ .option = name##_options\ } -- ffmpeg-codebot ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5] Mark C globals with small code model
Ping. On Tue, Apr 15, 2025 at 4:22 PM Pranav Kant wrote: > Hello again. Is there anything else I can do here? > > On Fri, Apr 4, 2025 at 11:40 AM Pranav Kant wrote: > >> Any thoughts on this? >> >> On Thu, Mar 20, 2025 at 5:30 PM Pranav Kant wrote: >> >>> Patch version v5: >>> - Uses two new macros DECLARE_ASM_VAR (used for both external and inline >>> asm) and DECLARE_EXTERNAL_ASM_VAR (used only for external asm) >>> - I intend to remove explicit existing use of >>> attribute_visibility_hidden in follow-up patch and instead use >>> DECLARE_EXTERNAL_ASM_VAR for those variables >>> - Other variables will be marked with these two new macros in a >>> follow-up patch. I want to settle down on the infrastructure first with a >>> handful of variables. >>> >>> Let me know if you have any questions. >>> >>> On Thu, Mar 20, 2025 at 5:28 PM Pranav Kant wrote: >>> By default, all globals in C/C++ compiled by clang are allocated in non-large data sections. See [1] for background on code models. For PIC (Position independent code), this is fine as long as binary is small but as binary size increases, users maybe want to use medium/large code models (-mcmodel=medium) which moves data in to large sections. As data in these large sections cannot be accessed using PIC code anymore (as it may be too far away), compiler ends up using a different instruction sequence when building C/C++ code -- using GOT to access these globals (which can be relaxed by linker at link time if binary ends up being smaller). However, external assembly and inline assembly continue to access these globals using older PC-relative addressing which may not work because globals may be placed too far away. Introduce new macros for such variables that mark them with small code model attribute. This ensures that these variables are never allocated in large data sections, and continue to be validly accessed from assembly code. This patch should not have any affect on builds that use small code model, which is the default mode. [1] https://eli.thegreenplace.net/2012/01/03/understanding-the-x64-code-models Signed-off-by: Pranav Kant --- libavcodec/ac3dsp.h | 4 +++- libavcodec/cabac.h | 4 +++- libavcodec/x86/constants.h | 12 +++- libavutil/attributes.h | 6 ++ libavutil/attributes_internal.h | 16 libavutil/mem_internal.h| 13 + 6 files changed, 48 insertions(+), 7 deletions(-) diff --git a/libavcodec/ac3dsp.h b/libavcodec/ac3dsp.h index b1b2bced8f..a3c55a833b 100644 --- a/libavcodec/ac3dsp.h +++ b/libavcodec/ac3dsp.h @@ -25,11 +25,13 @@ #include #include +#include "libavutil/mem_internal.h" + /** * Number of mantissa bits written for each bap value. * bap values with fractional bits are set to 0 and are calculated separately. */ -extern const uint16_t ff_ac3_bap_bits[16]; +extern DECLARE_EXTERNAL_ASM_VAR(16, const uint16_t, ff_ac3_bap_bits)[16]; typedef struct AC3DSPContext { /** diff --git a/libavcodec/cabac.h b/libavcodec/cabac.h index 38d06b2842..df352258c6 100644 --- a/libavcodec/cabac.h +++ b/libavcodec/cabac.h @@ -29,7 +29,9 @@ #include -extern const uint8_t ff_h264_cabac_tables[512 + 4*2*64 + 4*64 + 63]; +#include "libavutil/mem_internal.h" + +extern DECLARE_ASM_VAR(1, const uint8_t, ff_h264_cabac_tables)[512 + 4*2*64 + 4*64 + 63]; #define H264_NORM_SHIFT_OFFSET 0 #define H264_LPS_RANGE_OFFSET 512 #define H264_MLPS_STATE_OFFSET 1024 diff --git a/libavcodec/x86/constants.h b/libavcodec/x86/constants.h index 0c6bf41fa0..2561302604 100644 --- a/libavcodec/x86/constants.h +++ b/libavcodec/x86/constants.h @@ -23,13 +23,14 @@ #include +#include "libavutil/mem_internal.h" #include "libavutil/x86/asm.h" -extern const ymm_reg ff_pw_1; +extern DECLARE_EXTERNAL_ASM_VAR(32, const ymm_reg, ff_pw_1); extern const ymm_reg ff_pw_2; extern const xmm_reg ff_pw_3; -extern const ymm_reg ff_pw_4; -extern const xmm_reg ff_pw_5; +extern DECLARE_ASM_VAR(32, const ymm_reg, ff_pw_4); +extern DECLARE_ASM_VAR(16, const xmm_reg, ff_pw_5); extern const xmm_reg ff_pw_8; extern const xmm_reg ff_pw_9; extern const uint64_t ff_pw_15; @@ -43,7 +44,7 @@ extern const uint64_t ff_pw_128; extern const ymm_reg ff_pw_255; extern const ymm_reg ff_pw_256; extern const ymm_reg ff_pw_512; -extern const ymm_reg ff_pw_1023; +extern DECLARE_EXTERNAL_ASM_VAR(32, const ymm_reg, ff_pw_1023); extern const ymm_reg ff_pw_1024; extern const ymm_reg ff_pw_20
[FFmpeg-devel] [PATCH] avfilter/vf_lut3d_opencl Initial support for OpenCL implementation of vf_lut3d.
--- libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/opencl/lut3d.cl | 177 ++ libavfilter/opencl_source.h | 2 + libavfilter/vf_lut3d_opencl.c | 444 ++ 5 files changed, 625 insertions(+) create mode 100644 libavfilter/opencl/lut3d.cl create mode 100644 libavfilter/vf_lut3d_opencl.c diff --git a/libavfilter/Makefile b/libavfilter/Makefile index 7c0d879ec9..6524d0f91a 100644 --- a/libavfilter/Makefile +++ b/libavfilter/Makefile @@ -378,6 +378,7 @@ OBJS-$(CONFIG_LUT1D_FILTER) += vf_lut3d.o OBJS-$(CONFIG_LUT_FILTER)+= vf_lut.o OBJS-$(CONFIG_LUT2_FILTER) += vf_lut2.o framesync.o OBJS-$(CONFIG_LUT3D_FILTER) += vf_lut3d.o framesync.o +OBJS-$(CONFIG_LUT3D_OPENCL_FILTER) += vf_lut3d_opencl.o opencl.o opencl/lut3d.o OBJS-$(CONFIG_LUTRGB_FILTER) += vf_lut.o OBJS-$(CONFIG_LUTYUV_FILTER) += vf_lut.o OBJS-$(CONFIG_MASKEDCLAMP_FILTER)+= vf_maskedclamp.o framesync.o diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c index 740d9ab265..72c2f48ac4 100644 --- a/libavfilter/allfilters.c +++ b/libavfilter/allfilters.c @@ -353,6 +353,7 @@ extern const FFFilter ff_vf_lut; extern const FFFilter ff_vf_lut1d; extern const FFFilter ff_vf_lut2; extern const FFFilter ff_vf_lut3d; +extern const FFFilter ff_vf_lut3d_opencl; extern const FFFilter ff_vf_lutrgb; extern const FFFilter ff_vf_lutyuv; extern const FFFilter ff_vf_maskedclamp; diff --git a/libavfilter/opencl/lut3d.cl b/libavfilter/opencl/lut3d.cl new file mode 100644 index 00..16dfecdc4e --- /dev/null +++ b/libavfilter/opencl/lut3d.cl @@ -0,0 +1,177 @@ +/* + * Copyright (c) 2025 Jan Studeny + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +typedef struct rgbvec { +float r, g, b, a; +} rgbvec; + +#define MIN(X, Y) (((X) < (Y)) ? (X) : (Y)) + +#define NEAR(x) ((int)((x) + .5)) +#define PREV(x) ((int)(x)) +#define NEXT(x) (MIN((int)(x) + 1, lut_edge_size - 1)) + +/** + * Get the nearest defined point + */ +static rgbvec interp_nearest(float4 px, __global const rgbvec *lut, int lut_edge_size) +{ +int r = NEAR(px[0]); +int g = NEAR(px[1]); +int b = NEAR(px[2]); +int index = r * lut_edge_size * lut_edge_size + g * lut_edge_size + b; +return lut[index]; +} + +static float lerpf(float v0, float v1, float f) +{ +return v0 + (v1 - v0) * f; +} + +static rgbvec lerp(const rgbvec *v0, const rgbvec *v1, float f) +{ +rgbvec v = { +lerpf(v0->r, v1->r, f), lerpf(v0->g, v1->g, f), lerpf(v0->b, v1->b, f) +}; +return v; +} +/** + * Interpolate using the 8 vertices of a cube + * @see https://en.wikipedia.org/wiki/Trilinear_interpolation + */ +static rgbvec interp_trilinear(float4 px, __global const rgbvec *lut, int lut_edge_size) +{ +const int lutsize2 = lut_edge_size * lut_edge_size; +const int lutsize = lut_edge_size; + +const int prev[] = { PREV(px[0]), PREV(px[1]), PREV(px[2]) }; +const int next[] = { NEXT(px[0]), NEXT(px[1]), NEXT(px[2]) }; + +const rgbvec d = { +px[0] - prev[0], +px[1] - prev[1], +px[2] - prev[2] +}; + +const rgbvec c000 = lut[prev[0] * lutsize2 + prev[1] * lutsize + prev[2]]; +const rgbvec c001 = lut[prev[0] * lutsize2 + prev[1] * lutsize + next[2]]; +const rgbvec c010 = lut[prev[0] * lutsize2 + next[1] * lutsize + prev[2]]; +const rgbvec c011 = lut[prev[0] * lutsize2 + next[1] * lutsize + next[2]]; +const rgbvec c100 = lut[next[0] * lutsize2 + prev[1] * lutsize + prev[2]]; +const rgbvec c101 = lut[next[0] * lutsize2 + prev[1] * lutsize + next[2]]; +const rgbvec c110 = lut[next[0] * lutsize2 + next[1] * lutsize + prev[2]]; +const rgbvec c111 = lut[next[0] * lutsize2 + next[1] * lutsize + next[2]]; + +const rgbvec c00 = lerp(&c000, &c100, d.r); +const rgbvec c10 = lerp(&c010, &c110, d.r); +const rgbvec c01 = lerp(&c001, &c101, d.r); +const rgbvec c11 = lerp(&c011, &c111, d.r); + +const rgbvec c0 = lerp(&c00, &c10, d.g); +const rgbvec c1 = lerp(&c01, &c11, d.g); + +return lerp(&c0, &c1, d.b); +} + +/** + * Tetrahedral interpolation. Based
Re: [FFmpeg-devel] [PATCH] avformat/mov: Fix decoding fragmented MP4 with multiple sample entries and empty stsc
On 19 Apr 2025, at 16:27, Dimitry Andric wrote: > > On 10 Apr 2025, at 11:03, Dimitry Andric > wrote: >> >> On 3 Apr 2025, at 22:02, Dimitry Andric >> wrote: >>> >>> When decoding fragmented MP4 files that have an empty stsc box, and >>> instead contain sample description indexes in their tfhd boxes, the mov >>> demuxer does not notify the decoder whenever the current sample >>> description index changes. If the SPS or PPS changed sufficiently, this >>> can lead to unexpected decoding errors. >>> >>> To fix this, in mov_finalize_packet(), when stsc_data is not available, >>> use get_frag_stream_info_from_pkt() to get at the current fragment >>> stream info, and retrieve the current sample description index from >>> there. Then use that index in a similar manner as the stsc case. >>> >>> Signed-off-by: Dimitry Andric >>> --- >>> libavformat/mov.c | 50 --- >>> 1 file changed, 30 insertions(+), 20 deletions(-) >>> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index 452690090c..ead89192f4 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -10756,25 +10756,29 @@ static int mov_switch_root(AVFormatContext *s, >>> int64_t target, int index) >>> return 1; >>> } >>> >>> -static int mov_change_extradata(AVStream *st, AVPacket *pkt) >>> +static int mov_change_extradata(AVStream *st, AVPacket *pkt, int stsd_id) >>> { >>> MOVStreamContext *sc = st->priv_data; >>> uint8_t *side, *extradata; >>> int extradata_size; >>> >>> -/* Save the current index. */ >>> -sc->last_stsd_index = sc->stsc_data[sc->stsc_index].id - 1; >>> +if (stsd_id > 0 && >>> +stsd_id - 1 < sc->stsd_count && >>> +stsd_id - 1 != sc->last_stsd_index) { >>> +/* Save the current index. */ >>> +sc->last_stsd_index = stsd_id - 1; >>> >>> -/* Notify the decoder that extradata changed. */ >>> -extradata_size = sc->extradata_size[sc->last_stsd_index]; >>> -extradata = sc->extradata[sc->last_stsd_index]; >>> -if (st->discard != AVDISCARD_ALL && extradata_size > 0 && extradata) { >>> -side = av_packet_new_side_data(pkt, >>> - AV_PKT_DATA_NEW_EXTRADATA, >>> - extradata_size); >>> -if (!side) >>> -return AVERROR(ENOMEM); >>> -memcpy(side, extradata, extradata_size); >>> +/* Notify the decoder that extradata changed. */ >>> +extradata_size = sc->extradata_size[sc->last_stsd_index]; >>> +extradata = sc->extradata[sc->last_stsd_index]; >>> +if (st->discard != AVDISCARD_ALL && extradata_size > 0 && >>> extradata) { >>> +side = av_packet_new_side_data(pkt, >>> + AV_PKT_DATA_NEW_EXTRADATA, >>> + extradata_size); >>> +if (!side) >>> +return AVERROR(ENOMEM); >>> +memcpy(side, extradata, extradata_size); >>> +} >>> } >>> >>> return 0; >>> @@ -10893,13 +10897,10 @@ static int mov_finalize_packet(AVFormatContext >>> *s, AVStream *st, AVIndexEntry *s >>> >>> /* Multiple stsd handling. */ >>> if (sc->stsc_data) { >>> -if (sc->stsc_data[sc->stsc_index].id > 0 && >>> -sc->stsc_data[sc->stsc_index].id - 1 < sc->stsd_count && >>> -sc->stsc_data[sc->stsc_index].id - 1 != sc->last_stsd_index) { >>> -int ret = mov_change_extradata(st, pkt); >>> -if (ret < 0) >>> -return ret; >>> -} >>> +int stsd_id = sc->stsc_data[sc->stsc_index].id; >>> +int ret = mov_change_extradata(st, pkt, stsd_id); >>> +if (ret < 0) >>> +return ret; >>> >>> /* Update the stsc index for the next sample */ >>> sc->stsc_sample++; >>> @@ -10908,6 +10909,15 @@ static int mov_finalize_packet(AVFormatContext *s, >>> AVStream *st, AVIndexEntry *s >>> sc->stsc_index++; >>> sc->stsc_sample = 0; >>> } >>> +} else { >>> +MOVContext *mov = s->priv_data; >>> +MOVFragmentStreamInfo *frag_stream_info = >>> get_frag_stream_info_from_pkt(&mov->frag_index, pkt, sc->id); >>> +if (frag_stream_info) { >>> +int stsd_id = frag_stream_info->stsd_id; >>> +int ret = mov_change_extradata(st, pkt, stsd_id); >>> +if (ret < 0) >>> +return ret; >>> +} >>> } >>> >>> return 0; >>> -- >>> 2.43.0 >>> >> >> Any comments on this patch? > > Ping :) Is there any particular group of persons that "own" the mov muxer? -Dimitry ___ 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] lavc/vvc: Detect subpic overlaps at CTU level
On 28/04/2025 14:33, Nuo Mi wrote: > Hi Frank, > Thank you for the v2. > Could we remove all asserts? > Asserts can cause the application to crash at runtime. Hi, I think av_assert2s are the right thing to use here. In case it was not clear, these asserts should never be triggered by any bitstream (legal or illegal). The alternatives, as I see it, are both less favourable: * Don't check the return value at all. If the assumption above that pps_add_ctus shouldn't fail in these cases is incorrect, then all of a sudden there is a rather unscrutable error arising from subtracting a value from off, which might be rather difficult to debug. An assertion is better because it makes the issue obvious by crashing, and immediately points to the location in the code which is problematic. * Add a runtime check for these cases. If the assumption above is correct, then we're incurring needless runtime penalty checking for things which are always true. An av_assert2 is better because it is only enabled in debug builds, and not where performance is essential. > > On Sun, Apr 27, 2025 at 4:48 PM Frank Plowman wrote: > >> In d5dbcc00d889fb17948b025a468b00ddbea9e058, it was hoped that detection >> of subpicture overlaps could be performed at the tile level, so as to >> avoid introducing per-CTU checks. Unfortunately since that patch, >> fuzzing has indicated there are some structures involving >> pps_subpic_one_or_more_tiles_slice where tile-level checking is not >> sufficient. Performing the check at the CTU level should (touch wood) >> be the be-all and and-all of this, as CTUs are the lowest common >> denominator of the picture partitioning. >> >> Signed-off-by: Frank Plowman >> --- >> Changes since v1: >> * Merge pps_add_ctus and pps_add_ctus_check >> * Change if/else for early-exit where possible >> >> --- >> libavcodec/vvc/ps.c | 71 - >> 1 file changed, 31 insertions(+), 40 deletions(-) >> >> diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c >> index e8c312d8ac..ed96268bae 100644 >> --- a/libavcodec/vvc/ps.c >> +++ b/libavcodec/vvc/ps.c >> @@ -408,6 +408,8 @@ static int pps_add_ctus(VVCPPS *pps, int *off, const >> int rx, const int ry, >> int start = *off; >> for (int y = 0; y < h; y++) { >> for (int x = 0; x < w; x++) { >> +if (*off >= pps->ctb_count) >> +return AVERROR_INVALIDDATA; >> pps->ctb_addr_in_slice[*off] = ctu_rs(rx + x, ry + y, pps); >> (*off)++; >> } >> @@ -420,9 +422,11 @@ static void pps_single_slice_picture(VVCPPS *pps, int >> *off) >> pps->num_ctus_in_slice[0] = 0; >> for (int j = 0; j < pps->r->num_tile_rows; j++) { >> for (int i = 0; i < pps->r->num_tile_columns; i++) { >> -pps->num_ctus_in_slice[0] += pps_add_ctus(pps, off, >> +const int ret = pps_add_ctus(pps, off, >> pps->col_bd[i], pps->row_bd[j], >> pps->r->col_width_val[i], pps->r->row_height_val[j]); >> +av_assert2(ret >= 0); >> +pps->num_ctus_in_slice[0] += ret; >> } >> } >> } >> @@ -451,50 +455,36 @@ static void subpic_tiles(int *tile_x, int *tile_y, >> int *tile_x_end, int *tile_y_ >> (*tile_y_end)++; >> } >> >> -static bool mark_tile_as_used(bool *tile_in_subpic, const int tx, const >> int ty, const int tile_columns) >> -{ >> -const size_t tile_idx = ty * tile_columns + tx; >> -if (tile_in_subpic[tile_idx]) { >> -/* the tile is covered by other subpictures */ >> -return false; >> -} >> -tile_in_subpic[tile_idx] = true; >> -return true; >> -} >> - >> -static int pps_subpic_less_than_one_tile_slice(VVCPPS *pps, const VVCSPS >> *sps, const int i, const int tx, const int ty, int *off, bool >> *tile_in_subpic) >> +static int pps_subpic_less_than_one_tile_slice(VVCPPS *pps, const VVCSPS >> *sps, const int i, const int tx, const int ty, int *off) >> { >> -const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] + >> sps->r->sps_subpic_height_minus1[i]; >> -const int tile_bottom = pps->row_bd[ty] + pps->r->row_height_val[ty] >> - 1; >> -const bool is_final_subpic_in_tile = subpic_bottom == tile_bottom; >> - >> -if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic, tx, >> ty, pps->r->num_tile_columns)) >> -return AVERROR_INVALIDDATA; >> - >> -pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, >> +const int ret = pps_add_ctus(pps, off, >> sps->r->sps_subpic_ctu_top_left_x[i], >> sps->r->sps_subpic_ctu_top_left_y[i], >> sps->r->sps_subpic_width_minus1[i] + 1, >> sps->r->sps_subpic_height_minus1[i] + 1); >> +if (ret < 0) >> +return ret; >> >> +pps->num_ctus_in_slice[i] = ret; >> return 0; >> } >> >> static int pps_subpic_one_or_more_tiles_slice(VVCPPS *pps, const int >> tile_x, const int tile_y, const int x_end, const int y_end, >> -const int i, int *of
Re: [FFmpeg-devel] [PATCH] postproc/tests: Add test tools to .gitignore
On 28 Apr 2025, at 16:02, Andreas Rheinhardt wrote: > Patch attached. > LGTM > - 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 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 v7 02/13] fftools/textformat: Apply quality improvements
On date Friday 2025-04-25 23:30:57 +, softworkz wrote: > From: softworkz > > Perform multiple improvements to increase code robustness. > In particular: > - favor unsigned counters for loops > - add missing checks > - avoid possible leaks > - move variable declarations to inner scopes when feasible > - provide explicit type-casting when needed > > Signed-off-by: softworkz > --- > fftools/textformat/avtextformat.c | 85 --- > fftools/textformat/avtextformat.h | 6 +-- > fftools/textformat/tf_default.c | 8 ++- > fftools/textformat/tf_ini.c | 2 +- > fftools/textformat/tf_json.c | 17 --- > fftools/textformat/tf_xml.c | 3 -- > fftools/textformat/tw_avio.c | 11 +++- > 7 files changed, 83 insertions(+), 49 deletions(-) > > diff --git a/fftools/textformat/avtextformat.c > b/fftools/textformat/avtextformat.c > index 74d179c516..1939a1f739 100644 > --- a/fftools/textformat/avtextformat.c > +++ b/fftools/textformat/avtextformat.c > @@ -93,9 +93,8 @@ static const AVClass textcontext_class = { > > static void bprint_bytes(AVBPrint *bp, const uint8_t *ubuf, size_t ubuf_size) > { > -int i; > av_bprintf(bp, "0X"); > -for (i = 0; i < ubuf_size; i++) > +for (unsigned i = 0; i < ubuf_size; i++) > av_bprintf(bp, "%02X", ubuf[i]); > } > > @@ -137,6 +136,9 @@ int avtext_context_open(AVTextFormatContext **ptctx, > const AVTextFormatter *form > AVTextFormatContext *tctx; > int i, ret = 0; > > +if (!ptctx || !formatter) > +return AVERROR(EINVAL); assert0? > + > if (!(tctx = av_mallocz(sizeof(AVTextFormatContext { > ret = AVERROR(ENOMEM); > goto fail; > @@ -209,25 +211,26 @@ int avtext_context_open(AVTextFormatContext **ptctx, > const AVTextFormatter *form > av_log(NULL, AV_LOG_ERROR, " %s", n); > av_log(NULL, AV_LOG_ERROR, "\n"); > } > -return ret; > +goto fail; > } > > /* validate replace string */ > { > -const uint8_t *p = tctx->string_validation_replacement; > -const uint8_t *endp = p + strlen(p); > +const uint8_t *p = (uint8_t *)tctx->string_validation_replacement; > +const uint8_t *endp = p + strlen((const char *)p); > while (*p) { > const uint8_t *p0 = p; > int32_t code; > ret = av_utf8_decode(&code, &p, endp, > tctx->string_validation_utf8_flags); > if (ret < 0) { > AVBPrint bp; > -av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); > +av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); > bprint_bytes(&bp, p0, p - p0), > av_log(tctx, AV_LOG_ERROR, > "Invalid UTF8 sequence %s found in string > validation replace '%s'\n", > bp.str, tctx->string_validation_replacement); > -return ret; > +av_bprint_finalize(&bp, NULL); I see no advantage in this, since it's adding dynamic allocation and the need to free which was previously not needed > +goto fail; > } > } > } > @@ -255,6 +258,9 @@ static const char unit_bit_per_second_str[] = "bit/s"; > > void avtext_print_section_header(AVTextFormatContext *tctx, const void > *data, int section_id) > { > +if (section_id < 0 || section_id >= tctx->nb_sections) > +return; Given this is a check on the input data, we should probably make it fail with a log message, meaning we should also check the return value from the callee. Given this is not a public API I'm not against the assert though. [...] > int avtextwriter_create_file(AVTextWriterContext **pwctx, const char > *output_filename) > { > +if (!output_filename || !output_filename[0]) { > +av_log(NULL, AV_LOG_ERROR, "The output_filename cannot be NULL or > empty\n"); > +return AVERROR(EINVAL); > +} > + > IOWriterContext *ctx; > int ret; nit here and below: move declarations before actual code for stylistic consistency [...] ___ 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 v7 02/13] fftools/textformat: Apply quality improvements
> -Original Message- > From: Stefano Sabatini > Sent: Montag, 28. April 2025 22:24 > To: softworkz . > Cc: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply quality > improvements > > On date Monday 2025-04-28 20:05:25 +, softworkz . wrote: > > > > > > > -Original Message- > > > From: Stefano Sabatini > > > Sent: Montag, 28. April 2025 21:56 > > > To: FFmpeg development discussions and patches > > > Cc: softworkz > > > Subject: Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply > quality > > > improvements > > > > > > On date Friday 2025-04-25 23:30:57 +, softworkz wrote: > > > > From: softworkz > > > > > > > > Perform multiple improvements to increase code robustness. > > > > In particular: > > > > - favor unsigned counters for loops > > > > - add missing checks > > > > - avoid possible leaks > > > > - move variable declarations to inner scopes when feasible > > > > - provide explicit type-casting when needed > > > > > > > > Signed-off-by: softworkz > > > > --- > > > > fftools/textformat/avtextformat.c | 85 --- > > > > fftools/textformat/avtextformat.h | 6 +-- > > > > fftools/textformat/tf_default.c | 8 ++- > > > > fftools/textformat/tf_ini.c | 2 +- > > > > fftools/textformat/tf_json.c | 17 --- > > > > fftools/textformat/tf_xml.c | 3 -- > > > > fftools/textformat/tw_avio.c | 11 +++- > > > > 7 files changed, 83 insertions(+), 49 deletions(-) > > > > > > > > diff --git a/fftools/textformat/avtextformat.c > > > b/fftools/textformat/avtextformat.c > > > > index 74d179c516..1939a1f739 100644 > > > > --- a/fftools/textformat/avtextformat.c > > > > +++ b/fftools/textformat/avtextformat.c > > > > @@ -93,9 +93,8 @@ static const AVClass textcontext_class = { > > > > > > > > static void bprint_bytes(AVBPrint *bp, const uint8_t *ubuf, size_t > > > ubuf_size) > > > > { > > > > -int i; > > > > av_bprintf(bp, "0X"); > > > > -for (i = 0; i < ubuf_size; i++) > > > > +for (unsigned i = 0; i < ubuf_size; i++) > > > > av_bprintf(bp, "%02X", ubuf[i]); > > > > } > > > > > > > > @@ -137,6 +136,9 @@ int avtext_context_open(AVTextFormatContext **ptctx, > > > const AVTextFormatter *form > > > > AVTextFormatContext *tctx; > > > > int i, ret = 0; > > > > > > > > > > > +if (!ptctx || !formatter) > > > > +return AVERROR(EINVAL); > > > > > > assert0? > > > > OK. > > > > > > > > > > > + > > > > if (!(tctx = av_mallocz(sizeof(AVTextFormatContext { > > > > ret = AVERROR(ENOMEM); > > > > goto fail; > > > > @@ -209,25 +211,26 @@ int avtext_context_open(AVTextFormatContext > **ptctx, > > > const AVTextFormatter *form > > > > av_log(NULL, AV_LOG_ERROR, " %s", n); > > > > av_log(NULL, AV_LOG_ERROR, "\n"); > > > > } > > > > -return ret; > > > > +goto fail; > > > > } > > > > > > > > /* validate replace string */ > > > > { > > > > -const uint8_t *p = tctx->string_validation_replacement; > > > > -const uint8_t *endp = p + strlen(p); > > > > +const uint8_t *p = (uint8_t *)tctx- > >string_validation_replacement; > > > > +const uint8_t *endp = p + strlen((const char *)p); > > > > while (*p) { > > > > const uint8_t *p0 = p; > > > > int32_t code; > > > > ret = av_utf8_decode(&code, &p, endp, tctx- > > > >string_validation_utf8_flags); > > > > if (ret < 0) { > > > > > > > AVBPrint bp; > > > > -av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); > > > > +av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); > > > > bprint_bytes(&bp, p0, p - p0), > > > > av_log(tctx, AV_LOG_ERROR, > > > > "Invalid UTF8 sequence %s found in string > > > validation replace '%s'\n", > > > > bp.str, tctx- > >string_validation_replacement); > > > > -return ret; > > > > +av_bprint_finalize(&bp, NULL); > > > > > > > I see no advantage in this, since it's adding dynamic allocation and > > > the need to free which was previously not needed > > > > The dynamic allocation does not happen until the buffer content size > > does not fit anymore into the stack-alloced memory. > > Yes, but is this ever happening? Are we sure that it's not? Because unless we are 100% sure, we need to check the return value and when that check indicates an overflow of the buffer - what then? How to remedy? The check for the return value alone - even without any remediation - is more complicated code (more lines) than a single line with av_bprint_finalize(). That's my general rationale behind favoring _UNLIMITED. There are more cases where I have that, and where it's really relevant but for this o
Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply quality improvements
> -Original Message- > From: Stefano Sabatini > Sent: Montag, 28. April 2025 21:56 > To: FFmpeg development discussions and patches > Cc: softworkz > Subject: Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply quality > improvements > > On date Friday 2025-04-25 23:30:57 +, softworkz wrote: > > From: softworkz > > > > Perform multiple improvements to increase code robustness. > > In particular: > > - favor unsigned counters for loops > > - add missing checks > > - avoid possible leaks > > - move variable declarations to inner scopes when feasible > > - provide explicit type-casting when needed > > > > Signed-off-by: softworkz > > --- > > fftools/textformat/avtextformat.c | 85 --- > > fftools/textformat/avtextformat.h | 6 +-- > > fftools/textformat/tf_default.c | 8 ++- > > fftools/textformat/tf_ini.c | 2 +- > > fftools/textformat/tf_json.c | 17 --- > > fftools/textformat/tf_xml.c | 3 -- > > fftools/textformat/tw_avio.c | 11 +++- > > 7 files changed, 83 insertions(+), 49 deletions(-) > > > > diff --git a/fftools/textformat/avtextformat.c > b/fftools/textformat/avtextformat.c > > index 74d179c516..1939a1f739 100644 > > --- a/fftools/textformat/avtextformat.c > > +++ b/fftools/textformat/avtextformat.c > > @@ -93,9 +93,8 @@ static const AVClass textcontext_class = { > > > > static void bprint_bytes(AVBPrint *bp, const uint8_t *ubuf, size_t > ubuf_size) > > { > > -int i; > > av_bprintf(bp, "0X"); > > -for (i = 0; i < ubuf_size; i++) > > +for (unsigned i = 0; i < ubuf_size; i++) > > av_bprintf(bp, "%02X", ubuf[i]); > > } > > > > @@ -137,6 +136,9 @@ int avtext_context_open(AVTextFormatContext **ptctx, > const AVTextFormatter *form > > AVTextFormatContext *tctx; > > int i, ret = 0; > > > > > +if (!ptctx || !formatter) > > +return AVERROR(EINVAL); > > assert0? OK. > > > + > > if (!(tctx = av_mallocz(sizeof(AVTextFormatContext { > > ret = AVERROR(ENOMEM); > > goto fail; > > @@ -209,25 +211,26 @@ int avtext_context_open(AVTextFormatContext **ptctx, > const AVTextFormatter *form > > av_log(NULL, AV_LOG_ERROR, " %s", n); > > av_log(NULL, AV_LOG_ERROR, "\n"); > > } > > -return ret; > > +goto fail; > > } > > > > /* validate replace string */ > > { > > -const uint8_t *p = tctx->string_validation_replacement; > > -const uint8_t *endp = p + strlen(p); > > +const uint8_t *p = (uint8_t *)tctx->string_validation_replacement; > > +const uint8_t *endp = p + strlen((const char *)p); > > while (*p) { > > const uint8_t *p0 = p; > > int32_t code; > > ret = av_utf8_decode(&code, &p, endp, tctx- > >string_validation_utf8_flags); > > if (ret < 0) { > > > AVBPrint bp; > > -av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); > > +av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); > > bprint_bytes(&bp, p0, p - p0), > > av_log(tctx, AV_LOG_ERROR, > > "Invalid UTF8 sequence %s found in string > validation replace '%s'\n", > > bp.str, tctx->string_validation_replacement); > > -return ret; > > +av_bprint_finalize(&bp, NULL); > > I see no advantage in this, since it's adding dynamic allocation and > the need to free which was previously not needed The dynamic allocation does not happen until the buffer content size does not fit anymore into the stack-alloced memory. > > > +goto fail; > > } > > } > > } > > @@ -255,6 +258,9 @@ static const char unit_bit_per_second_str[] = "bit/s"; > > > > > void avtext_print_section_header(AVTextFormatContext *tctx, const void > *data, int section_id) > > { > > +if (section_id < 0 || section_id >= tctx->nb_sections) > > +return; > > Given this is a check on the input data, we should probably make it > fail with a log message, meaning we should also check the return value > from the callee. > > Given this is not a public API I'm not against the assert though. Let's discuss this in the other thread (Shaping...). Did you see my recent reply? Thanks sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply quality improvements
On date Monday 2025-04-28 20:05:25 +, softworkz . wrote: > > > > -Original Message- > > From: Stefano Sabatini > > Sent: Montag, 28. April 2025 21:56 > > To: FFmpeg development discussions and patches > > Cc: softworkz > > Subject: Re: [FFmpeg-devel] [PATCH v7 02/13] fftools/textformat: Apply > > quality > > improvements > > > > On date Friday 2025-04-25 23:30:57 +, softworkz wrote: > > > From: softworkz > > > > > > Perform multiple improvements to increase code robustness. > > > In particular: > > > - favor unsigned counters for loops > > > - add missing checks > > > - avoid possible leaks > > > - move variable declarations to inner scopes when feasible > > > - provide explicit type-casting when needed > > > > > > Signed-off-by: softworkz > > > --- > > > fftools/textformat/avtextformat.c | 85 --- > > > fftools/textformat/avtextformat.h | 6 +-- > > > fftools/textformat/tf_default.c | 8 ++- > > > fftools/textformat/tf_ini.c | 2 +- > > > fftools/textformat/tf_json.c | 17 --- > > > fftools/textformat/tf_xml.c | 3 -- > > > fftools/textformat/tw_avio.c | 11 +++- > > > 7 files changed, 83 insertions(+), 49 deletions(-) > > > > > > diff --git a/fftools/textformat/avtextformat.c > > b/fftools/textformat/avtextformat.c > > > index 74d179c516..1939a1f739 100644 > > > --- a/fftools/textformat/avtextformat.c > > > +++ b/fftools/textformat/avtextformat.c > > > @@ -93,9 +93,8 @@ static const AVClass textcontext_class = { > > > > > > static void bprint_bytes(AVBPrint *bp, const uint8_t *ubuf, size_t > > ubuf_size) > > > { > > > -int i; > > > av_bprintf(bp, "0X"); > > > -for (i = 0; i < ubuf_size; i++) > > > +for (unsigned i = 0; i < ubuf_size; i++) > > > av_bprintf(bp, "%02X", ubuf[i]); > > > } > > > > > > @@ -137,6 +136,9 @@ int avtext_context_open(AVTextFormatContext **ptctx, > > const AVTextFormatter *form > > > AVTextFormatContext *tctx; > > > int i, ret = 0; > > > > > > > > +if (!ptctx || !formatter) > > > +return AVERROR(EINVAL); > > > > assert0? > > OK. > > > > > > > + > > > if (!(tctx = av_mallocz(sizeof(AVTextFormatContext { > > > ret = AVERROR(ENOMEM); > > > goto fail; > > > @@ -209,25 +211,26 @@ int avtext_context_open(AVTextFormatContext **ptctx, > > const AVTextFormatter *form > > > av_log(NULL, AV_LOG_ERROR, " %s", n); > > > av_log(NULL, AV_LOG_ERROR, "\n"); > > > } > > > -return ret; > > > +goto fail; > > > } > > > > > > /* validate replace string */ > > > { > > > -const uint8_t *p = tctx->string_validation_replacement; > > > -const uint8_t *endp = p + strlen(p); > > > +const uint8_t *p = (uint8_t > > > *)tctx->string_validation_replacement; > > > +const uint8_t *endp = p + strlen((const char *)p); > > > while (*p) { > > > const uint8_t *p0 = p; > > > int32_t code; > > > ret = av_utf8_decode(&code, &p, endp, tctx- > > >string_validation_utf8_flags); > > > if (ret < 0) { > > > > > AVBPrint bp; > > > -av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC); > > > +av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED); > > > bprint_bytes(&bp, p0, p - p0), > > > av_log(tctx, AV_LOG_ERROR, > > > "Invalid UTF8 sequence %s found in string > > validation replace '%s'\n", > > > bp.str, tctx->string_validation_replacement); > > > -return ret; > > > +av_bprint_finalize(&bp, NULL); > > > > I see no advantage in this, since it's adding dynamic allocation and > > the need to free which was previously not needed > > The dynamic allocation does not happen until the buffer content size > does not fit anymore into the stack-alloced memory. Yes, but is this ever happening? Since in this case the size of the buffer is guaranteed to be short (since it's the max return size of the UTF8 decode function). It won't hurt, but it won't help either so I'd rather keep as is and fix the indent weirdness (the , in place of ; for instance) and the ret. > > > > > +goto fail; > > > } > > > } > > > } > > > @@ -255,6 +258,9 @@ static const char unit_bit_per_second_str[] = "bit/s"; > > > > > > > > void avtext_print_section_header(AVTextFormatContext *tctx, const void > > *data, int section_id) > > > { > > > +if (section_id < 0 || section_id >= tctx->nb_sections) > > > +return; > > > > Given this is a check on the input data, we should probably make it > > fail with a log message, meaning we should also check the return value > > from the callee. > > > > Given this is not a public API I'm not against the assert though. > > Let's discuss this in the other thr