[FFmpeg-devel] [PATCH] libavcodec/x86/vvc/vvc_sad: fix assembler error
From: Haihao Xiang X86ASMlibavcodec/x86/vvc/vvc_sad.o libavcodec/x86/vvc/vvc_sad.asm:85: error: invalid number of operands libavcodec/x86/vvc/vvc_sad.asm:87: error: invalid number of operands Signed-off-by: Haihao Xiang --- libavcodec/x86/vvc/vvc_sad.asm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavcodec/x86/vvc/vvc_sad.asm b/libavcodec/x86/vvc/vvc_sad.asm index 13224583a5..b468d89ac2 100644 --- a/libavcodec/x86/vvc/vvc_sad.asm +++ b/libavcodec/x86/vvc/vvc_sad.asm @@ -82,9 +82,9 @@ cglobal vvc_sad, 6, 9, 5, src1, src2, dx, dy, block_w, block_h, off1, off2, row_ vvc_sad_8: .loop_height: movu xm0, [src1q] -vinserti128m0, [src1q + MAX_PB_SIZE * ROWS * 2], 1 +vinserti128m0, m0, [src1q + MAX_PB_SIZE * ROWS * 2], 1 movu xm1, [src2q] -vinserti128m1, [src2q + MAX_PB_SIZE * ROWS * 2], 1 +vinserti128m1, m1, [src2q + MAX_PB_SIZE * ROWS * 2], 1 MIN_MAX_SADm1, m0, m2 pmaddwdm1, m4 -- 2.34.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/5] replace scale2ref by scale=rw:rh
Quoting Niklas Haas (2024-04-24 12:51:55) > Incidentally, I think it would be nice if lavfi could *automatically* > split filter links referenced multiple times, so we could just write > e.g. [i][ri]scale=rw:rh[o]; [ri][o]overlay[out] and have it work. Maybe > I'll try getting that to work, next. While I agree that it would be nice, using duplicate link labels currently does not break, and I've seen multiple people using those. So we'll probably need to deprecate this behaviour first. -- Anton Khirnov ___ 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] avcodec/dovi - disable metadata compression by default
From: Cosmin Stejerean
not all clients support metadata compression, output when
vdr_dm_metadata_changed fails the DV verifier.
---
libavcodec/dovi_rpu.h| 10 ++
libavcodec/dovi_rpuenc.c | 8 ++--
libavcodec/libaomenc.c | 3 +--
libavcodec/libsvtav1.c | 3 +--
libavcodec/libx265.c | 3 +--
5 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h
index 8ce0c88e9d..dee34165c9 100644
--- a/libavcodec/dovi_rpu.h
+++ b/libavcodec/dovi_rpu.h
@@ -28,6 +28,11 @@
#include "libavutil/frame.h"
#include "avcodec.h"
+#define DOVI_ENCODING_OPTS \
+{ "dolbyvision", "Enable Dolby Vision RPU coding", OFFSET(dovi.enable),
AV_OPT_TYPE_BOOL, {.i64 = FF_DOVI_AUTOMATIC }, -1, 1, VE, .unit = "dovi" }, \
+{ "auto", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_DOVI_AUTOMATIC}, .flags
= VE, .unit = "dovi" }, \
+{ "dv_enable_compression", "Enable Dolby Vision metadata compression",
OFFSET(dovi.enable_compression), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, VE },
+
#define DOVI_MAX_DM_ID 15
typedef struct DOVIContext {
void *logctx;
@@ -71,6 +76,11 @@ typedef struct DOVIContext {
AVDOVIDmData *ext_blocks;
int num_ext_blocks;
+/**
+ * Enable metadata compression in the output. Currently this is
experimental.
+ */
+int enable_compression;
+
/**
* Private fields internal to dovi_rpu.c
*/
diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c
index 3c3e0f84c0..26ed25733a 100644
--- a/libavcodec/dovi_rpuenc.c
+++ b/libavcodec/dovi_rpuenc.c
@@ -512,8 +512,12 @@ int ff_dovi_rpu_generate(DOVIContext *s, const
AVDOVIMetadata *metadata,
}
}
-vdr_dm_metadata_changed = !s->color || memcmp(s->color, color,
sizeof(*color));
-use_prev_vdr_rpu = !memcmp(&s->vdr[vdr_rpu_id]->mapping, mapping,
sizeof(*mapping));
+// the output when vdr_dm_metadata_changed is 0 fails the DV verifier
+// force it to 1 until we can get some samples or documentation on correct
syntax
+vdr_dm_metadata_changed = 1; // !s->color || memcmp(s->color, color,
sizeof(*color));
+
+// not all clients support metadata compression
+use_prev_vdr_rpu = s->enable_compression &&
!memcmp(&s->vdr[vdr_rpu_id]->mapping, mapping, sizeof(*mapping));
buffer_size = 12 /* vdr seq info */ + 5 /* CRC32 + terminator */;
buffer_size += num_ext_blocks_v1 * 13;
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index dec74ebecd..3837eaaec2 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -1487,8 +1487,7 @@ static const AVOption options[] = {
{ "ssim",NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
AOM_TUNE_SSIM}, 0, 0, VE, .unit = "tune"},
FF_AV1_PROFILE_OPTS
{ "still-picture", "Encode in single frame mode (typically used for still
AVIF images).", OFFSET(still_picture), AV_OPT_TYPE_BOOL, {.i64 = 0}, -1, 1, VE
},
-{ "dolbyvision", "Enable Dolby Vision RPU coding",
OFFSET(dovi.enable), AV_OPT_TYPE_BOOL, {.i64 = FF_DOVI_AUTOMATIC }, -1, 1, VE,
.unit = "dovi" },
-{ "auto", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_DOVI_AUTOMATIC}, .flags
= VE, .unit = "dovi" },
+DOVI_ENCODING_OPTS
{ "enable-rect-partitions", "Enable rectangular partitions",
OFFSET(enable_rect_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
{ "enable-1to4-partitions", "Enable 1:4/4:1 partitions",
OFFSET(enable_1to4_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
{ "enable-ab-partitions", "Enable ab shape partitions",
OFFSET(enable_ab_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
index 2fef8c8971..d747c31824 100644
--- a/libavcodec/libsvtav1.c
+++ b/libavcodec/libsvtav1.c
@@ -731,8 +731,7 @@ static const AVOption options[] = {
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 63, VE },
{ "svtav1-params", "Set the SVT-AV1 configuration using a :-separated list
of key=value parameters", OFFSET(svtav1_opts), AV_OPT_TYPE_DICT, { 0 }, 0, 0,
VE },
-{ "dolbyvision", "Enable Dolby Vision RPU coding", OFFSET(dovi.enable),
AV_OPT_TYPE_BOOL, {.i64 = FF_DOVI_AUTOMATIC }, -1, 1, VE, .unit = "dovi" },
-{ "auto", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_DOVI_AUTOMATIC}, .flags
= VE, .unit = "dovi" },
+DOVI_ENCODING_OPTS
{NULL},
};
diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index ac1dbc4f97..30a0b3d205 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -951,8 +951,7 @@ static const AVOption options[] = {
{ "a53cc", "Use A53 Closed Captions (if available)",
OFFSET(a53_cc),AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1,
VE },
{ "x265-params", "set the x265 configuration using a :-separated list of
key=value parameters", OFFSET(x265_opts), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE },
#if X265_BUILD >= 167
-{ "dolbyvision", "Enable Dolby Vision RPU coding", OFFSE
Re: [FFmpeg-devel] [PATCH] avcodec/dovi - disable metadata compression by default
> On May 21, 2024, at 3:19 AM, Niklas Haas wrote:
>
> On Tue, 21 May 2024 04:03:43 + Cosmin Stejerean via ffmpeg-devel
> wrote:
>>
>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
>> index dec74ebecd..c6104f5522 100644
>> --- a/libavcodec/libaomenc.c
>> +++ b/libavcodec/libaomenc.c
>> @@ -1489,6 +1489,7 @@ static const AVOption options[] = {
>> { "still-picture", "Encode in single frame mode (typically used for
>> still AVIF images).", OFFSET(still_picture), AV_OPT_TYPE_BOOL, {.i64 = 0},
>> -1, 1, VE },
>> { "dolbyvision", "Enable Dolby Vision RPU coding",
>> OFFSET(dovi.enable), AV_OPT_TYPE_BOOL, {.i64 = FF_DOVI_AUTOMATIC }, -1, 1,
>> VE, .unit = "dovi" },
>> { "auto", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_DOVI_AUTOMATIC},
>> .flags = VE, .unit = "dovi" },
>> +{ "dv_enable_compression", "Enable Dolby Vision metadata compression",
>> OFFSET(dovi.enable_compression), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, VE },
>> { "enable-rect-partitions", "Enable rectangular partitions",
>> OFFSET(enable_rect_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
>> { "enable-1to4-partitions", "Enable 1:4/4:1 partitions",
>> OFFSET(enable_1to4_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
>> { "enable-ab-partitions", "Enable ab shape partitions",
>> OFFSET(enable_ab_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
>> diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
>> index 2fef8c8971..86bb6686dd 100644
>> --- a/libavcodec/libsvtav1.c
>> +++ b/libavcodec/libsvtav1.c
>> @@ -733,6 +733,7 @@ static const AVOption options[] = {
>>
>> { "dolbyvision", "Enable Dolby Vision RPU coding", OFFSET(dovi.enable),
>> AV_OPT_TYPE_BOOL, {.i64 = FF_DOVI_AUTOMATIC }, -1, 1, VE, .unit = "dovi" },
>> { "auto", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_DOVI_AUTOMATIC},
>> .flags = VE, .unit = "dovi" },
>> +{ "dv_enable_compression", "Enable Dolby Vision metadata compression",
>> OFFSET(dovi.enable_compression), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, VE },
>>
>> {NULL},
>> };
>> diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
>> index ac1dbc4f97..2a79a5e6da 100644
>> --- a/libavcodec/libx265.c
>> +++ b/libavcodec/libx265.c
>> @@ -953,6 +953,7 @@ static const AVOption options[] = {
>> #if X265_BUILD >= 167
>> { "dolbyvision", "Enable Dolby Vision RPU coding", OFFSET(dovi.enable),
>> AV_OPT_TYPE_BOOL, {.i64 = FF_DOVI_AUTOMATIC }, -1, 1, VE, .unit = "dovi" },
>> { "auto", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = FF_DOVI_AUTOMATIC},
>> .flags = VE, .unit = "dovi" },
>> +{ "dv_enable_compression", "Enable Dolby Vision metadata compression",
>> OFFSET(dovi.enable_compression), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, VE },
>> #endif
>
> Setting up an extra AVClass here seems more hassle than it's worth, but
> maybe we could at least hide these options behind a preprocessor
> definition so that multiple files can reference them without blatantly
> duplicating code?
>
Sending a revised v2 that moves these to a shared preprocessor definition.
- Cosmin
___
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] lavc/rv34dsp: optimise R-V V idct_dc_add
Unfortunately I only test to obtain benchmarks and basic correctness. I always feel the need for a professional to write the tests. Rémi Denis-Courmont 于2024年5月23日周四 04:35写道： > > > Le 22 mai 2024 23:28:54 GMT+03:00, "Rémi Denis-Courmont" > a écrit : > >This removes one stray LI and reworks the vector arithmetic to avoid > >changing the vector configuration. On K230, this takes the 46.5 cycle > >count down from 46.5 to 43.5. > >--- > > libavcodec/riscv/rv34dsp_rvv.S | 13 ++--- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > >diff --git a/libavcodec/riscv/rv34dsp_rvv.S > b/libavcodec/riscv/rv34dsp_rvv.S > >index f1f6345012..e8aff7e570 100644 > >--- a/libavcodec/riscv/rv34dsp_rvv.S > >+++ b/libavcodec/riscv/rv34dsp_rvv.S > >@@ -36,16 +36,15 @@ func ff_rv34_idct_dc_add_rvv, zve32x > > vsetivli zero, 4, e8, mf4, ta, ma > > vlse32.v v0, (a0), a1 > > lit1, 169 > >+lit2, 128 > > mul t1, t1, a2 > >-lia2, 255 > >+vsetivli zero, 4*4, e8, m1, ta, ma > >+vwsubu.vx v2, v0, t2 > > addi t1, t1, 512 > > srai t1, t1, 10 > >-vsetivli zero, 4*4, e16, m2, ta, ma > >-vzext.vf2 v2, v0 > >-vadd.vx v2, v2, t1 > >-vmax.vx v2, v2, zero > >-vsetvli zero, zero, e8, m1, ta, ma > >-vnclipu.wiv0, v2, 0 > >+vwadd.wx v2, v2, t1 > > Hmm, this should not work, as t1 has more than 8 bits. Maybe checkasm is > sloppy here. > > >+vnclip.wi v0, v2, 0 > >+vxor.vx v0, v0, t2 > > vsetivli zero, 4, e8, mf4, ta, ma > > vsse32.v v0, (a0), a1 > > > ___ > 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 v12 0/8] [WIP] webp: add support for animated WebP decoding
On Tue, May 21, 2024 at 9:50 AM Thilo Borgmann via ffmpeg-devel wrote: > > Hi, > > [...] > >>> Tests mostly work for me. There are a few images (that I reported > >>> earlier) that give: > >> > >> thanks for testing! > >> > >> > >>> Canvas change detected. The output will be damaged. Use -threads 1 > >>> to try decoding with best effort. > >>> They don't animate without that option and with it render incorrectly. > >> > >> That issue yields from the canvas frame being the synchronization object > >> (ThreadFrame) - doing so prevents the canvas size changed mid-stream. > >> _Maybe_ this can be fixed switching the whole frame multithreading away > >> from ThreadFrame to sth else, not sure though and no experience with the > >> alternatives (AVExecutor?). Maybe Andreas can predict if it's > >> worth/valid to change that whole part of it? I'm not against putting > >> more effort into it to get it right. > > I could fix 488x488.webp and have an almost identical output to libwebp. > > 488x488.webp features an ARGB canvas and has both, ARGB & YUVA420P > p-frames. > > Do you have more files with other variations of canvas & p-frames? If > they at all exist... e.g. canvas YUV and p-frames RGB? > Sent a few created with `gif2webp -mixed` off list. A more exhaustive set can be created using cwebp and webpmux to assemble them. > Pinged Meta as well for real-world samples. Will take some more days > until I get feedback. Will then post the next iteration... > > Thanks, > Thilo ___ 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 00/13] flvdec/flvenc: add support for enhanced rtmp codecs and multitrack/multichannel
Timo Rothenpieler 于2024年5月21日周二 17:03写道： > > This is based on the preliminary spec for enhanced rtmp v2: > https://veovera.org/docs/enhanced/enhanced-rtmp-v2 > > The spec is not final, and can still undergo breaking changes, hence this set > is purely for comments and review, and not ready to be merged until the final > v2 spec is published. > > There are no samples out in the wild yet, so testing interoperability with > other software has not happened yet either. > Specially the two other multitrack modes, where multiple tracks are in the > same packet, have not been tested at all, since no software can write such > files. Yes, there have reconnect function in v2 And i cannot sure there maybe have some problem in multitrack mode, different codecs type, change amount the tracks, maybe need more process, need check GOP、 keyframes 、VFR mode, because they will get different timestamps. > > The set can also be found on GitHub, where ignoring whitespaces makes > specially the last patch a lot more readable: > https://github.com/BtbN/FFmpeg/tree/enhanced-flv > > > Dennis Sädtler via ffmpeg-devel (2): > avformat/flvenc: Implement support for multi-track video > avformat/flvdec: Add support for demuxing multi-track FLV > > Timo Rothenpieler (11): > avformat/flvenc: add enhanced audio codecs > avformat/flvenc: remove !size check for audio packets > avformat/flvdec: add enhanced audio codecs > avformat/flvenc: refactor fourcc writing > avformat/flvenc: write enhanced rtmp multichannel info for audio with > more than two channels > avformat/flvdec: parse enhanced rtmp multichannel info > avformat/flvenc: add support for writing multi track audio > avformat/flvdec: add support for reading multi track audio > avformat/rtmpproto: add more enhanced rtmp codecs > avformat/flvdec: stop shadowing local variables > avformat/flvdec: support all multi-track modes > > libavformat/flv.h | 21 ++ > libavformat/flvdec.c| 654 +++- > libavformat/flvenc.c| 443 +-- > libavformat/rtmpproto.c | 11 +- > 4 files changed, 819 insertions(+), 310 deletions(-) > > -- > 2.43.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". Thanks Steven ___ 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 v3 4/4] tests/checkasm/vvc_alf: add check_alf_classify
On 5/14/2024 8:25 AM, Nuo Mi wrote: On Mon, May 13, 2024 at 8:32 PM wrote: From: Wu Jianhua Perforamnce Test (fps): clip before after delta Tango2_3840x2160_60_10_420_27_LD.266 56 115 105.36% RitualDance_1920x1080_60_10_420_32_LD.266 272 481 76.83% RitualDance_1920x1080_60_10_420_37_RA.266 303 426 40.59% Applied. Thank you. Some tests fails with certain seeds tests/checkasm/checkasm 2325607578 --test=vvc_alf checkasm: using random seed 2325607578 AVX2: vvc_alf_filter_luma_120x20_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x24_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x28_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x32_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x36_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x40_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x44_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x48_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x52_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x56_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x60_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x64_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x68_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x72_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x76_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x80_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x84_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x88_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x92_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x96_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x100_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x104_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x108_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x112_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x116_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x120_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x124_12_avx2 (vvc_alf.c:104) vvc_alf_filter_luma_120x128_12_avx2 (vvc_alf.c:104) - vvc_alf.alf_filter [FAILED] - vvc_alf.alf_classify [OK] checkasm: 28 of 9216 tests have failed ___ 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 7/7] avcodec/tests/bitstream_template: Make it clear that the return is intentionally not checked
Helps: CID1518967 Unchecked return value Helps: CID1518968 Unchecked return value Sponsored-by: Sovereign Tech Fund Signed-off-by: Michael Niedermayer --- libavcodec/tests/bitstream_template.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/tests/bitstream_template.c b/libavcodec/tests/bitstream_template.c index ef59845154d..d8cf980bee1 100644 --- a/libavcodec/tests/bitstream_template.c +++ b/libavcodec/tests/bitstream_template.c @@ -74,7 +74,7 @@ int main(int argc, char **argv) for (unsigned i = 0; i < SIZE; i++) buf[i] = av_lfg_get(&lfg); -bits_init8 (&bc, buf, SIZE); +(void)bits_init8 (&bc, buf, SIZE); init_put_bits(&pb, dst, SIZE); /* use a random sequence of bitreading operations to transfer data -- 2.45.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 6/7] avcodec/sga: Make it clear that the return is intentionally not checked
Related: CID1473496 Unchecked return value
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer
---
libavcodec/sga.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/sga.c b/libavcodec/sga.c
index 0f42cf912b2..618df000ada 100644
--- a/libavcodec/sga.c
+++ b/libavcodec/sga.c
@@ -73,7 +73,7 @@ static int decode_palette(GetByteContext *gb, uint32_t *pal)
return AVERROR_INVALIDDATA;
memset(pal, 0, 16 * sizeof(*pal));
-init_get_bits8(&gbit, gb->buffer, 18);
+(void)init_get_bits8(&gbit, gb->buffer, 18);
for (int RGBIndex = 0; RGBIndex < 3; RGBIndex++) {
for (int index = 0; index < 16; index++) {
--
2.45.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 5/7] avformat/asfdec_f: Use 64bit for preroll computation
Fixes: CID1500342 Unintentional integer overflow Sponsored-by: Sovereign Tech Fund Signed-off-by: Michael Niedermayer --- libavformat/asfdec_f.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index fcc2b98a2c4..2441cadb444 100644 --- a/libavformat/asfdec_f.c +++ b/libavformat/asfdec_f.c @@ -675,7 +675,7 @@ static int asf_read_marker(AVFormatContext *s) avio_rl64(pb); // offset, 8 bytes pres_time = avio_rl64(pb); // presentation time -pres_time = av_sat_sub64(pres_time, asf->hdr.preroll * 1); +pres_time = av_sat_sub64(pres_time, asf->hdr.preroll * 1LL); avio_rl16(pb); // entry length avio_rl32(pb); // send time avio_rl32(pb); // flags -- 2.45.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 4/7] avformat/argo_asf: Use 64bit in offset intermediate
Fixes: CID1467435 Unintentional integer overflow Sponsored-by: Sovereign Tech Fund Signed-off-by: Michael Niedermayer --- libavformat/argo_asf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c index 61bfc6de1fc..e08f029f80c 100644 --- a/libavformat/argo_asf.c +++ b/libavformat/argo_asf.c @@ -259,7 +259,7 @@ static int argo_asf_seek(AVFormatContext *s, int stream_index, return -1; offset = asf->fhdr.chunk_offset + ASF_CHUNK_HEADER_SIZE + - (block * st->codecpar->block_align); + block * (int64_t)st->codecpar->block_align; if ((offset = avio_seek(s->pb, offset, SEEK_SET)) < 0) return offset; -- 2.45.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/7] avformat/ape: Use 64bit for final frame size
Fixes: CID1505963 Unintentional integer overflow
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer
---
libavformat/ape.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/ape.c b/libavformat/ape.c
index c0e3e9f4fe6..f86ca5e894c 100644
--- a/libavformat/ape.c
+++ b/libavformat/ape.c
@@ -292,7 +292,7 @@ static int ape_read_header(AVFormatContext * s)
final_size -= final_size & 3;
}
if (file_size <= 0 || final_size <= 0)
-final_size = ape->finalframeblocks * 8;
+final_size = ape->finalframeblocks * 8LL;
ape->frames[ape->totalframes - 1].size = final_size;
for (i = 0; i < ape->totalframes; i++) {
--
2.45.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/7] avformat/ac4dec: Check remaining space in ac4_probe()
Fixes: CID1538298 Untrusted loop bound Fixes: undefined behavior Sponsored-by: Sovereign Tech Fund Signed-off-by: Michael Niedermayer --- libavformat/ac4dec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavformat/ac4dec.c b/libavformat/ac4dec.c index f647f557ccd..dc6638de3a4 100644 --- a/libavformat/ac4dec.c +++ b/libavformat/ac4dec.c @@ -43,6 +43,8 @@ static int ac4_probe(const AVProbeData *p) size += 4; if (buf[1] == 0x41) size += 2; +if (left < size) +break; max_frames++; left -= size; buf += size; -- 2.45.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/7] avdevice/pulse_audio_enc: Use av_rescale() to avoid integer overflow
Fixes: CID1503075 Unintentional integer overflow
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer
---
libavdevice/pulse_audio_enc.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/libavdevice/pulse_audio_enc.c b/libavdevice/pulse_audio_enc.c
index 3e2cc91f697..80136d1e20c 100644
--- a/libavdevice/pulse_audio_enc.c
+++ b/libavdevice/pulse_audio_enc.c
@@ -471,10 +471,11 @@ static av_cold int pulse_write_header(AVFormatContext *h)
s->nonblocking = (h->flags & AVFMT_FLAG_NONBLOCK);
if (s->buffer_duration) {
-int64_t bytes = s->buffer_duration;
-bytes *= st->codecpar->ch_layout.nb_channels *
st->codecpar->sample_rate *
- av_get_bytes_per_sample(st->codecpar->format);
-bytes /= 1000;
+int64_t bytes = av_rescale(s->buffer_duration,
+ st->codecpar->ch_layout.nb_channels *
+(int64_t)st->codecpar->sample_rate *
+
av_get_bytes_per_sample(st->codecpar->format),
+ 1000);
buffer_attributes.tlength = FFMAX(s->buffer_size, av_clip64(bytes, 0,
UINT32_MAX - 1));
av_log(s, AV_LOG_DEBUG,
"Buffer duration: %ums recalculated into %"PRId64" bytes
buffer.\n",
--
2.45.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting
On Wed, May 22, 2024, 02:59 Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:
> The earlier code distinguished between a partial reset
> (yae_clear()) and a complete reset (yae_release_buffers()
> which also releases the buffers); this separation existed
> to avoid allocations, as buffers were reallocated on reconfigs.
>
> Yet it is pointless since a5704659e3e41b7698812b89f67d9a7467a74d20,
> so simply use yae_release_buffers() everywhere.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavfilter/af_atempo.c | 69 +
> 1 file changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
> index 1aedb82060..110f792eec 100644
> --- a/libavfilter/af_atempo.c
> +++ b/libavfilter/af_atempo.c
> @@ -244,18 +244,6 @@ static void yae_release_buffers(ATempoContext *atempo)
> av_tx_uninit(&atempo->complex_to_real);
> }
>
> -/* av_realloc is not aligned enough; fortunately, the data does not need
> to
> - * be preserved */
> -#define RE_MALLOC_OR_FAIL(field, field_size, element_size) \
> -do {\
> -av_freep(&field); \
> -field = av_calloc(field_size, element_size);\
> -if (!field) { \
> -yae_release_buffers(atempo);\
> -return AVERROR(ENOMEM); \
> -} \
> -} while (0)
> -
> /**
> * Prepare filter for processing audio data of given format,
> * sample rate and number of channels.
> @@ -289,40 +277,51 @@ static int yae_reset(ATempoContext *atempo,
> nlevels++;
> }
>
> +/* av_realloc is not aligned enough, so simply discard all the old
> buffers
> + * (fortunately, their data does not need to be preserved) */
> +yae_release_buffers(atempo);
> +
> // initialize audio fragment buffers:
> -RE_MALLOC_OR_FAIL(atempo->frag[0].data, atempo->window,
> atempo->stride);
> -RE_MALLOC_OR_FAIL(atempo->frag[1].data, atempo->window,
> atempo->stride);
> -RE_MALLOC_OR_FAIL(atempo->frag[0].xdat_in, (atempo->window + 1),
> sizeof(AVComplexFloat));
> -RE_MALLOC_OR_FAIL(atempo->frag[1].xdat_in, (atempo->window + 1),
> sizeof(AVComplexFloat));
> -RE_MALLOC_OR_FAIL(atempo->frag[0].xdat, (atempo->window + 1),
> sizeof(AVComplexFloat));
> -RE_MALLOC_OR_FAIL(atempo->frag[1].xdat, (atempo->window + 1),
> sizeof(AVComplexFloat));
> +if (!(atempo->frag[0].data= av_calloc(atempo->window,
> atempo->stride)) ||
> +!(atempo->frag[1].data= av_calloc(atempo->window,
> atempo->stride)) ||
> +!(atempo->frag[0].xdat_in = av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat))) ||
> +!(atempo->frag[1].xdat_in = av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat))) ||
> +!(atempo->frag[0].xdat= av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat))) ||
> +!(atempo->frag[1].xdat= av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
>
> // initialize rDFT contexts:
> -av_tx_uninit(&atempo->real_to_complex);
> -av_tx_uninit(&atempo->complex_to_real);
> -
> ret = av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn,
> AV_TX_FLOAT_RDFT, 0, 1 << (nlevels + 1), &scale, 0);
> -if (ret < 0) {
> -yae_release_buffers(atempo);
> -return ret;
> -}
> +if (ret < 0)
> +goto fail;
>
> ret = av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn,
> AV_TX_FLOAT_RDFT, 1, 1 << (nlevels + 1), &iscale, 0);
> -if (ret < 0) {
> -yae_release_buffers(atempo);
> -return ret;
> -}
> +if (ret < 0)
> +goto fail;
>
> -RE_MALLOC_OR_FAIL(atempo->correlation_in, (atempo->window + 1),
> sizeof(AVComplexFloat));
> -RE_MALLOC_OR_FAIL(atempo->correlation, atempo->window,
> sizeof(AVComplexFloat));
> +if (!(atempo->correlation_in = av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat))) ||
> +!(atempo->correlation= av_calloc(atempo->window,
> sizeof(AVComplexFloat {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
>
> atempo->ring = atempo->window * 3;
> -RE_MALLOC_OR_FAIL(atempo->buffer, atempo->ring, atempo->stride);
> +atempo->buffer = av_calloc(atempo->ring, atempo->stride);
> +if (!atempo->buffer) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
>
> // initialize the Hann window function:
> -RE_MALLOC_OR_FAIL(atempo->hann, atempo->window, sizeof(float));
> +atempo->hann = av_malloc_array(atempo->window, sizeof(float));
> +if (!atempo->hann) {
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
>
> for (i = 0; i < atempo->window; i++) {
Re: [FFmpeg-devel] [PATCH v5 1/2][GSoC 2024] libavcodec/x86/vvc: Add AVX2 DMVR SAD functions for VVC
On 5/21/2024 10:01 PM, Ronald S. Bultje wrote: Hi, On Tue, May 21, 2024 at 8:01 PM Stone Chen wrote: Implements AVX2 DMVR (decoder-side motion vector refinement) SAD functions. DMVR SAD is only calculated if w >= 8, h >= 8, and w * h > 128. To reduce complexity, SAD is only calculated on even rows. This is calculated for all video bitdepths, but the values passed to the function are always 16bit (even if the original video bitdepth is 8). The AVX2 implementation uses min/max/sub. Additionally this changes parameters dx and dy from int to intptr_t. This allows dx & dy to be used as pointer offsets without needing to use movsxd. Benchmarks ( AMD 7940HS ) Before: BQTerrace_1920x1080_60_10_420_22_RA.vvc | 106.0 | Chimera_8bit_1080P_1000_frames.vvc | 204.3 | NovosobornayaSquare_1920x1080.bin | 197.3 | RitualDance_1920x1080_60_10_420_37_RA.266 | 174.0 | After: BQTerrace_1920x1080_60_10_420_22_RA.vvc | 109.3 | Chimera_8bit_1080P_1000_frames.vvc | 216.0 | NovosobornayaSquare_1920x1080.bin | 204.0| RitualDance_1920x1080_60_10_420_37_RA.266 | 181.7 | --- libavcodec/vvc/dsp.c | 2 +- libavcodec/vvc/dsp.h | 2 +- libavcodec/x86/vvc/Makefile | 3 +- libavcodec/x86/vvc/vvc_sad.asm | 130 +++ libavcodec/x86/vvc/vvcdsp_init.c | 6 ++ 5 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 libavcodec/x86/vvc/vvc_sad.asm LGTM. Ronald Implemented my changes and applied. ___ 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 2/2][GSoC 2024] tests/checkasm: Add check_vvc_sad to vvc_mc.c
On 5/21/2024 10:12 PM, Ronald S. Bultje wrote: Hi, On Tue, May 21, 2024 at 8:01 PM Stone Chen wrote: Adds checkasm for DMVR SAD AVX2 implementation. Benchmarks ( AMD 7940HS ) vvc_sad_8x8_c: 50.3 vvc_sad_8x8_avx2: 0.3 vvc_sad_16x16_c: 250.3 vvc_sad_16x16_avx2: 10.3 vvc_sad_32x32_c: 1020.3 vvc_sad_32x32_avx2: 60.3 vvc_sad_64x64_c: 3850.3 vvc_sad_64x64_avx2: 220.3 vvc_sad_128x128_c: 14100.3 vvc_sad_128x128_avx2: 840.3 --- tests/checkasm/vvc_mc.c | 38 ++ 1 file changed, 38 insertions(+) LGTM. Ronald Applied after removing the trailing whitespaces. ___ 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 1/2][GSoC 2024] libavcodec/x86/vvc: Add AVX2 DMVR SAD functions for VVC
On 5/22/2024 2:02 AM, Andreas Rheinhardt wrote: Stone Chen: Implements AVX2 DMVR (decoder-side motion vector refinement) SAD functions. DMVR SAD is only calculated if w >= 8, h >= 8, and w * h > 128. To reduce complexity, SAD is only calculated on even rows. This is calculated for all video bitdepths, but the values passed to the function are always 16bit (even if the original video bitdepth is 8). The AVX2 implementation uses min/max/sub. Additionally this changes parameters dx and dy from int to intptr_t. This allows dx & dy to be used as pointer offsets without needing to use movsxd. Benchmarks ( AMD 7940HS ) Before: BQTerrace_1920x1080_60_10_420_22_RA.vvc | 106.0 | Chimera_8bit_1080P_1000_frames.vvc | 204.3 | NovosobornayaSquare_1920x1080.bin | 197.3 | RitualDance_1920x1080_60_10_420_37_RA.266 | 174.0 | After: BQTerrace_1920x1080_60_10_420_22_RA.vvc | 109.3 | Chimera_8bit_1080P_1000_frames.vvc | 216.0 | NovosobornayaSquare_1920x1080.bin | 204.0| RitualDance_1920x1080_60_10_420_37_RA.266 | 181.7 | --- libavcodec/vvc/dsp.c | 2 +- libavcodec/vvc/dsp.h | 2 +- libavcodec/x86/vvc/Makefile | 3 +- libavcodec/x86/vvc/vvc_sad.asm | 130 +++ libavcodec/x86/vvc/vvcdsp_init.c | 6 ++ 5 files changed, 140 insertions(+), 3 deletions(-) create mode 100644 libavcodec/x86/vvc/vvc_sad.asm diff --git a/libavcodec/x86/vvc/vvcdsp_init.c b/libavcodec/x86/vvc/vvcdsp_init.c index 0e68971b2c..aa6c916760 100644 --- a/libavcodec/x86/vvc/vvcdsp_init.c +++ b/libavcodec/x86/vvc/vvcdsp_init.c @@ -311,6 +311,9 @@ ALF_FUNCS(16, 12, avx2) c->alf.filter[CHROMA] = ff_vvc_alf_filter_chroma_##bd##_avx2;\ c->alf.classify = ff_vvc_alf_classify_##bd##_avx2; \ } while (0) + +int ff_vvc_sad_avx2(const int16_t *src0, const int16_t *src1, intptr_t dx, intptr_t dy, int block_w, int block_h); +#define SAD_INIT() c->inter.sad = ff_vvc_sad_avx2 You are adding an AVX2 function to an ARCH_X86_64 #if block. I expect this to lead to linking failures if AVX2 is disabled. It's a prototype, so no linking failures. And SAD_INIT() is called on a block that both needs ARCH_X86_64 and EXTERNAL_AVX2_FAST to be true. ___ 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] lavc/rv34dsp: optimise R-V V idct_dc_add
Le 22 mai 2024 23:28:54 GMT+03:00, "Rémi Denis-Courmont" a écrit : >This removes one stray LI and reworks the vector arithmetic to avoid >changing the vector configuration. On K230, this takes the 46.5 cycle >count down from 46.5 to 43.5. >--- > libavcodec/riscv/rv34dsp_rvv.S | 13 ++--- > 1 file changed, 6 insertions(+), 7 deletions(-) > >diff --git a/libavcodec/riscv/rv34dsp_rvv.S b/libavcodec/riscv/rv34dsp_rvv.S >index f1f6345012..e8aff7e570 100644 >--- a/libavcodec/riscv/rv34dsp_rvv.S >+++ b/libavcodec/riscv/rv34dsp_rvv.S >@@ -36,16 +36,15 @@ func ff_rv34_idct_dc_add_rvv, zve32x > vsetivli zero, 4, e8, mf4, ta, ma > vlse32.v v0, (a0), a1 > lit1, 169 >+lit2, 128 > mul t1, t1, a2 >-lia2, 255 >+vsetivli zero, 4*4, e8, m1, ta, ma >+vwsubu.vx v2, v0, t2 > addi t1, t1, 512 > srai t1, t1, 10 >-vsetivli zero, 4*4, e16, m2, ta, ma >-vzext.vf2 v2, v0 >-vadd.vx v2, v2, t1 >-vmax.vx v2, v2, zero >-vsetvli zero, zero, e8, m1, ta, ma >-vnclipu.wiv0, v2, 0 >+vwadd.wx v2, v2, t1 Hmm, this should not work, as t1 has more than 8 bits. Maybe checkasm is sloppy here. >+vnclip.wi v0, v2, 0 >+vxor.vx v0, v0, t2 > vsetivli zero, 4, e8, mf4, ta, ma > vsse32.v v0, (a0), a1 > ___ 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] lavc/rv34dsp: optimise R-V V idct_dc_add
This removes one stray LI and reworks the vector arithmetic to avoid changing the vector configuration. On K230, this takes the 46.5 cycle count down from 46.5 to 43.5. --- libavcodec/riscv/rv34dsp_rvv.S | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/libavcodec/riscv/rv34dsp_rvv.S b/libavcodec/riscv/rv34dsp_rvv.S index f1f6345012..e8aff7e570 100644 --- a/libavcodec/riscv/rv34dsp_rvv.S +++ b/libavcodec/riscv/rv34dsp_rvv.S @@ -36,16 +36,15 @@ func ff_rv34_idct_dc_add_rvv, zve32x vsetivli zero, 4, e8, mf4, ta, ma vlse32.v v0, (a0), a1 lit1, 169 +lit2, 128 mul t1, t1, a2 -lia2, 255 +vsetivli zero, 4*4, e8, m1, ta, ma +vwsubu.vx v2, v0, t2 addi t1, t1, 512 srai t1, t1, 10 -vsetivli zero, 4*4, e16, m2, ta, ma -vzext.vf2 v2, v0 -vadd.vx v2, v2, t1 -vmax.vx v2, v2, zero -vsetvli zero, zero, e8, m1, ta, ma -vnclipu.wiv0, v2, 0 +vwadd.wx v2, v2, t1 +vnclip.wi v0, v2, 0 +vxor.vx v0, v0, t2 vsetivli zero, 4, e8, mf4, ta, ma vsse32.v v0, (a0), a1 -- 2.45.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 8/8] aacdec: add a decoder for AAC USAC (xHE-AAC)
On 22/05/2024 22:15, Marton Balint wrote:
On Wed, 22 May 2024, Lynne via ffmpeg-devel wrote:
On 21/05/2024 23:33, Hendrik Leppkes wrote:
On Tue, May 21, 2024 at 9:52 PM Lynne via ffmpeg-devel
wrote:
It should be the case here, we shouldn't need reordering as NATIVE
just
lets you specify what order the elements appear in the bitstream.
NATIVE means "the FFmpeg native ordering", not "bitstream order".
CUSTOM lets you specify an arbitrary order but requires metadata to
that effect, but it makes it particularly hard to map to any standard
when playing or transcoding, so some efforts to try to unify it into a
NATIVE format is always appreciated if possible.
Right, I forgot about that, thanks.
Amended in my git repo to use Marton's code.
ret = av_channel_layout_custom_init(&ac->oc[1].ch_layout, nb_channels);
if (ret < 0)
return ret;
for (int i = 0; i < nb_channels; i++) {
AVChannelCustom *cm = &ac->oc[1].ch_layout.u.map[i];
cm->id = usac_ch_pos_to_av[get_bits(gb, 5)]; /* bsOutputChannelPos */
if (cm->id)
cm->id = AV_CHAN_UNKNOWN;
if (cm->id == AV_CHAN_NONE)
cm->id = AV_CHAN_UNKNOWN;
}
ret = av_channel_layout_retype(&ac->oc[1].ch_layout,
AV_CHANNEL_ORDER_NATIVE,
AV_CHANNEL_LAYOUT_RETYPE_FLAG_CANONICAL);
You can simply pass 0 instead of AV_CHANNEL_ORDER_NATIVE as the order
parameter, because AV_CHANNEL_LAYOUT_RETYPE_FLAG_CANONICAL automatically
uses the canonical order and ignores the order parameter.
if (ret < 0)
return ret;
av_channel_layout_copy(&avctx->ch_layout, &ac->oc[1].ch_layout);
Missing error check.
Thanks,
Marton
___
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".
Fixed both, thanks.
I'll keep AV_CHANNEL_ORDER_NATIVE as-is because its just more readable.
OpenPGP_0xA2FEA5F03F034464.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 8/8] aacdec: add a decoder for AAC USAC (xHE-AAC)
On Wed, 22 May 2024, Lynne via ffmpeg-devel wrote:
On 21/05/2024 23:33, Hendrik Leppkes wrote:
On Tue, May 21, 2024 at 9:52 PM Lynne via ffmpeg-devel
wrote:
It should be the case here, we shouldn't need reordering as NATIVE just
lets you specify what order the elements appear in the bitstream.
NATIVE means "the FFmpeg native ordering", not "bitstream order".
CUSTOM lets you specify an arbitrary order but requires metadata to
that effect, but it makes it particularly hard to map to any standard
when playing or transcoding, so some efforts to try to unify it into a
NATIVE format is always appreciated if possible.
Right, I forgot about that, thanks.
Amended in my git repo to use Marton's code.
ret = av_channel_layout_custom_init(&ac->oc[1].ch_layout, nb_channels);
if (ret < 0)
return ret;
for (int i = 0; i < nb_channels; i++) {
AVChannelCustom *cm = &ac->oc[1].ch_layout.u.map[i];
cm->id = usac_ch_pos_to_av[get_bits(gb, 5)]; /* bsOutputChannelPos */
if (cm->id)
cm->id = AV_CHAN_UNKNOWN;
if (cm->id == AV_CHAN_NONE)
cm->id = AV_CHAN_UNKNOWN;
}
ret = av_channel_layout_retype(&ac->oc[1].ch_layout,
AV_CHANNEL_ORDER_NATIVE,
AV_CHANNEL_LAYOUT_RETYPE_FLAG_CANONICAL);
You can simply pass 0 instead of AV_CHANNEL_ORDER_NATIVE as the order
parameter, because AV_CHANNEL_LAYOUT_RETYPE_FLAG_CANONICAL automatically
uses the canonical order and ignores the order parameter.
if (ret < 0)
return ret;
av_channel_layout_copy(&avctx->ch_layout, &ac->oc[1].ch_layout);
Missing error check.
Thanks,
Marton
___
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 13/13] avformat/flvdec: support all multi-track modes
---
libavformat/flvdec.c | 574 +++
1 file changed, 311 insertions(+), 263 deletions(-)
diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index 4f98ff348c..fe5a44a715 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -1273,6 +1273,7 @@ static int flv_read_packet(AVFormatContext *s, AVPacket
*pkt)
{
FLVContext *flv = s->priv_data;
int ret, i, size, flags;
+int res = 0;
enum FlvTagType type;
int stream_type=-1;
int64_t next, pos, meta_pos;
@@ -1287,6 +1288,7 @@ static int flv_read_packet(AVFormatContext *s, AVPacket
*pkt)
int pkt_type = 0;
uint8_t track_idx = 0;
uint32_t codec_id = 0;
+int multitrack_type = MultitrackTypeOneTrack;
retry:
/* pkt size is repeated at end. skip it */
@@ -1337,14 +1339,9 @@ retry:
if (pkt_type == AudioPacketTypeMultitrack) {
uint8_t types = avio_r8(s->pb);
-int multitrack_type = types >> 4;
+multitrack_type = types & 0xF0;
pkt_type = types & 0xF;
-if (multitrack_type != MultitrackTypeOneTrack) {
-av_log(s, AV_LOG_ERROR, "Audio multitrack types other than
MultitrackTypeOneTrack are unsupported!\n");
-return AVERROR_PATCHWELCOME;
-}
-
multitrack = 1;
size--;
}
@@ -1371,14 +1368,9 @@ retry:
if (pkt_type == PacketTypeMultitrack) {
uint8_t types = avio_r8(s->pb);
-int multitrack_type = types >> 4;
+multitrack_type = types & 0xF0;
pkt_type = types & 0xF;
-if (multitrack_type != MultitrackTypeOneTrack) {
-av_log(s, AV_LOG_ERROR, "Multitrack types other than
MultitrackTypeOneTrack are unsupported!\n");
-return AVERROR_PATCHWELCOME;
-}
-
multitrack = 1;
size--;
}
@@ -1447,293 +1439,349 @@ skip:
goto leave;
}
-/* now find stream */
-for (i = 0; i < s->nb_streams; i++) {
-st = s->streams[i];
-if (stream_type == FLV_STREAM_TYPE_AUDIO) {
-if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO &&
-(s->audio_codec_id || flv_same_audio_codec(st->codecpar,
flags, codec_id)) &&
-st->id == track_idx)
-break;
-} else if (stream_type == FLV_STREAM_TYPE_VIDEO) {
-if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
-(s->video_codec_id || flv_same_video_codec(st->codecpar,
codec_id)) &&
-st->id == track_idx)
-break;
-} else if (stream_type == FLV_STREAM_TYPE_SUBTITLE) {
-if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
-break;
-} else if (stream_type == FLV_STREAM_TYPE_DATA) {
-if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA)
-break;
+for (;;) {
+int track_size = size;
+
+if (multitrack_type != MultitrackTypeOneTrack) {
+track_size = avio_rb24(s->pb);
+size -= 3;
}
-}
-if (i == s->nb_streams) {
-static const enum AVMediaType stream_types[] = {AVMEDIA_TYPE_VIDEO,
AVMEDIA_TYPE_AUDIO, AVMEDIA_TYPE_SUBTITLE, AVMEDIA_TYPE_DATA};
-st = create_stream(s, stream_types[stream_type], track_idx);
-if (!st)
-return AVERROR(ENOMEM);
-}
-av_log(s, AV_LOG_TRACE, "%d %X %d \n", stream_type, flags, st->discard);
-if (flv->time_pos <= pos) {
-dts += flv->time_offset;
-}
+/* now find stream */
+for (i = 0; i < s->nb_streams; i++) {
+st = s->streams[i];
+if (stream_type == FLV_STREAM_TYPE_AUDIO) {
+if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO &&
+(s->audio_codec_id || flv_same_audio_codec(st->codecpar,
flags, codec_id)) &&
+st->id == track_idx)
+break;
+} else if (stream_type == FLV_STREAM_TYPE_VIDEO) {
+if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
+(s->video_codec_id || flv_same_video_codec(st->codecpar,
codec_id)) &&
+st->id == track_idx)
+break;
+} else if (stream_type == FLV_STREAM_TYPE_SUBTITLE) {
+if (st->codecpar->codec_type == AVMEDIA_TYPE_SUBTITLE)
+break;
+} else if (stream_type == FLV_STREAM_TYPE_DATA) {
+if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA)
+break;
+}
+}
+if (i == s->nb_streams) {
+static const enum AVMediaType stream_types[] =
{AVMEDIA_TYPE_VIDEO, AVMEDIA_TYPE_AUDIO, AVMEDIA_TYPE_SUBTITLE,
AVMEDIA_TYPE_DATA};
+st = create_stream(s, stream_types[stream_type], track_idx);
+if (!st)
+
Re: [FFmpeg-devel] [PATCH 13/13] avformat/flvdec: support all multi-track modes
On 22.05.2024 02:02, Michael Niedermayer wrote: On Tue, May 21, 2024 at 11:02:22AM +0200, Timo Rothenpieler wrote: --- libavformat/flvdec.c | 570 +++ 1 file changed, 306 insertions(+), 264 deletions(-) infinite loops [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org) [flv @ 0x555e803d2940] Video codec (0) is not implemented. Update your FFmpeg version to the newest one from Git. If the problem still occurs, it means that your file has a feature which has not been implemented. [flv @ 0x555e803d2940] If you want to help, upload a sample of this file to https://streams.videolan.org/upload/ and contact th
[FFmpeg-devel] [PATCH] avcodec/h265_metadata: Add option to set width/height after crop
From: Zhao Zhili
It's a common usecase to request a video size after crop. Before
this patch, user must know the video size before crop, then set
crop_right/crop_bottom accordingly. Since HEVC can have different
CTU size, it's not easy to get/deduce the video size before crop.
With the new width/height options, there is no such requirement.
Signed-off-by: Zhao Zhili
---
doc/bitstream_filters.texi | 4 ++
libavcodec/bsf/h265_metadata.c | 101 -
libavcodec/version.h | 2 +-
3 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index 3d4dda04fc..c03f04f858 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -456,6 +456,10 @@ will replace the current ones if the stream is already
cropped.
These fields are set in pixels. Note that some sizes may not be
representable if the chroma is subsampled (H.265 section 7.4.3.2.1).
+@item width
+@item height
+Set width and height after crop.
+
@item level
Set the level in the VPS and SPS. See H.265 section A.4 and tables
A.6 and A.7.
diff --git a/libavcodec/bsf/h265_metadata.c b/libavcodec/bsf/h265_metadata.c
index c9e1cc3eed..eba00c20d5 100644
--- a/libavcodec/bsf/h265_metadata.c
+++ b/libavcodec/bsf/h265_metadata.c
@@ -58,6 +58,8 @@ typedef struct H265MetadataContext {
int crop_right;
int crop_top;
int crop_bottom;
+int width;
+int height;
int level;
int level_guess;
@@ -187,12 +189,94 @@ static int h265_metadata_update_vps(AVBSFContext *bsf,
return 0;
}
+static int h265_metadata_deduce_crop(AVBSFContext *bsf, const H265RawSPS *sps,
+ int *crop_left, int *crop_right,
+ int *crop_top, int *crop_bottom)
+{
+const H265MetadataContext *ctx = bsf->priv_data;
+int left = ctx->crop_left;
+int right = ctx->crop_right;
+int top = ctx->crop_top;
+int bottom = ctx->crop_bottom;
+
+if (ctx->width > 0) {
+if (ctx->width > sps->pic_width_in_luma_samples) {
+av_log(bsf, AV_LOG_ERROR,
+ "The width option value %d is larger than picture width
%d\n",
+ ctx->width, sps->pic_width_in_luma_samples);
+return AVERROR(EINVAL);
+}
+
+if (left < 0) {
+if (right > 0)
+left = sps->pic_width_in_luma_samples - ctx->width - right;
+else
+left = 0;
+}
+
+if (right < 0)
+right = sps->pic_width_in_luma_samples - ctx->width - left;
+
+if (left < 0 || right < 0 || (left + right + ctx->width) !=
+sps->pic_width_in_luma_samples) {
+av_log(bsf, AV_LOG_ERROR,
+ "Invalid value for crop_left %d, crop_right %d, width after
"
+ "crop %d, with picture width %d\n",
+ ctx->crop_left, ctx->crop_right, ctx->width,
+ sps->pic_width_in_luma_samples);
+return AVERROR(EINVAL);
+}
+}
+
+if (ctx->height > 0) {
+if (ctx->height > sps->pic_height_in_luma_samples) {
+av_log(bsf, AV_LOG_ERROR,
+ "The height option value %d is larger than picture height
%d\n",
+ ctx->height, sps->pic_height_in_luma_samples);
+return AVERROR(EINVAL);
+}
+
+if (top < 0) {
+if (bottom > 0)
+top = sps->pic_height_in_luma_samples - ctx->height - bottom;
+else
+top = 0;
+}
+
+if (bottom < 0)
+bottom = sps->pic_height_in_luma_samples - ctx->height - top;
+
+if (top < 0 || bottom < 0 || (top + bottom + ctx->height) !=
+sps->pic_height_in_luma_samples) {
+av_log(bsf, AV_LOG_ERROR,
+ "Invalid value for crop_top %d, crop_bottom %d, height
after "
+ "crop %d, with picture height %d\n",
+ ctx->crop_top, ctx->crop_bottom, ctx->height,
+ sps->pic_height_in_luma_samples);
+return AVERROR(EINVAL);
+}
+}
+
+*crop_left = left;
+*crop_right = right;
+*crop_top = top;
+*crop_bottom = bottom;
+
+return 0;
+}
+
static int h265_metadata_update_sps(AVBSFContext *bsf,
H265RawSPS *sps)
{
H265MetadataContext *ctx = bsf->priv_data;
int need_vui = 0;
int crop_unit_x, crop_unit_y;
+/* Use local variables to avoid modifying context fields in case of video
+ * resolution changed. Crop doesn't work well with resolution change, this
+ * is the best we can do.
+ */
+int crop_left, crop_right, crop_top, crop_bottom;
+int ret;
if (ctx->sample_aspect_ratio.num && ctx->sample_aspect_ratio.den) {
int num, den, i;
@@ -289,6 +373,11 @@ static int h265_metadata_update_sps(AVBSFContext *bsf,
Re: [FFmpeg-devel] [PATCH] lavc/vp9: set update_map to 0 when segmentation.enabled is 0
Hi, On Wed, May 22, 2024 at 1:28 PM Hendrik Leppkes wrote: > On Wed, May 22, 2024 at 7:16 PM Lynne via ffmpeg-devel > wrote: > > I'd hate to apply fixes with no information in shared code. This can get > > removed with no information about what relies on it. > > Changing 5 different hwaccel modules to avoid one line here seems > rather silly, doesn't it? > We can add a comment, if that helps. > Comment is a good idea, I think Lynne is right there's a risk we accidentally remove it. Generally, I'd like to see more hwaccel fate coverage, e.g. a vaapi (or whatever) fate machine that runs relevant tests using hw decoder instead of sw decoder. That would protect against the same risk. Ronald ___ 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] tests/checkasm/vvc_alf: Don't use declare_func_emms
On Wed, 22 May 2024, Andreas Rheinhardt wrote: VVC does not have MMX code at all, so one can use the stricter declare_func to also check that the MMX state has not been clobbered with (which would be an ABI violation). Signed-off-by: Andreas Rheinhardt --- tests/checkasm/vvc_alf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/checkasm/vvc_alf.c b/tests/checkasm/vvc_alf.c index 6dd89bfafc..f35fd2cd3e 100644 --- a/tests/checkasm/vvc_alf.c +++ b/tests/checkasm/vvc_alf.c @@ -83,7 +83,7 @@ static void check_alf_filter(VVCDSPContext *c, const int bit_depth) ptrdiff_t dst_stride = DST_PIXEL_STRIDE * SIZEOF_PIXEL; int offset = (3 * SRC_PIXEL_STRIDE + 3) * SIZEOF_PIXEL; -declare_func_emms(AV_CPU_FLAG_AVX2, void, uint8_t *dst, ptrdiff_t dst_stride, const uint8_t *src, ptrdiff_t src_stride, +declare_func(void, uint8_t *dst, ptrdiff_t dst_stride, const uint8_t *src, ptrdiff_t src_stride, int width, int height, const int16_t *filter, const int16_t *clip, const int vb_pos); randomize_buffers(src0, src1, SRC_BUF_SIZE); @@ -137,7 +137,7 @@ static void check_alf_classify(VVCDSPContext *c, const int bit_depth) ptrdiff_t stride = SRC_PIXEL_STRIDE * SIZEOF_PIXEL; int offset = (3 * SRC_PIXEL_STRIDE + 3) * SIZEOF_PIXEL; -declare_func_emms(AV_CPU_FLAG_AVX2, void, int *class_idx, int *transpose_idx, +declare_func(void, int *class_idx, int *transpose_idx, const uint8_t *src, ptrdiff_t src_stride, int width, int height, int vb_pos, int *gradient_tmp); randomize_buffers(src0, src1, SRC_BUF_SIZE); -- 2.40.1 LGTM // Martin ___ 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] lavc/vp9: set update_map to 0 when segmentation.enabled is 0
On Wed, May 22, 2024 at 7:16 PM Lynne via ffmpeg-devel
wrote:
>
> On 22/05/2024 18:17, Philip Langdale via ffmpeg-devel wrote:
> > On Wed, 22 May 2024 11:10:31 -0400
> > "Ronald S. Bultje" wrote:
> >
> >> Hi,
> >>
> >> On Wed, May 22, 2024 at 10:36 AM Hendrik Leppkes
> >> wrote:
> >>
> >>> On Thu, Feb 29, 2024 at 7:19 AM llyyr wrote:
>
> segmentation.update_map is never reset to 0 on a new frame, and
> retains the value from the previous frame. This bugs out a bunch
> of hwaccel drivers when segmentation.enabled is 0 but update_map
> isn't because they don't ignore values behind switches. We also
> do this for vp8* so this commit is just mirroring the vp8 logic.
>
> This fixes an issue with certain samples** that causes blocky
> artifacts with vaapi and d3d11va (as far as known hwaccel drivers
> go). Mesa worked around*** this by ignoring this field if
> segmentation.enabled is 0, but d3d11va still doesn't work.
>
> *
> >>> https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811
> >>>
> ** https://github.com/mpv-player/mpv/issues/13533
> ***
> https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27816
>
> Signed-off-by: llyyr
> ---
> libavcodec/vp9.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> index 855936cdc1c7..4a628625131e 100644
> --- a/libavcodec/vp9.c
> +++ b/libavcodec/vp9.c
> @@ -717,6 +717,8 @@ static int decode_frame_header(AVCodecContext
> *avctx, s->s.h.segmentation.feat[i].skip_enabled =
> >>> get_bits1(&s->gb);
> }
> }
> +} else {
> +s->s.h.segmentation.update_map = 0;
> }
>
> // set qmul[] based on Y/UV, AC/DC and segmentation Q idx
> deltas
>
> base-commit: d263fce2b209e86a5a1e8f1b6aa33430ecc2c187
> --
> >>>
> >>> Change LGTM.
> >>> I was debugging the same issue today, and found the same problem
> >>> with some hwaccels not properly ignoring update_map when
> >>> segmentation is disabled.
> >>>
> >>> Will apply soon if there are no further comments.
> >>>
> >>
> >> Is fine, please apply.
> >>
> >
> > Another LGTM. We've been seeing this reported on the mpv side as well.
> >
> > --phil
> > ___
> > 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".
>
> Can't this get fixed by hwaccel code rather than globally?
> I'd hate to apply fixes with no information in shared code. This can get
> removed with no information about what relies on it.
Changing 5 different hwaccel modules to avoid one line here seems
rather silly, doesn't it?
We can add a comment, if that helps.
- Hendrik
___
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] lavc/vp9: set update_map to 0 when segmentation.enabled is 0
On 22/05/2024 18:17, Philip Langdale via ffmpeg-devel wrote:
On Wed, 22 May 2024 11:10:31 -0400
"Ronald S. Bultje" wrote:
Hi,
On Wed, May 22, 2024 at 10:36 AM Hendrik Leppkes
wrote:
On Thu, Feb 29, 2024 at 7:19 AM llyyr wrote:
segmentation.update_map is never reset to 0 on a new frame, and
retains the value from the previous frame. This bugs out a bunch
of hwaccel drivers when segmentation.enabled is 0 but update_map
isn't because they don't ignore values behind switches. We also
do this for vp8* so this commit is just mirroring the vp8 logic.
This fixes an issue with certain samples** that causes blocky
artifacts with vaapi and d3d11va (as far as known hwaccel drivers
go). Mesa worked around*** this by ignoring this field if
segmentation.enabled is 0, but d3d11va still doesn't work.
*
https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811
** https://github.com/mpv-player/mpv/issues/13533
***
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27816
Signed-off-by: llyyr
---
libavcodec/vp9.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 855936cdc1c7..4a628625131e 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -717,6 +717,8 @@ static int decode_frame_header(AVCodecContext
*avctx, s->s.h.segmentation.feat[i].skip_enabled =
get_bits1(&s->gb);
}
}
+} else {
+s->s.h.segmentation.update_map = 0;
}
// set qmul[] based on Y/UV, AC/DC and segmentation Q idx
deltas
base-commit: d263fce2b209e86a5a1e8f1b6aa33430ecc2c187
--
Change LGTM.
I was debugging the same issue today, and found the same problem
with some hwaccels not properly ignoring update_map when
segmentation is disabled.
Will apply soon if there are no further comments.
Is fine, please apply.
Another LGTM. We've been seeing this reported on the mpv side as well.
--phil
___
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".
Can't this get fixed by hwaccel code rather than globally?
I'd hate to apply fixes with no information in shared code. This can get
removed with no information about what relies on it.
OpenPGP_0xA2FEA5F03F034464.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] tests/checkasm/vvc_alf: Don't use declare_func_emms
VVC does not have MMX code at all, so one can use the stricter declare_func to also check that the MMX state has not been clobbered with (which would be an ABI violation). Signed-off-by: Andreas Rheinhardt --- tests/checkasm/vvc_alf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/checkasm/vvc_alf.c b/tests/checkasm/vvc_alf.c index 6dd89bfafc..f35fd2cd3e 100644 --- a/tests/checkasm/vvc_alf.c +++ b/tests/checkasm/vvc_alf.c @@ -83,7 +83,7 @@ static void check_alf_filter(VVCDSPContext *c, const int bit_depth) ptrdiff_t dst_stride = DST_PIXEL_STRIDE * SIZEOF_PIXEL; int offset = (3 * SRC_PIXEL_STRIDE + 3) * SIZEOF_PIXEL; -declare_func_emms(AV_CPU_FLAG_AVX2, void, uint8_t *dst, ptrdiff_t dst_stride, const uint8_t *src, ptrdiff_t src_stride, +declare_func(void, uint8_t *dst, ptrdiff_t dst_stride, const uint8_t *src, ptrdiff_t src_stride, int width, int height, const int16_t *filter, const int16_t *clip, const int vb_pos); randomize_buffers(src0, src1, SRC_BUF_SIZE); @@ -137,7 +137,7 @@ static void check_alf_classify(VVCDSPContext *c, const int bit_depth) ptrdiff_t stride = SRC_PIXEL_STRIDE * SIZEOF_PIXEL; int offset = (3 * SRC_PIXEL_STRIDE + 3) * SIZEOF_PIXEL; -declare_func_emms(AV_CPU_FLAG_AVX2, void, int *class_idx, int *transpose_idx, +declare_func(void, int *class_idx, int *transpose_idx, const uint8_t *src, ptrdiff_t src_stride, int width, int height, int vb_pos, int *gradient_tmp); randomize_buffers(src0, src1, SRC_BUF_SIZE); -- 2.40.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 4/4] lavf: Add documentation for private "Context" classes
On Wed, May 22, 2024 at 05:24:36PM +0200, Andreas Rheinhardt wrote: > These structures should be renamed instead of adding these comments > (which are pointless for internal developers). I just sent a patch for that. > Thanks for pointing out the issue. Oh, great! So the next version of this patchset will skip this patch, and will reduce links like this: > + * @brief @ref md_doc_2context "Context" for a cache down to: > + * @brief @ref Context for a cache I don't see a way of removing the "@ref" without doing something nasty, like making a fake "Context" struct and shoving the documentation in there. Also, if someone does something strange like this: > + * @brief @ref Context "structure" (actually an enum) doxygen won't render it the way the author expects. I don't expect that to happen much in the real world though. ___ 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] lavc/vp9: set update_map to 0 when segmentation.enabled is 0
On Wed, 22 May 2024 11:10:31 -0400
"Ronald S. Bultje" wrote:
> Hi,
>
> On Wed, May 22, 2024 at 10:36 AM Hendrik Leppkes
> wrote:
>
> > On Thu, Feb 29, 2024 at 7:19 AM llyyr wrote:
> > >
> > > segmentation.update_map is never reset to 0 on a new frame, and
> > > retains the value from the previous frame. This bugs out a bunch
> > > of hwaccel drivers when segmentation.enabled is 0 but update_map
> > > isn't because they don't ignore values behind switches. We also
> > > do this for vp8* so this commit is just mirroring the vp8 logic.
> > >
> > > This fixes an issue with certain samples** that causes blocky
> > > artifacts with vaapi and d3d11va (as far as known hwaccel drivers
> > > go). Mesa worked around*** this by ignoring this field if
> > > segmentation.enabled is 0, but d3d11va still doesn't work.
> > >
> > > *
> > https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811
> >
> > > ** https://github.com/mpv-player/mpv/issues/13533
> > > ***
> > > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27816
> > >
> > > Signed-off-by: llyyr
> > > ---
> > > libavcodec/vp9.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> > > index 855936cdc1c7..4a628625131e 100644
> > > --- a/libavcodec/vp9.c
> > > +++ b/libavcodec/vp9.c
> > > @@ -717,6 +717,8 @@ static int decode_frame_header(AVCodecContext
> > > *avctx, s->s.h.segmentation.feat[i].skip_enabled =
> > get_bits1(&s->gb);
> > > }
> > > }
> > > +} else {
> > > +s->s.h.segmentation.update_map = 0;
> > > }
> > >
> > > // set qmul[] based on Y/UV, AC/DC and segmentation Q idx
> > > deltas
> > >
> > > base-commit: d263fce2b209e86a5a1e8f1b6aa33430ecc2c187
> > > --
> >
> > Change LGTM.
> > I was debugging the same issue today, and found the same problem
> > with some hwaccels not properly ignoring update_map when
> > segmentation is disabled.
> >
> > Will apply soon if there are no further comments.
> >
>
> Is fine, please apply.
>
Another LGTM. We've been seeing this reported on the mpv side as well.
--phil
___
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 v4 1/4] doc: Explain what "context" means
On Wed, May 22, 2024 at 11:31:52AM +0200, Stefano Sabatini wrote:
> Sorry for the slow reply.
Welcome back :)
I've gathered some critiques of my own over the past week, which I'll pepper
throughout the reply. Starting with...
The document assumes (or is at least designed to be secure against) readers
starting at the top and reading through to the bottom. I found doxygen's
@tableofcontents command while writing this e-mail, which I will definitely
use in the next version, and which might provoke a rewrite aimed at people
jumping around the document looking for answers to specific questions.
>
> On date Wednesday 2024-05-15 16:54:19 +0100, Andrew Sayers wrote:
> > Derived from detailed explanations kindly provided by Stefano Sabatini:
> > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325903.html
> > ---
> > doc/context.md | 394 +
> > 1 file changed, 394 insertions(+)
> > create mode 100644 doc/context.md
> >
> > diff --git a/doc/context.md b/doc/context.md
> > new file mode 100644
> > index 00..fb85b3f366
> > --- /dev/null
> > +++ b/doc/context.md
> > @@ -0,0 +1,394 @@
> > +# Introduction to contexts
> > +
> > +“%Context”
>
> Is this style of quoting needed? Especially I'd avoid special markup
> to simplify unredendered text reading (which is the point of markdown
> afterall).
Short answer: I'll change it in the next patch and see what happens.
Long answer: HTML quotes are ugly for everyone, UTF-8 is great until someone
turns up complaining we broke their Latin-1 workflow. I've always preferred
ASCII-only representations for that reason, but happy to try the other way
and see if anyone still cares.
>
> > is a name for a widely-used programming idiom.
>
> > +This document explains the general idiom and the conventions FFmpeg has
> > built around it.
> > +
> > +This document uses object-oriented analogies to help readers familiar with
> > +[object-oriented
> > programming](https://en.wikipedia.org/wiki/Object-oriented_programming)
> > +learn about contexts. But contexts can also be used outside of OOP,
> > +and even in situations where OOP isn't helpful. So these analogies
> > +should only be used as a first step towards understanding contexts.
> > +
> > +## “Context” as a way to think about code
> > +
> > +A context is any data structure that is passed to several functions
> > +(or several instances of the same function) that all operate on the same
> > entity.
> > +For example, [object-oriented
> > programming](https://en.wikipedia.org/wiki/Object-oriented_programming)
> > +languages usually provide member functions with a `this` or `self` value:
> > +
>
> > +```c
> > +class my_cxx_class {
> > + void my_member_function() {
> > +// the implicit object parameter provides context for the member
> > function:
> > +std::cout << this;
> > + }
> > +};
> > +```
>
> I'm not convinced this is really useful: if you know C++ this is
> redundant, if you don't this is confusing and don't add much information.
The example is there to break up a wall of text (syntax-highlighted in the
rendered output), and to let the reader know that this is going to be one of
those documents that alternates between text and code, so they're ready for the
more substantive examples later on. I take the point about C++ though -
would this Python example be more readable?
class MyClass:
def my_func(self):
# If a Python function is part of a class,
# its first parameter must be an instance of that class
>
> > +
> > +Contexts are a fundamental building block of OOP, but can also be used in
> > procedural code.
>
> I'd drop this line, and drop the anchor on OOP at the same time since
> it's adding no much information.
Fundamentally, this document addresses two audiences:
1. people coming from a non-OOP background, who want to learn contexts
from first principles, and at best see OOP stuff as background information
2. people coming from an OOP background. There's no polite way to say this -
their incentive is to write FFmpeg off as a failed attempt at OOP, so they
don't have to learn a new way of working that's just different enough to
make them feel dumb
I think a good way to evaluate the document might be to read it through twice,
stopping after each paragraph to ask two unfair questions...
1. what has this told me about FFmpeg itself, as opposed to some other thing
you wish I cared about?
2. couldn't you have just done this the standard OOP way?
The earlier paragraph acknowledged that contexts resemble OOP (telling the OOP
audience we get it), then this paragraph adds "but they're not the same"
(telling the OOP audience we disagree). To be more useful to non-OOP folk,
how about:
Contexts can be a fundamental building block of OOP, but can also be used in
procedural projects like FFmpeg.
>
> > +For example, most callback functions can be understood to use contexts:
>
>
[FFmpeg-devel] [PATCH v3 0/2] avcodec/dovi - correctly read el_bit_depth_minus8 and ext_mapping_idc
From: Cosmin Stejerean Some DolbyVision samples fail to parse currently due to mis-reading the el_bit_depth_minus8 field. Upon investigation it seems that the RPU syntax has been extended in an as of yet undocumented way by adding ext_mapping_idc and coding it together with el_bit_depth_minus8 together into a single 16 bit integer with the upper 8 bits for ext_mapping_idc and the lower 8 bits for el_bit_depth_minus8. This can be observed in the output of the DoVi verifier, which shows how this is laid out. This patchset adds the new fields to dovi_meta and implements the code to parse and write this back out. Compared to the previous version it moves the fields to the end for ABI compatibility, bumps the minor version in lavu and splits this into a separate commit. Cosmin Stejerean (2): lavu/dovi_meta - add fields for ext_mapping_idc avcodec/dovi - correctly read el_bit_depth_minus8 and ext_mapping_idc libavcodec/dovi_rpudec.c | 7 ++- libavcodec/dovi_rpuenc.c | 4 +++- libavutil/dovi_meta.h| 2 ++ libavutil/version.h | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) -- 2.42.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 2/2] avcodec/dovi - correctly read el_bit_depth_minus8 and ext_mapping_idc
From: Cosmin Stejerean
These two fields are coded together into a single 16 bit integer with upper 8
bits for ext_mapping_idc and lower 8 bits for el_bit_depth_minus8.
Furthermore ext_mapping_idc has two components, upper 3 bits and lower 5 bits.
---
libavcodec/dovi_rpudec.c | 7 ++-
libavcodec/dovi_rpuenc.c | 4 +++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c
index 7c7eda9d09..af41ab5827 100644
--- a/libavcodec/dovi_rpudec.c
+++ b/libavcodec/dovi_rpudec.c
@@ -411,13 +411,18 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu,
size_t rpu_size,
if ((hdr->rpu_format & 0x700) == 0) {
int bl_bit_depth_minus8 = get_ue_golomb_31(gb);
-int el_bit_depth_minus8 = get_ue_golomb_31(gb);
+int el_bit_depth_minus8_and_ext_mapping_idc =
get_ue_golomb_long(gb);
+int el_bit_depth_minus8 = el_bit_depth_minus8_and_ext_mapping_idc
& 0xFF; // lowest 8 bits
+int ext_mapping_idc = (el_bit_depth_minus8_and_ext_mapping_idc &
0xFF00) >> 8; // upper 8 bits
+
int vdr_bit_depth_minus8 = get_ue_golomb_31(gb);
VALIDATE(bl_bit_depth_minus8, 0, 8);
VALIDATE(el_bit_depth_minus8, 0, 8);
VALIDATE(vdr_bit_depth_minus8, 0, 8);
hdr->bl_bit_depth = bl_bit_depth_minus8 + 8;
hdr->el_bit_depth = el_bit_depth_minus8 + 8;
+hdr->ext_mapping_idc_0_4 = ext_mapping_idc & 0x1F; // lowest 5
bits of ext_mapping_idc
+hdr->ext_mapping_idc_5_7 = (ext_mapping_idc & 0xE0) >> 5; // upper
3 bits of ext_mapping_idc
hdr->vdr_bit_depth = vdr_bit_depth_minus8 + 8;
hdr->spatial_resampling_filter_flag = get_bits1(gb);
skip_bits(gb, 3); /* reserved_zero_3bits */
diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c
index 3c3e0f84c0..91c0a85050 100644
--- a/libavcodec/dovi_rpuenc.c
+++ b/libavcodec/dovi_rpuenc.c
@@ -444,6 +444,7 @@ int ff_dovi_rpu_generate(DOVIContext *s, const
AVDOVIMetadata *metadata,
int vdr_dm_metadata_changed, vdr_rpu_id, use_prev_vdr_rpu, profile,
buffer_size, rpu_size, pad, zero_run;
int num_ext_blocks_v1, num_ext_blocks_v2;
+uint8_t ext_mapping_idc;
uint32_t crc;
uint8_t *dst;
if (!metadata) {
@@ -551,7 +552,8 @@ int ff_dovi_rpu_generate(DOVIContext *s, const
AVDOVIMetadata *metadata,
put_bits(pb, 1, hdr->bl_video_full_range_flag);
if ((hdr->rpu_format & 0x700) == 0) {
set_ue_golomb(pb, hdr->bl_bit_depth - 8);
-set_ue_golomb(pb, hdr->el_bit_depth - 8);
+ext_mapping_idc = (hdr->ext_mapping_idc_5_7 << 5) |
hdr->ext_mapping_idc_0_4;
+set_ue_golomb(pb, (ext_mapping_idc << 8) | hdr->el_bit_depth - 8);
set_ue_golomb(pb, hdr->vdr_bit_depth - 8);
put_bits(pb, 1, hdr->spatial_resampling_filter_flag);
put_bits(pb, 3, 0); /* reserved_zero_3bits */
--
2.42.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/2] lavu/dovi_meta - add fields for ext_mapping_idc
From: Cosmin Stejerean
---
libavutil/dovi_meta.h | 2 ++
libavutil/version.h | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
index e10332f8d7..e168075a24 100644
--- a/libavutil/dovi_meta.h
+++ b/libavutil/dovi_meta.h
@@ -91,6 +91,8 @@ typedef struct AVDOVIRpuDataHeader {
uint8_t spatial_resampling_filter_flag;
uint8_t el_spatial_resampling_filter_flag;
uint8_t disable_residual_flag;
+uint8_t ext_mapping_idc_0_4; /* extended base layer inverse mapping
indicator */
+uint8_t ext_mapping_idc_5_7; /* reserved */
} AVDOVIRpuDataHeader;
enum AVDOVIMappingMethod {
diff --git a/libavutil/version.h b/libavutil/version.h
index 3221c4c592..9c7146c228 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
*/
#define LIBAVUTIL_VERSION_MAJOR 59
-#define LIBAVUTIL_VERSION_MINOR 19
+#define LIBAVUTIL_VERSION_MINOR 20
#define LIBAVUTIL_VERSION_MICRO 100
#define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
--
2.42.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avcodec/dovi - correctly read el_bit_depth_minus8 and ext_mapping_idc
> On May 21, 2024, at 9:19 PM, Andreas Rheinhardt
> wrote:
>
> Cosmin Stejerean via ffmpeg-devel:
>> From: Cosmin Stejerean
>>
>> These two fields are coded together into a single 16 bit integer with upper 8
>> bits for ext_mapping_idc and lower 8 bits for el_bit_depth_minus8.
>>
>> Furthermore ext_mapping_idc has two components, upper 3 bits and lower 5
>> bits.
>
> How do you know about these fields? You seem to know something that
> Niklas doesn't.
I can see them in the output of the DoVi verifier.
RPU Header
...
| Sequence header
...
|BL_video_full_range_flag 0
|BL_bit_depth 10
|EL_bit_depth 10
|ext_mapping_idc[4:0] 1
|ext_mapping_idc[7:5] 4
|vdr_bit_depth 12
...
>> --- a/libavutil/dovi_meta.h
>> +++ b/libavutil/dovi_meta.h
>> @@ -87,6 +87,8 @@ typedef struct AVDOVIRpuDataHeader {
>> uint8_t bl_video_full_range_flag;
>> uint8_t bl_bit_depth; /* [8, 16] */
>> uint8_t el_bit_depth; /* [8, 16] */
>> +uint8_t ext_mapping_idc_0_4; /* extended base layer inverse mapping
>> indicator */
>> +uint8_t ext_mapping_idc_5_7; /* reserved */
>
> This is an ABI break. All new additions need to be put at the end.
> Furthermore this needs an entry in APIChanges and a lavu minor version
> bump. And it should be in a patch of its own.
Ok, sending v3.
- Cosmin
___
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 v4 4/4] lavf: Add documentation for private "Context" classes
Andrew Sayers:
> Doxygen thinks any text like "Context for foo" is a link to a struct called
> "Context".
> Add a description and a better link, to avoid confusing readers.
> ---
> libavformat/async.c | 3 +++
> libavformat/cache.c | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/libavformat/async.c b/libavformat/async.c
> index e096b0bc6f..3c28d418ae 100644
> --- a/libavformat/async.c
> +++ b/libavformat/async.c
> @@ -53,6 +53,9 @@ typedef struct RingBuffer
> int read_pos;
> } RingBuffer;
>
> +/**
> + * @brief @ref md_doc_2context "Context" for testing async
> + */
> typedef struct Context {
> AVClass*class;
> URLContext *inner;
> diff --git a/libavformat/cache.c b/libavformat/cache.c
> index 5f78adba9d..3cc0edec82 100644
> --- a/libavformat/cache.c
> +++ b/libavformat/cache.c
> @@ -52,6 +52,9 @@ typedef struct CacheEntry {
> int size;
> } CacheEntry;
>
> +/**
> + * @brief @ref md_doc_2context "Context" for a cache
> + */
> typedef struct Context {
> AVClass *class;
> int fd;
These structures should be renamed instead of adding these comments
(which are pointless for internal developers). I just sent a patch for that.
Thanks for pointing out the issue.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/async, cache: Use more unique context names
Otherwise Doxygen thinks any text like "Context for foo"
is a link to the async protocol's struct called "Context".
Reported-by: Andrew Sayers
Signed-off-by: Andreas Rheinhardt
---
libavformat/async.c | 22 +++---
libavformat/cache.c | 18 +-
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/libavformat/async.c b/libavformat/async.c
index e096b0bc6f..e0329e23ec 100644
--- a/libavformat/async.c
+++ b/libavformat/async.c
@@ -53,7 +53,7 @@ typedef struct RingBuffer
int read_pos;
} RingBuffer;
-typedef struct Context {
+typedef struct AsyncContext {
AVClass*class;
URLContext *inner;
@@ -78,7 +78,7 @@ typedef struct Context {
int abort_request;
AVIOInterruptCB interrupt_callback;
-} Context;
+} AsyncContext;
static int ring_init(RingBuffer *ring, unsigned int capacity, int
read_back_capacity)
{
@@ -132,7 +132,7 @@ static int ring_read(RingBuffer *ring, void *dest, int
buf_size)
static int wrapped_url_read(void *src, void *dst, size_t *size)
{
URLContext *h = src;
-Context*c = h->priv_data;
+AsyncContext *c = h->priv_data;
int ret;
ret = ffurl_read(c->inner, dst, *size);
@@ -170,7 +170,7 @@ static int ring_drain(RingBuffer *ring, int offset)
static int async_check_interrupt(void *arg)
{
URLContext *h = arg;
-Context*c = h->priv_data;
+AsyncContext *c = h->priv_data;
if (c->abort_request)
return 1;
@@ -184,7 +184,7 @@ static int async_check_interrupt(void *arg)
static void *async_buffer_task(void *arg)
{
URLContext *h= arg;
-Context *c= h->priv_data;
+AsyncContext *c= h->priv_data;
RingBuffer *ring = &c->ring;
int ret = 0;
int64_t seek_ret;
@@ -249,7 +249,7 @@ static void *async_buffer_task(void *arg)
static int async_open(URLContext *h, const char *arg, int flags, AVDictionary
**options)
{
-Context *c = h->priv_data;
+AsyncContext *c = h->priv_data;
int ret;
AVIOInterruptCB interrupt_callback = {.callback = async_check_interrupt,
.opaque = h};
@@ -316,7 +316,7 @@ fifo_fail:
static int async_close(URLContext *h)
{
-Context *c = h->priv_data;
+AsyncContext *c = h->priv_data;
int ret;
pthread_mutex_lock(&c->mutex);
@@ -339,7 +339,7 @@ static int async_close(URLContext *h)
static int async_read_internal(URLContext *h, void *dest, int size)
{
-Context *c = h->priv_data;
+AsyncContext *c = h->priv_data;
RingBuffer *ring= &c->ring;
int read_complete = !dest;
int to_read = size;
@@ -391,7 +391,7 @@ static int async_read(URLContext *h, unsigned char *buf,
int size)
static int64_t async_seek(URLContext *h, int64_t pos, int whence)
{
-Context *c= h->priv_data;
+AsyncContext *c= h->priv_data;
RingBuffer *ring = &c->ring;
int64_t ret;
int64_t new_logical_pos;
@@ -472,7 +472,7 @@ static int64_t async_seek(URLContext *h, int64_t pos, int
whence)
return ret;
}
-#define OFFSET(x) offsetof(Context, x)
+#define OFFSET(x) offsetof(AsyncContext, x)
#define D AV_OPT_FLAG_DECODING_PARAM
static const AVOption options[] = {
@@ -495,7 +495,7 @@ const URLProtocol ff_async_protocol = {
.url_read= async_read,
.url_seek= async_seek,
.url_close = async_close,
-.priv_data_size = sizeof(Context),
+.priv_data_size = sizeof(AsyncContext),
.priv_data_class = &async_context_class,
};
diff --git a/libavformat/cache.c b/libavformat/cache.c
index 5f78adba9d..5d71e56f3d 100644
--- a/libavformat/cache.c
+++ b/libavformat/cache.c
@@ -52,7 +52,7 @@ typedef struct CacheEntry {
int size;
} CacheEntry;
-typedef struct Context {
+typedef struct CacheContext {
AVClass *class;
int fd;
char *filename;
@@ -65,7 +65,7 @@ typedef struct Context {
URLContext *inner;
int64_t cache_hit, cache_miss;
int read_ahead_limit;
-} Context;
+} CacheContext;
static int cmp(const void *key, const void *node)
{
@@ -74,9 +74,9 @@ static int cmp(const void *key, const void *node)
static int cache_open(URLContext *h, const char *arg, int flags, AVDictionary
**options)
{
+CacheContext *c = h->priv_data;
int ret;
char *buffername;
-Context *c= h->priv_data;
av_strstart(arg, "cache:", &arg);
@@ -99,7 +99,7 @@ static int cache_open(URLContext *h, const char *arg, int
flags, AVDictionary **
static int add_entry(URLContext *h, const unsigned char *buf, int size)
{
-Context *c= h->priv_data;
+CacheContext *c = h->priv_data;
int64_t pos = -1;
int ret;
CacheEntry *entry = NULL, *next[2] = {NULL, NULL};
@@ -162,7 +162,7 @@ fail:
static int cache_read(URLContext *h, unsigned char *buf, int size)
{
-Context *c= h->priv_data;
+CacheC
Re: [FFmpeg-devel] [PATCH] lavc/vp9: set update_map to 0 when segmentation.enabled is 0
Hi,
On Wed, May 22, 2024 at 10:36 AM Hendrik Leppkes
wrote:
> On Thu, Feb 29, 2024 at 7:19 AM llyyr wrote:
> >
> > segmentation.update_map is never reset to 0 on a new frame, and retains
> > the value from the previous frame. This bugs out a bunch of hwaccel
> > drivers when segmentation.enabled is 0 but update_map isn't because
> > they don't ignore values behind switches. We also do this for vp8* so
> > this commit is just mirroring the vp8 logic.
> >
> > This fixes an issue with certain samples** that causes blocky
> > artifacts with vaapi and d3d11va (as far as known hwaccel drivers go).
> > Mesa worked around*** this by ignoring this field if
> > segmentation.enabled is 0, but d3d11va still doesn't work.
> >
> > *
> https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811
> > ** https://github.com/mpv-player/mpv/issues/13533
> > *** https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27816
> >
> > Signed-off-by: llyyr
> > ---
> > libavcodec/vp9.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> > index 855936cdc1c7..4a628625131e 100644
> > --- a/libavcodec/vp9.c
> > +++ b/libavcodec/vp9.c
> > @@ -717,6 +717,8 @@ static int decode_frame_header(AVCodecContext *avctx,
> > s->s.h.segmentation.feat[i].skip_enabled =
> get_bits1(&s->gb);
> > }
> > }
> > +} else {
> > +s->s.h.segmentation.update_map = 0;
> > }
> >
> > // set qmul[] based on Y/UV, AC/DC and segmentation Q idx deltas
> >
> > base-commit: d263fce2b209e86a5a1e8f1b6aa33430ecc2c187
> > --
>
> Change LGTM.
> I was debugging the same issue today, and found the same problem with
> some hwaccels not properly ignoring update_map when segmentation is
> disabled.
>
> Will apply soon if there are no further comments.
>
Is fine, please apply.
Ronald
___
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 v4 4/4] lavf: Add documentation for private "Context" classes
On Wed, May 22, 2024 at 12:08:29PM +0200, Stefano Sabatini wrote:
> On date Wednesday 2024-05-15 16:54:22 +0100, Andrew Sayers wrote:
> > Doxygen thinks any text like "Context for foo" is a link to a struct called
> > "Context".
> > Add a description and a better link, to avoid confusing readers.
> > ---
> > libavformat/async.c | 3 +++
> > libavformat/cache.c | 3 +++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/libavformat/async.c b/libavformat/async.c
> > index e096b0bc6f..3c28d418ae 100644
> > --- a/libavformat/async.c
> > +++ b/libavformat/async.c
> > @@ -53,6 +53,9 @@ typedef struct RingBuffer
> > int read_pos;
> > } RingBuffer;
> >
> > +/**
> > + * @brief @ref md_doc_2context "Context" for testing async
> > + */
> > typedef struct Context {
> > AVClass*class;
> > URLContext *inner;
> > diff --git a/libavformat/cache.c b/libavformat/cache.c
> > index 5f78adba9d..3cc0edec82 100644
> > --- a/libavformat/cache.c
> > +++ b/libavformat/cache.c
> > @@ -52,6 +52,9 @@ typedef struct CacheEntry {
> > int size;
> > } CacheEntry;
> >
> > +/**
> > + * @brief @ref md_doc_2context "Context" for a cache
> > + */
> > typedef struct Context {
> > AVClass *class;
> > int fd;
>
> Not sure, these are private structs and we enforce no use of internal
> markup for those, and we can assume internals developers are already
> acquainted with contexts and such so this is probably adding no value.
Ah, yeah the use case isn't obvious if you haven't tripped over it...
Imagine you're a new user trying to learn FFmpeg, and you find yourself at
https://ffmpeg.org/doxygen/trunk/structAVAudioFifo.html - the first word
appears to be a link for this "context" thing you've been hearing about[1],
so you drop what you're doing to investigate. It links you to a struct that
looks promisingly general, but eventually turns out to be some random internal
struct with a misleading name. Now you've wasted a bunch of time and forgotten
what you were doing.
The other patch fixes the examples I've found in the code, but doxygen links
all instances of the word "Context" to this struct, confusing newbies. This
patch ensures that when future developers say "Context" in a comment, the page
doxygen links them to will point them in the right direction.
I had previously assumed it was a deliberate decision to include private files
in the documentation, so I didn't look that carefully into workarounds that
would break links to these private structures. But you've implied elsewhere
that's not the case, so is it worth looking into solutions that made "Context"
link to the context document, at the cost of making it impossible to link to
these private structs at all?
[1] to be clear, this is the current behaviour of the page on the live site
___
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] lavc/vp9: set update_map to 0 when segmentation.enabled is 0
On Thu, Feb 29, 2024 at 7:19 AM llyyr wrote:
>
> segmentation.update_map is never reset to 0 on a new frame, and retains
> the value from the previous frame. This bugs out a bunch of hwaccel
> drivers when segmentation.enabled is 0 but update_map isn't because
> they don't ignore values behind switches. We also do this for vp8* so
> this commit is just mirroring the vp8 logic.
>
> This fixes an issue with certain samples** that causes blocky
> artifacts with vaapi and d3d11va (as far as known hwaccel drivers go).
> Mesa worked around*** this by ignoring this field if
> segmentation.enabled is 0, but d3d11va still doesn't work.
>
> * https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/libavcodec/vp8.c#l811
> ** https://github.com/mpv-player/mpv/issues/13533
> *** https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27816
>
> Signed-off-by: llyyr
> ---
> libavcodec/vp9.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> index 855936cdc1c7..4a628625131e 100644
> --- a/libavcodec/vp9.c
> +++ b/libavcodec/vp9.c
> @@ -717,6 +717,8 @@ static int decode_frame_header(AVCodecContext *avctx,
> s->s.h.segmentation.feat[i].skip_enabled = get_bits1(&s->gb);
> }
> }
> +} else {
> +s->s.h.segmentation.update_map = 0;
> }
>
> // set qmul[] based on Y/UV, AC/DC and segmentation Q idx deltas
>
> base-commit: d263fce2b209e86a5a1e8f1b6aa33430ecc2c187
> --
Change LGTM.
I was debugging the same issue today, and found the same problem with
some hwaccels not properly ignoring update_map when segmentation is
disabled.
Will apply soon if there are no further comments.
- Hendrik
___
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] avformat/mov: store sample_sizes as unsigned ints
On 5/19/2024 10:41 PM, James Almer wrote:
As defined in Section 8.7.3.2.1 of ISO 14496-12.
Any unsupported value will be rejected in mov_build_index() without outright
aborting demuxing.
Fixes ticket #11005.
Signed-off-by: James Almer
---
libavformat/isom.h | 2 +-
libavformat/mov.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/libavformat/isom.h b/libavformat/isom.h
index 07f09d6eff..c0a5788e08 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -193,7 +193,7 @@ typedef struct MOVStreamContext {
unsigned int sample_size; ///< may contain value calculated from stsd or
value from stsz atom
unsigned int stsz_sample_size; ///< always contains sample size from stsz
atom
unsigned int sample_count;
-int *sample_sizes;
+unsigned int *sample_sizes;
int keyframe_absent;
unsigned int keyframe_count;
int *keyframes;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index b3fa748f27..54c2d1eebc 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3308,9 +3308,9 @@ static int mov_read_stsz(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
for (i = 0; i < entries; i++) {
sc->sample_sizes[i] = get_bits_long(&gb, field_size);
-if (sc->sample_sizes[i] < 0) {
+if (sc->sample_sizes[i] > INT64_MAX - sc->data_size) {
av_free(buf);
-av_log(c->fc, AV_LOG_ERROR, "Invalid sample size %d\n",
sc->sample_sizes[i]);
+av_log(c->fc, AV_LOG_ERROR, "Sample size overflow in STSZ\n");
return AVERROR_INVALIDDATA;
}
sc->data_size += sc->sample_sizes[i];
Will apply.
___
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] avformat/vvc: fix parsing sps_subpic_id
On 5/19/2024 10:25 AM, James Almer wrote:
The length of the sps_subpic_id[i] syntax element is sps_subpic_id_len_minus1 +
1 bits.
Signed-off-by: James Almer
---
libavformat/vvc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/libavformat/vvc.c b/libavformat/vvc.c
index 34c0aaf58b..ac3209a01b 100644
--- a/libavformat/vvc.c
+++ b/libavformat/vvc.c
@@ -371,6 +371,7 @@ static int vvcc_parse_sps(GetBitContext *gb,
const int tmp_height_val =
AV_CEIL_RSHIFT(sps_pic_height_max_in_luma_samples, ctb_log2_size_y);
const int wlen= av_ceil_log2(tmp_width_val);
const int hlen= av_ceil_log2(tmp_height_val);
+unsigned int sps_subpic_id_len;
if (sps_num_subpics_minus1 > 0) { // sps_num_subpics_minus1
sps_independent_subpics_flag = get_bits1(gb);
sps_subpic_same_size_flag = get_bits1(gb);
@@ -390,11 +391,11 @@ static int vvcc_parse_sps(GetBitContext *gb,
skip_bits(gb, 2); // sps_subpic_treated_as_pic_flag &&
sps_loop_filter_across_subpic_enabled_flag
}
}
-get_ue_golomb_long(gb); // sps_subpic_id_len_minus1
+sps_subpic_id_len = get_ue_golomb_long(gb) + 1;
if (get_bits1(gb)) {//
sps_subpic_id_mapping_explicitly_signalled_flag
if (get_bits1(gb)) // sps_subpic_id_mapping_present_flag
for (int i = 0; i <= sps_num_subpics_minus1; i++) {
-skip_bits1(gb); // sps_subpic_id[i]
+skip_bits_long(gb, sps_subpic_id_len); // sps_subpic_id[i]
}
}
}
Will apply.
___
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] x86/vvc_alf: use the x86inc instruction macros
On Wed, May 22, 2024 at 12:29 AM Wu Jianhua wrote: > > 发件人: ffmpeg-devel 代表 James Almer < > jamr...@gmail.com> > > 发送时间: 2024年5月21日 6:52 > > 收件人: ffmpeg-devel@ffmpeg.org > > 主题: [FFmpeg-devel] [PATCH] x86/vvc_alf: use the x86inc instruction macros > > > > Let its magic figure out the correct mnemonic based on target > instruction set. > > > > Signed-off-by: James Almer > > --- > > libavcodec/x86/vvc/vvc_alf.asm | 202 - > > 1 file changed, 101 insertions(+), 101 deletions(-) > > I tested this patch and LGTM. Thanks for updating them. > > And would it be better to add avcodec to the path of the commit message? > Hi Jianhua, vvc is clear in this case. Applied, thank you, Jianhua and James > Thanks, > Jianhua > ___ > 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 v3 1/3] doc: Explain what "context" means
On Wed, May 22, 2024 at 12:37:37PM +0200, Stefano Sabatini wrote: > On date Sunday 2024-05-05 22:04:36 +0100, Andrew Sayers wrote: > > I'm still travelling, so the following thoughts might be a bit > > half-formed. But I wanted to get some feedback before sitting down > > for a proper think. > [...] > > > > I've also gone through the code looking for edge cases we haven't > > > > covered. > > > > Here are some questions trying to prompt an "oh yeah I forgot to mention > > > > that"-type answer. Anything where the answer is more like "that should > > > > probably be rewritten to be clearer", let me know and I'll avoid > > > > confusing > > > > newbies with it. > > > > > > > > > > > av_ambient_viewing_environment_create_side_data() takes an AVFrame as > > > > its > > > > first argument, and returns a new AVAmbientViewingEnvironment. What is > > > > the > > > > context object for that function - AVFrame or > > > > AVAmbientViewingEnvironment? > > > > > > But this should be clear from the doxy: > > > > > > /** > > > * Allocate and add an AVAmbientViewingEnvironment structure to an > > > existing > > > * AVFrame as side data. > > > * > > > * @return the newly allocated struct, or NULL on failure > > > */ > > > AVAmbientViewingEnvironment > > > *av_ambient_viewing_environment_create_side_data(AVFrame *frame); > > > > I'm afraid it's not clear, at least to me. I think you're saying the > > AVFrame is the context because a "create" function can't have a > > context any more than a C++ "new" can be called as a member. But the > > function's prefix points to the other conclusion, and neither signal > > is clear enough on its own. > > No, what I'm saying is that in some cases you don't need to think in > terms of contexts, in this case there is no context at all, the > function takes a frame and modify it, and returns the ambient > environment to be used by the following functions. This should be very > clear by reading the doxy. There is no rule dictating the first param > of each FFmpeg function should be a "context". I'm still writing up a reply to your longer feedback, but on this topic... This function is in the "av_ambient_viewing_environment" namespace, and returns an object that is clearly used as a context for other functions. So saying "stop thinking about contexts" just leaves a negative space and a bad thing to fill it with (confusion in my case). I've found it useful to think about "receiving" vs. "producing" a context: * avcodec_alloc_context3() produces a context, but does not receive one * sws_init_context() receives a context, but does not produce one * av_ambient_viewing_environment_create_side_data() receives one context, and produces another How about if the document mostly talks about functions as having contexts, then follows it up with something like: There are some edge cases where this doesn't work. . If you find contexts a useful metaphor in these cases, you might prefer to think about them as "receiving" and "producing" contexts. ... or something similar that acknowledges contexts are unnecessary here, but provides a solution for people that want to use them anyway. ___ 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 PATCH 2/2] tests/fate/source-check: Relax BSD licence check
Le 22 mai 2024 14:04:50 GMT+03:00, Andreas Rheinhardt a écrit : >Several files already had standard license header (namely >2-clause BSD files), yet due to the 80 char line length limit, >they were not treated as such by source-check.sh (which >fate-source uses). Therefore relax the BSD check. LGTM. ___ 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] STF 2025
Le 22 mai 2024 00:34:03 GMT+03:00, Thilo Borgmann via ffmpeg-devel a écrit : >>> I hope you realize what you argue in favor of. >> >> Yes. It's quoted above. >> >> Are you claiming that *no* review is better than *some* review done in >> *public* for all to see by a paid professional just because the person is >> maybe biased? >> >> First, even volunteers have their own biases. Any expert should have opinions >> from their experience, and that by definition makes them "biased". >> >> And second, you can't have it both ways. Either we want people to be paid for >> review, and they will be answerable to their sponsor, or we want people to >> continue to work on their free time. > >I think that is what you don't understand. You're not answering the question here. The current STF funding of 153k€ for 2 years is roughly enough to pay for ONE full-time entry-level software engineer in Germany. Even if this were doubled with another similar round of funding next year, and even if that was to be reliably renewed year on year, and assuming that STF keeps an hands-off approach of not influencing the work, that will *not* be enough to pay all reviewers. So is it better to have no reviews or reviews by skilled corporate employees? (...) >> Ideally so but that's the land of utopia. > >Of course, we talk about what should be, don't we? Of course *not*. There is no point debating ideals that we can all agree on and that will never come to fruition. Rather this is all about how to concretely apply or not apply to STF, and more generally how to try to improve the sustainability of FFmpeg in a realistic manner. >> And "I hope you realise that you are arguing for" Intel, Loongson, etc. >> employees to stop reviewing patches. > >Syntax error. What exactly do you mean? I fail to see a syntax error. You're saying that corporate employees should not review because "they [will] want to get [their]" or their colleagues' "stuff in" (your words). Intel and Loongson are obvious current examples of companies whose employees are pushing and reviewing enablement patches for their commercial hardware. That is very definitely not "unbiased" nor "independent". >According to my assumptions: No, I value reviews of company employees in >general which have been proven to be useful and unbiased e.g. in getting part >of the community reviewing 'stuf' but not their 'own stuff'. I never said that I wanted biased reviews. I said some reviews were better than none, in spite of the risk of bias. So much for your grandstanding against my alleged not realising what I am advocating for, if you end up agreeing with me... ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [RFC PATCH 2/2] tests/fate/source-check: Relax BSD licence check
Several files already had standard license header (namely 2-clause BSD files), yet due to the 80 char line length limit, they were not treated as such by source-check.sh (which fate-source uses). Therefore relax the BSD check. Signed-off-by: Andreas Rheinhardt --- This is the first variant. The second variant follows shortly. tests/fate/source-check.sh | 2 +- tests/ref/fate/source | 5 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/fate/source-check.sh b/tests/fate/source-check.sh index 658823fc0b..4d7e175784 100755 --- a/tests/fate/source-check.sh +++ b/tests/fate/source-check.sh @@ -11,7 +11,7 @@ git grep -L -E "This file is part of FFmpeg|This file is part of libswresample|" "Permission is hereby granted to use, copy, modify, and distribute this|"\ "Permission is granted to anyone to use this software for any purpose|"\ "This work is licensed under the terms of the GNU GPL|"\ -"Redistribution and use in source and binary forms, with or without modification|"\ +"Redistribution and use in source and binary forms, with or without|"\ "This library is free software; you can redistribute it and/or|"\ "This program is free software; you can redistribute it and/or modify|"\ "Licensed under the Apache License|"\ diff --git a/tests/ref/fate/source b/tests/ref/fate/source index 723e2e06c7..a3beb35093 100644 --- a/tests/ref/fate/source +++ b/tests/ref/fate/source @@ -1,15 +1,10 @@ Files without standard license headers: libavcodec/file_open.c -libavcodec/ilbcdata.h -libavcodec/ilbcdec.c libavcodec/interplayacm.c libavcodec/log2_tab.c libavcodec/reverse.c -libavcodec/riscv/startcode_rvb.S -libavcodec/riscv/startcode_rvv.S libavdevice/file_open.c libavdevice/reverse.c -libavfilter/af_arnndn.c libavfilter/file_open.c libavfilter/log2_tab.c libavformat/bitstream.c -- 2.40.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [RFC PATCH] all: Adapt BSD license headers to avoid fate-source exceptions
This is against the 80 char line length limit, but it is not a hard rule anyway. Signed-off-by: Andreas Rheinhardt --- This is an alternative to modifying check-source.sh (for which I also sent a patch). doc/mips.txt | 5 ++--- libavcodec/aacsbr_fixed.c | 5 ++--- libavcodec/ac3dec_fixed.c | 5 ++--- libavcodec/aom_film_grain_template.c | 4 ++-- libavcodec/arm/vp8dsp_armv6.S | 5 ++--- libavcodec/ilbcdata.h | 5 ++--- libavcodec/ilbcdec.c | 5 ++--- libavcodec/j2kenc.c | 5 ++--- libavcodec/mips/ac3dsp_mips.c | 5 ++--- libavcodec/mips/acelp_filters_mips.c | 5 ++--- libavcodec/mips/acelp_vectors_mips.c | 5 ++--- libavcodec/mips/amrwbdec_mips.c | 5 ++--- libavcodec/mips/amrwbdec_mips.h | 5 ++--- libavcodec/mips/celp_filters_mips.c | 5 ++--- libavcodec/mips/celp_math_mips.c | 5 ++--- libavcodec/mips/compute_antialias_fixed.h | 5 ++--- libavcodec/mips/compute_antialias_float.h | 5 ++--- libavcodec/mips/fmtconvert_mips.c | 5 ++--- libavcodec/mips/iirfilter_mips.c | 5 ++--- libavcodec/mips/lsp_mips.h| 5 ++--- libavcodec/mips/mpegaudiodsp_mips_fixed.c | 5 ++--- libavcodec/mips/mpegaudiodsp_mips_float.c | 5 ++--- libavcodec/riscv/startcode_rvb.S | 4 ++-- libavcodec/riscv/startcode_rvv.S | 4 ++-- libavcodec/speexdata.h| 5 ++--- libavcodec/speexdec.c | 5 ++--- libavcodec/vvc/itx_1d.c | 4 ++-- libavfilter/af_arnndn.c | 5 ++--- libavformat/imf.h | 4 ++-- libavformat/imf_cpl.c | 4 ++-- libavformat/imfdec.c | 4 ++-- libavformat/tests/imf.c | 4 ++-- libavutil/fixed_dsp.c | 5 ++--- libavutil/fixed_dsp.h | 5 ++--- libavutil/mips/float_dsp_mips.c | 5 ++--- libavutil/mips/libm_mips.h| 5 ++--- libavutil/softfloat_tables.h | 5 ++--- libavutil/uuid.c | 5 ++--- tests/ref/fate/source | 5 - 39 files changed, 76 insertions(+), 111 deletions(-) diff --git a/doc/mips.txt b/doc/mips.txt index d66ce3b447..65ae48cf1d 100644 --- a/doc/mips.txt +++ b/doc/mips.txt @@ -15,9 +15,8 @@ Example of copyright notice: * Copyright (c) 2012 * MIPS Technologies, Inc., California. * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: + * Redistribution and use in source and binary forms, with or without modification, + * are permitted provided that the following conditions are met: * 1. Redistributions of source code must retain the above copyright *notice, this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright diff --git a/libavcodec/aacsbr_fixed.c b/libavcodec/aacsbr_fixed.c index 06d07e1941..422bb21c20 100644 --- a/libavcodec/aacsbr_fixed.c +++ b/libavcodec/aacsbr_fixed.c @@ -2,9 +2,8 @@ * Copyright (c) 2013 * MIPS Technologies, Inc., California. * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: + * Redistribution and use in source and binary forms, with or without modification, + * are permitted provided that the following conditions are met: * 1. Redistributions of source code must retain the above copyright *notice, this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright diff --git a/libavcodec/ac3dec_fixed.c b/libavcodec/ac3dec_fixed.c index c9e5cda69c..59b7492733 100644 --- a/libavcodec/ac3dec_fixed.c +++ b/libavcodec/ac3dec_fixed.c @@ -2,9 +2,8 @@ * Copyright (c) 2012 * MIPS Technologies, Inc., California. * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: + * Redistribution and use in source and binary forms, with or without modification, + * are permitted provided that the following conditions are met: * 1. Redistributions of source code must retain the above copyright *notice, this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright diff --git a/libavcodec/aom_film_grain_template.c b/libavcodec/aom_film_grain_template.c index 5f9f29f1fa..e510b5c6a7 100644 --- a/libavcodec/aom_film_grain_template.c +++ b/libavcodec/aom_film_grain_template.c @@ -25,8 +25,8 @@ * Copyright © 2018, Two Orioles, LLC * All rights reserved. * - * Redistribution and use in source and binary forms, with or without - * m
[FFmpeg-devel] [PATCH 1/2] doc/mips: Update list of files with MIPS copyright notice
E.g. the AAC stuff has been removed in 03cf10164578aed33f4d0cb5b69d63669c01a538. Signed-off-by: Andreas Rheinhardt --- Alternatively, we could just nuke this file. doc/mips.txt | 6 -- 1 file changed, 6 deletions(-) diff --git a/doc/mips.txt b/doc/mips.txt index d66ce3b447..a42546f0cd 100644 --- a/doc/mips.txt +++ b/doc/mips.txt @@ -49,11 +49,6 @@ Files that have MIPS copyright notice in them: libm_mips.h softfloat_tables.h * libavcodec/mips/ - aacdec_fixed.c - aacsbr_fixed.c - aacsbr_template.c - aaccoder_mips.c - aacpsy_mips.h ac3dsp_mips.c acelp_filters_mips.c acelp_vectors_mips.c @@ -64,7 +59,6 @@ Files that have MIPS copyright notice in them: compute_antialias_fixed.h compute_antialias_float.h lsp_mips.h - dsputil_mips.c fmtconvert_mips.c iirfilter_mips.c mpegaudiodsp_mips_fixed.c -- 2.40.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 1/3] doc: Explain what "context" means
On date Sunday 2024-05-05 22:04:36 +0100, Andrew Sayers wrote:
> I'm still travelling, so the following thoughts might be a bit
> half-formed. But I wanted to get some feedback before sitting down
> for a proper think.
[...]
> > > I've also gone through the code looking for edge cases we haven't covered.
> > > Here are some questions trying to prompt an "oh yeah I forgot to mention
> > > that"-type answer. Anything where the answer is more like "that should
> > > probably be rewritten to be clearer", let me know and I'll avoid confusing
> > > newbies with it.
> > >
> >
> > > av_ambient_viewing_environment_create_side_data() takes an AVFrame as its
> > > first argument, and returns a new AVAmbientViewingEnvironment. What is
> > > the
> > > context object for that function - AVFrame or AVAmbientViewingEnvironment?
> >
> > But this should be clear from the doxy:
> >
> > /**
> > * Allocate and add an AVAmbientViewingEnvironment structure to an existing
> > * AVFrame as side data.
> > *
> > * @return the newly allocated struct, or NULL on failure
> > */
> > AVAmbientViewingEnvironment
> > *av_ambient_viewing_environment_create_side_data(AVFrame *frame);
>
> I'm afraid it's not clear, at least to me. I think you're saying the
> AVFrame is the context because a "create" function can't have a
> context any more than a C++ "new" can be called as a member. But the
> function's prefix points to the other conclusion, and neither signal
> is clear enough on its own.
No, what I'm saying is that in some cases you don't need to think in
terms of contexts, in this case there is no context at all, the
function takes a frame and modify it, and returns the ambient
environment to be used by the following functions. This should be very
clear by reading the doxy. There is no rule dictating the first param
of each FFmpeg function should be a "context".
>
> My current thinking is to propose separate patches renaming arguments
> to `ctx` whenever I find functions I can't parse. That's not as good
> as a simple rule like "the first argument is always the context", but
> better than adding a paragraph or two about how to read the docs.
There cannot be such rule, because it would be false in many cases.
> > Also, you are assuming that all the function should have a
> > context. That's not the case, as you don't always need to keep track
> > of a "context" when performing operations.
>
[...]
> > > av_channel_description_bprint() takes a `struct AVBPrint *` as its first
> > > argument, then `enum AVChannel`. Is the context AVBPrint, AVChannel,
> > > or both? Does it make sense for a function to have two contexts?
> >
> > Again, this should be clear from the doxy:
> > /**
> > * Get a human readable string describing a given channel.
> > *
> > * @param buf pre-allocated buffer where to put the generated string
> > * @param buf_size size in bytes of the buffer.
> > * @param channel the AVChannel whose description to get
> > * @return amount of bytes needed to hold the output string, or a negative
> > AVERROR
> > * on failure. If the returned value is bigger than buf_size, then
> > the
> > * string was truncated.
> > */
> > int av_channel_description(char *buf, size_t buf_size, enum AVChannel
> > channel);
> >
> > /**
> > * bprint variant of av_channel_description().
> > *
> > * @note the string will be appended to the bprint buffer.
> > */
> > void av_channel_description_bprint(struct AVBPrint *bp, enum AVChannel
> > channel_id);
>
> I think you're saying that I should look at which word appears more
> often in the doxy ("channel") rather than which word appears first in
> the argument list ("buf")? As above, the solution might be to rename
> the variable in a separate patch rather than teach people another
> special case.
This is more about the semantics described in English language by the
doxy (which is normative). Again, thinking in terms of "contexts" is
misleading in this case.
In this case you have two functions, av_channel_description writing a
string to a buffer with fixed size, the second modifying an "AVBPrint"
struct, which is a high-level buffer providing more flexibility (and
"bprint" is used as a verb in the doxy, which might be misleading).
Note that both signatures are mimicing the standard C library
convention:
memcpy(dst, dst_size, src)
which in turn is a mnemonics for:
dst = src
meaning that we are copying data from src to dst.
You might think that in fact you are operating on a context (the dst
buffer or the AVBPrint struct), but you don't need to introduce the
concept of context for these simple functions.
___
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 v4 4/4] lavf: Add documentation for private "Context" classes
On date Wednesday 2024-05-15 16:54:22 +0100, Andrew Sayers wrote:
> Doxygen thinks any text like "Context for foo" is a link to a struct called
> "Context".
> Add a description and a better link, to avoid confusing readers.
> ---
> libavformat/async.c | 3 +++
> libavformat/cache.c | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/libavformat/async.c b/libavformat/async.c
> index e096b0bc6f..3c28d418ae 100644
> --- a/libavformat/async.c
> +++ b/libavformat/async.c
> @@ -53,6 +53,9 @@ typedef struct RingBuffer
> int read_pos;
> } RingBuffer;
>
> +/**
> + * @brief @ref md_doc_2context "Context" for testing async
> + */
> typedef struct Context {
> AVClass*class;
> URLContext *inner;
> diff --git a/libavformat/cache.c b/libavformat/cache.c
> index 5f78adba9d..3cc0edec82 100644
> --- a/libavformat/cache.c
> +++ b/libavformat/cache.c
> @@ -52,6 +52,9 @@ typedef struct CacheEntry {
> int size;
> } CacheEntry;
>
> +/**
> + * @brief @ref md_doc_2context "Context" for a cache
> + */
> typedef struct Context {
> AVClass *class;
> int fd;
Not sure, these are private structs and we enforce no use of internal
markup for those, and we can assume internals developers are already
acquainted with contexts and such so this is probably adding no value.
___
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 v4 2/4] lavu: Clarify relationship between AVClass, AVOption and context
On date Wednesday 2024-05-15 16:54:20 +0100, Andrew Sayers wrote:
> ---
> libavutil/log.h | 11 ---
> libavutil/opt.h | 7 ---
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/libavutil/log.h b/libavutil/log.h
> index ab7ceabe22..cfbf416679 100644
> --- a/libavutil/log.h
> +++ b/libavutil/log.h
> @@ -59,9 +59,14 @@ typedef enum {
> struct AVOptionRanges;
>
> /**
> - * Describe the class of an AVClass context structure. That is an
> - * arbitrary struct of which the first field is a pointer to an
> - * AVClass struct (e.g. AVCodecContext, AVFormatContext etc.).
> + * Metadata about an arbitrary data structure
The previous definition was circular, while this one is lacking
information (for example it is neglecting the logging facilities
provided by AVClass).
My take:
Provide access to generic logging and introspection facilities, used to
describe a class of structs.
An AVClass should be defined as the first element of the struct using
the AVClass facilities, so that functions operating on the AVClass
will work by providing the pointer to the structure.
Note: we might move the AVClass definition to its own file class.h.
> + *
> + * A @ref md_doc_2context "context struct" whose first member is a pointer
> + * to an AVClass object is called an "AVClass context structure"
> + * (e.g. AVCodecContext, AVFormatContext etc.).
> + *
> + * AVClass is often combined with @ref avoptions "AVOptions" to create
> + * "AVOptions-enabled structs" that can be easily configured by users.
> */
> typedef struct AVClass {
> /**
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 07e27a9208..e314e134c2 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -39,9 +39,10 @@
> * @defgroup avoptions AVOptions
> * @ingroup lavu_data
> * @{
> - * AVOptions provide a generic system to declare options on arbitrary structs
> - * ("objects").
This should be kept. In addition you can mention that it requires the
first element of the struct to be an AVClass (which is already
mentioned later in the text).
> An option can have a help text, a type and a range of possible
> - * values. Options may then be enumerated, read and written to.
> + * Builds on AVClass, adding a generic system to declare options.
> + *
> + * An option can have a help text, a type and a range of possible values.
> + * Options may then be enumerated, read and written to.
> *
> * There are two modes of access to members of AVOption and its child
> structs.
> * One is called 'native access', and refers to access from the code that
[...]
___
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 v4 1/4] doc: Explain what "context" means
Sorry for the slow reply.
On date Wednesday 2024-05-15 16:54:19 +0100, Andrew Sayers wrote:
> Derived from detailed explanations kindly provided by Stefano Sabatini:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325903.html
> ---
> doc/context.md | 394 +
> 1 file changed, 394 insertions(+)
> create mode 100644 doc/context.md
>
> diff --git a/doc/context.md b/doc/context.md
> new file mode 100644
> index 00..fb85b3f366
> --- /dev/null
> +++ b/doc/context.md
> @@ -0,0 +1,394 @@
> +# Introduction to contexts
> +
> +“%Context”
Is this style of quoting needed? Especially I'd avoid special markup
to simplify unredendered text reading (which is the point of markdown
afterall).
> is a name for a widely-used programming idiom.
> +This document explains the general idiom and the conventions FFmpeg has
> built around it.
> +
> +This document uses object-oriented analogies to help readers familiar with
> +[object-oriented
> programming](https://en.wikipedia.org/wiki/Object-oriented_programming)
> +learn about contexts. But contexts can also be used outside of OOP,
> +and even in situations where OOP isn't helpful. So these analogies
> +should only be used as a first step towards understanding contexts.
> +
> +## “Context” as a way to think about code
> +
> +A context is any data structure that is passed to several functions
> +(or several instances of the same function) that all operate on the same
> entity.
> +For example, [object-oriented
> programming](https://en.wikipedia.org/wiki/Object-oriented_programming)
> +languages usually provide member functions with a `this` or `self` value:
> +
> +```c
> +class my_cxx_class {
> + void my_member_function() {
> +// the implicit object parameter provides context for the member
> function:
> +std::cout << this;
> + }
> +};
> +```
I'm not convinced this is really useful: if you know C++ this is
redundant, if you don't this is confusing and don't add much information.
> +
> +Contexts are a fundamental building block of OOP, but can also be used in
> procedural code.
I'd drop this line, and drop the anchor on OOP at the same time since
it's adding no much information.
> +For example, most callback functions can be understood to use contexts:
> +
> +```c
> +struct MyStruct {
> + int counter;
> +};
> +
> +void my_callback( void *my_var_ ) {
> + // my_var provides context for the callback function:
> + struct MyStruct *my_var = (struct MyStruct *)my_var_;
> + printf("Called %d time(s)", ++my_var->counter);
> +}
> +
> +void init() {
> + struct MyStruct my_var;
> + my_var.counter = 0;
> + register_callback( my_callback, &my_var );
style: fun(my_callback, ...) (so spaces around parentheses) here and
below
> +}
> +```
> +
> +In the broadest sense, “context” is just a way to think about
> code.
> +You can even use it to think about code written by people who have never
> +heard the term, or who would disagree with you about what it means.
> +
> +## “Context” as a tool of communication
> +
> +“%Context“ can just be a word to understand code in your own
> head,
> +but it can also be a term you use to explain your interfaces.
> +Here is a version of the callback example that makes the context explicit:
> +
> +```c
> +struct CallbackContext {
> + int counter;
> +};
> +
> +void my_callback( void *ctx_ ) {
> + // ctx provides context for the callback function:
> + struct CallbackContext *ctx = (struct CallbackContext *)ctx_;
> + printf("Called %d time(s)", ++ctx->counter);
> +}
> +
> +void init() {
> + struct CallbackContext ctx;
> + ctx.counter = 0;
> + register_callback( my_callback, &ctx );
> +}
> +```
> +
> +The difference here is subtle, but important. If a piece of code
> +*appears compatible with contexts*, then you are *allowed to think
> +that way*, but if a piece of code *explicitly states it uses
> +contexts*, then you are *required to follow that approach*.
> +
> +For example, imagine someone modified `MyStruct` in the earlier example
> +to count several unrelated events across the whole program. That would mean
> +it contained information about multiple entities, so was not a context.
> +But nobody ever *said* it was a context, so that isn't necessarily wrong.
> +However, proposing the same change to the `CallbackContext` in the later
> example
> +would violate a guarantee, and should be pointed out in a code review.
> +
I'm not very convinced by the callback example. The use of contexts in
the FFmpeg API is very much simpler, it is used to keep track of
configuration and state (that is they track the "object" where to
operate on), so the callback example here is a bit misleading.
Callbacks are used in the internals to implement different elements
(codecs, protocols, filters, etc...) implementing a common API, but in
this case the relation with "contexts" is less straightforward.
> +@warning Guaranteeing to use contexts does not mean guaranteeing to use
> +o
Re: [FFmpeg-devel] [PATCH v2] lavf/dash: Forward strict flag to component demuxers
Frank Plowman:
> Before the patch, opening a DASH file containing streams which require
> experimental decoders was problematic. No matter where the -strict -2
> was put on the command line, the option was not passed to the demuxer
> for that component. This resulted in an error, prompting the user to
> add the -strict -2 flag, which is already present. Decoding appeared to
> continue correctly however.
>
> Patch removes the error message by creating an options object for the
> demuxer created for the component, which inherits from the parent
> demuxer.
>
> Signed-off-by: Frank Plowman
> ---
> Changes since v1: Use nb_streams stream_info_opts, not just one.
>
> libavformat/dashdec.c | 19 +--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 555e21bf69..9c5ef5801a 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -1911,13 +1911,28 @@ static int reopen_demux_for_component(AVFormatContext
> *s, struct representation
> if (ret < 0)
> goto fail;
> if (pls->n_fragments) {
> +AVDictionary **stream_info_opts;
> +
> +stream_info_opts = av_calloc(pls->ctx->nb_streams,
> sizeof(*stream_info_opts));
> +if (!stream_info_opts)
> +goto fail;
Presumably you should set ret here.
> +
> #if FF_API_R_FRAME_RATE
> if (pls->framerate.den) {
> -for (i = 0; i < pls->ctx->nb_streams; i++)
> +for (i = 0; i < pls->ctx->nb_streams; i++) {
> pls->ctx->streams[i]->r_frame_rate = pls->framerate;
> +av_dict_set_int(&stream_info_opts[i], "strict",
> s->strict_std_compliance, 0);
You are setting this in a loop intended to be removed; and one that is
furthermore only executed under certain conditions. This is surely not
what you intended.
> +}
> +
> }
> #endif
> -ret = avformat_find_stream_info(pls->ctx, NULL);
> +
> +ret = avformat_find_stream_info(pls->ctx, stream_info_opts);
> +
> +for (i = 0; i < pls->ctx->nb_streams; i++)
We prefer loop-based iterators nowadays.
> +av_dict_free(&stream_info_opts[i]);
> +av_dict_free(stream_info_opts);
This is the wrong deallocator; stream_info_opts will leak.
> +
> if (ret < 0)
> goto fail;
> }
Apart from that my comment about using
AVFormatContext.stridt_std_compliance still applies.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/dash: Forward strict flag to component demuxers
Thanks for your review Andreas.
On 20/05/2024 21:41, Andreas Rheinhardt wrote:
> Frank Plowman:
>> Before the patch, opening a DASH file containing streams which require
>> experimental decoders was problematic. No matter where the -strict -2
>> was put on the command line, the option was not passed to the demuxer
>> for that component. This resulted in an error, prompting the user to
>> add the -strict -2 flag, which is already present. Decoding appeared to
>> continue correctly however.
>>
>> Patch removes the error message by creating an options object for the
>> demuxer created for the component, which inherits from the parent
>> demuxer.
>>
>> Signed-off-by: Frank Plowman
>> ---
>> PS: Can anyone think of other options which should be propagated to the
>> component demuxers?
>>
>> libavformat/dashdec.c | 7 ++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
>> index 555e21bf69..40abb5ebba 100644
>> --- a/libavformat/dashdec.c
>> +++ b/libavformat/dashdec.c
>> @@ -1911,13 +1911,18 @@ static int
>> reopen_demux_for_component(AVFormatContext *s, struct representation
>> if (ret < 0)
>> goto fail;
>> if (pls->n_fragments) {
>> +AVDictionary *stream_info_opts = NULL;
>> +
>> #if FF_API_R_FRAME_RATE
>> if (pls->framerate.den) {
>> for (i = 0; i < pls->ctx->nb_streams; i++)
>> pls->ctx->streams[i]->r_frame_rate = pls->framerate;
>> }
>> #endif
>> -ret = avformat_find_stream_info(pls->ctx, NULL);
>> +
>> +av_dict_set_int(&stream_info_opts, "strict",
>> s->strict_std_compliance, 0);
>> +
>> +ret = avformat_find_stream_info(pls->ctx, &stream_info_opts);
>> if (ret < 0)
>> goto fail;
>> }
>
> The loop over pls->ctx indicates that pls->ctx->nb_streams can be > 1
> before avformat_find_stream_info(). But then using a single AVDictionary
> is wrong, as avformat_find_stream_info() expects an array of
> pls->ctx->nb_streams AVDictionary*.
Thanks, v2 sent which addresses this.
>
> Furthermore, the mixing between AVFormatContext and AVCodecContext
> options here does not seem good (e.g. for ordinary demuxers setting
> strict_std_compliance does not affect the AVCodecContext's values at all).
Can you see an alternative? In the case of fftools, the -strict flag is
always set for both the codec_opts and format_opts but yes I see the
concern assuming this logic in libav*.
--
Frank
___
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] lavf/dash: Forward strict flag to component demuxers
Before the patch, opening a DASH file containing streams which require
experimental decoders was problematic. No matter where the -strict -2
was put on the command line, the option was not passed to the demuxer
for that component. This resulted in an error, prompting the user to
add the -strict -2 flag, which is already present. Decoding appeared to
continue correctly however.
Patch removes the error message by creating an options object for the
demuxer created for the component, which inherits from the parent
demuxer.
Signed-off-by: Frank Plowman
---
Changes since v1: Use nb_streams stream_info_opts, not just one.
libavformat/dashdec.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 555e21bf69..9c5ef5801a 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1911,13 +1911,28 @@ static int reopen_demux_for_component(AVFormatContext
*s, struct representation
if (ret < 0)
goto fail;
if (pls->n_fragments) {
+AVDictionary **stream_info_opts;
+
+stream_info_opts = av_calloc(pls->ctx->nb_streams,
sizeof(*stream_info_opts));
+if (!stream_info_opts)
+goto fail;
+
#if FF_API_R_FRAME_RATE
if (pls->framerate.den) {
-for (i = 0; i < pls->ctx->nb_streams; i++)
+for (i = 0; i < pls->ctx->nb_streams; i++) {
pls->ctx->streams[i]->r_frame_rate = pls->framerate;
+av_dict_set_int(&stream_info_opts[i], "strict",
s->strict_std_compliance, 0);
+}
+
}
#endif
-ret = avformat_find_stream_info(pls->ctx, NULL);
+
+ret = avformat_find_stream_info(pls->ctx, stream_info_opts);
+
+for (i = 0; i < pls->ctx->nb_streams; i++)
+av_dict_free(&stream_info_opts[i]);
+av_dict_free(stream_info_opts);
+
if (ret < 0)
goto fail;
}
--
2.44.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 5/5] avformat/riffenc: Fix outdated comment
Signed-off-by: Andreas Rheinhardt --- libavformat/riffenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/riffenc.c b/libavformat/riffenc.c index 8accb69541..59c9932c36 100644 --- a/libavformat/riffenc.c +++ b/libavformat/riffenc.c @@ -72,7 +72,7 @@ int ff_put_wav_header(AVFormatContext *s, AVIOContext *pb, } /* We use the known constant frame size for the codec if known, otherwise - * fall back on using AVCodecContext.frame_size, which is not as reliable + * fall back on using AVCodecParameters.frame_size, which is not as reliable * for indicating packet duration. */ frame_size = av_get_audio_frame_duration2(par, par->block_align); -- 2.40.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: Check ff_put_wav_header() failure
Fixes Coverity issue #1506706. Signed-off-by: Andreas Rheinhardt --- libavformat/matroskaenc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 63bae9fe1c..76c542d50b 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1250,7 +1250,9 @@ static int mkv_assemble_codecprivate(AVFormatContext *s, AVIOContext *dyn_cp, par->codec_tag = tag; /* Same comment as for ff_put_bmp_header applies here. */ -ff_put_wav_header(s, dyn_cp, par, FF_PUT_WAV_HEADER_FORCE_WAVEFORMATEX); +ret = ff_put_wav_header(s, dyn_cp, par, FF_PUT_WAV_HEADER_FORCE_WAVEFORMATEX); +if (ret < 0) +return ret; #endif } -- 2.40.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/5] avfilter/af_atempo: Fix indentation
Forgotten after b8f74ee57a6e9e75a507adcddf30f84dd4a39d39.
Signed-off-by: Andreas Rheinhardt
---
libavfilter/af_atempo.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
index 110f792eec..3658348c45 100644
--- a/libavfilter/af_atempo.c
+++ b/libavfilter/af_atempo.c
@@ -998,20 +998,20 @@ static av_cold void uninit(AVFilterContext *ctx)
yae_release_buffers(atempo);
}
-// WSOLA necessitates an internal sliding window ring buffer
-// for incoming audio stream.
-//
-// Planar sample formats are too cumbersome to store in a ring buffer,
-// therefore planar sample formats are not supported.
-//
-static const enum AVSampleFormat sample_fmts[] = {
-AV_SAMPLE_FMT_U8,
-AV_SAMPLE_FMT_S16,
-AV_SAMPLE_FMT_S32,
-AV_SAMPLE_FMT_FLT,
-AV_SAMPLE_FMT_DBL,
-AV_SAMPLE_FMT_NONE
-};
+// WSOLA necessitates an internal sliding window ring buffer
+// for incoming audio stream.
+//
+// Planar sample formats are too cumbersome to store in a ring buffer,
+// therefore planar sample formats are not supported.
+//
+static const enum AVSampleFormat sample_fmts[] = {
+AV_SAMPLE_FMT_U8,
+AV_SAMPLE_FMT_S16,
+AV_SAMPLE_FMT_S32,
+AV_SAMPLE_FMT_FLT,
+AV_SAMPLE_FMT_DBL,
+AV_SAMPLE_FMT_NONE
+};
static int config_props(AVFilterLink *inlink)
{
--
2.40.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting
The earlier code distinguished between a partial reset
(yae_clear()) and a complete reset (yae_release_buffers()
which also releases the buffers); this separation existed
to avoid allocations, as buffers were reallocated on reconfigs.
Yet it is pointless since a5704659e3e41b7698812b89f67d9a7467a74d20,
so simply use yae_release_buffers() everywhere.
Signed-off-by: Andreas Rheinhardt
---
libavfilter/af_atempo.c | 69 +
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
index 1aedb82060..110f792eec 100644
--- a/libavfilter/af_atempo.c
+++ b/libavfilter/af_atempo.c
@@ -244,18 +244,6 @@ static void yae_release_buffers(ATempoContext *atempo)
av_tx_uninit(&atempo->complex_to_real);
}
-/* av_realloc is not aligned enough; fortunately, the data does not need to
- * be preserved */
-#define RE_MALLOC_OR_FAIL(field, field_size, element_size) \
-do {\
-av_freep(&field); \
-field = av_calloc(field_size, element_size);\
-if (!field) { \
-yae_release_buffers(atempo);\
-return AVERROR(ENOMEM); \
-} \
-} while (0)
-
/**
* Prepare filter for processing audio data of given format,
* sample rate and number of channels.
@@ -289,40 +277,51 @@ static int yae_reset(ATempoContext *atempo,
nlevels++;
}
+/* av_realloc is not aligned enough, so simply discard all the old buffers
+ * (fortunately, their data does not need to be preserved) */
+yae_release_buffers(atempo);
+
// initialize audio fragment buffers:
-RE_MALLOC_OR_FAIL(atempo->frag[0].data, atempo->window, atempo->stride);
-RE_MALLOC_OR_FAIL(atempo->frag[1].data, atempo->window, atempo->stride);
-RE_MALLOC_OR_FAIL(atempo->frag[0].xdat_in, (atempo->window + 1),
sizeof(AVComplexFloat));
-RE_MALLOC_OR_FAIL(atempo->frag[1].xdat_in, (atempo->window + 1),
sizeof(AVComplexFloat));
-RE_MALLOC_OR_FAIL(atempo->frag[0].xdat, (atempo->window + 1),
sizeof(AVComplexFloat));
-RE_MALLOC_OR_FAIL(atempo->frag[1].xdat, (atempo->window + 1),
sizeof(AVComplexFloat));
+if (!(atempo->frag[0].data= av_calloc(atempo->window, atempo->stride))
||
+!(atempo->frag[1].data= av_calloc(atempo->window, atempo->stride))
||
+!(atempo->frag[0].xdat_in = av_calloc(atempo->window + 1,
sizeof(AVComplexFloat))) ||
+!(atempo->frag[1].xdat_in = av_calloc(atempo->window + 1,
sizeof(AVComplexFloat))) ||
+!(atempo->frag[0].xdat= av_calloc(atempo->window + 1,
sizeof(AVComplexFloat))) ||
+!(atempo->frag[1].xdat= av_calloc(atempo->window + 1,
sizeof(AVComplexFloat {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
// initialize rDFT contexts:
-av_tx_uninit(&atempo->real_to_complex);
-av_tx_uninit(&atempo->complex_to_real);
-
ret = av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn,
AV_TX_FLOAT_RDFT, 0, 1 << (nlevels + 1), &scale, 0);
-if (ret < 0) {
-yae_release_buffers(atempo);
-return ret;
-}
+if (ret < 0)
+goto fail;
ret = av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn,
AV_TX_FLOAT_RDFT, 1, 1 << (nlevels + 1), &iscale, 0);
-if (ret < 0) {
-yae_release_buffers(atempo);
-return ret;
-}
+if (ret < 0)
+goto fail;
-RE_MALLOC_OR_FAIL(atempo->correlation_in, (atempo->window + 1),
sizeof(AVComplexFloat));
-RE_MALLOC_OR_FAIL(atempo->correlation, atempo->window,
sizeof(AVComplexFloat));
+if (!(atempo->correlation_in = av_calloc(atempo->window + 1,
sizeof(AVComplexFloat))) ||
+!(atempo->correlation= av_calloc(atempo->window,
sizeof(AVComplexFloat {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
atempo->ring = atempo->window * 3;
-RE_MALLOC_OR_FAIL(atempo->buffer, atempo->ring, atempo->stride);
+atempo->buffer = av_calloc(atempo->ring, atempo->stride);
+if (!atempo->buffer) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
// initialize the Hann window function:
-RE_MALLOC_OR_FAIL(atempo->hann, atempo->window, sizeof(float));
+atempo->hann = av_malloc_array(atempo->window, sizeof(float));
+if (!atempo->hann) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
for (i = 0; i < atempo->window; i++) {
double t = (double)i / (double)(atempo->window - 1);
@@ -330,8 +329,10 @@ static int yae_reset(ATempoContext *atempo,
atempo->hann[i] = (float)h;
}
-yae_clear(atempo);
return 0;
+fail:
+yae_release_buffers(atempo);
+return ret;
}
static int yae_update(AVFilte
[FFmpeg-devel] [PATCH 1/5] avfilter/af_atempo: Properly check av_tx_init()
Fixes Coverity issue #1516804.
Signed-off-by: Andreas Rheinhardt
---
libavfilter/af_atempo.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
index 0c36eb4dd7..1aedb82060 100644
--- a/libavfilter/af_atempo.c
+++ b/libavfilter/af_atempo.c
@@ -269,6 +269,7 @@ static int yae_reset(ATempoContext *atempo,
uint32_t nlevels = 0;
float scale = 1.f, iscale = 1.f;
uint32_t pot;
+int ret;
int i;
atempo->format = format;
@@ -300,16 +301,18 @@ static int yae_reset(ATempoContext *atempo,
av_tx_uninit(&atempo->real_to_complex);
av_tx_uninit(&atempo->complex_to_real);
-av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn, AV_TX_FLOAT_RDFT, 0,
1 << (nlevels + 1), &scale, 0);
-if (!atempo->real_to_complex) {
+ret = av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn,
+ AV_TX_FLOAT_RDFT, 0, 1 << (nlevels + 1), &scale, 0);
+if (ret < 0) {
yae_release_buffers(atempo);
-return AVERROR(ENOMEM);
+return ret;
}
-av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn, AV_TX_FLOAT_RDFT, 1,
1 << (nlevels + 1), &iscale, 0);
-if (!atempo->complex_to_real) {
+ret = av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn,
+ AV_TX_FLOAT_RDFT, 1, 1 << (nlevels + 1), &iscale, 0);
+if (ret < 0) {
yae_release_buffers(atempo);
-return AVERROR(ENOMEM);
+return ret;
}
RE_MALLOC_OR_FAIL(atempo->correlation_in, (atempo->window + 1),
sizeof(AVComplexFloat));
--
2.40.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
