Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
Le 2 mai 2024 21:38:13 GMT+03:00, "Ronald S. Bultje" a écrit : >Hi, > >On Thu, May 2, 2024 at 1:44 PM Vittorio Giovara >wrote: > >> I believe the path forward would be designing a system that can accommodate >> both workflows >> > >I agree with this. I vehemently disagree with this. Unless you are volunteering to write such a tool, this is wishful thinking and the result is that we stick to the mailing list workflow. ___ 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] 5 year plan & Inovation
Le 2 mai 2024 22:32:16 GMT+03:00, "Ondřej Fiala" a écrit : >On Thu May 2, 2024 at 4:38 PM CEST, Rémi Denis-Courmont wrote: >> Le torstaina 2. toukokuuta 2024, 17.25.06 EEST Ondřej Fiala a écrit : >> > On Wed May 1, 2024 at 7:27 AM CEST, Rémi Denis-Courmont wrote: >> > > I don't use Gmail, and using email for review still sucks. No matter how >> > > you slice it, email was not meant for threaded code reviews. >> > >> > Email was not meant for a lot of what it's used for today. >> >> And Gitlab and Github are meant for what they are used. >> That's the whole point. >This argument can actually go in both directions No, it can't. > Since the Web and web >browsers weren't meant for performing code review either. I was obviously and explicitly talking about Github and Gitlab web applications, not the browsers. You're being ridiculous. Your OS was originally meant to run bash, not a mail client, by that logic. And in the end, I could be wrong, but I haven't seen you doing much code review here. This is all about optimising the workflow for people doing code reviews and code merges, so why do you even care? ___ 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 v1] scale: Bring back the old yuv2yuvX, use it when disable-x86asm.
于2024年4月26日周五 20:21写道：
>
> From: huheng
>
> rename old inline yuv2yuvX to yuv2yuv_X, to avoid conflicts with
> the names of standalone asm functions. When ffmpeg is compiled with
> --disable-x86asm, using the scale function will cause the video to
> be blurred. The reason is that when disable-x86asm, INLINE_MMXEXT
> is 1 and use_mmx_vfilter is 1, but c->yuv2planeX uses the c language
> version, which causes a problem of mismatch with the vfilter. This
> problem has persisted from version 4.4 to the present. Fix it by using
> inline yuv2yuv_X_mmxext, that can maintain the consistency of
> use_mmx_vfilter.
>
> reproduce the issue:
> 1. ./configure --disable-x86asm --enable-gpl --enable-libx264
> 2. ./ffmpeg -i input.mp4 -vf "scale=1280x720" -c:v libx264 output.mp4
> the output.mp4 is abnormal
>
> Signed-off-by: huheng
> ---
> libswscale/x86/swscale.c | 6 +++-
> libswscale/x86/swscale_template.c | 53 +++
> 2 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
> index ff16398988..1bb9d1d51a 100644
> --- a/libswscale/x86/swscale.c
> +++ b/libswscale/x86/swscale.c
> @@ -452,8 +452,12 @@ av_cold void ff_sws_init_swscale_x86(SwsContext *c)
> int cpu_flags = av_get_cpu_flags();
>
> #if HAVE_MMXEXT_INLINE
> -if (INLINE_MMXEXT(cpu_flags))
> +if (INLINE_MMXEXT(cpu_flags)) {
> sws_init_swscale_mmxext(c);
> +if (c->use_mmx_vfilter && !(c->flags & SWS_ACCURATE_RND)) {
> +c->yuv2planeX = yuv2yuv_X_mmxext;
> +}
> +}
> #endif
> if(c->use_mmx_vfilter && !(c->flags & SWS_ACCURATE_RND)) {
> #if HAVE_MMXEXT_EXTERNAL
> diff --git a/libswscale/x86/swscale_template.c
> b/libswscale/x86/swscale_template.c
> index 6190fcb4fe..1b8794480d 100644
> --- a/libswscale/x86/swscale_template.c
> +++ b/libswscale/x86/swscale_template.c
> @@ -33,6 +33,59 @@
> #define MOVNTQ2 "movntq "
> #define MOVNTQ(a,b) REAL_MOVNTQ(a,b)
>
> +static void RENAME(yuv2yuv_X)(const int16_t *filter, int filterSize,
> + const int16_t **src, uint8_t *dest, int dstW,
> + const uint8_t *dither, int offset)
> +{
> +filterSize--;
> +__asm__ volatile(
> +"movd %0, %%mm1\n\t"
> +"punpcklwd %%mm1, %%mm1\n\t"
> +"punpckldq %%mm1, %%mm1\n\t"
> +"psllw$3, %%mm1\n\t"
> +"paddw %%mm1, %%mm3\n\t"
> +"paddw %%mm1, %%mm4\n\t"
> +"psraw$4, %%mm3\n\t"
> +"psraw$4, %%mm4\n\t"
> +::"m"(filterSize)
> + );
> +
> +__asm__ volatile(\
> +"movq%%mm3, %%mm6\n\t"
> +"movq%%mm4, %%mm7\n\t"
> +"movl %3, %%ecx\n\t"
> +"mov %0, %%"FF_REG_d" \n\t"\
> +"mov(%%"FF_REG_d"), %%"FF_REG_S"\n\t"\
> +".p2align 4 \n\t" /*
> FIXME Unroll? */\
> +"1: \n\t"\
> +"movq 8(%%"FF_REG_d"), %%mm0 \n\t" /*
> filterCoeff */\
> +"movq(%%"FF_REG_S", %%"FF_REG_c", 2), %%mm2 \n\t" /*
> srcData */\
> +"movq 8(%%"FF_REG_S", %%"FF_REG_c", 2), %%mm5 \n\t" /*
> srcData */\
> +"add$16, %%"FF_REG_d" \n\t"\
> +"mov(%%"FF_REG_d"), %%"FF_REG_S"\n\t"\
> +"test %%"FF_REG_S", %%"FF_REG_S"\n\t"\
> +"pmulhw %%mm0, %%mm2 \n\t"\
> +"pmulhw %%mm0, %%mm5 \n\t"\
> +"paddw%%mm2, %%mm3 \n\t"\
> +"paddw%%mm5, %%mm4 \n\t"\
> +" jnz1b \n\t"\
> +"psraw $3, %%mm3 \n\t"\
> +"psraw $3, %%mm4 \n\t"\
> +"packuswb %%mm4, %%mm3 \n\t"
> +MOVNTQ2 " %%mm3, (%1, %%"FF_REG_c")\n\t"
> +"add $8, %%"FF_REG_c" \n\t"\
> +"cmp %2, %%"FF_REG_c" \n\t"\
> +"movq%%mm6, %%mm3\n\t"
> +"movq%%mm7, %%mm4\n\t"
> +"mov %0, %%"FF_REG_d" \n\t"\
> +"mov(%%"FF_REG_d"), %%"FF_REG_S" \n\t"\
> +"jb 1b \n\t"\
> +:: "g" (filter),
> + "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m" (offset)
> +: "%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
> +);
> +}
> +
> #define YSCALEYUV2PACKEDX_UV \
> __asm__ volatile(\
> "xor%%"FF_REG_a", %%"FF_REG_a" \n\t"\
> --
> 2.20.1
Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: only check index_edit_rate when calculating the index tables
On Thu, May 02, 2024 at 11:01:02PM +0200, Marton Balint wrote: [...] > If we add a new FATE file for every fixed file or workflow, the amount of > FATE samples (and the time fate will run) will increase significantly, I am > not sure that is intended. In this case, I could only craft an MXF file, > because I don't have access to the software. IMO add as many tests as you like. If we really hit a "too many tests" situation, then we can look at them all and remove the most expensive/least usefull tests. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When you are offended at any man's fault, turn to yourself and study your own failings. Then you will forget your anger. -- Epictetus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] avformat/framecrcenc: compute the checksum for side data
Hi On Thu, May 02, 2024 at 11:23:13PM +0200, Marton Balint wrote: [...] > surely some users depend on it behaving in a certain > way... who ? I dont think you should assume something without evidence especially for framecrc, which isnt that usefull except for testing the output is changed vs identical. This patch (or a variation of it) improves this very usecase Also whats your oppinion Marton ? should we test side data or not test side data. In teh tests in FFmpeg (FATE) ? And framecrc is what tests the produced AVPackets I think james replied to the rest already thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No great genius has ever existed without some touch of madness. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12
On Wed, May 01, 2024 at 09:56:53PM -0300, James Almer wrote:
> On 5/1/2024 9:41 PM, Michael Niedermayer wrote:
> > Fixes: CID1544265 Logically dead code
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer
> > ---
> > libavcodec/av1dec.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> > index 79a30a114da..4f9222cca27 100644
> > --- a/libavcodec/av1dec.c
> > +++ b/libavcodec/av1dec.c
> > @@ -493,7 +493,7 @@ static enum AVPixelFormat get_sw_pixel_format(void
> > *logctx,
> > else if (bit_depth == 12)
> > pix_fmt = AV_PIX_FMT_YUV444P12;
> > else
> > -av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel
> > format.\n");
> > +av_assert0(0);
> > } else if (seq->color_config.subsampling_x == 1 &&
> > seq->color_config.subsampling_y == 0) {
> > if (bit_depth == 8)
> > @@ -503,7 +503,7 @@ static enum AVPixelFormat get_sw_pixel_format(void
> > *logctx,
> > else if (bit_depth == 12)
> > pix_fmt = AV_PIX_FMT_YUV422P12;
> > else
> > -av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel
> > format.\n");
> > +av_assert0(0);
> > } else if (seq->color_config.subsampling_x == 1 &&
> > seq->color_config.subsampling_y == 1) {
> > if (bit_depth == 8)
> > @@ -513,7 +513,7 @@ static enum AVPixelFormat get_sw_pixel_format(void
> > *logctx,
> > else if (bit_depth == 12)
> > pix_fmt = AV_PIX_FMT_YUV420P12;
> > else
> > -av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel
> > format.\n");
> > +av_assert0(0);
> > }
> > } else {
> > if (bit_depth == 8)
> > @@ -523,7 +523,7 @@ static enum AVPixelFormat get_sw_pixel_format(void
> > *logctx,
> > else if (bit_depth == 12)
> > pix_fmt = AV_PIX_FMT_GRAY12;
> > else
> > -av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> > +av_assert0(0);
> > }
> > return pix_fmt;
>
> LGTM.
will apply
>
> Can you change bit_depth into an int while at it? no reason for it to be
> uint8_t as a scalar.
ok, but its unrelated
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test
On Thu, May 02, 2024 at 08:58:17AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1439654 Untrusted pointer read
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer
> > ---
> > libavcodec/cbs_jpeg.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> > index b1b58dcd65e..406147c082c 100644
> > --- a/libavcodec/cbs_jpeg.c
> > +++ b/libavcodec/cbs_jpeg.c
> > @@ -146,13 +146,13 @@ static int
> > cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
> > }
> > } else {
> > i = start;
> > -if (i + 2 > frag->data_size) {
> > +if (i > frag->data_size - 2) {
> > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> > "truncated at %02x marker.\n", marker);
> > return AVERROR_INVALIDDATA;
> > }
> > length = AV_RB16(frag->data + i);
> > -if (i + length > frag->data_size) {
> > +if (length > frag->data_size - i) {
> > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> > "truncated at %02x marker segment.\n", marker);
> > return AVERROR_INVALIDDATA;
>
> You should always mention when you are not fixing bugs in our code, but
> rather intend to apply workaround for coverity crazyness (i.e. the
> requirement that reading values in non-native endianness needs to be
> sanitized).
writing code like
if (i + length > frag->data_size)
is IMHO not proper and that has nothing to do with coverity
the reason why this is not proper is that i + length could in
principle overflow. It depends on teh whole loop and calling code
if that is possible or not.
to check length, length should be alone on one side of the check
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is what and why we do it that matters, not just one of them.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 5/7] avcodec/avs3_parser: Check the return value of init_get_bits8()
On Thu, May 02, 2024 at 10:28:36AM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: CID1492867 Unchecked return value > > > > Sponsored-by: Sovereign Tech Fund > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/avs3_parser.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/avs3_parser.c b/libavcodec/avs3_parser.c > > index a819b5783d6..0f9076befe1 100644 > > --- a/libavcodec/avs3_parser.c > > +++ b/libavcodec/avs3_parser.c > > @@ -73,7 +73,9 @@ static void parse_avs3_nal_units(AVCodecParserContext *s, > > const uint8_t *buf, > > GetBitContext gb; > > int profile, ratecode, low_delay; > > > > -init_get_bits8(, buf + 4, buf_size - 4); > > +int ret = init_get_bits8(, buf + 4, buf_size - 4); > > +if (ret < 0) > > +return; > > > > s->key_frame = 1; > > s->pict_type = AV_PICTURE_TYPE_I; > > This code only reads/skips a few bits here (at most 100 if I counted > correctly), so one could initialize the reader for a shorter length and > assert that the call succeeds. will apply: -init_get_bits8(, buf + 4, buf_size - 4); +av_unused int ret = init_get_bits(, buf + 4, 100); +av_assert1(ret >= 0); thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
On Thu, May 02, 2024 at 11:34:48PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Thu, May 02, 2024 at 09:12:36AM +0200, Andreas Rheinhardt wrote:
> >> Michael Niedermayer:
> >>> Fixes: CID1596605 Uninitialized scalar variable
> >>>
> >>> Sponsored-by: Sovereign Tech Fund
> >>> Signed-off-by: Michael Niedermayer
> >>> ---
> >>> libavcodec/av1dec.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> >>> index 4f9222cca27..93ab04eb378 100644
> >>> --- a/libavcodec/av1dec.c
> >>> +++ b/libavcodec/av1dec.c
> >>> @@ -1262,7 +1262,7 @@ static int
> >>> av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >>> {
> >>> AV1DecContext *s = avctx->priv_data;
> >>> AV1RawTileGroup *raw_tile_group = NULL;
> >>> -int i = 0, ret;
> >>> +int i = 0, ret = 0;
> >>>
> >>> for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
> >>> CodedBitstreamUnit *unit = >current_obu.units[i];
> >>
> >> A better approach is to actually initialize ret before every goto end in
> >> order to ensure that only the actually desired ret is returned and not
> >> some earlier value.
> >
> > ok, will apply with ret = 0 before the 2 goto end lacking it that i see
> >
>
> I already sent a patch for this.
ok, ill drop mine then
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] avformat/movenc: Avoid loop for writing array
Signed-off-by: Andreas Rheinhardt
---
libavformat/movenc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index b4c1db2774..f907f67752 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1244,8 +1244,7 @@ static int mov_write_chnl_tag(AVFormatContext *s,
AVIOContext *pb, MOVTrack *tra
if (config) {
avio_wb64(pb, 0);
} else {
-for (int i = 0; i < layout->nb_channels; i++)
-avio_w8(pb, speaker_pos[i]);
+avio_write(pb, speaker_pos, layout->nb_channels);
av_freep(_pos);
}
--
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 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
Michael Niedermayer:
> On Thu, May 02, 2024 at 09:12:36AM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> Fixes: CID1596605 Uninitialized scalar variable
>>>
>>> Sponsored-by: Sovereign Tech Fund
>>> Signed-off-by: Michael Niedermayer
>>> ---
>>> libavcodec/av1dec.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>> index 4f9222cca27..93ab04eb378 100644
>>> --- a/libavcodec/av1dec.c
>>> +++ b/libavcodec/av1dec.c
>>> @@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext
>>> *avctx, AVFrame *frame)
>>> {
>>> AV1DecContext *s = avctx->priv_data;
>>> AV1RawTileGroup *raw_tile_group = NULL;
>>> -int i = 0, ret;
>>> +int i = 0, ret = 0;
>>>
>>> for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
>>> CodedBitstreamUnit *unit = >current_obu.units[i];
>>
>> A better approach is to actually initialize ret before every goto end in
>> order to ensure that only the actually desired ret is returned and not
>> some earlier value.
>
> ok, will apply with ret = 0 before the 2 goto end lacking it that i see
>
I already sent a patch for this.
- 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 1/2] avformat/movenc: Check av_malloc()
Fixes Coverity issue #1596735.
Signed-off-by: Andreas Rheinhardt
---
libavformat/movenc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index e9bbfd67cf..b4c1db2774 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1221,6 +1221,8 @@ static int mov_write_chnl_tag(AVFormatContext *s,
AVIOContext *pb, MOVTrack *tra
if (ret || !config) {
config = 0;
speaker_pos = av_malloc(layout->nb_channels);
+if (!speaker_pos)
+return AVERROR(ENOMEM);
ret = ff_mov_get_channel_positions_from_layout(layout,
speaker_pos, layout->nb_channels);
if (ret) {
--
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 4/7] avcodec/avs2_parser: Assert init_get_bits8() success with const size 15
On Thu, May 02, 2024 at 10:25:24AM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: CID1506708 Unchecked return value > > > > Sponsored-by: Sovereign Tech Fund > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/avs2_parser.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/avs2_parser.c b/libavcodec/avs2_parser.c > > index 200134f91db..8d4bc3cee0d 100644 > > --- a/libavcodec/avs2_parser.c > > +++ b/libavcodec/avs2_parser.c > > @@ -72,13 +72,15 @@ static void parse_avs2_seq_header(AVCodecParserContext > > *s, const uint8_t *buf, > > unsigned aspect_ratio; > > unsigned frame_rate_code; > > int low_delay; > > +int ret; > > // update buf_size_min if parse more deeper > > const int buf_size_min = 15; > > > > if (buf_size < buf_size_min) > > return; > > > > -init_get_bits8(, buf, buf_size_min); > > +ret = init_get_bits8(, buf, buf_size_min); > > +av_assert2(ret >= 0); > > > > s->key_frame = 1; > > s->pict_type = AV_PICTURE_TYPE_I; > > Code like this may trigger set-but-unused variable warnings when > ASSERT_LEVEL is <= 1. Add av_unused to ret to fix this. > Apart from that, it should be av_assert1 (this is not hot). will apply with these changes thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
On Thu, May 02, 2024 at 09:12:36AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1596605 Uninitialized scalar variable
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer
> > ---
> > libavcodec/av1dec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> > index 4f9222cca27..93ab04eb378 100644
> > --- a/libavcodec/av1dec.c
> > +++ b/libavcodec/av1dec.c
> > @@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext
> > *avctx, AVFrame *frame)
> > {
> > AV1DecContext *s = avctx->priv_data;
> > AV1RawTileGroup *raw_tile_group = NULL;
> > -int i = 0, ret;
> > +int i = 0, ret = 0;
> >
> > for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
> > CodedBitstreamUnit *unit = >current_obu.units[i];
>
> A better approach is to actually initialize ret before every goto end in
> order to ensure that only the actually desired ret is returned and not
> some earlier value.
ok, will apply with ret = 0 before the 2 goto end lacking it that i see
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] avformat/framecrcenc: compute the checksum for side data
On 5/2/2024 6:23 PM, Marton Balint wrote: On Wed, 1 May 2024, Michael Niedermayer wrote: This allows detecting issues in side data related code, same as what framecrc does for before already for packet data itself. This basically reverts c6ae560a18d67b9ddaa25a0338b7fb55e3312e57. Can you at least add an option which allows disabling dumping the side data? Changing the format of framecrc output again and again is not very The framehash/framemd5 muxer is versioned, which is what you should use if you want parseable output. nice, as it is a public muxer, surely some users depend on it behaving in a certain way... Also some feedback from Anton would be nice, as he made the original commit which removed side data support... Anton afaik is on holidays, but agree, this patch can wait a bit. ___ 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/mxfdec: only check index_edit_rate when calculating the index tables
On Mon, 29 Apr 2024, Tomas Härdin wrote: mån 2024-04-15 klockan 21:34 +0200 skrev Marton Balint: Commit ed49391961999f028e0bc55767d0eef6eeb15e49 started rejecting negative index segment edit rates to avoid negative av_rescale parameters. There are two problems with this: 1) there is already a validation for zero (uninitialized) rates later on 2) it rejects files with 0/0 index edit rates which do exist and we should continue to support those There are no such files in FATE last time I checked. At the very least we need to know which vendor is producing such broken files. Without tests covering the workflows we want to support, we have no confidence in refactoriing. This leads to cargo culting. And to be sure, every workflow we support means a non-trivial amount of work, especially when it comes to MXF. I can add a comment to the code or the commit message, we did this plenty of times in the past. The broken software is Marquise Technologies MT Mediabase 4.7.2 by the way. I can also rework the patch to keep rejecting negative values and only allow zero, as that is the only thing I *know* to exist. If we add a new FATE file for every fixed file or workflow, the amount of FATE samples (and the time fate will run) will increase significantly, I am not sure that is intended. In this case, I could only craft an MXF file, because I don't have access to the software. Regards, 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".
Re: [FFmpeg-devel] [PATCH 01/10, v2] avutil: add hwcontext_amf.
On 01/05/2024 19:38, Dmitrii Ovchinnikov wrote:
> Adds hwcontext_amf, which allows to use shared AMF
> context for the encoder, decoder and AMF-based filters,
> without copy to the host memory.
> It will also allow you to use some optimizations in
> the interaction of components (for example, SAV) and make a more
> manageable and optimal setup for using GPU devices with AMF
> in the case of a fully AMF pipeline.
> It will be a significant performance uplift when full AMF pipeline
> with filters is used.
>
> We also plan to add Compression artefact removal filter in near feature.
> v2: cleanup header files, fixed naming and formats
> ---
> libavutil/Makefile | 4 +
> libavutil/hwcontext.c | 4 +
> libavutil/hwcontext.h | 1 +
> libavutil/hwcontext_amf.c | 588 +
> libavutil/hwcontext_amf.h | 41 ++
> libavutil/hwcontext_amf_internal.h | 77
> libavutil/hwcontext_internal.h | 1 +
> libavutil/pixdesc.c| 4 +
> libavutil/pixfmt.h | 5 +
> 9 files changed, 725 insertions(+)
> create mode 100644 libavutil/hwcontext_amf.c
> create mode 100644 libavutil/hwcontext_amf.h
> create mode 100644 libavutil/hwcontext_amf_internal.h
>
> ...
> diff --git a/libavutil/hwcontext_amf.c b/libavutil/hwcontext_amf.c
> new file mode 100644
> index 00..8edfb20fbb
> --- /dev/null
> +++ b/libavutil/hwcontext_amf.c
> ...
> +
> +static void amf_dummy_free(void *opaque, uint8_t *data)
> +{
> +
> +}
> +
> +static AVBufferRef *amf_pool_alloc(void *opaque, size_t size)
> +{
> +AVHWFramesContext *hwfc = (AVHWFramesContext *)opaque;
> +AVBufferRef *buf;
> +
> +buf = av_buffer_create(NULL, NULL, amf_dummy_free, hwfc,
> AV_BUFFER_FLAG_READONLY);
> +if (!buf) {
> +av_log(hwfc, AV_LOG_ERROR, "Failed to create buffer for AMF
> context.\n");
> +return NULL;
> +}
> +return buf;
> +}
This seems to have forgotten to actually allocate anything? It will always
assign NULL to frame->data[0] below.
> +
> +static int amf_frames_init(AVHWFramesContext *ctx)
> +{
> +int i;
> +
> +for (i = 0; i < FF_ARRAY_ELEMS(supported_formats); i++) {
> +if (ctx->sw_format == supported_formats[i])
> +break;
> +}
> +if (i == FF_ARRAY_ELEMS(supported_formats)) {
> +av_log(ctx, AV_LOG_ERROR, "Pixel format '%s' is not supported\n",
> + av_get_pix_fmt_name(ctx->sw_format));
> +return AVERROR(ENOSYS);
> +}
> +
> +ffhwframesctx(ctx)->pool_internal =
> +av_buffer_pool_init2(sizeof(AMFSurface), ctx,
> + amf_pool_alloc, NULL);
> +
> +
> +return 0;
> +}
> +
> +static int amf_get_buffer(AVHWFramesContext *ctx, AVFrame *frame)
> +{
> +frame->buf[0] = av_buffer_pool_get(ctx->pool);
> +if (!frame->buf[0])
> +return AVERROR(ENOMEM);
> +
> +frame->data[0] = frame->buf[0]->data;
> +frame->format = AV_PIX_FMT_AMF_SURFACE;
> +frame->width = ctx->width;
> +frame->height = ctx->height;
> +return 0;
> +}
> +
> ...
> +
> +static int amf_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> + const AVFrame *src)
> +{
> +AMFSurface* surface = (AMFSurface*)dst->data[0];
> +AMFPlane *plane;
> +uint8_t *dst_data[4];
> +int dst_linesize[4];
> +int planes;
> +int i;
> +int w = FFMIN(dst->width, src->width);
> +int h = FFMIN(dst->height, src->height);
> +
> +planes = (int)surface->pVtbl->GetPlanesCount(surface);
> +av_assert0(planes < FF_ARRAY_ELEMS(dst_data));
> +
> +for (i = 0; i < planes; i++) {
> +plane = surface->pVtbl->GetPlaneAt(surface, i);
> +dst_data[i] = plane->pVtbl->GetNative(plane);
> +dst_linesize[i] = plane->pVtbl->GetHPitch(plane);
> +}
> +av_image_copy(dst_data, dst_linesize,
> +(const uint8_t**)src->data, src->linesize, src->format,
> +w, h);
> +
> +return 0;
> +}
> +static int amf_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> +const AVFrame *src)
> +{
> +AMFSurface* surface = (AMFSurface*)src->data[0];
> +AMFPlane *plane;
> +uint8_t *src_data[4];
> +int src_linesize[4];
> +int planes;
> +int i;
> +int w = FFMIN(dst->width, src->width);
> +int h = FFMIN(dst->height, src->height);
> +int ret;
> +
> +ret = surface->pVtbl->Convert(surface, AMF_MEMORY_HOST);
> +AMF_RETURN_IF_FALSE(ctx, ret == AMF_OK, AVERROR_UNKNOWN,
> "Convert(amf::AMF_MEMORY_HOST) failed with error %d\n", AVERROR_UNKNOWN);
> +
> +planes = (int)surface->pVtbl->GetPlanesCount(surface);
> +av_assert0(planes < FF_ARRAY_ELEMS(src_data));
> +
> +for (i = 0; i < planes; i++) {
> +plane = surface->pVtbl->GetPlaneAt(surface, i);
> +src_data[i] = plane->pVtbl->GetNative(plane);
> +
Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
On 2 May 2024, at 21:32, Ondřej Fiala wrote:
> On Thu May 2, 2024 at 4:38 PM CEST, Rémi Denis-Courmont wrote:
>> Le torstaina 2. toukokuuta 2024, 17.25.06 EEST Ondřej Fiala a écrit :
>>> On Wed May 1, 2024 at 7:27 AM CEST, Rémi Denis-Courmont wrote:
I don't use Gmail, and using email for review still sucks. No matter how
you slice it, email was not meant for threaded code reviews.
>>>
>>> Email was not meant for a lot of what it's used for today.
>>
>> And Gitlab and Github are meant for what they are used.
>> That's the whole point.
> This argument can actually go in both directions since the Web and web
> browsers weren't meant for performing code review either. If you remember,
> the Web was originally intended for sharing *documents*, which a MR page
> on GitLab definitely isn't.
>
> IMHO, the fact that something was intended for some use case does not imply
> that it's actually good for that use case. My point was that what it was
> meant for does not matter, what's important is if and how well it works
> for that use case.
>
>>> and unlike GitHub allow threads of arbitrary depth.
>>
>> I don't care because *GitHub* is out of the race for other reasons anyway. I
>> have never had a situation whence *Gitlab* refused to add more comments to a
>> thread.
> I said *depth*, not *length*. AFAIK GitLab can't create message threads like:
>
> message
> |- reply
> | |- reply
> | | '- reply
> | '- reply
> '- reply
> '- reply
>
>>> Using such a client with commands for moving between
>>> messages in a a thread etc. makes threaded code review over email quite
>>> usably in my opinion.
>>
>> So how do I ask my mail agent to pull more existing code for context? Or to
>> get back to the code that started a thread?
> Why would you do that in a mail client? You have your personal copy of the
> repo, so you can just import the patch by piping it to `git am` and then
> use any of the wide array of git-supporting *specialized code review software*
> to look at the changes!
How do I see the review comments that way?
>
> Of course, the quality of your toolings matters a lot. If your email client
> can't pipe a bunch of emails to a shell command, it's not fit for being used
> to review git patches. On the other hand, if you possess just some basic shell
> scripting skills, you can make it do pretty cool things.
So I first have to get proficient in some shell scripting gymnastics
(and also switch to a completely different terminal-based mail client)
so I can do proper reviews?
Thats incredibly gatekeeping.
>
> Since you felt that there is no way to see additional context, I put together
> a quick demo[1] showing how easily you can review all files affected by a
> patch
> and look at *all* the context. Of course, you could do a bunch of other
> things to
> adjust the email-based workflow as desired. And don't forget this is just a
> demo;
> I am sure you could come up with something better.
>
> [1] https://paste.c-net.org/HansenWeekends
That seems to download some binary file? I have no idea what it is supposed to
be.
>
> It's just sway, aerc, and a fuzzy picker combined. The command "changes" is
> just:
> changes() {
> while p="$(git diff --name-only origin/master | pick)"; do
> git diff -U999 origin/master "$p"
> done
> }
>
Also while I can use git-send-email, not everyone can. And patches as
attachments are simply awful. Unfortunately I can't dictate that people
don't send patches that way.
>>>
>>> How can anyone use git, but not git send-email? Any decent email provider
>>> has support for external clients over SMTP.
>>
>> Simply put: no, that is simply not true.
>>
>> Not everybody can pick a decent email provider with outbound SMTP and a good
>> reputation. Also not everybody gets to pick their mail agent or their ISP.
>>
>> You are just being unwittingly elistist here.
> I must admit I did not realize how bad some email/internet providers can be
> when writing this, as I have a fairly average setup and never ran into such
> issues.
>
> But the problem with accessibility is not aleviated by switching away from
> email, since those forges aren't universally accessible either. I remember how
> I used to run Pale Moon like 2 years ago. In case you don't know, it's a
> Firefox
> fork maintained by a small team. GitHub didn't run on it. Oh, sorry, you don't
> care about GitHub. But they share the same desig -- hugely complex "web app"
> that only runs on latest version of major browsers. Everyone else is excluded.
> When I wanted to contribute to a project I really cared about, I had to
> download
> mainline Firefox and do it over that. If I cared even a bit less about it, I
> wouldn't bother.
>
> So how is that any different?
How is it different to download a well maintained recent software and open a
website,
in comparison to learn how to setup a (complex) combination of tools just to be
able
to easily contribute?
>
> I think
Re: [FFmpeg-devel] [PATCH 02/10, v2] avcodec: add amfdec.
On 01/05/2024 19:38, Dmitrii Ovchinnikov wrote:
> From: Evgeny Pavlov
>
> Added AMF based h264, hevc, av1 decoders.
> Co-authored-by: Dmitrii Ovchinnikov
> v2: added encoder reinitialisation
> ---
> libavcodec/Makefile| 7 +-
> libavcodec/allcodecs.c | 3 +
> libavcodec/amfdec.c| 719 +
> libavcodec/amfdec.h| 72 +
> 4 files changed, 799 insertions(+), 2 deletions(-)
> create mode 100644 libavcodec/amfdec.c
> create mode 100644 libavcodec/amfdec.h
>
> ...
> +
> +static int amf_decode_init(AVCodecContext *avctx)
> +{
> +AvAmfDecoderContext *ctx = avctx->priv_data;
> +int ret;
> +enum AVPixelFormat pix_fmts[3] = {
> +AV_PIX_FMT_AMF_SURFACE,
> +avctx->pix_fmt,
> +AV_PIX_FMT_NONE };
> +
> +ret = ff_get_format(avctx, pix_fmts);
> ...
> +const FFCodec ff_##x##_amf_decoder = { \
> ...
> +.init = amf_decode_init, \
> ...
This is still fundamentally incorrect.
See my reply to the previous posting of this patch:
.
Thanks,
- Mark
___
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 02/10, v2] avcodec: add amfdec.
On Wed, May 01, 2024 at 08:38:01PM +0200, Dmitrii Ovchinnikov wrote: > From: Evgeny Pavlov > > Added AMF based h264, hevc, av1 decoders. > Co-authored-by: Dmitrii Ovchinnikov > v2: added encoder reinitialisation > --- > libavcodec/Makefile| 7 +- > libavcodec/allcodecs.c | 3 + > libavcodec/amfdec.c| 719 + > libavcodec/amfdec.h| 72 + > 4 files changed, 799 insertions(+), 2 deletions(-) > create mode 100644 libavcodec/amfdec.c > create mode 100644 libavcodec/amfdec.h fails to build here: libavcodec/amfdec.c:19:10: fatal error: AMF/core/Variant.h: No such file or directory 19 | #include | ^~~~ compilation terminated. make: *** [ffbuild/common.mak:81: libavcodec/amfdec.o] Error 1 thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
On Thu May 2, 2024 at 7:44 PM CEST, Vittorio Giovara wrote:
> On Thu, May 2, 2024 at 10:35 AM Ondřej Fiala wrote:
> > > [...]
> > You will get similar selection bias anywhere else. Even if you handled
> > such a conversation on a discussion site, the technology powering such
> > site will influence what kind of people use it. While discussing this
> > here likely excludes people who don't know how to use a mailing list,
> > having such a debate on Reddit, for example, would exclude people who
> > don't use social media (anyone valuing their privacy and mental health),
> > etc. IMHO there is no way to avoid that.
> >
>
> I think the point is not where the bias is, but how to facilitate new blood
> in ffmpeg. While mailing list reviews may work well for you, there are
> hundreds of developers that won't even get close to FFmpeg because they
> cannot use git{lab,hub}, regardless of the pros or cons of email.
Yeah, that's true.
> I believe the path forward would be designing a system that can accommodate
> both workflows, a main git{hub,lab} interface which can send and mirror the
> discussion happening on the mailing list for those who prefer emails. Such
> a project would be another good use of SPI funds.
I can see the value in that; but I still feel like SourceHut has some valuable
features over the current setup: a unified interface, not having to sign up
anywhere to contribute or comment on issues, and a significantly friendlier
UI for the mailing list archive. I believe it should be considered even if just
as an upgrade for the "email people".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/9] lavu/hwcontext_qsv: update AVQSVFramesContext to support dynamic frame pool
On 28/04/2024 08:39, Xiang, Haihao wrote:
> From: Haihao Xiang
>
> Add AVQSVFramesContext.info and update the description.
>
> Signed-off-by: Haihao Xiang
> ---
> doc/APIchanges| 3 +++
> libavutil/hwcontext_qsv.c | 4 ++--
> libavutil/hwcontext_qsv.h | 28 +---
> libavutil/version.h | 4 ++--
> 4 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 0566fcdcc5..4a434b2877 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
>
> API changes, most recent first:
>
> +2024-04-xx - xx - lavu 59.17.100 - hwcontext_qsv.h
> + Add AVQSVFramesContext.info
> +
> 2024-04-24 - 8616cfe0890 - lavu 59.16.100 - opt.h
>Add AV_OPT_SERIALIZE_SEARCH_CHILDREN.
>
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index c7c7878644..f552811346 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -627,7 +627,7 @@ static mfxStatus frame_alloc(mfxHDL pthis,
> mfxFrameAllocRequest *req,
> QSVFramesContext *s = ctx->hwctx;
> AVQSVFramesContext *hwctx = >p;
> mfxFrameInfo *i = >Info;
> -mfxFrameInfo *i1 = >surfaces[0].Info;
> +mfxFrameInfo *i1 = hwctx->nb_surfaces ? >surfaces[0].Info :
> hwctx->info;
>
> if (!(req->Type & MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET) ||
> !(req->Type & (MFX_MEMTYPE_FROM_VPPIN | MFX_MEMTYPE_FROM_VPPOUT)) ||
> @@ -1207,7 +1207,7 @@ static int qsv_init_internal_session(AVHWFramesContext
> *ctx,
>MFX_IOPATTERN_OUT_SYSTEM_MEMORY;
> par.AsyncDepth = 1;
>
> -par.vpp.In = frames_hwctx->surfaces[0].Info;
> +par.vpp.In = frames_hwctx->nb_surfaces ? frames_hwctx->surfaces[0].Info
> : *frames_hwctx->info;
>
> /* Apparently VPP requires the frame rate to be set to some value,
> otherwise
> * init will fail (probably for the framerate conversion filter). Since
> we
> diff --git a/libavutil/hwcontext_qsv.h b/libavutil/hwcontext_qsv.h
> index e2dba8ad83..9e43a237d4 100644
> --- a/libavutil/hwcontext_qsv.h
> +++ b/libavutil/hwcontext_qsv.h
> @@ -25,8 +25,8 @@
> * @file
> * An API-specific header for AV_HWDEVICE_TYPE_QSV.
> *
> - * This API does not support dynamic frame pools. AVHWFramesContext.pool must
> - * contain AVBufferRefs whose data pointer points to an mfxFrameSurface1
> struct.
> + * AVHWFramesContext.pool must contain AVBufferRefs whose data pointer points
> + * to a mfxFrameSurface1 struct.
> */
>
> /**
> @@ -51,7 +51,29 @@ typedef struct AVQSVDeviceContext {
> * This struct is allocated as AVHWFramesContext.hwctx
> */
> typedef struct AVQSVFramesContext {
> -mfxFrameSurface1 *surfaces;
> +/**
> + * A pointer to a mfxFrameSurface1 or mfxFrameInfo struct
> + *
> + * When nb_surfaces is non-zero, it is a pointer to a mfxFrameSurface1
> + * struct.
> + *
> + * When nb_surfaces is 0, it is a pointer to a mfxFrameInfo struct, all
> + * buffers allocated from the pool have the same mfxFrameInfo.
> + */
> +union {
> +mfxFrameSurface1 *surfaces;
> +mfxFrameInfo *info;
> +};
doc/developer.texi:
"FFmpeg is mainly programmed in the ISO C11 language, except for the public
headers which must stay C99 compatible."
Anonymous unions are therefore not allowed in public headers.
Can you explain what you need the info field for, though? (What is needed but
can't be inferred elsewhere? VAAPI in particular is can be mapped here but
doesn't contain any special information like this.)
> +
> +/**
> + * Number of frames in the pool
> + *
> + * It is 0 for dynamic frame pools or AVHWFramesContext.initial_pool_size
> + * for fixed frame pools.
> + *
> + * Note only oneVPL GPU runtime 2.9+ can support dynamic frame pools
> + * on d3d11va or vaapi
> + */
> intnb_surfaces;
+1 to adding proper documentation for these fields.
>
> /**
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 735f6832e3..3b5a2e7aaa 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,8 +79,8 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 59
> -#define LIBAVUTIL_VERSION_MINOR 16
> -#define LIBAVUTIL_VERSION_MICRO 101
> +#define LIBAVUTIL_VERSION_MINOR 17
> +#define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> LIBAVUTIL_VERSION_MINOR, \
Thanks,
- Mark
___
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] 5 year plan & Inovation
On Thu May 2, 2024 at 4:38 PM CEST, Rémi Denis-Courmont wrote:
> Le torstaina 2. toukokuuta 2024, 17.25.06 EEST Ondřej Fiala a écrit :
> > On Wed May 1, 2024 at 7:27 AM CEST, Rémi Denis-Courmont wrote:
> > > I don't use Gmail, and using email for review still sucks. No matter how
> > > you slice it, email was not meant for threaded code reviews.
> >
> > Email was not meant for a lot of what it's used for today.
>
> And Gitlab and Github are meant for what they are used.
> That's the whole point.
This argument can actually go in both directions since the Web and web
browsers weren't meant for performing code review either. If you remember,
the Web was originally intended for sharing *documents*, which a MR page
on GitLab definitely isn't.
IMHO, the fact that something was intended for some use case does not imply
that it's actually good for that use case. My point was that what it was
meant for does not matter, what's important is if and how well it works
for that use case.
> > and unlike GitHub allow threads of arbitrary depth.
>
> I don't care because *GitHub* is out of the race for other reasons anyway. I
> have never had a situation whence *Gitlab* refused to add more comments to a
> thread.
I said *depth*, not *length*. AFAIK GitLab can't create message threads like:
message
|- reply
| |- reply
| | '- reply
| '- reply
'- reply
'- reply
> > Using such a client with commands for moving between
> > messages in a a thread etc. makes threaded code review over email quite
> > usably in my opinion.
>
> So how do I ask my mail agent to pull more existing code for context? Or to
> get back to the code that started a thread?
Why would you do that in a mail client? You have your personal copy of the
repo, so you can just import the patch by piping it to `git am` and then
use any of the wide array of git-supporting *specialized code review software*
to look at the changes!
Of course, the quality of your toolings matters a lot. If your email client
can't pipe a bunch of emails to a shell command, it's not fit for being used
to review git patches. On the other hand, if you possess just some basic shell
scripting skills, you can make it do pretty cool things.
Since you felt that there is no way to see additional context, I put together
a quick demo[1] showing how easily you can review all files affected by a patch
and look at *all* the context. Of course, you could do a bunch of other things
to
adjust the email-based workflow as desired. And don't forget this is just a
demo;
I am sure you could come up with something better.
[1] https://paste.c-net.org/HansenWeekends
It's just sway, aerc, and a fuzzy picker combined. The command "changes" is
just:
changes() {
while p="$(git diff --name-only origin/master | pick)"; do
git diff -U999 origin/master "$p"
done
}
> > > Also while I can use git-send-email, not everyone can. And patches as
> > > attachments are simply awful. Unfortunately I can't dictate that people
> > > don't send patches that way.
> >
> > How can anyone use git, but not git send-email? Any decent email provider
> > has support for external clients over SMTP.
>
> Simply put: no, that is simply not true.
>
> Not everybody can pick a decent email provider with outbound SMTP and a good
> reputation. Also not everybody gets to pick their mail agent or their ISP.
>
> You are just being unwittingly elistist here.
I must admit I did not realize how bad some email/internet providers can be
when writing this, as I have a fairly average setup and never ran into such
issues.
But the problem with accessibility is not aleviated by switching away from
email, since those forges aren't universally accessible either. I remember how
I used to run Pale Moon like 2 years ago. In case you don't know, it's a Firefox
fork maintained by a small team. GitHub didn't run on it. Oh, sorry, you don't
care about GitHub. But they share the same desig -- hugely complex "web app"
that only runs on latest version of major browsers. Everyone else is excluded.
When I wanted to contribute to a project I really cared about, I had to download
mainline Firefox and do it over that. If I cared even a bit less about it, I
wouldn't bother.
So how is that any different?
I think the solution to the email issues you mentioned could be to have the
ability to upload patches through the SourceHut UI directly. Since SourceHut
is still not feature-finished AFAIK, it could actually be added if there was
enough interest.
___
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] 5 year plan & Inovation
Hi, On Thu, May 2, 2024 at 1:44 PM Vittorio Giovara wrote: > I believe the path forward would be designing a system that can accommodate > both workflows > I agree with this. 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] avformat/framecrcenc: support calculating checksum of IAMF side data
On 5/2/2024 12:16 PM, Andreas Rheinhardt wrote:
James Almer:
The total allocated size for types is arch dependent, so instead calculate a
checksum of the fields only.
Signed-off-by: James Almer
---
Depends on "[PATCH v3] avformat/framecrcenc: compute the checksum for side data"
libavformat/framecrcenc.c | 76 ---
1 file changed, 71 insertions(+), 5 deletions(-)
diff --git a/libavformat/framecrcenc.c b/libavformat/framecrcenc.c
index 8cc4a93053..6d46b51489 100644
--- a/libavformat/framecrcenc.c
+++ b/libavformat/framecrcenc.c
@@ -24,6 +24,7 @@
#include "config.h"
#include "libavutil/adler32.h"
#include "libavutil/avstring.h"
+#include "libavutil/iamf.h"
#include "libavutil/intreadwrite.h"
#include "libavcodec/codec_id.h"
@@ -76,7 +77,8 @@ static int framecrc_write_packet(struct AVFormatContext *s,
AVPacket *pkt)
for (int i = 0; i < pkt->side_data_elems; i++) {
const AVPacketSideData *const sd = >side_data[i];
const uint8_t *data = sd->data;
-int64_t side_data_crc = 0;
+size_t size = sd->size;
+uint32_t side_data_crc = 0;
switch (sd->type) {
#if HAVE_BIGENDIAN
@@ -127,12 +129,76 @@ static int framecrc_write_packet(struct AVFormatContext
*s, AVPacket *pkt)
case AV_PKT_DATA_IAMF_MIX_GAIN_PARAM:
case AV_PKT_DATA_IAMF_DEMIXING_INFO_PARAM:
case AV_PKT_DATA_IAMF_RECON_GAIN_INFO_PARAM:
-side_data_crc = -1;
+{
+const AVIAMFParamDefinition *param = (AVIAMFParamDefinition
*)sd->data;
+uint8_t buf[8];
+ptrdiff_t offset = 0;
+size = 0;
+#define READ_UINT32(struct, parent, child) do { \
+if ((offset + offsetof(struct, child) + sizeof(parent->child)) > sd->size)
\
+goto iamf_end; \
+AV_WL32(buf, parent->child); \
+side_data_crc = av_adler32_update(side_data_crc, buf, 4); \
+size += 4; \
+} while (0)
+#define READ_RATIONAL(struct, parent, child) do { \
+if ((offset + offsetof(struct, child) + sizeof(parent->child)) > sd->size)
\
+goto iamf_end; \
+AV_WL32(buf + 0, parent->child.num); \
+AV_WL32(buf + 4, parent->child.den); \
+side_data_crc = av_adler32_update(side_data_crc, buf, 8); \
+size += 8; \
+} while (0)
+READ_UINT32(AVIAMFParamDefinition, param, nb_subblocks);
+READ_UINT32(AVIAMFParamDefinition, param, type);
+READ_UINT32(AVIAMFParamDefinition, param, parameter_id);
+READ_UINT32(AVIAMFParamDefinition, param, parameter_rate);
+READ_UINT32(AVIAMFParamDefinition, param, duration);
+READ_UINT32(AVIAMFParamDefinition, param,
constant_subblock_duration);
+for (unsigned int i = 0; i < param->nb_subblocks; i++) {
+void *subblock =
av_iamf_param_definition_get_subblock(param, i);
+
+offset = (intptr_t)subblock - (intptr_t)sd->data;
+switch (param->type) {
+case AV_IAMF_PARAMETER_DEFINITION_MIX_GAIN: {
+const AVIAMFMixGain *mix = subblock;
+READ_UINT32(AVIAMFMixGain, mix, subblock_duration);
+READ_UINT32(AVIAMFMixGain, mix, animation_type);
+READ_RATIONAL(AVIAMFMixGain, mix, start_point_value);
+READ_RATIONAL(AVIAMFMixGain, mix, end_point_value);
+READ_RATIONAL(AVIAMFMixGain, mix, control_point_value);
+READ_RATIONAL(AVIAMFMixGain, mix,
control_point_relative_time);
+break;
+}
+case AV_IAMF_PARAMETER_DEFINITION_DEMIXING: {
+const AVIAMFDemixingInfo *demix = subblock;
+READ_UINT32(AVIAMFDemixingInfo, demix,
subblock_duration);
+READ_UINT32(AVIAMFDemixingInfo, demix, dmixp_mode);
+break;
+}
+case AV_IAMF_PARAMETER_DEFINITION_RECON_GAIN: {
+const AVIAMFReconGain *recon = subblock;
+READ_UINT32(AVIAMFReconGain, recon, subblock_duration);
+if ((offset + offsetof(AVIAMFReconGain, recon_gain)
++ sizeof(recon->recon_gain)) > sd->size)
+goto iamf_end;
+side_data_crc = av_adler32_update(side_data_crc,
+ (uint8_t
*)recon->recon_gain,
+
sizeof(recon->recon_gain));
+size += sizeof(recon->recon_gain);
+break;
+}
+default:
+break;
+}
+
Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: Always set ret before goto end
On 5/2/2024 6:48 AM, Andreas Rheinhardt wrote:
Before 0f8763fbea4e8816cd54c2a481d4c048fec58394, av1_frame_ref()
and update_reference_list() could fail and therefore needed to
be checked, which incidentally set ret. This is no longer happening,
leading to a potential use of an uninitialized value which is
also the subject of Coverity ticket #1596605.
Fix this by always setting ret before goto end; do not return
some random ancient value.
Signed-off-by: Andreas Rheinhardt
---
Here is a different approach that uses the translation from 0 to EAGAIN.
libavcodec/av1dec.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 79a30a114d..75cc3fba48 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1333,12 +1333,15 @@ static int av1_receive_frame_internal(AVCodecContext
*avctx, AVFrame *frame)
if (s->cur_frame.f) {
ret = set_output_frame(avctx, frame);
-if (ret < 0)
+if (ret < 0) {
av_log(avctx, AV_LOG_ERROR, "Set output frame
error.\n");
+goto end;
+}
}
s->raw_frame_header = NULL;
i++;
+ret = 0;
goto end;
}
@@ -1439,17 +1442,20 @@ static int av1_receive_frame_internal(AVCodecContext
*avctx, AVFrame *frame)
update_reference_list(avctx);
-if (s->raw_frame_header->show_frame && s->cur_frame.f) {
-ret = set_output_frame(avctx, frame);
-if (ret < 0) {
-av_log(avctx, AV_LOG_ERROR, "Set output frame error\n");
-goto end;
-}
-}
-raw_tile_group = NULL;
+raw_tile_group = NULL;
s->raw_frame_header = NULL;
+
if (show_frame) {
+// cur_frame.f needn't exist due to skip_frame.
+if (s->cur_frame.f) {
+ret = set_output_frame(avctx, frame);
+if (ret < 0) {
+av_log(avctx, AV_LOG_ERROR, "Set output frame
error\n");
+goto end;
+}
+}
i++;
+ret = 0;
goto end;
}
}
Should be ok.
___
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/2] checkasm: add test for fdct
Le keskiviikkona 17. huhtikuuta 2024, 21.01.37 EEST Ramiro Polla a écrit :
> Reviewed-by: Martin Storsjö
> ---
> tests/checkasm/Makefile | 1 +
> tests/checkasm/checkasm.c | 3 ++
> tests/checkasm/checkasm.h | 1 +
> tests/checkasm/fdctdsp.c | 68 +++
> tests/fate/checkasm.mak | 1 +
> 5 files changed, 74 insertions(+)
> create mode 100644 tests/checkasm/fdctdsp.c
>
> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
> index 2673e1d098..70a6120c70 100644
> --- a/tests/checkasm/Makefile
> +++ b/tests/checkasm/Makefile
> @@ -4,6 +4,7 @@ AVCODECOBJS-$(CONFIG_AC3DSP)+= ac3dsp.o
> AVCODECOBJS-$(CONFIG_AUDIODSP) += audiodsp.o
> AVCODECOBJS-$(CONFIG_BLOCKDSP) += blockdsp.o
> AVCODECOBJS-$(CONFIG_BSWAPDSP) += bswapdsp.o
> +AVCODECOBJS-$(CONFIG_FDCTDSP) += fdctdsp.o
> AVCODECOBJS-$(CONFIG_FMTCONVERT)+= fmtconvert.o
> AVCODECOBJS-$(CONFIG_G722DSP) += g722dsp.o
> AVCODECOBJS-$(CONFIG_H264CHROMA)+= h264chroma.o
> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
> index 8be6cb0f55..92c3a30ad3 100644
> --- a/tests/checkasm/checkasm.c
> +++ b/tests/checkasm/checkasm.c
> @@ -106,6 +106,9 @@ static const struct {
> #if CONFIG_EXR_DECODER
> { "exrdsp", checkasm_check_exrdsp },
> #endif
> +#if CONFIG_FDCTDSP
> +{ "fdctdsp", checkasm_check_fdctdsp },
> +#endif
> #if CONFIG_FLAC_DECODER
> { "flacdsp", checkasm_check_flacdsp },
> #endif
> diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
> index f90920dee7..d3e8f9a37a 100644
> --- a/tests/checkasm/checkasm.h
> +++ b/tests/checkasm/checkasm.h
> @@ -85,6 +85,7 @@ void checkasm_check_blockdsp(void);
> void checkasm_check_bswapdsp(void);
> void checkasm_check_colorspace(void);
> void checkasm_check_exrdsp(void);
> +void checkasm_check_fdctdsp(void);
> void checkasm_check_fixed_dsp(void);
> void checkasm_check_flacdsp(void);
> void checkasm_check_float_dsp(void);
> diff --git a/tests/checkasm/fdctdsp.c b/tests/checkasm/fdctdsp.c
> new file mode 100644
> index 00..68a9b5e435
> --- /dev/null
> +++ b/tests/checkasm/fdctdsp.c
> @@ -0,0 +1,68 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include
> +
> +#include "checkasm.h"
> +
> +#include "libavcodec/avcodec.h"
> +#include "libavcodec/fdctdsp.h"
> +
> +#include "libavutil/common.h"
> +#include "libavutil/internal.h"
> +#include "libavutil/mem_internal.h"
> +
> +static int int16_cmp_off_by_n(const int16_t *ref, const int16_t *test,
> size_t n, int accuracy) +{
> +for (size_t i = 0; i < n; i++) {
> +if (abs(ref[i] - test[i]) > accuracy)
> +return 1;
> +}
> +return 0;
> +}
> +
> +static void check_fdct(void)
> +{
> +LOCAL_ALIGNED_16(int16_t, block0, [64]);
> +LOCAL_ALIGNED_16(int16_t, block1, [64]);
> +
> +AVCodecContext avctx = { 0 };
AFAICT, that is not a legal context for ff_fdctdst_init(), which expect
bits_per_raw_sample to be one of 8, 9 or 10. It would also be good manners to
initialise dct_algo.
> +FDCTDSPContext h;
> +
> +ff_fdctdsp_init(, );
> +
> +if (check_func(h.fdct, "fdct")) {
> +declare_func(void, int16_t *);
> +for (int i = 0; i < 64; i++) {
> +uint8_t r = rnd();
> +block0[i] = r;
> +block1[i] = r;
> +}
> +call_ref(block0);
> +call_new(block1);
> +if (int16_cmp_off_by_n(block0, block1, 64, 2))
> +fail();
> +bench_new(block1);
> +}
> +}
> +
> +void checkasm_check_fdctdsp(void)
> +{
> +check_fdct();
> +report("fdctdsp");
> +}
> diff --git a/tests/fate/checkasm.mak b/tests/fate/checkasm.mak
> index 3b5b867a97..10a42f2f9d 100644
> --- a/tests/fate/checkasm.mak
> +++ b/tests/fate/checkasm.mak
> @@ -8,6 +8,7 @@ FATE_CHECKASM = fate-checkasm-aacencdsp
>\ fate-checkasm-blockdsp \
> fate-checkasm-bswapdsp \
> fate-checkasm-exrdsp\ +
> fate-checkasm-fdctdsp \
> fate-checkasm-fixed_dsp
Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
On Thu, May 2, 2024 at 10:35 AM Ondřej Fiala wrote:
> On Thu May 2, 2024 at 4:20 PM CEST, Kieran Kunhya wrote:
> > > [...]
> > I feel it's a huge selection bias to have arguments about Gitlab vs
> Mailing
> > list handled on a mailing list.
> >
> > [...]
> You will get similar selection bias anywhere else. Even if you handled
> such a conversation on a discussion site, the technology powering such
> site will influence what kind of people use it. While discussing this
> here likely excludes people who don't know how to use a mailing list,
> having such a debate on Reddit, for example, would exclude people who
> don't use social media (anyone valuing their privacy and mental health),
> etc. IMHO there is no way to avoid that.
>
I think the point is not where the bias is, but how to facilitate new blood
in ffmpeg. While mailing list reviews may work well for you, there are
hundreds of developers that won't even get close to FFmpeg because they
cannot use git{lab,hub}, regardless of the pros or cons of email.
I believe the path forward would be designing a system that can accommodate
both workflows, a main git{hub,lab} interface which can send and mirror the
discussion happening on the mailing list for those who prefer emails. Such
a project would be another good use of SPI funds.
--
Vittorio
___
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] 5 year plan & Inovation
> On May 2, 2024, at 9:35 AM, Zhao Zhili wrote: > > I know a developer which have contributed to FFmpeg and stop doing so after > losing his git-send-email environment. I'm not surprised, getting git-send-email to work can be fairly daunting. First you have to know enough about secure SMTP to know the difference between ports 465 and 587 and properly configuring SMTP encryption in git config (quick, which one is "tls" and which one is "ssl"). Then you may need to know enough about Perl to install some modules from CPAN, for example I always need to install Net::SMTP::SSL on a new machine. Lastly you need to figure out how to integrate git with keychain on your particular platform to avoid having your email password in a plaintext file. - 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] [RFC] 5 year plan & Inovation
> -Original Message- > From: ffmpeg-devel On Behalf Of Ondřej Fiala > Sent: 2024年5月2日 22:25 > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation > > On Wed May 1, 2024 at 7:27 AM CEST, Rémi Denis-Courmont wrote: > > Le 30 avril 2024 22:15:10 GMT+03:00, "Ondřej Fiala" a > > écrit : > > >On Tue Apr 30, 2024 at 9:06 PM CEST, Hendrik Leppkes wrote: > > >> I will take the replacement instead, thanks. Email is archaic. The > > >> entire point is to get away from email, not dress it up. > > >> SourceHut usage would likely make me even less interested then today. > > >> > > >> - Hendrik > > >I guess that depends on how (and with what) you use it. Using it with > > >Gmail UI for example is obviously not a great idea. No idea whether you > > >do, but if you do, you should be upset at Gmail, not email. > > > > I don't use Gmail, and using email for review still sucks. No matter how you > > slice it, email was not meant for threaded code reviews. > Email was not meant for a lot of what it's used for today. Many email clients > have support for threading, and unlike GitHub allow threads of arbitrary > depth. Using such a client with commands for moving between messages in a > a thread etc. makes threaded code review over email quite usably in my > opinion. > > > Also while I can use git-send-email, not everyone can. And patches as > > attachments are simply awful. Unfortunately I can't dictate that people > > don't > > send patches that way. > How can anyone use git, but not git send-email? Any decent email provider > has support for external clients over SMTP. And I believe you *can* actually > dictate that people don't attach patches -- if you have control over the > mailing list software, you can set up a filter that rejects such emails > and auto-replies with instructions on how to send them properly. I have tested a few email providers trying to find one which works with git send-email. Some email providers blocked my emails because they don't know git send-email and treat patches in email as spam. And there are email client issues. Michael's email is blank in a very popular email client. I reported the bug to the developer of that email client, they don't understand the use case and have no interest to fix it. I know a developer which have contributed to FFmpeg and stop doing so after losing his git-send-email environment. > > > >But you did not answer my question: which specific code review features > > >are you missing? > > > > Proper threaded reviews with state tracking, ability to collapse and expand > > context and files, and proper listing of open MR (*not* like patchwork). > I can sort of understand everything except the last one. What is "a proper > listing of open MR" supposed to mean...? (I know what a merge request is, > of course, but I don't get how the way GitLab lists them is supposedly > superior to SourceHut's list of patches.) > ___ > 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 2/2] checkasm/h264dsp: support checking more idct depths
On Wed, Apr 24, 2024 at 10:10 AM J. Dekker wrote:
>
> Signed-off-by: J. Dekker
> ---
> tests/checkasm/h264dsp.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c
> index 0f484e3f43..5cb646ae49 100644
> --- a/tests/checkasm/h264dsp.c
> +++ b/tests/checkasm/h264dsp.c
> @@ -173,6 +173,7 @@ static void dct8x8(int16_t *coef, int bit_depth)
>
> static void check_idct(void)
> {
> +static const int depths[5] = { 8, 9, 10, 12, 14 };
> LOCAL_ALIGNED_16(uint8_t, src, [8 * 8 * 2]);
> LOCAL_ALIGNED_16(uint8_t, dst, [8 * 8 * 2]);
> LOCAL_ALIGNED_16(uint8_t, dst0, [8 * 8 * 2]);
> @@ -181,10 +182,11 @@ static void check_idct(void)
> LOCAL_ALIGNED_16(int16_t, subcoef0, [8 * 8 * 2]);
> LOCAL_ALIGNED_16(int16_t, subcoef1, [8 * 8 * 2]);
> H264DSPContext h;
> -int bit_depth, sz, align, dc;
> +int bit_depth, sz, align, dc, i;
> declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, int16_t *block,
> int stride);
>
> -for (bit_depth = 8; bit_depth <= 10; bit_depth++) {
> +for (i = 0; i < 5; i++) {
> +bit_depth = depths[i];
Perhaps this should use FF_ARRAY_ELEMS(depths) rather than a hard-coded "5"?
Devin
--
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com e: devin.heitmuel...@ltnglobal.com
___
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/2] checkasm/h264dsp: support checking more idct depths
Le keskiviikkona 24. huhtikuuta 2024, 17.09.44 EEST J. Dekker a écrit :
> Signed-off-by: J. Dekker
> ---
> tests/checkasm/h264dsp.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/checkasm/h264dsp.c b/tests/checkasm/h264dsp.c
> index 0f484e3f43..5cb646ae49 100644
> --- a/tests/checkasm/h264dsp.c
> +++ b/tests/checkasm/h264dsp.c
> @@ -173,6 +173,7 @@ static void dct8x8(int16_t *coef, int bit_depth)
>
> static void check_idct(void)
> {
> +static const int depths[5] = { 8, 9, 10, 12, 14 };
> LOCAL_ALIGNED_16(uint8_t, src, [8 * 8 * 2]);
> LOCAL_ALIGNED_16(uint8_t, dst, [8 * 8 * 2]);
> LOCAL_ALIGNED_16(uint8_t, dst0, [8 * 8 * 2]);
> @@ -181,10 +182,11 @@ static void check_idct(void)
> LOCAL_ALIGNED_16(int16_t, subcoef0, [8 * 8 * 2]);
> LOCAL_ALIGNED_16(int16_t, subcoef1, [8 * 8 * 2]);
> H264DSPContext h;
> -int bit_depth, sz, align, dc;
> +int bit_depth, sz, align, dc, i;
> declare_func_emms(AV_CPU_FLAG_MMX, void, uint8_t *dst, int16_t *block,
> int stride);
>
> -for (bit_depth = 8; bit_depth <= 10; bit_depth++) {
> +for (i = 0; i < 5; i++) {
> +bit_depth = depths[i];
> ff_h264dsp_init(, bit_depth, 1);
> for (sz = 4; sz <= 8; sz += 4) {
> randomize_buffers();
Fine with me, but then again, this is not RISC-V-specific, and other people are
ostensibly better for reviewing checkasm changes.
--
Rémi Denis-Courmont
http://www.remlab.net/
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avfilter/riscv: build afir only if required
Le keskiviikkona 24. huhtikuuta 2024, 17.09.43 EEST J. Dekker a écrit : > Signed-off-by: J. Dekker > --- > libavfilter/riscv/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/riscv/Makefile b/libavfilter/riscv/Makefile > index 0b968a9c0d..277dde2aed 100644 > --- a/libavfilter/riscv/Makefile > +++ b/libavfilter/riscv/Makefile > @@ -1,2 +1,2 @@ > -OBJS += riscv/af_afir_init.o > -RVV-OBJS += riscv/af_afir_rvv.o > +OBJS-$(CONFIG_AFIR_FILTER) += riscv/af_afir_init.o > +RVV-OBJS-$(CONFIG_AFIR_FILTER) += riscv/af_afir_rvv.o LGTM -- レミ・デニ-クールモン http://www.remlab.net/ ___ 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/4] lavc/vp9dsp: R-V V ipred vert
Sorry, this is because a 'bpp == 8' was missed. It has been fixed in this link Rémi Denis-Courmont 于2024年5月2日周四 22:11写道： > Le tiistaina 30. huhtikuuta 2024, 2.36.22 EEST flow gg a écrit : > > updated it in the reply and https://github.com/hleft/FFmpeg/tree/vp8vp9 > > VP9 checkasm does not pass on that branch. > > > Rémi Denis-Courmont 于2024年4月30日周二 01:57写道： > > > > > Le perjantaina 22. maaliskuuta 2024, 8.02.38 EEST flow gg a écrit : > > > > Because the previous patch was updated, so it was updated in this > > > > > > response > > > > > > Seemingly needs rebase since April 7. > > > > > > -- > > > レミ・デニ-クールモン > > > http://www.remlab.net/ > > > > > > > > > > > > ___ > > > 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". > > > -- > レミ・デニ-クールモン > http://www.remlab.net/ > > > > ___ > 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] avformat/framecrcenc: support calculating checksum of IAMF side data
James Almer:
> The total allocated size for types is arch dependent, so instead calculate a
> checksum of the fields only.
>
> Signed-off-by: James Almer
> ---
> Depends on "[PATCH v3] avformat/framecrcenc: compute the checksum for side
> data"
>
> libavformat/framecrcenc.c | 76 ---
> 1 file changed, 71 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/framecrcenc.c b/libavformat/framecrcenc.c
> index 8cc4a93053..6d46b51489 100644
> --- a/libavformat/framecrcenc.c
> +++ b/libavformat/framecrcenc.c
> @@ -24,6 +24,7 @@
> #include "config.h"
> #include "libavutil/adler32.h"
> #include "libavutil/avstring.h"
> +#include "libavutil/iamf.h"
> #include "libavutil/intreadwrite.h"
>
> #include "libavcodec/codec_id.h"
> @@ -76,7 +77,8 @@ static int framecrc_write_packet(struct AVFormatContext *s,
> AVPacket *pkt)
> for (int i = 0; i < pkt->side_data_elems; i++) {
> const AVPacketSideData *const sd = >side_data[i];
> const uint8_t *data = sd->data;
> -int64_t side_data_crc = 0;
> +size_t size = sd->size;
> +uint32_t side_data_crc = 0;
>
> switch (sd->type) {
> #if HAVE_BIGENDIAN
> @@ -127,12 +129,76 @@ static int framecrc_write_packet(struct AVFormatContext
> *s, AVPacket *pkt)
> case AV_PKT_DATA_IAMF_MIX_GAIN_PARAM:
> case AV_PKT_DATA_IAMF_DEMIXING_INFO_PARAM:
> case AV_PKT_DATA_IAMF_RECON_GAIN_INFO_PARAM:
> -side_data_crc = -1;
> +{
> +const AVIAMFParamDefinition *param = (AVIAMFParamDefinition
> *)sd->data;
> +uint8_t buf[8];
> +ptrdiff_t offset = 0;
> +size = 0;
> +#define READ_UINT32(struct, parent, child) do { \
> +if ((offset + offsetof(struct, child) + sizeof(parent->child)) >
> sd->size) \
> +goto iamf_end; \
> +AV_WL32(buf, parent->child); \
> +side_data_crc = av_adler32_update(side_data_crc, buf, 4); \
> +size += 4; \
> +} while (0)
> +#define READ_RATIONAL(struct, parent, child) do { \
> +if ((offset + offsetof(struct, child) + sizeof(parent->child)) >
> sd->size) \
> +goto iamf_end; \
> +AV_WL32(buf + 0, parent->child.num); \
> +AV_WL32(buf + 4, parent->child.den); \
> +side_data_crc = av_adler32_update(side_data_crc, buf, 8); \
> +size += 8; \
> +} while (0)
> +READ_UINT32(AVIAMFParamDefinition, param, nb_subblocks);
> +READ_UINT32(AVIAMFParamDefinition, param, type);
> +READ_UINT32(AVIAMFParamDefinition, param, parameter_id);
> +READ_UINT32(AVIAMFParamDefinition, param, parameter_rate);
> +READ_UINT32(AVIAMFParamDefinition, param, duration);
> +READ_UINT32(AVIAMFParamDefinition, param,
> constant_subblock_duration);
> +for (unsigned int i = 0; i < param->nb_subblocks; i++) {
> +void *subblock =
> av_iamf_param_definition_get_subblock(param, i);
> +
> +offset = (intptr_t)subblock - (intptr_t)sd->data;
> +switch (param->type) {
> +case AV_IAMF_PARAMETER_DEFINITION_MIX_GAIN: {
> +const AVIAMFMixGain *mix = subblock;
> +READ_UINT32(AVIAMFMixGain, mix, subblock_duration);
> +READ_UINT32(AVIAMFMixGain, mix, animation_type);
> +READ_RATIONAL(AVIAMFMixGain, mix, start_point_value);
> +READ_RATIONAL(AVIAMFMixGain, mix, end_point_value);
> +READ_RATIONAL(AVIAMFMixGain, mix,
> control_point_value);
> +READ_RATIONAL(AVIAMFMixGain, mix,
> control_point_relative_time);
> +break;
> +}
> +case AV_IAMF_PARAMETER_DEFINITION_DEMIXING: {
> +const AVIAMFDemixingInfo *demix = subblock;
> +READ_UINT32(AVIAMFDemixingInfo, demix,
> subblock_duration);
> +READ_UINT32(AVIAMFDemixingInfo, demix, dmixp_mode);
> +break;
> +}
> +case AV_IAMF_PARAMETER_DEFINITION_RECON_GAIN: {
> +const AVIAMFReconGain *recon = subblock;
> +READ_UINT32(AVIAMFReconGain, recon,
> subblock_duration);
> +if ((offset + offsetof(AVIAMFReconGain, recon_gain)
> ++ sizeof(recon->recon_gain)) > sd->size)
> +goto iamf_end;
> +side_data_crc = av_adler32_update(side_data_crc,
> + (uint8_t
> *)recon->recon_gain,
> +
> sizeof(recon->recon_gain));
> +size += sizeof(recon->recon_gain);
> +
[FFmpeg-devel] [PATCH] avformat/framecrcenc: support calculating checksum of IAMF side data
The total allocated size for types is arch dependent, so instead calculate a
checksum of the fields only.
Signed-off-by: James Almer
---
Depends on "[PATCH v3] avformat/framecrcenc: compute the checksum for side data"
libavformat/framecrcenc.c | 76 ---
1 file changed, 71 insertions(+), 5 deletions(-)
diff --git a/libavformat/framecrcenc.c b/libavformat/framecrcenc.c
index 8cc4a93053..6d46b51489 100644
--- a/libavformat/framecrcenc.c
+++ b/libavformat/framecrcenc.c
@@ -24,6 +24,7 @@
#include "config.h"
#include "libavutil/adler32.h"
#include "libavutil/avstring.h"
+#include "libavutil/iamf.h"
#include "libavutil/intreadwrite.h"
#include "libavcodec/codec_id.h"
@@ -76,7 +77,8 @@ static int framecrc_write_packet(struct AVFormatContext *s,
AVPacket *pkt)
for (int i = 0; i < pkt->side_data_elems; i++) {
const AVPacketSideData *const sd = >side_data[i];
const uint8_t *data = sd->data;
-int64_t side_data_crc = 0;
+size_t size = sd->size;
+uint32_t side_data_crc = 0;
switch (sd->type) {
#if HAVE_BIGENDIAN
@@ -127,12 +129,76 @@ static int framecrc_write_packet(struct AVFormatContext
*s, AVPacket *pkt)
case AV_PKT_DATA_IAMF_MIX_GAIN_PARAM:
case AV_PKT_DATA_IAMF_DEMIXING_INFO_PARAM:
case AV_PKT_DATA_IAMF_RECON_GAIN_INFO_PARAM:
-side_data_crc = -1;
+{
+const AVIAMFParamDefinition *param = (AVIAMFParamDefinition
*)sd->data;
+uint8_t buf[8];
+ptrdiff_t offset = 0;
+size = 0;
+#define READ_UINT32(struct, parent, child) do { \
+if ((offset + offsetof(struct, child) + sizeof(parent->child)) > sd->size)
\
+goto iamf_end; \
+AV_WL32(buf, parent->child); \
+side_data_crc = av_adler32_update(side_data_crc, buf, 4); \
+size += 4; \
+} while (0)
+#define READ_RATIONAL(struct, parent, child) do { \
+if ((offset + offsetof(struct, child) + sizeof(parent->child)) > sd->size)
\
+goto iamf_end; \
+AV_WL32(buf + 0, parent->child.num); \
+AV_WL32(buf + 4, parent->child.den); \
+side_data_crc = av_adler32_update(side_data_crc, buf, 8); \
+size += 8; \
+} while (0)
+READ_UINT32(AVIAMFParamDefinition, param, nb_subblocks);
+READ_UINT32(AVIAMFParamDefinition, param, type);
+READ_UINT32(AVIAMFParamDefinition, param, parameter_id);
+READ_UINT32(AVIAMFParamDefinition, param, parameter_rate);
+READ_UINT32(AVIAMFParamDefinition, param, duration);
+READ_UINT32(AVIAMFParamDefinition, param,
constant_subblock_duration);
+for (unsigned int i = 0; i < param->nb_subblocks; i++) {
+void *subblock =
av_iamf_param_definition_get_subblock(param, i);
+
+offset = (intptr_t)subblock - (intptr_t)sd->data;
+switch (param->type) {
+case AV_IAMF_PARAMETER_DEFINITION_MIX_GAIN: {
+const AVIAMFMixGain *mix = subblock;
+READ_UINT32(AVIAMFMixGain, mix, subblock_duration);
+READ_UINT32(AVIAMFMixGain, mix, animation_type);
+READ_RATIONAL(AVIAMFMixGain, mix, start_point_value);
+READ_RATIONAL(AVIAMFMixGain, mix, end_point_value);
+READ_RATIONAL(AVIAMFMixGain, mix, control_point_value);
+READ_RATIONAL(AVIAMFMixGain, mix,
control_point_relative_time);
+break;
+}
+case AV_IAMF_PARAMETER_DEFINITION_DEMIXING: {
+const AVIAMFDemixingInfo *demix = subblock;
+READ_UINT32(AVIAMFDemixingInfo, demix,
subblock_duration);
+READ_UINT32(AVIAMFDemixingInfo, demix, dmixp_mode);
+break;
+}
+case AV_IAMF_PARAMETER_DEFINITION_RECON_GAIN: {
+const AVIAMFReconGain *recon = subblock;
+READ_UINT32(AVIAMFReconGain, recon, subblock_duration);
+if ((offset + offsetof(AVIAMFReconGain, recon_gain)
++ sizeof(recon->recon_gain)) > sd->size)
+goto iamf_end;
+side_data_crc = av_adler32_update(side_data_crc,
+ (uint8_t
*)recon->recon_gain,
+
sizeof(recon->recon_gain));
+size += sizeof(recon->recon_gain);
+break;
+}
+default:
+break;
+}
+}
+iamf_end:
+break;
+}
Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
Le torstaina 2. toukokuuta 2024, 17.25.06 EEST Ondřej Fiala a écrit : > On Wed May 1, 2024 at 7:27 AM CEST, Rémi Denis-Courmont wrote: > > Le 30 avril 2024 22:15:10 GMT+03:00, "Ondřej Fiala" a écrit : > > >On Tue Apr 30, 2024 at 9:06 PM CEST, Hendrik Leppkes wrote: > > >> I will take the replacement instead, thanks. Email is archaic. The > > >> entire point is to get away from email, not dress it up. > > >> SourceHut usage would likely make me even less interested then today. > > >> > > >> - Hendrik > > > > > >I guess that depends on how (and with what) you use it. Using it with > > >Gmail UI for example is obviously not a great idea. No idea whether you > > >do, but if you do, you should be upset at Gmail, not email. > > > > I don't use Gmail, and using email for review still sucks. No matter how > > you slice it, email was not meant for threaded code reviews. > > Email was not meant for a lot of what it's used for today. And Gitlab and Github are meant for what they are used. That's the whole point. Maybe (probably) email is better than any other tool that was not meant for code review, but not than a dedicated tool. But to claim that email is better for code review that dedicated tools is very highly disingenuous. People wouldn't have spent so much effort developping those tools, and they would not be so popular. > Many email clients have support for threading, No, they don't. Email threading is *not* the same as code review threading. And then email clients also can't track open/closed states. > and unlike GitHub allow threads of arbitrary depth. I don't care because *GitHub* is out of the race for other reasons anyway. I have never had a situation whence *Gitlab* refused to add more comments to a thread. > Using such a client with commands for moving between > messages in a a thread etc. makes threaded code review over email quite > usably in my opinion. So how do I ask my mail agent to pull more existing code for context? Or to get back to the code that started a thread? > > Also while I can use git-send-email, not everyone can. And patches as > > attachments are simply awful. Unfortunately I can't dictate that people > > don't send patches that way. > > How can anyone use git, but not git send-email? Any decent email provider > has support for external clients over SMTP. Simply put: no, that is simply not true. Not everybody can pick a decent email provider with outbound SMTP and a good reputation. Also not everybody gets to pick their mail agent or their ISP. You are just being unwittingly elistist here. > And I believe you *can* actually > dictate that people don't attach patches -- if you have control over the > mailing list software, you can set up a filter that rejects such emails and > auto-replies with instructions on how to send them properly. Yes but then those people can't contribute at all. -- Rémi Denis-Courmont http://www.remlab.net/ ___ 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] 5 year plan & Inovation
On Thu May 2, 2024 at 4:20 PM CEST, Kieran Kunhya wrote: > > [...] > I feel it's a huge selection bias to have arguments about Gitlab vs Mailing > list handled on a mailing list. > > [...] You will get similar selection bias anywhere else. Even if you handled such a conversation on a discussion site, the technology powering such site will influence what kind of people use it. While discussing this here likely excludes people who don't know how to use a mailing list, having such a debate on Reddit, for example, would exclude people who don't use social media (anyone valuing their privacy and mental health), etc. IMHO there is no way to avoid 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] [RFC] 5 year plan & Inovation
On Wed May 1, 2024 at 7:27 AM CEST, Rémi Denis-Courmont wrote: > Le 30 avril 2024 22:15:10 GMT+03:00, "Ondřej Fiala" a > écrit : > >On Tue Apr 30, 2024 at 9:06 PM CEST, Hendrik Leppkes wrote: > >> I will take the replacement instead, thanks. Email is archaic. The > >> entire point is to get away from email, not dress it up. > >> SourceHut usage would likely make me even less interested then today. > >> > >> - Hendrik > >I guess that depends on how (and with what) you use it. Using it with > >Gmail UI for example is obviously not a great idea. No idea whether you > >do, but if you do, you should be upset at Gmail, not email. > > I don't use Gmail, and using email for review still sucks. No matter how you > slice it, email was not meant for threaded code reviews. Email was not meant for a lot of what it's used for today. Many email clients have support for threading, and unlike GitHub allow threads of arbitrary depth. Using such a client with commands for moving between messages in a a thread etc. makes threaded code review over email quite usably in my opinion. > Also while I can use git-send-email, not everyone can. And patches as > attachments are simply awful. Unfortunately I can't dictate that people don't > send patches that way. How can anyone use git, but not git send-email? Any decent email provider has support for external clients over SMTP. And I believe you *can* actually dictate that people don't attach patches -- if you have control over the mailing list software, you can set up a filter that rejects such emails and auto-replies with instructions on how to send them properly. > >But you did not answer my question: which specific code review features > >are you missing? > > Proper threaded reviews with state tracking, ability to collapse and expand > context and files, and proper listing of open MR (*not* like patchwork). I can sort of understand everything except the last one. What is "a proper listing of open MR" supposed to mean...? (I know what a merge request is, of course, but I don't get how the way GitLab lists them is supposedly superior to SourceHut's list of patches.) ___ 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] 5 year plan & Inovation
Sent from my mobile device On Thu, 2 May 2024, 15:54 Ondřej Fiala, wrote: > On Wed May 1, 2024 at 1:01 AM CEST, Andrew Sayers wrote: > > On Tue, Apr 30, 2024 at 09:05:05PM +0200, Ondřej Fiala wrote: > > > [...] > > > > IMHO, GitHub have improved that user experience significantly in recent > years. > > Yes you're still making a fork and pushing it, but the experience is > more like > > click the edit button -> make changes (in an admittedly clunky web > editor) -> > > save and push. The rest is just kinda presented as implementation > details. > > > > That's a bit of a nitpick, but the wider point is interesting - > > GitHub etc. are fast-moving targets, so today's friction points become > > tomorrow's selling points, then the next day's lock-in opportunities. > > That makes it hard to compare to a mailing list, which is unlikely to be > > better or worse ten years from now. > That's an interesting point, and I guess it also shows how different > perspectives result in very different conclusions. To me, GitHub being > fast-moving is a negative for the same reason the whole Web tech stack > being fast-moving is. > I feel it's a huge selection bias to have arguments about Gitlab vs Mailing list handled on a mailing list. [insert meme of plane with holes in it] Kieran > ___ 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/4] lavc/vp9dsp: R-V V ipred vert
Le tiistaina 30. huhtikuuta 2024, 2.36.22 EEST flow gg a écrit : > updated it in the reply and https://github.com/hleft/FFmpeg/tree/vp8vp9 VP9 checkasm does not pass on that branch. > Rémi Denis-Courmont 于2024年4月30日周二 01:57写道： > > > Le perjantaina 22. maaliskuuta 2024, 8.02.38 EEST flow gg a écrit : > > > Because the previous patch was updated, so it was updated in this > > > > response > > > > Seemingly needs rebase since April 7. > > > > -- > > レミ・デニ-クールモン > > http://www.remlab.net/ > > > > > > > > ___ > > 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". -- レミ・デニ-クールモン http://www.remlab.net/ ___ 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] 5 year plan & Inovation
On Wed May 1, 2024 at 1:01 AM CEST, Andrew Sayers wrote: > On Tue, Apr 30, 2024 at 09:05:05PM +0200, Ondřej Fiala wrote: > > [...] > > IMHO, GitHub have improved that user experience significantly in recent years. > Yes you're still making a fork and pushing it, but the experience is more like > click the edit button -> make changes (in an admittedly clunky web editor) -> > save and push. The rest is just kinda presented as implementation details. > > That's a bit of a nitpick, but the wider point is interesting - > GitHub etc. are fast-moving targets, so today's friction points become > tomorrow's selling points, then the next day's lock-in opportunities. > That makes it hard to compare to a mailing list, which is unlikely to be > better or worse ten years from now. That's an interesting point, and I guess it also shows how different perspectives result in very different conclusions. To me, GitHub being fast-moving is a negative for the same reason the whole Web tech stack being fast-moving is. It means that unless I use the latest Chrome or Firefox or something built on their engines, I am going to be locked out from participating. Worse, because contemporary JS technologies are getting increasingly power-hungry, one needs a relatively recent desktop to be able to use many of these things, which, apart from leading to needless electronic waste, could be a serious barrier for people in poorer parts of the world. Of course, big tech companies intentionally using inefficient software to drive up sales of new hardware sounds completely like a conspiracy theory... until you look at the news and read about Microsoft's Windows 11 lacking support for older hardware without any apparent reason, as people were able to modify the OS to run on such hardware and it worked just fine. I don't need to remind anyone that GitHub is owned by Microsoft. I believe stability and simplicity are virtues, not drawbacks. > > > [...] > > I actually agree that the mailing list can be somewhat annoying as well, > > which is why I like that on SourceHut you can send a patch to their mailing > > lists without being subscribed and it's standard practice that people Cc you > > on the replies. I really feel like this should be standard practice; > > subscribing to the mailing list makes no sense if you only want to send in a > > single patch, and it increases the effort required by flooding you with > > emails > > which aren't relevant to you, as you say. > > > > [...] > > I haven't properly tried this, and it's an ugly hack if it works at all, but > it > might be possible to support logged-out comments with a web-based trigger. > > Triggers are designed to let you e.g. ping a URL on github.com when some > third-party dependency is updated, and have code on their servers > automatically > pull in that dependency and rebuild your package without manual intervention. > But you could equally ping "my-web-hook?name=...=..." then have your > bot turn that into a comment. I must admit that this is an interesting idea, but unless people can also contribute that way it's not going to be very useful. And I am afraid that such "bridging" of email to GitHub would degrade the user experience on both sides, as I have seen happen in similar cases elsewhere, e.g. with Matrix being bridged to IRC. I mean -- if it's decided to switch to GitHub because its code review is supposedly better, then surely the last thing those people would want is others sending in their reviews as emails, completely avoiding GitHub's code review facilities. > This isn't unique to GitHub - a quick look suggests GitLab can do the same, > and I wouldn't be surprised if SourceHut can too. And a self-hosted solution > could presumably use this as the basis for a general anonymous comment thing. SourceHut needs no such hacks -- it already accepts comments sent in through email to its issue tracker, and code review is done directly on mailing lists. ___ 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 3/4] all: Link to "context" from all contexts with documentation
> On May 2, 2024, at 21:27, Andrew Sayers wrote:
>
> On Thu, May 02, 2024 at 09:00:42PM +0800, Zhao Zhili wrote:
>>
>>
>>> On May 2, 2024, at 19:01, Lynne wrote:
>>>
>>> Apr 29, 2024, 11:24 by ffmpeg-de...@pileofstuff.org:
>>>
Some headings needed to be rewritten to accomodate the text,
(hopefully) without changing the meaning.
---
libavcodec/aac/aacdec.h| 2 +-
libavcodec/aacenc.h| 2 +-
libavcodec/ac3enc.h| 2 +-
libavcodec/amfenc.h| 2 +-
libavcodec/atrac.h | 2 +-
libavcodec/avcodec.h | 3 ++-
libavcodec/bsf.h | 2 +-
libavcodec/cbs.h | 2 +-
libavcodec/d3d11va.h | 3 +--
libavcodec/h264dsp.h | 2 +-
libavcodec/h264pred.h | 2 +-
libavcodec/mediacodec.h| 2 +-
libavcodec/mpegaudiodec_template.c | 2 +-
libavcodec/pthread_frame.c | 4 ++--
libavcodec/qsv.h | 6 --
libavcodec/sbr.h | 2 +-
libavcodec/smacker.c | 2 +-
libavcodec/vdpau.h | 3 ++-
libavcodec/videotoolbox.h | 5 +++--
libavfilter/avfilter.h | 2 +-
libavformat/avformat.h | 3 ++-
libavformat/avio.h | 3 ++-
libavutil/audio_fifo.h | 2 +-
libavutil/hwcontext.h | 21 -
libavutil/hwcontext_cuda.h | 2 +-
libavutil/hwcontext_d3d11va.h | 4 ++--
libavutil/hwcontext_d3d12va.h | 6 +++---
libavutil/hwcontext_drm.h | 2 +-
libavutil/hwcontext_dxva2.h| 4 ++--
libavutil/hwcontext_mediacodec.h | 2 +-
libavutil/hwcontext_opencl.h | 4 ++--
libavutil/hwcontext_qsv.h | 4 ++--
libavutil/hwcontext_vaapi.h| 6 +++---
libavutil/hwcontext_vdpau.h| 2 +-
libavutil/hwcontext_vulkan.h | 4 ++--
libavutil/lfg.h| 2 +-
36 files changed, 66 insertions(+), 57 deletions(-)
diff --git a/libavcodec/aac/aacdec.h b/libavcodec/aac/aacdec.h
index 4cf764e2e9..71d61813f4 100644
--- a/libavcodec/aac/aacdec.h
+++ b/libavcodec/aac/aacdec.h
@@ -248,7 +248,7 @@ typedef struct AACDecDSP {
} AACDecDSP;
/**
- * main AAC decoding context
+ * main AAC decoding @ref md_doc_2context "context"
*/
struct AACDecContext {
const struct AVClass *class;
diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h
index d07960620e..1a645f4719 100644
--- a/libavcodec/aacenc.h
+++ b/libavcodec/aacenc.h
@@ -207,7 +207,7 @@ typedef struct AACPCEInfo {
} AACPCEInfo;
/**
- * AAC encoder context
+ * AAC encoder @ref md_doc_2context "context"
*/
typedef struct AACEncContext {
AVClass *av_class;
diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
index 30812617cc..c725007077 100644
--- a/libavcodec/ac3enc.h
+++ b/libavcodec/ac3enc.h
@@ -152,7 +152,7 @@ typedef struct AC3Block {
} AC3Block;
/**
- * AC-3 encoder private context.
+ * AC-3 encoder private @ref md_doc_2context "context"
*/
typedef struct AC3EncodeContext {
AVClass *av_class; ///< AVClass used for AVOption
diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
index 2dbd378ef8..f142ede63a 100644
--- a/libavcodec/amfenc.h
+++ b/libavcodec/amfenc.h
@@ -43,7 +43,7 @@ typedef struct AmfTraceWriter {
} AmfTraceWriter;
/**
-* AMF encoder context
+* AMF encoder @ref md_doc_2context "context"
*/
typedef struct AmfContext {
diff --git a/libavcodec/atrac.h b/libavcodec/atrac.h
index 05208bbee6..1527e376a9 100644
--- a/libavcodec/atrac.h
+++ b/libavcodec/atrac.h
@@ -39,7 +39,7 @@ typedef struct AtracGainInfo {
} AtracGainInfo;
/**
- * Gain compensation context structure.
+ * Gain compensation @ref md_doc_2context "context"
*/
typedef struct AtracGCContext {
float gain_tab1[16]; ///< gain compensation level table
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 968009a192..9180fedca7 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -430,7 +430,8 @@ typedef struct RcOverride{
#define AV_GET_ENCODE_BUFFER_FLAG_REF (1 << 0)
/**
- * main external API structure.
+ * @ref md_doc_2context "Context" for an encode or decode session
+ *
* New fields can be added to the end with minor version bumps.
* Removal, reordering and changes to existing fields require a major
* version bump.
diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
index a09c69f242..bf79afa7cc
Re: [FFmpeg-devel] [PATCH v4 3/4] all: Link to "context" from all contexts with documentation
On Thu, May 02, 2024 at 09:00:42PM +0800, Zhao Zhili wrote:
>
>
> > On May 2, 2024, at 19:01, Lynne wrote:
> >
> > Apr 29, 2024, 11:24 by ffmpeg-de...@pileofstuff.org:
> >
> >> Some headings needed to be rewritten to accomodate the text,
> >> (hopefully) without changing the meaning.
> >> ---
> >> libavcodec/aac/aacdec.h| 2 +-
> >> libavcodec/aacenc.h| 2 +-
> >> libavcodec/ac3enc.h| 2 +-
> >> libavcodec/amfenc.h| 2 +-
> >> libavcodec/atrac.h | 2 +-
> >> libavcodec/avcodec.h | 3 ++-
> >> libavcodec/bsf.h | 2 +-
> >> libavcodec/cbs.h | 2 +-
> >> libavcodec/d3d11va.h | 3 +--
> >> libavcodec/h264dsp.h | 2 +-
> >> libavcodec/h264pred.h | 2 +-
> >> libavcodec/mediacodec.h| 2 +-
> >> libavcodec/mpegaudiodec_template.c | 2 +-
> >> libavcodec/pthread_frame.c | 4 ++--
> >> libavcodec/qsv.h | 6 --
> >> libavcodec/sbr.h | 2 +-
> >> libavcodec/smacker.c | 2 +-
> >> libavcodec/vdpau.h | 3 ++-
> >> libavcodec/videotoolbox.h | 5 +++--
> >> libavfilter/avfilter.h | 2 +-
> >> libavformat/avformat.h | 3 ++-
> >> libavformat/avio.h | 3 ++-
> >> libavutil/audio_fifo.h | 2 +-
> >> libavutil/hwcontext.h | 21 -
> >> libavutil/hwcontext_cuda.h | 2 +-
> >> libavutil/hwcontext_d3d11va.h | 4 ++--
> >> libavutil/hwcontext_d3d12va.h | 6 +++---
> >> libavutil/hwcontext_drm.h | 2 +-
> >> libavutil/hwcontext_dxva2.h| 4 ++--
> >> libavutil/hwcontext_mediacodec.h | 2 +-
> >> libavutil/hwcontext_opencl.h | 4 ++--
> >> libavutil/hwcontext_qsv.h | 4 ++--
> >> libavutil/hwcontext_vaapi.h| 6 +++---
> >> libavutil/hwcontext_vdpau.h| 2 +-
> >> libavutil/hwcontext_vulkan.h | 4 ++--
> >> libavutil/lfg.h| 2 +-
> >> 36 files changed, 66 insertions(+), 57 deletions(-)
> >>
> >> diff --git a/libavcodec/aac/aacdec.h b/libavcodec/aac/aacdec.h
> >> index 4cf764e2e9..71d61813f4 100644
> >> --- a/libavcodec/aac/aacdec.h
> >> +++ b/libavcodec/aac/aacdec.h
> >> @@ -248,7 +248,7 @@ typedef struct AACDecDSP {
> >> } AACDecDSP;
> >>
> >> /**
> >> - * main AAC decoding context
> >> + * main AAC decoding @ref md_doc_2context "context"
> >> */
> >> struct AACDecContext {
> >> const struct AVClass *class;
> >> diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h
> >> index d07960620e..1a645f4719 100644
> >> --- a/libavcodec/aacenc.h
> >> +++ b/libavcodec/aacenc.h
> >> @@ -207,7 +207,7 @@ typedef struct AACPCEInfo {
> >> } AACPCEInfo;
> >>
> >> /**
> >> - * AAC encoder context
> >> + * AAC encoder @ref md_doc_2context "context"
> >> */
> >> typedef struct AACEncContext {
> >> AVClass *av_class;
> >> diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
> >> index 30812617cc..c725007077 100644
> >> --- a/libavcodec/ac3enc.h
> >> +++ b/libavcodec/ac3enc.h
> >> @@ -152,7 +152,7 @@ typedef struct AC3Block {
> >> } AC3Block;
> >>
> >> /**
> >> - * AC-3 encoder private context.
> >> + * AC-3 encoder private @ref md_doc_2context "context"
> >> */
> >> typedef struct AC3EncodeContext {
> >> AVClass *av_class; ///< AVClass used for AVOption
> >> diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> >> index 2dbd378ef8..f142ede63a 100644
> >> --- a/libavcodec/amfenc.h
> >> +++ b/libavcodec/amfenc.h
> >> @@ -43,7 +43,7 @@ typedef struct AmfTraceWriter {
> >> } AmfTraceWriter;
> >>
> >> /**
> >> -* AMF encoder context
> >> +* AMF encoder @ref md_doc_2context "context"
> >> */
> >>
> >> typedef struct AmfContext {
> >> diff --git a/libavcodec/atrac.h b/libavcodec/atrac.h
> >> index 05208bbee6..1527e376a9 100644
> >> --- a/libavcodec/atrac.h
> >> +++ b/libavcodec/atrac.h
> >> @@ -39,7 +39,7 @@ typedef struct AtracGainInfo {
> >> } AtracGainInfo;
> >>
> >> /**
> >> - * Gain compensation context structure.
> >> + * Gain compensation @ref md_doc_2context "context"
> >> */
> >> typedef struct AtracGCContext {
> >> float gain_tab1[16]; ///< gain compensation level table
> >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >> index 968009a192..9180fedca7 100644
> >> --- a/libavcodec/avcodec.h
> >> +++ b/libavcodec/avcodec.h
> >> @@ -430,7 +430,8 @@ typedef struct RcOverride{
> >> #define AV_GET_ENCODE_BUFFER_FLAG_REF (1 << 0)
> >>
> >> /**
> >> - * main external API structure.
> >> + * @ref md_doc_2context "Context" for an encode or decode session
> >> + *
> >> * New fields can be added to the end with minor version bumps.
> >> * Removal, reordering and changes to existing fields require a major
> >> * version bump.
> >> diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
> >> index a09c69f242..bf79afa7cc 100644
> >> --- a/libavcodec/bsf.h
> >> +++
Re: [FFmpeg-devel] [PATCH v4 3/4] all: Link to "context" from all contexts with documentation
> On May 2, 2024, at 19:01, Lynne wrote:
>
> Apr 29, 2024, 11:24 by ffmpeg-de...@pileofstuff.org:
>
>> Some headings needed to be rewritten to accomodate the text,
>> (hopefully) without changing the meaning.
>> ---
>> libavcodec/aac/aacdec.h| 2 +-
>> libavcodec/aacenc.h| 2 +-
>> libavcodec/ac3enc.h| 2 +-
>> libavcodec/amfenc.h| 2 +-
>> libavcodec/atrac.h | 2 +-
>> libavcodec/avcodec.h | 3 ++-
>> libavcodec/bsf.h | 2 +-
>> libavcodec/cbs.h | 2 +-
>> libavcodec/d3d11va.h | 3 +--
>> libavcodec/h264dsp.h | 2 +-
>> libavcodec/h264pred.h | 2 +-
>> libavcodec/mediacodec.h| 2 +-
>> libavcodec/mpegaudiodec_template.c | 2 +-
>> libavcodec/pthread_frame.c | 4 ++--
>> libavcodec/qsv.h | 6 --
>> libavcodec/sbr.h | 2 +-
>> libavcodec/smacker.c | 2 +-
>> libavcodec/vdpau.h | 3 ++-
>> libavcodec/videotoolbox.h | 5 +++--
>> libavfilter/avfilter.h | 2 +-
>> libavformat/avformat.h | 3 ++-
>> libavformat/avio.h | 3 ++-
>> libavutil/audio_fifo.h | 2 +-
>> libavutil/hwcontext.h | 21 -
>> libavutil/hwcontext_cuda.h | 2 +-
>> libavutil/hwcontext_d3d11va.h | 4 ++--
>> libavutil/hwcontext_d3d12va.h | 6 +++---
>> libavutil/hwcontext_drm.h | 2 +-
>> libavutil/hwcontext_dxva2.h| 4 ++--
>> libavutil/hwcontext_mediacodec.h | 2 +-
>> libavutil/hwcontext_opencl.h | 4 ++--
>> libavutil/hwcontext_qsv.h | 4 ++--
>> libavutil/hwcontext_vaapi.h| 6 +++---
>> libavutil/hwcontext_vdpau.h| 2 +-
>> libavutil/hwcontext_vulkan.h | 4 ++--
>> libavutil/lfg.h| 2 +-
>> 36 files changed, 66 insertions(+), 57 deletions(-)
>>
>> diff --git a/libavcodec/aac/aacdec.h b/libavcodec/aac/aacdec.h
>> index 4cf764e2e9..71d61813f4 100644
>> --- a/libavcodec/aac/aacdec.h
>> +++ b/libavcodec/aac/aacdec.h
>> @@ -248,7 +248,7 @@ typedef struct AACDecDSP {
>> } AACDecDSP;
>>
>> /**
>> - * main AAC decoding context
>> + * main AAC decoding @ref md_doc_2context "context"
>> */
>> struct AACDecContext {
>> const struct AVClass *class;
>> diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h
>> index d07960620e..1a645f4719 100644
>> --- a/libavcodec/aacenc.h
>> +++ b/libavcodec/aacenc.h
>> @@ -207,7 +207,7 @@ typedef struct AACPCEInfo {
>> } AACPCEInfo;
>>
>> /**
>> - * AAC encoder context
>> + * AAC encoder @ref md_doc_2context "context"
>> */
>> typedef struct AACEncContext {
>> AVClass *av_class;
>> diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
>> index 30812617cc..c725007077 100644
>> --- a/libavcodec/ac3enc.h
>> +++ b/libavcodec/ac3enc.h
>> @@ -152,7 +152,7 @@ typedef struct AC3Block {
>> } AC3Block;
>>
>> /**
>> - * AC-3 encoder private context.
>> + * AC-3 encoder private @ref md_doc_2context "context"
>> */
>> typedef struct AC3EncodeContext {
>> AVClass *av_class; ///< AVClass used for AVOption
>> diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
>> index 2dbd378ef8..f142ede63a 100644
>> --- a/libavcodec/amfenc.h
>> +++ b/libavcodec/amfenc.h
>> @@ -43,7 +43,7 @@ typedef struct AmfTraceWriter {
>> } AmfTraceWriter;
>>
>> /**
>> -* AMF encoder context
>> +* AMF encoder @ref md_doc_2context "context"
>> */
>>
>> typedef struct AmfContext {
>> diff --git a/libavcodec/atrac.h b/libavcodec/atrac.h
>> index 05208bbee6..1527e376a9 100644
>> --- a/libavcodec/atrac.h
>> +++ b/libavcodec/atrac.h
>> @@ -39,7 +39,7 @@ typedef struct AtracGainInfo {
>> } AtracGainInfo;
>>
>> /**
>> - * Gain compensation context structure.
>> + * Gain compensation @ref md_doc_2context "context"
>> */
>> typedef struct AtracGCContext {
>> float gain_tab1[16]; ///< gain compensation level table
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 968009a192..9180fedca7 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -430,7 +430,8 @@ typedef struct RcOverride{
>> #define AV_GET_ENCODE_BUFFER_FLAG_REF (1 << 0)
>>
>> /**
>> - * main external API structure.
>> + * @ref md_doc_2context "Context" for an encode or decode session
>> + *
>> * New fields can be added to the end with minor version bumps.
>> * Removal, reordering and changes to existing fields require a major
>> * version bump.
>> diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
>> index a09c69f242..bf79afa7cc 100644
>> --- a/libavcodec/bsf.h
>> +++ b/libavcodec/bsf.h
>> @@ -56,7 +56,7 @@
>> */
>>
>> /**
>> - * The bitstream filter state.
>> + * Bitstream filter @ref md_doc_2context "context"
>> *
>> * This struct must be allocated with av_bsf_alloc() and freed with
>> * av_bsf_free().
>> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
>> index d479b1ac2d..0ff64d2fef 100644
Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: Always set ret before goto end
James Almer:
> On 5/2/2024 6:05 AM, Andreas Rheinhardt wrote:
>> Hendrik Leppkes:
>>> On Thu, May 2, 2024 at 10:22 AM Andreas Rheinhardt
>>> wrote:
Before 0f8763fbea4e8816cd54c2a481d4c048fec58394, av1_frame_ref()
and update_reference_list() could fail and therefore needed to
be checked, which incidentally set ret. This is no longer happening,
leading to a potential use of an uninitialized value which is
also the subject of Coverity ticket #1596605.
Fix this by always setting ret before goto end; do not return
some random ancient value.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/av1dec.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 79a30a114d..c3f255a29a 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1335,6 +1335,12 @@ static int
av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
ret = set_output_frame(avctx, frame);
if (ret < 0)
av_log(avctx, AV_LOG_ERROR, "Set output
frame error.\n");
+ } else {
+ // CBS checks for us that the frame to be shown
actually existed
+ // in the bitstream; if it doesn't it could be
e.g. due to
+ // skip_frame setting. Return EAGAIN to
indicate that we are
+ // currently unable to produce output.
+ ret = AVERROR(EAGAIN);
}
>>>
>>> In the vein of this comment, set_output_frame will also return 0
>>> without returning a frame in some cases - eg. with multiple layers.
>>> Should this equally return EAGAIN rather than claiming success without
>>> a frame?
>>>
>>
>> Thanks for pointing this out. There is a translation 0->AVERROR(EAGAIN)
>> at the end of this function if the output frame is unset. Maybe this
>> commit should rather set ret to 0 and rely on this (as it was before
>> 0f8763fbea)? Let's hear what James says about this.
>
> Yes, i prefer that.
I already send a v2 that does this.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: Always set ret before goto end
On 5/2/2024 6:05 AM, Andreas Rheinhardt wrote:
Hendrik Leppkes:
On Thu, May 2, 2024 at 10:22 AM Andreas Rheinhardt
wrote:
Before 0f8763fbea4e8816cd54c2a481d4c048fec58394, av1_frame_ref()
and update_reference_list() could fail and therefore needed to
be checked, which incidentally set ret. This is no longer happening,
leading to a potential use of an uninitialized value which is
also the subject of Coverity ticket #1596605.
Fix this by always setting ret before goto end; do not return
some random ancient value.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/av1dec.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 79a30a114d..c3f255a29a 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1335,6 +1335,12 @@ static int av1_receive_frame_internal(AVCodecContext
*avctx, AVFrame *frame)
ret = set_output_frame(avctx, frame);
if (ret < 0)
av_log(avctx, AV_LOG_ERROR, "Set output frame
error.\n");
+} else {
+// CBS checks for us that the frame to be shown actually
existed
+// in the bitstream; if it doesn't it could be e.g. due to
+// skip_frame setting. Return EAGAIN to indicate that we
are
+// currently unable to produce output.
+ret = AVERROR(EAGAIN);
}
In the vein of this comment, set_output_frame will also return 0
without returning a frame in some cases - eg. with multiple layers.
Should this equally return EAGAIN rather than claiming success without
a frame?
Thanks for pointing this out. There is a translation 0->AVERROR(EAGAIN)
at the end of this function if the output frame is unset. Maybe this
commit should rather set ret to 0 and rely on this (as it was before
0f8763fbea)? Let's hear what James says about this.
Yes, i prefer 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 3/4] all: Link to "context" from all contexts with documentation
On Thu, May 02, 2024 at 01:01:43PM +0200, Lynne wrote:
> Apr 29, 2024, 11:24 by ffmpeg-de...@pileofstuff.org:
>
> > Some headings needed to be rewritten to accomodate the text,
> > (hopefully) without changing the meaning.
> > ---
> > libavcodec/aac/aacdec.h| 2 +-
> > libavcodec/aacenc.h| 2 +-
> > libavcodec/ac3enc.h| 2 +-
> > libavcodec/amfenc.h| 2 +-
> > libavcodec/atrac.h | 2 +-
> > libavcodec/avcodec.h | 3 ++-
> > libavcodec/bsf.h | 2 +-
> > libavcodec/cbs.h | 2 +-
> > libavcodec/d3d11va.h | 3 +--
> > libavcodec/h264dsp.h | 2 +-
> > libavcodec/h264pred.h | 2 +-
> > libavcodec/mediacodec.h| 2 +-
> > libavcodec/mpegaudiodec_template.c | 2 +-
> > libavcodec/pthread_frame.c | 4 ++--
> > libavcodec/qsv.h | 6 --
> > libavcodec/sbr.h | 2 +-
> > libavcodec/smacker.c | 2 +-
> > libavcodec/vdpau.h | 3 ++-
> > libavcodec/videotoolbox.h | 5 +++--
> > libavfilter/avfilter.h | 2 +-
> > libavformat/avformat.h | 3 ++-
> > libavformat/avio.h | 3 ++-
> > libavutil/audio_fifo.h | 2 +-
> > libavutil/hwcontext.h | 21 -
> > libavutil/hwcontext_cuda.h | 2 +-
> > libavutil/hwcontext_d3d11va.h | 4 ++--
> > libavutil/hwcontext_d3d12va.h | 6 +++---
> > libavutil/hwcontext_drm.h | 2 +-
> > libavutil/hwcontext_dxva2.h| 4 ++--
> > libavutil/hwcontext_mediacodec.h | 2 +-
> > libavutil/hwcontext_opencl.h | 4 ++--
> > libavutil/hwcontext_qsv.h | 4 ++--
> > libavutil/hwcontext_vaapi.h| 6 +++---
> > libavutil/hwcontext_vdpau.h| 2 +-
> > libavutil/hwcontext_vulkan.h | 4 ++--
> > libavutil/lfg.h| 2 +-
> > 36 files changed, 66 insertions(+), 57 deletions(-)
> >
> > diff --git a/libavcodec/aac/aacdec.h b/libavcodec/aac/aacdec.h
> > index 4cf764e2e9..71d61813f4 100644
> > --- a/libavcodec/aac/aacdec.h
> > +++ b/libavcodec/aac/aacdec.h
> > @@ -248,7 +248,7 @@ typedef struct AACDecDSP {
> > } AACDecDSP;
> >
> > /**
> > - * main AAC decoding context
> > + * main AAC decoding @ref md_doc_2context "context"
> > */
> > struct AACDecContext {
> > const struct AVClass *class;
> > diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h
> > index d07960620e..1a645f4719 100644
> > --- a/libavcodec/aacenc.h
> > +++ b/libavcodec/aacenc.h
> > @@ -207,7 +207,7 @@ typedef struct AACPCEInfo {
> > } AACPCEInfo;
> >
> > /**
> > - * AAC encoder context
> > + * AAC encoder @ref md_doc_2context "context"
> > */
> > typedef struct AACEncContext {
> > AVClass *av_class;
> > diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
> > index 30812617cc..c725007077 100644
> > --- a/libavcodec/ac3enc.h
> > +++ b/libavcodec/ac3enc.h
> > @@ -152,7 +152,7 @@ typedef struct AC3Block {
> > } AC3Block;
> >
> > /**
> > - * AC-3 encoder private context.
> > + * AC-3 encoder private @ref md_doc_2context "context"
> > */
> > typedef struct AC3EncodeContext {
> > AVClass *av_class; ///< AVClass used for AVOption
> > diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> > index 2dbd378ef8..f142ede63a 100644
> > --- a/libavcodec/amfenc.h
> > +++ b/libavcodec/amfenc.h
> > @@ -43,7 +43,7 @@ typedef struct AmfTraceWriter {
> > } AmfTraceWriter;
> >
> > /**
> > -* AMF encoder context
> > +* AMF encoder @ref md_doc_2context "context"
> > */
> >
> > typedef struct AmfContext {
> > diff --git a/libavcodec/atrac.h b/libavcodec/atrac.h
> > index 05208bbee6..1527e376a9 100644
> > --- a/libavcodec/atrac.h
> > +++ b/libavcodec/atrac.h
> > @@ -39,7 +39,7 @@ typedef struct AtracGainInfo {
> > } AtracGainInfo;
> >
> > /**
> > - * Gain compensation context structure.
> > + * Gain compensation @ref md_doc_2context "context"
> > */
> > typedef struct AtracGCContext {
> > float gain_tab1[16]; ///< gain compensation level table
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 968009a192..9180fedca7 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -430,7 +430,8 @@ typedef struct RcOverride{
> > #define AV_GET_ENCODE_BUFFER_FLAG_REF (1 << 0)
> >
> > /**
> > - * main external API structure.
> > + * @ref md_doc_2context "Context" for an encode or decode session
> > + *
> > * New fields can be added to the end with minor version bumps.
> > * Removal, reordering and changes to existing fields require a major
> > * version bump.
> > diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
> > index a09c69f242..bf79afa7cc 100644
> > --- a/libavcodec/bsf.h
> > +++ b/libavcodec/bsf.h
> > @@ -56,7 +56,7 @@
> > */
> >
> > /**
> > - * The bitstream filter state.
> > + * Bitstream filter @ref
Re: [FFmpeg-devel] [PATCH v4 3/4] all: Link to "context" from all contexts with documentation
Apr 29, 2024, 11:24 by ffmpeg-de...@pileofstuff.org:
> Some headings needed to be rewritten to accomodate the text,
> (hopefully) without changing the meaning.
> ---
> libavcodec/aac/aacdec.h| 2 +-
> libavcodec/aacenc.h| 2 +-
> libavcodec/ac3enc.h| 2 +-
> libavcodec/amfenc.h| 2 +-
> libavcodec/atrac.h | 2 +-
> libavcodec/avcodec.h | 3 ++-
> libavcodec/bsf.h | 2 +-
> libavcodec/cbs.h | 2 +-
> libavcodec/d3d11va.h | 3 +--
> libavcodec/h264dsp.h | 2 +-
> libavcodec/h264pred.h | 2 +-
> libavcodec/mediacodec.h| 2 +-
> libavcodec/mpegaudiodec_template.c | 2 +-
> libavcodec/pthread_frame.c | 4 ++--
> libavcodec/qsv.h | 6 --
> libavcodec/sbr.h | 2 +-
> libavcodec/smacker.c | 2 +-
> libavcodec/vdpau.h | 3 ++-
> libavcodec/videotoolbox.h | 5 +++--
> libavfilter/avfilter.h | 2 +-
> libavformat/avformat.h | 3 ++-
> libavformat/avio.h | 3 ++-
> libavutil/audio_fifo.h | 2 +-
> libavutil/hwcontext.h | 21 -
> libavutil/hwcontext_cuda.h | 2 +-
> libavutil/hwcontext_d3d11va.h | 4 ++--
> libavutil/hwcontext_d3d12va.h | 6 +++---
> libavutil/hwcontext_drm.h | 2 +-
> libavutil/hwcontext_dxva2.h| 4 ++--
> libavutil/hwcontext_mediacodec.h | 2 +-
> libavutil/hwcontext_opencl.h | 4 ++--
> libavutil/hwcontext_qsv.h | 4 ++--
> libavutil/hwcontext_vaapi.h| 6 +++---
> libavutil/hwcontext_vdpau.h| 2 +-
> libavutil/hwcontext_vulkan.h | 4 ++--
> libavutil/lfg.h| 2 +-
> 36 files changed, 66 insertions(+), 57 deletions(-)
>
> diff --git a/libavcodec/aac/aacdec.h b/libavcodec/aac/aacdec.h
> index 4cf764e2e9..71d61813f4 100644
> --- a/libavcodec/aac/aacdec.h
> +++ b/libavcodec/aac/aacdec.h
> @@ -248,7 +248,7 @@ typedef struct AACDecDSP {
> } AACDecDSP;
>
> /**
> - * main AAC decoding context
> + * main AAC decoding @ref md_doc_2context "context"
> */
> struct AACDecContext {
> const struct AVClass *class;
> diff --git a/libavcodec/aacenc.h b/libavcodec/aacenc.h
> index d07960620e..1a645f4719 100644
> --- a/libavcodec/aacenc.h
> +++ b/libavcodec/aacenc.h
> @@ -207,7 +207,7 @@ typedef struct AACPCEInfo {
> } AACPCEInfo;
>
> /**
> - * AAC encoder context
> + * AAC encoder @ref md_doc_2context "context"
> */
> typedef struct AACEncContext {
> AVClass *av_class;
> diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
> index 30812617cc..c725007077 100644
> --- a/libavcodec/ac3enc.h
> +++ b/libavcodec/ac3enc.h
> @@ -152,7 +152,7 @@ typedef struct AC3Block {
> } AC3Block;
>
> /**
> - * AC-3 encoder private context.
> + * AC-3 encoder private @ref md_doc_2context "context"
> */
> typedef struct AC3EncodeContext {
> AVClass *av_class; ///< AVClass used for AVOption
> diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> index 2dbd378ef8..f142ede63a 100644
> --- a/libavcodec/amfenc.h
> +++ b/libavcodec/amfenc.h
> @@ -43,7 +43,7 @@ typedef struct AmfTraceWriter {
> } AmfTraceWriter;
>
> /**
> -* AMF encoder context
> +* AMF encoder @ref md_doc_2context "context"
> */
>
> typedef struct AmfContext {
> diff --git a/libavcodec/atrac.h b/libavcodec/atrac.h
> index 05208bbee6..1527e376a9 100644
> --- a/libavcodec/atrac.h
> +++ b/libavcodec/atrac.h
> @@ -39,7 +39,7 @@ typedef struct AtracGainInfo {
> } AtracGainInfo;
>
> /**
> - * Gain compensation context structure.
> + * Gain compensation @ref md_doc_2context "context"
> */
> typedef struct AtracGCContext {
> float gain_tab1[16]; ///< gain compensation level table
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 968009a192..9180fedca7 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -430,7 +430,8 @@ typedef struct RcOverride{
> #define AV_GET_ENCODE_BUFFER_FLAG_REF (1 << 0)
>
> /**
> - * main external API structure.
> + * @ref md_doc_2context "Context" for an encode or decode session
> + *
> * New fields can be added to the end with minor version bumps.
> * Removal, reordering and changes to existing fields require a major
> * version bump.
> diff --git a/libavcodec/bsf.h b/libavcodec/bsf.h
> index a09c69f242..bf79afa7cc 100644
> --- a/libavcodec/bsf.h
> +++ b/libavcodec/bsf.h
> @@ -56,7 +56,7 @@
> */
>
> /**
> - * The bitstream filter state.
> + * Bitstream filter @ref md_doc_2context "context"
> *
> * This struct must be allocated with av_bsf_alloc() and freed with
> * av_bsf_free().
> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> index d479b1ac2d..0ff64d2fef 100644
> --- a/libavcodec/cbs.h
> +++ b/libavcodec/cbs.h
> @@ -214,7 +214,7 @@ typedef void
Re: [FFmpeg-devel] Massive memory leak in 6.1.1 (fixed on master)
On 01/05/2024 18:52, Ville Syrjälä wrote: [...] So those should be cherry-picked to the next 6.1 release (assuming there will be one). Both cherry-pick cleanly, and afterwards the leak is gone from the 6.1 branch as well. From the release/6.1 branch it seems that a 6.1.2 release has been prepared by Michael in commit 9593b727e2751e5a79be86a7327a98f3422fa505 but not tagged yet. Regards, Tobias ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/11] avcodec: add avcodec_get_supported_config()
On Thu, 11 Apr 2024 00:09:05 +0200 Michael Niedermayer wrote: > On Mon, Apr 08, 2024 at 11:55:02PM +0200, Niklas Haas wrote: > > On Mon, 08 Apr 2024 22:18:33 +0200 Michael Niedermayer > > wrote: > > > On Fri, Apr 05, 2024 at 08:57:11PM +0200, Niklas Haas wrote: > > > > From: Niklas Haas > > > > > > > > This replaces the myriad of existing lists in AVCodec by a unified API > > > > call, allowing us to (ultimately) trim down the sizeof(AVCodec) quite > > > > substantially, while also making this more trivially extensible. > > > > > > > > In addition to the already covered lists, add two new entries for color > > > > space and color range, mirroring the newly added negotiable fields in > > > > libavfilter. > > > > > > > > I decided to drop the explicit length field from the API proposed by > > > > Andreas Rheinhardt, because having it in place ended up complicating > > > > both the codec side and the client side implementations, while also > > > > being strictly less flexible (it's trivial to recover a length given > > > > a terminator, but requires allocation to add a terminator given > > > > a length). Using a terminator also presents less of a porting challenge > > > > for existing users of the current API. > > > > > > > > Once the deprecation period passes for the existing public fields, the > > > > rough plan is to move the commonly used fields (such as > > > > pix_fmt/sample_fmt) into FFCodec, possibly as a union of audio and video > > > > configuration types, and then implement the rarely used fields with > > > > custom callbacks. > > > > --- > > > > doc/APIchanges | 5 > > > > libavcodec/avcodec.c| 51 + > > > > libavcodec/avcodec.h| 27 > > > > libavcodec/codec.h | 19 +++--- > > > > libavcodec/codec_internal.h | 21 +++ > > > > libavcodec/version.h| 4 +-- > > > > 6 files changed, 121 insertions(+), 6 deletions(-) > > > > > > This patchset seems to overlap a bit with AVOptionRanges > > > > > > I think ideally the user should at some point be able to query using some > > > API on a AVCodecContext/AVCodecParameters/AVFormatContex/AVStream > > > what for that specific instance are supported settings for each field > > > > > > The API here seems to use a enum, which can make sense but it differs from > > > how AVOption works which doesnt use enums to identify fields > > > > I didn't know AVOptionRanges exists; indeed it can be seen as somewhat > > overlapping here. That said, there is a vital distinction here: > > AVOptionRanges > > represents *static* limits on what options can be set (e.g. via > > `av_opt_set_int`), whereas this API represents *dynamic* limits on what can > > be > > coded. > > AVOptionRanges where definitly not intended to be static > > see the docs: > * The returned list may depend on other fields in obj like for example > profile. > > that would not be static > > > > > > > In particular, the list of supported colorspaces etc. can *depend* on the > > value > > of other options. Currently, only strict_std_compliance, but I can easily > > see > > this list growing in the future (e.g. for different profiles, dolbyvision, > > ...). > > > > That aside, I personally find an API based on strings and doubles rather > > cumbersome to use, especially when downstream clients expect an enum list. > > AVOption* is intended to be a generic API. > For example something that an App can query to build a drop down menu in a GUI > for each parameter > > For this, it must be possible to iterate over all paremeters, then for each > iterate over all possible settings if its a discrete type or range if its a > continous type. And then present the user with the corresponding GUI elements > > thx Okay, then I do not present as strong an objection as I thought. That said, I still think that downstream API users will be very thankful for having a replacement API that's easy to migrate to, rather one that's IMO rather difficult to use (and which requires both FPU and malloc to access what used to be just a static list). What do other people think? Should we introduce this new API as-is, or should it be scrapped in favor of reusing AVOptionRanges? ___ 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
On Wed, 24 Apr 2024 12:51:55 +0200 Niklas Haas wrote: > As discussed in my previous series for fixing scale2ref[1], this filter > is fundamentally broken, and the only real fix would be to switch to > activate(), or ideally FFFrameSync. > > [1] https://ffmpeg.org//pipermail/ffmpeg-devel/2024-March/323382.html > > The main thing making this difficult is the fact that scale2ref also > wants to output ref frames to its secondary output, which FFFrameSync > does not support, and which is ultimately at least part of the root > cause of trac #10795. > > Since this is in principle completely unnecessary (users can just > 'split' the ref input and have it be consumed by vf_scale), and to make > the design of this filter a bit more robust and maintainable, switch to > an approach where vf_scale itself gains the ability to reference > a secondary input stream, using the "ref_*" series of variables. > > This makes the current [i][ri]scale2ref[o][ro] equivalent to the only > slightly more verbose [ri]split[t][ro]; [i][t]scale=rw:rh[o]. (And > conversely, it is no longer necessary to use nullsink to consume an > unused [ro]) > > 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. > > Either way, after the deprecation period has elapsed, we can delete > scale2ref from the codebase in order to make vf_scale.c IMHO > significantly simpler versus the status quo. > > ___ > 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". Will merge in around 24h if there is no objection, as this is fixing a bug marked important. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/6] avutil/frame: add av_frame_remove_side_data_changed
On Fri, 26 Apr 2024 16:29:03 -0300 James Almer wrote: > On 4/26/2024 9:27 AM, Niklas Haas wrote: > > From: Niklas Haas > > > > Many filters modify certain aspects of frame data, e.g. through resizing > > (vf_*scale* family), color volume mapping (vf_lut*, vf_tonemap*), or > > possibly others. > > > > When this happens, we should strip all frame side data that will no > > longer be correct/relevant after the operation. For example, changing > > the image size should invalidate AV_FRAME_DATA_PANSCAN because the crop > > window (given in pixels) no longer corresponds to the actual image size. > > For another example, tone-mapping filters (e.g. from HDR to SDR) should > > strip all of the dynamic HDR related metadata. > > > > Since there are a lot of similar with basically similar operations, it > > make sense to consolidate this stripping logic into a common helper > > function. I decided to put it into libavutil as it may be useful for API > > users as well, who often have their own internal processing and > > filtering. > > Maybe instead of "changed", which is a concept that doesn't belong to a > frame but to a process, use a name that references side data that > depends on specific properties (dimensions, color props, etc). > > Also, don't make it depend on AVFrame, but AVFrameSideData instead, so > it can be reused in other contexts (Use the av_frame_side_data_* > namespace for it). Do you have a proposed name? I can think of: - av_frame_side_data_remove_aspect() - av_frame_side_data_remove_dependent() - av_frame_side_data_remove_category() Not sure if I like any more (which is why I went with _changed(), which tells you exactly when this function was intended to be used). ___ 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 Mon, Apr 29, 2024 at 10:10:35AM +0100, Andrew Sayers wrote: > > 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? > > av_register_bitstream_filter() (deprecated 4.0, removed 5.0) took an > `AVBitStreamFilter *` as its first argument, but I don't think you'd say > the argument provided "context" for the function. So would I be right in > saying `AVBitStreamFilter *` is not a context, despite looking like one? > > av_buffersink_*() all take a `const AVFilterContext *` argument. > What does the difference between av_buffersink prefix and AVFilter type mean? > > 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? > > Related to the previous question, does `av_cmp_q()` count as a function > with two contexts? Or no contexts? > > Finally, a general question - functions of the form "avfoo" seem like they > are more consistent than "av_foo". Does the underscore mean anything? One extra question I hadn't thought to ask before - when, if at all, would an ordinary user be expected to access the contents of AVClass directly? Or AVOption for that matter? For example, it's not clear to me how people are supposed to use AVClass.category - is this an important use case we haven't covered? Either way, happy to propose an AVClass patch to make it clearer. It's my turn to be off for a few days, so will pick up any responses next week. ___ 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/av1dec: Always set ret before goto end
Before 0f8763fbea4e8816cd54c2a481d4c048fec58394, av1_frame_ref()
and update_reference_list() could fail and therefore needed to
be checked, which incidentally set ret. This is no longer happening,
leading to a potential use of an uninitialized value which is
also the subject of Coverity ticket #1596605.
Fix this by always setting ret before goto end; do not return
some random ancient value.
Signed-off-by: Andreas Rheinhardt
---
Here is a different approach that uses the translation from 0 to EAGAIN.
libavcodec/av1dec.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 79a30a114d..75cc3fba48 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1333,12 +1333,15 @@ static int av1_receive_frame_internal(AVCodecContext
*avctx, AVFrame *frame)
if (s->cur_frame.f) {
ret = set_output_frame(avctx, frame);
-if (ret < 0)
+if (ret < 0) {
av_log(avctx, AV_LOG_ERROR, "Set output frame
error.\n");
+goto end;
+}
}
s->raw_frame_header = NULL;
i++;
+ret = 0;
goto end;
}
@@ -1439,17 +1442,20 @@ static int av1_receive_frame_internal(AVCodecContext
*avctx, AVFrame *frame)
update_reference_list(avctx);
-if (s->raw_frame_header->show_frame && s->cur_frame.f) {
-ret = set_output_frame(avctx, frame);
-if (ret < 0) {
-av_log(avctx, AV_LOG_ERROR, "Set output frame error\n");
-goto end;
-}
-}
-raw_tile_group = NULL;
+raw_tile_group = NULL;
s->raw_frame_header = NULL;
+
if (show_frame) {
+// cur_frame.f needn't exist due to skip_frame.
+if (s->cur_frame.f) {
+ret = set_output_frame(avctx, frame);
+if (ret < 0) {
+av_log(avctx, AV_LOG_ERROR, "Set output frame
error\n");
+goto end;
+}
+}
i++;
+ret = 0;
goto end;
}
}
--
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] avcodec/av1dec: Always set ret before goto end
Hendrik Leppkes:
> On Thu, May 2, 2024 at 10:22 AM Andreas Rheinhardt
> wrote:
>>
>> Before 0f8763fbea4e8816cd54c2a481d4c048fec58394, av1_frame_ref()
>> and update_reference_list() could fail and therefore needed to
>> be checked, which incidentally set ret. This is no longer happening,
>> leading to a potential use of an uninitialized value which is
>> also the subject of Coverity ticket #1596605.
>>
>> Fix this by always setting ret before goto end; do not return
>> some random ancient value.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavcodec/av1dec.c | 12 ++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>> index 79a30a114d..c3f255a29a 100644
>> --- a/libavcodec/av1dec.c
>> +++ b/libavcodec/av1dec.c
>> @@ -1335,6 +1335,12 @@ static int av1_receive_frame_internal(AVCodecContext
>> *avctx, AVFrame *frame)
>> ret = set_output_frame(avctx, frame);
>> if (ret < 0)
>> av_log(avctx, AV_LOG_ERROR, "Set output frame
>> error.\n");
>> +} else {
>> +// CBS checks for us that the frame to be shown
>> actually existed
>> +// in the bitstream; if it doesn't it could be e.g. due
>> to
>> +// skip_frame setting. Return EAGAIN to indicate that
>> we are
>> +// currently unable to produce output.
>> +ret = AVERROR(EAGAIN);
>> }
>>
>
> In the vein of this comment, set_output_frame will also return 0
> without returning a frame in some cases - eg. with multiple layers.
> Should this equally return EAGAIN rather than claiming success without
> a frame?
>
Thanks for pointing this out. There is a translation 0->AVERROR(EAGAIN)
at the end of this function if the output frame is unset. Maybe this
commit should rather set ret to 0 and rely on this (as it was before
0f8763fbea)? Let's hear what James says about this.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: Always set ret before goto end
On Thu, May 2, 2024 at 10:22 AM Andreas Rheinhardt
wrote:
>
> Before 0f8763fbea4e8816cd54c2a481d4c048fec58394, av1_frame_ref()
> and update_reference_list() could fail and therefore needed to
> be checked, which incidentally set ret. This is no longer happening,
> leading to a potential use of an uninitialized value which is
> also the subject of Coverity ticket #1596605.
>
> Fix this by always setting ret before goto end; do not return
> some random ancient value.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/av1dec.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 79a30a114d..c3f255a29a 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -1335,6 +1335,12 @@ static int av1_receive_frame_internal(AVCodecContext
> *avctx, AVFrame *frame)
> ret = set_output_frame(avctx, frame);
> if (ret < 0)
> av_log(avctx, AV_LOG_ERROR, "Set output frame
> error.\n");
> +} else {
> +// CBS checks for us that the frame to be shown actually
> existed
> +// in the bitstream; if it doesn't it could be e.g. due
> to
> +// skip_frame setting. Return EAGAIN to indicate that we
> are
> +// currently unable to produce output.
> +ret = AVERROR(EAGAIN);
> }
>
In the vein of this comment, set_output_frame will also return 0
without returning a frame in some cases - eg. with multiple layers.
Should this equally return EAGAIN rather than claiming success without
a frame?
- 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 01/10, v2] avutil: add hwcontext_amf.
May 2, 2024, 10:04 by ovchinnikov.dmit...@gmail.com: > >>Is there a reason to add this code in now? > DX12 and Vulkan native encoders will expose less features compare to AMF, > at least in foreseeable feature. The missing features include low latency, > PreAnalysis including look-ahead etc. > Do you really not talk with each other over there? You should. This is a lot of vendor-specific code for which an overlap with a standard API already exists, and I'd just prefer to know why this should be merged and maintained now, as Vulkan video adoption is finally starting. ___ 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 5/7] avcodec/avs3_parser: Check the return value of init_get_bits8()
Michael Niedermayer: > Fixes: CID1492867 Unchecked return value > > Sponsored-by: Sovereign Tech Fund > Signed-off-by: Michael Niedermayer > --- > libavcodec/avs3_parser.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/avs3_parser.c b/libavcodec/avs3_parser.c > index a819b5783d6..0f9076befe1 100644 > --- a/libavcodec/avs3_parser.c > +++ b/libavcodec/avs3_parser.c > @@ -73,7 +73,9 @@ static void parse_avs3_nal_units(AVCodecParserContext *s, > const uint8_t *buf, > GetBitContext gb; > int profile, ratecode, low_delay; > > -init_get_bits8(, buf + 4, buf_size - 4); > +int ret = init_get_bits8(, buf + 4, buf_size - 4); > +if (ret < 0) > +return; > > s->key_frame = 1; > s->pict_type = AV_PICTURE_TYPE_I; This code only reads/skips a few bits here (at most 100 if I counted correctly), so one could initialize the reader for a shorter length and assert that the call succeeds. - 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 4/7] avcodec/avs2_parser: Assert init_get_bits8() success with const size 15
Michael Niedermayer: > Fixes: CID1506708 Unchecked return value > > Sponsored-by: Sovereign Tech Fund > Signed-off-by: Michael Niedermayer > --- > libavcodec/avs2_parser.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/avs2_parser.c b/libavcodec/avs2_parser.c > index 200134f91db..8d4bc3cee0d 100644 > --- a/libavcodec/avs2_parser.c > +++ b/libavcodec/avs2_parser.c > @@ -72,13 +72,15 @@ static void parse_avs2_seq_header(AVCodecParserContext > *s, const uint8_t *buf, > unsigned aspect_ratio; > unsigned frame_rate_code; > int low_delay; > +int ret; > // update buf_size_min if parse more deeper > const int buf_size_min = 15; > > if (buf_size < buf_size_min) > return; > > -init_get_bits8(, buf, buf_size_min); > +ret = init_get_bits8(, buf, buf_size_min); > +av_assert2(ret >= 0); > > s->key_frame = 1; > s->pict_type = AV_PICTURE_TYPE_I; Code like this may trigger set-but-unused variable warnings when ASSERT_LEVEL is <= 1. Add av_unused to ret to fix this. Apart from that, it should be av_assert1 (this is not hot). - 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] avcodec/av1dec: Always set ret before goto end
Before 0f8763fbea4e8816cd54c2a481d4c048fec58394, av1_frame_ref()
and update_reference_list() could fail and therefore needed to
be checked, which incidentally set ret. This is no longer happening,
leading to a potential use of an uninitialized value which is
also the subject of Coverity ticket #1596605.
Fix this by always setting ret before goto end; do not return
some random ancient value.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/av1dec.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 79a30a114d..c3f255a29a 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1335,6 +1335,12 @@ static int av1_receive_frame_internal(AVCodecContext
*avctx, AVFrame *frame)
ret = set_output_frame(avctx, frame);
if (ret < 0)
av_log(avctx, AV_LOG_ERROR, "Set output frame
error.\n");
+} else {
+// CBS checks for us that the frame to be shown actually
existed
+// in the bitstream; if it doesn't it could be e.g. due to
+// skip_frame setting. Return EAGAIN to indicate that we
are
+// currently unable to produce output.
+ret = AVERROR(EAGAIN);
}
s->raw_frame_header = NULL;
@@ -1439,13 +1445,15 @@ static int av1_receive_frame_internal(AVCodecContext
*avctx, AVFrame *frame)
update_reference_list(avctx);
-if (s->raw_frame_header->show_frame && s->cur_frame.f) {
+// cur_frame.f needn't exist due to skip_frame.
+if (show_frame && s->cur_frame.f) {
ret = set_output_frame(avctx, frame);
if (ret < 0) {
av_log(avctx, AV_LOG_ERROR, "Set output frame error\n");
goto end;
}
-}
+} else
+ret = AVERROR(EAGAIN);
raw_tile_group = NULL;
s->raw_frame_header = NULL;
if (show_frame) {
--
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 01/10, v2] avutil: add hwcontext_amf.
>>Is there a reason to add this code in now? DX12 and Vulkan native encoders will expose less features compare to AMF, at least in foreseeable feature. The missing features include low latency, PreAnalysis including look-ahead etc. AMF context on Windows allows fully enable SAV - ability to utilize VCNs in dGPU and APU in a single session. Eventually specialized multimedia AMD cards could be added seamlessly to FFmpeg with AMF integration. AMF FSR(VSR) includes YUV version with focus on videos which is not available in AMD FSR aimed for gaming. -- Sincerely, Ovchinnikov D.A. ___ 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/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
Michael Niedermayer:
> Fixes: CID1596605 Uninitialized scalar variable
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer
> ---
> libavcodec/av1dec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 4f9222cca27..93ab04eb378 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext
> *avctx, AVFrame *frame)
> {
> AV1DecContext *s = avctx->priv_data;
> AV1RawTileGroup *raw_tile_group = NULL;
> -int i = 0, ret;
> +int i = 0, ret = 0;
>
> for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
> CodedBitstreamUnit *unit = >current_obu.units[i];
A better approach is to actually initialize ret before every goto end in
order to ensure that only the actually desired ret is returned and not
some earlier value.
- 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 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test
Michael Niedermayer:
> Fixes: CID1439654 Untrusted pointer read
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer
> ---
> libavcodec/cbs_jpeg.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> index b1b58dcd65e..406147c082c 100644
> --- a/libavcodec/cbs_jpeg.c
> +++ b/libavcodec/cbs_jpeg.c
> @@ -146,13 +146,13 @@ static int
> cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
> }
> } else {
> i = start;
> -if (i + 2 > frag->data_size) {
> +if (i > frag->data_size - 2) {
> av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> "truncated at %02x marker.\n", marker);
> return AVERROR_INVALIDDATA;
> }
> length = AV_RB16(frag->data + i);
> -if (i + length > frag->data_size) {
> +if (length > frag->data_size - i) {
> av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> "truncated at %02x marker segment.\n", marker);
> return AVERROR_INVALIDDATA;
You should always mention when you are not fixing bugs in our code, but
rather intend to apply workaround for coverity crazyness (i.e. the
requirement that reading values in non-native endianness needs to be
sanitized).
- 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".
