date:20201027

---
 libswscale/x86/Makefile |   1 +
 libswscale/x86/swscale.c|  75 --
 libswscale/x86/yuv2yuvX.asm | 105 
 3 files changed, 116 insertions(+), 65 deletions(-)
 create mode 100644 libswscale/x86/yuv2yuvX.asm

diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
index 831d5359aa..bfe383364e 100644
--- a/libswscale/x86/Makefile
+++ b/libswscale/x86/Makefile
@@ -13,3 +13,4 @@ X86ASM-OBJS += x86/input.o
  \
x86/scale.o  \
x86/rgb_2_rgb.o  \
x86/yuv_2_rgb.o  \
+   x86/yuv2yuvX.o   \
diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
index 3160fedf04..fec9fa22e0 100644
--- a/libswscale/x86/swscale.c
+++ b/libswscale/x86/swscale.c
@@ -197,80 +197,25 @@ void ff_updateMMXDitherTables(SwsContext *c, int dstY)
 }
 
 #if HAVE_MMXEXT
+void ff_yuv2yuvX_sse3(const int16_t *filter, int filterSize,
+   uint8_t *dest, int dstW,
+   const uint8_t *dither, int offset);
+
 static void yuv2yuvX_sse3(const int16_t *filter, int filterSize,
const int16_t **src, uint8_t *dest, int dstW,
const uint8_t *dither, int offset)
 {
+int remainder = (dstW % 32);
+int pixelsProcessed = dstW - remainder;
 if(((uintptr_t)dest) & 15){
 yuv2yuvX_mmxext(filter, filterSize, src, dest, dstW, dither, offset);
 return;
 }
-filterSize--;
-#define MAIN_FUNCTION \
-"pxor   %%xmm0, %%xmm0 \n\t" \
-"punpcklbw  %%xmm0, %%xmm3 \n\t" \
-"movd   %4, %%xmm1 \n\t" \
-"punpcklwd  %%xmm1, %%xmm1 \n\t" \
-"punpckldq  %%xmm1, %%xmm1 \n\t" \
-"punpcklqdq %%xmm1, %%xmm1 \n\t" \
-"psllw  $3, %%xmm1 \n\t" \
-"paddw  %%xmm1, %%xmm3 \n\t" \
-"psraw  $4, %%xmm3 \n\t" \
-"movdqa %%xmm3, %%xmm4 \n\t" \
-"movdqa %%xmm3, %%xmm7 \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"\
-"movddup  8(%%"FF_REG_d"), %%xmm0   \n\t" /* 
filterCoeff */\
-"movdqa  (%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm2 \n\t" /* 
srcData */\
-"movdqa16(%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm5 \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   %%xmm0, %%xmm2  \n\t"\
-"pmulhw   %%xmm0, %%xmm5  \n\t"\
-"paddw%%xmm2, %%xmm3  \n\t"\
-"paddw%%xmm5, %%xmm4  \n\t"\
-" jnz1b \n\t"\
-"psraw   $3, %%xmm3  \n\t"\
-"psraw   $3, %%xmm4  \n\t"\
-"packuswb %%xmm4, %%xmm3  \n\t"\
-"movntdq  %%xmm3, (%1, %%"FF_REG_c") \n\t"\
-"add $16, %%"FF_REG_c"\n\t"\
-"cmp  %2, %%"FF_REG_c"\n\t"\
-"movdqa   %%xmm7, %%xmm3\n\t" \
-"movdqa   %%xmm7, %%xmm4\n\t" \
-"mov %0, %%"FF_REG_d"\n\t"\
-"mov(%%"FF_REG_d"), %%"FF_REG_S" \n\t"\
-"jb  1b \n\t"
-
-if (offset) {
-__asm__ volatile(
-"movq  %5, %%xmm3  \n\t"
-"movdqa%%xmm3, %%xmm4  \n\t"
-"psrlq$24, %%xmm3  \n\t"
-"psllq$40, %%xmm4  \n\t"
-"por   %%xmm4, %%xmm3  \n\t"
-MAIN_FUNCTION
-  :: "g" (filter),
-  "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m" (offset),
-  "m"(filterSize), "m"(((uint64_t *) dither)[0])
-  : XMM_CLOBBERS("%xmm0" , "%xmm1" , "%xmm2" , "%xmm3" , "%xmm4" , 
"%xmm5" , "%xmm7" ,)
-"%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
-  );
-} else {
-__asm__ volatile(
-"movq  %5, %%xmm3   \n\t"
-MAIN_FUNCTION
-  :: "g"

Re: [FFmpeg-devel] [PATCH] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup. AVX2 version is ready and tested, however, although local tests show a signifi

Thanks for the review, I have made the required changes. As I have changed
the subject the patch is in a new thread.

On Fri, Oct 23, 2020 at 4:10 PM James Almer  wrote:

> On 10/23/2020 10:17 AM, Alan Kelly wrote:
> >  Fixed. The wrong step size was used causing a write passed the end of
> >  the buffer. yuv2yuvX_mmxext is now called if there are any remaining
> pixels.
>
> Please fix the commit subject (It's too long and contains commentary),
> and keep comments about fixes between versions outside of the commit
> message body. You can manually place them after the --- below, or in a
> separate reply.
>
> > ---
> >  libswscale/x86/Makefile |   1 +
> >  libswscale/x86/swscale.c|  75 --
> >  libswscale/x86/yuv2yuvX.asm | 105 
> >  3 files changed, 116 insertions(+), 65 deletions(-)
> >  create mode 100644 libswscale/x86/yuv2yuvX.asm
> >
> > diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
> > index 831d5359aa..bfe383364e 100644
> > --- a/libswscale/x86/Makefile
> > +++ b/libswscale/x86/Makefile
> > @@ -13,3 +13,4 @@ X86ASM-OBJS += x86/input.o
>   \
> > x86/scale.o
> \
> > x86/rgb_2_rgb.o
> \
> > x86/yuv_2_rgb.o
> \
> > +   x86/yuv2yuvX.o
>  \
> > diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
> > index 3160fedf04..fec9fa22e0 100644
> > --- a/libswscale/x86/swscale.c
> > +++ b/libswscale/x86/swscale.c
> > @@ -197,80 +197,25 @@ void ff_updateMMXDitherTables(SwsContext *c, int
> dstY)
> >  }
> >
> >  #if HAVE_MMXEXT
> > +void ff_yuv2yuvX_sse3(const int16_t *filter, int filterSize,
> > +   uint8_t *dest, int dstW,
> > +   const uint8_t *dither, int offset);
> > +
> >  static void yuv2yuvX_sse3(const int16_t *filter, int filterSize,
> > const int16_t **src, uint8_t *dest, int dstW,
> > const uint8_t *dither, int offset)
> >  {
> > +int remainder = (dstW % 32);
> > +int pixelsProcessed = dstW - remainder;
> >  if(((uintptr_t)dest) & 15){
> >  yuv2yuvX_mmxext(filter, filterSize, src, dest, dstW, dither,
> offset);
> >  return;
> >  }
> > -filterSize--;
> > -#define MAIN_FUNCTION \
> > -"pxor   %%xmm0, %%xmm0 \n\t" \
> > -"punpcklbw  %%xmm0, %%xmm3 \n\t" \
> > -"movd   %4, %%xmm1 \n\t" \
> > -"punpcklwd  %%xmm1, %%xmm1 \n\t" \
> > -"punpckldq  %%xmm1, %%xmm1 \n\t" \
> > -"punpcklqdq %%xmm1, %%xmm1 \n\t" \
> > -"psllw  $3, %%xmm1 \n\t" \
> > -"paddw  %%xmm1, %%xmm3 \n\t" \
> > -"psraw  $4, %%xmm3 \n\t" \
> > -"movdqa %%xmm3, %%xmm4 \n\t" \
> > -"movdqa %%xmm3, %%xmm7 \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"\
> > -"movddup  8(%%"FF_REG_d"), %%xmm0   \n\t" /*
> filterCoeff */\
> > -"movdqa  (%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm2
> \n\t" /* srcData */\
> > -"movdqa16(%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm5
> \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   %%xmm0, %%xmm2  \n\t"\
> > -"pmulhw   %%xmm0, %%xmm5  \n\t"\
> > -"paddw%%xmm2, %%xmm3  \n\t"\
> > -"paddw%%xmm5, %%xmm4  \n\t"\
> > -" jnz1b \n\t"\
> > -"psraw   $3, %%xmm3  \n\t"\
> > -"psraw   $3, %%xmm4  \n\t"\
> > -"packuswb %%xmm4, %%xmm3  \n\t"\
> > -"movntdq  %%xmm3, (%1, %%"FF_REG_c")
> \n\t"\
> > -"add $16, %%"FF_REG_c"\n\t"\
> > -"cmp  %2, %%"FF_REG_c"\n\t"\
> > -"movdqa   %%xmm7, %%xmm3\n\t" \
> > -"movdqa   %%xmm7, %%xmm4\n\t" \
> > -"mov %0, %%"FF_REG_d"
> \n\t"\
> > -"mov(%%"FF_REG_d"), %%"FF_REG_S"
>  \n\t"\
> > -"jb

Re: [FFmpeg-devel] [PATCH] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup.

Apologies for the multiple threads, my git send-email was wrongly
configured. This has been fixed.

This code has been tested on AVX2 giving a significant speedup, however,
until the ff_hscale* functions are ported to avx2, this should not be
enabled as it results in an overall slowdown of swscale probably due to cpu
frequency scaling.

checkasm will follow in a separate patch.

On Tue, Oct 27, 2020 at 9:56 AM Alan Kelly  wrote:

> ---
>  libswscale/x86/Makefile |   1 +
>  libswscale/x86/swscale.c|  75 --
>  libswscale/x86/yuv2yuvX.asm | 105 
>  3 files changed, 116 insertions(+), 65 deletions(-)
>  create mode 100644 libswscale/x86/yuv2yuvX.asm
>
> diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
> index 831d5359aa..bfe383364e 100644
> --- a/libswscale/x86/Makefile
> +++ b/libswscale/x86/Makefile
> @@ -13,3 +13,4 @@ X86ASM-OBJS += x86/input.o
> \
> x86/scale.o  \
> x86/rgb_2_rgb.o  \
> x86/yuv_2_rgb.o  \
> +   x86/yuv2yuvX.o   \
> diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
> index 3160fedf04..fec9fa22e0 100644
> --- a/libswscale/x86/swscale.c
> +++ b/libswscale/x86/swscale.c
> @@ -197,80 +197,25 @@ void ff_updateMMXDitherTables(SwsContext *c, int
> dstY)
>  }
>
>  #if HAVE_MMXEXT
> +void ff_yuv2yuvX_sse3(const int16_t *filter, int filterSize,
> +   uint8_t *dest, int dstW,
> +   const uint8_t *dither, int offset);
> +
>  static void yuv2yuvX_sse3(const int16_t *filter, int filterSize,
> const int16_t **src, uint8_t *dest, int dstW,
> const uint8_t *dither, int offset)
>  {
> +int remainder = (dstW % 32);
> +int pixelsProcessed = dstW - remainder;
>  if(((uintptr_t)dest) & 15){
>  yuv2yuvX_mmxext(filter, filterSize, src, dest, dstW, dither,
> offset);
>  return;
>  }
> -filterSize--;
> -#define MAIN_FUNCTION \
> -"pxor   %%xmm0, %%xmm0 \n\t" \
> -"punpcklbw  %%xmm0, %%xmm3 \n\t" \
> -"movd   %4, %%xmm1 \n\t" \
> -"punpcklwd  %%xmm1, %%xmm1 \n\t" \
> -"punpckldq  %%xmm1, %%xmm1 \n\t" \
> -"punpcklqdq %%xmm1, %%xmm1 \n\t" \
> -"psllw  $3, %%xmm1 \n\t" \
> -"paddw  %%xmm1, %%xmm3 \n\t" \
> -"psraw  $4, %%xmm3 \n\t" \
> -"movdqa %%xmm3, %%xmm4 \n\t" \
> -"movdqa %%xmm3, %%xmm7 \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"\
> -"movddup  8(%%"FF_REG_d"), %%xmm0   \n\t" /*
> filterCoeff */\
> -"movdqa  (%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm2
> \n\t" /* srcData */\
> -"movdqa16(%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm5
> \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   %%xmm0, %%xmm2  \n\t"\
> -"pmulhw   %%xmm0, %%xmm5  \n\t"\
> -"paddw%%xmm2, %%xmm3  \n\t"\
> -"paddw%%xmm5, %%xmm4  \n\t"\
> -" jnz1b \n\t"\
> -"psraw   $3, %%xmm3  \n\t"\
> -"psraw   $3, %%xmm4  \n\t"\
> -"packuswb %%xmm4, %%xmm3  \n\t"\
> -"movntdq  %%xmm3, (%1, %%"FF_REG_c")
> \n\t"\
> -"add $16, %%"FF_REG_c"\n\t"\
> -"cmp  %2, %%"FF_REG_c"\n\t"\
> -"movdqa   %%xmm7, %%xmm3\n\t" \
> -"movdqa   %%xmm7, %%xmm4\n\t" \
> -"mov %0, %%"FF_REG_d"
> \n\t"\
> -"mov(%%"FF_REG_d"), %%"FF_REG_S"
>  \n\t"\
> -"jb  1b \n\t"
> -
> -if (offset) {
> -__asm__ volatile(
> -"movq  %5, %%xmm3  \n\t"
> -"movdqa%%xmm3, %%xmm4  \n\t"
> -"psrlq$24, %%xmm3  \n\t"
> -"psllq

[FFmpeg-devel] [PATCH] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup.

---
 libswscale/x86/Makefile |   1 +
 libswscale/x86/swscale.c|  75 -
 libswscale/x86/yuv2yuvX.asm | 109 
 3 files changed, 120 insertions(+), 65 deletions(-)
 create mode 100644 libswscale/x86/yuv2yuvX.asm

diff --git a/libswscale/x86/Makefile b/libswscale/x86/Makefile
index 831d5359aa..bfe383364e 100644
--- a/libswscale/x86/Makefile
+++ b/libswscale/x86/Makefile
@@ -13,3 +13,4 @@ X86ASM-OBJS += x86/input.o
  \
x86/scale.o  \
x86/rgb_2_rgb.o  \
x86/yuv_2_rgb.o  \
+   x86/yuv2yuvX.o   \
diff --git a/libswscale/x86/swscale.c b/libswscale/x86/swscale.c
index 3160fedf04..fec9fa22e0 100644
--- a/libswscale/x86/swscale.c
+++ b/libswscale/x86/swscale.c
@@ -197,80 +197,25 @@ void ff_updateMMXDitherTables(SwsContext *c, int dstY)
 }
 
 #if HAVE_MMXEXT
+void ff_yuv2yuvX_sse3(const int16_t *filter, int filterSize,
+   uint8_t *dest, int dstW,
+   const uint8_t *dither, int offset);
+
 static void yuv2yuvX_sse3(const int16_t *filter, int filterSize,
const int16_t **src, uint8_t *dest, int dstW,
const uint8_t *dither, int offset)
 {
+int remainder = (dstW % 32);
+int pixelsProcessed = dstW - remainder;
 if(((uintptr_t)dest) & 15){
 yuv2yuvX_mmxext(filter, filterSize, src, dest, dstW, dither, offset);
 return;
 }
-filterSize--;
-#define MAIN_FUNCTION \
-"pxor   %%xmm0, %%xmm0 \n\t" \
-"punpcklbw  %%xmm0, %%xmm3 \n\t" \
-"movd   %4, %%xmm1 \n\t" \
-"punpcklwd  %%xmm1, %%xmm1 \n\t" \
-"punpckldq  %%xmm1, %%xmm1 \n\t" \
-"punpcklqdq %%xmm1, %%xmm1 \n\t" \
-"psllw  $3, %%xmm1 \n\t" \
-"paddw  %%xmm1, %%xmm3 \n\t" \
-"psraw  $4, %%xmm3 \n\t" \
-"movdqa %%xmm3, %%xmm4 \n\t" \
-"movdqa %%xmm3, %%xmm7 \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"\
-"movddup  8(%%"FF_REG_d"), %%xmm0   \n\t" /* 
filterCoeff */\
-"movdqa  (%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm2 \n\t" /* 
srcData */\
-"movdqa16(%%"FF_REG_S", %%"FF_REG_c", 2), %%xmm5 \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   %%xmm0, %%xmm2  \n\t"\
-"pmulhw   %%xmm0, %%xmm5  \n\t"\
-"paddw%%xmm2, %%xmm3  \n\t"\
-"paddw%%xmm5, %%xmm4  \n\t"\
-" jnz1b \n\t"\
-"psraw   $3, %%xmm3  \n\t"\
-"psraw   $3, %%xmm4  \n\t"\
-"packuswb %%xmm4, %%xmm3  \n\t"\
-"movntdq  %%xmm3, (%1, %%"FF_REG_c") \n\t"\
-"add $16, %%"FF_REG_c"\n\t"\
-"cmp  %2, %%"FF_REG_c"\n\t"\
-"movdqa   %%xmm7, %%xmm3\n\t" \
-"movdqa   %%xmm7, %%xmm4\n\t" \
-"mov %0, %%"FF_REG_d"\n\t"\
-"mov(%%"FF_REG_d"), %%"FF_REG_S" \n\t"\
-"jb  1b \n\t"
-
-if (offset) {
-__asm__ volatile(
-"movq  %5, %%xmm3  \n\t"
-"movdqa%%xmm3, %%xmm4  \n\t"
-"psrlq$24, %%xmm3  \n\t"
-"psllq$40, %%xmm4  \n\t"
-"por   %%xmm4, %%xmm3  \n\t"
-MAIN_FUNCTION
-  :: "g" (filter),
-  "r" (dest-offset), "g" ((x86_reg)(dstW+offset)), "m" (offset),
-  "m"(filterSize), "m"(((uint64_t *) dither)[0])
-  : XMM_CLOBBERS("%xmm0" , "%xmm1" , "%xmm2" , "%xmm3" , "%xmm4" , 
"%xmm5" , "%xmm7" ,)
-"%"FF_REG_d, "%"FF_REG_S, "%"FF_REG_c
-  );
-} else {
-__asm__ volatile(
-"movq  %5, %%xmm3   \n\t"
-MAIN_FUNCTION
-  :: "g" (

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/bitstream: Check code length before truncating to uint8_t

Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt 
> ---
> The main motivation for this patch is actually the second patch and not
> the improved check.
> 
>  libavcodec/bitstream.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c
> index 77c2b9ce05..ffa352b28b 100644
> --- a/libavcodec/bitstream.c
> +++ b/libavcodec/bitstream.c
> @@ -302,15 +302,17 @@ int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int 
> nb_codes,
>  j = 0;
>  #define COPY(condition)\
>  for (i = 0; i < nb_codes; i++) {\
> -GET_DATA(buf[j].bits, bits, i, bits_wrap, bits_size);   \
> +unsigned len;   \
> +GET_DATA(len, bits, i, bits_wrap, bits_size);   \
>  if (!(condition))   \
>  continue;   \
> -if (buf[j].bits > 3*nb_bits || buf[j].bits>32) {\
> -av_log(NULL, AV_LOG_ERROR, "Too long VLC (%d) in init_vlc\n", 
> buf[j].bits);\
> +if (len > 3*nb_bits || len > 32) {  \
> +av_log(NULL, AV_LOG_ERROR, "Too long VLC (%u) in init_vlc\n", 
> len);\
>  if (buf != localbuf)\
>  av_free(buf);   \
>  return AVERROR(EINVAL); \
>  }   \
> +buf[j].bits = len;  \
>  GET_DATA(buf[j].code, codes, i, codes_wrap, codes_size);\
>  if (buf[j].code >= (1LL<  av_log(NULL, AV_LOG_ERROR, "Invalid code %"PRIx32" for %d in "  \
> @@ -329,10 +331,10 @@ int ff_init_vlc_sparse(VLC *vlc_arg, int nb_bits, int 
> nb_codes,
>  buf[j].symbol = i;  \
>  j++;\
>  }
> -COPY(buf[j].bits > nb_bits);
> +COPY(len > nb_bits);
>  // qsort is the slowest part of init_vlc, and could probably be improved 
> or avoided
>  AV_QSORT(buf, j, struct VLCcode, compare_vlcspec);
> -COPY(buf[j].bits && buf[j].bits <= nb_bits);
> +COPY(len && len <= nb_bits);
>  nb_codes = j;
>  
>  ret = build_table(vlc, nb_bits, nb_codes, buf, flags);
> 
Will apply this later today unless there are objections.

- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup.

Hi,
Quoting Alan Kelly (2020-10-27 10:10:14)
> ---
>  libswscale/x86/Makefile |   1 +
>  libswscale/x86/swscale.c|  75 -
>  libswscale/x86/yuv2yuvX.asm | 109 
>  3 files changed, 120 insertions(+), 65 deletions(-)
>  create mode 100644 libswscale/x86/yuv2yuvX.asm
>

No comments on the code itself (yet?), but as for your submission:
- when you send multiple iterations of the same patch, it is helpful to
  mention what changed, e.g. with git send-email --annotate
- the commit message should follow the standard format of:
* swscale: short summary of the change

  Extended description of the commit, if needed.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/6] put_bits: make avpriv_align_put_bits() inline

Quoting Andreas Rheinhardt (2020-10-26 15:21:26)
> Unnecessary: The code below also works for LE writers, now that the LE
> version of put_bits() is called.

Right, changed locally.

> 
> > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > index 02ffa95749..78c4dd64ee 100644
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -144,6 +144,8 @@
> >  #ifndef FF_API_UNUSED_CODEC_CAPS
> >  #define FF_API_UNUSED_CODEC_CAPS   (LIBAVCODEC_VERSION_MAJOR < 59)
> >  #endif
> > -
> > +#ifndef FF_API_AVPRIV_PUT_BITS
> > +#define FF_API_AVPRIV_PUT_BITS (LIBAVCODEC_VERSION_MAJOR < 59)
> > +#endif
> >  
> 
> Why are you adding this instead of checking for LIBAVCODEC_VERSION_MAJOR
> < 59? After all, avpriv functions don't require a deprecation period and
> so there is no reason to postpone removing them to version 60.

Mainly so we don't forget to remove it during the bump, in my experience
people tend to forget to check for things that are under plain
#if VERSION. But I have no strong feelings either way.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 6/6] lavc: un-avpriv avpriv_bprint_to_extradata()

Quoting James Almer (2020-10-27 01:10:47)
> On 10/26/2020 10:41 AM, Anton Khirnov wrote:
> > It has not been used outside of lavc since 6f69f7a8bf6.
> > 
> > Also, move it to the only place where it is used.
> 
> Shouldn't you keep the symbol around until the bump? Even though it was
> not used by other libraries, it was nonetheless still exported by lavc.
> A simple version define preprocessor check like Andreas mentioned is
> enough, no need to add a new FF_API define for avpriv functions.

Just being exported doesn't mean we have to maintain compatibility for
it, IMO. The only code that could have legally used it is in other libs,
but there is no such code since before last bump.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCHv2] Document community process

Quoting Michael Niedermayer (2020-10-26 19:01:47)
> On Sun, Oct 25, 2020 at 01:55:11PM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2020-10-19 23:57:31)
> > > On Mon, Oct 19, 2020 at 07:22:48PM +0200, Jean-Baptiste Kempf wrote:
> > > > Yo,
> > > > 
> > > > On Mon, 19 Oct 2020, at 19:02, Michael Niedermayer wrote:
> > > > > > +## Voting
> > > > > > +
> > > > > 
> > > > > > +Voting is done using a ranked voting system, currently running on 
> > > > > > https://vote.ffmpeg.org/ .
> > > > > 
> > > > > I think Voting should be defined more precissely
> > > > 
> > > > That's a good point. What would like to see here? The algo used? The 
> > > > software used?
> > > 
> > > I dont know what is best.
> > > 
> > > What is the goal having this information there serves ?
> > > I think there are 3 or 4 levels/classes of information that could be 
> > > provided
> > > at highest level, listing the properties of the vote system
> > 
> > In my view, this documented is intended to serve mainly as a statement
> > of intent rather than a strict legalistic definition of everything, so
> > it would be sufficient to mention that we are using a ranked Condorcet
> > method. I would think very few developers know or care what the exact
> > differences between the methods are, as long as they are in some sense
> > "reasonable".
> 
> The problem is elections with multiple winners, That is elections of seats
> in a committee or other group.
> Consider a 5 seat comittee
> and lets consider that there are blue and pink candidates
> if you have 100 people voting and 51 of them vote only for pink candidates and
> 49 only for blue candidates.
> repeated application of a Condorcet method will give you 5 pink candidates
> 
> OTOH something like schulze STV, also a Condorcet method should in this case
> give you 3 pink candidates and 2 blue ones.
> 
> The above is a bit oversimplified but basically there are 2 classes of voting
> systems. The first class is applying single winner election methods repeatedly
> to fill all seats. 
> The other is trying to fill seats so they are representing the set of voters.
> 
> The first class can skip over minorities even when they are quite large,
> but the people choosen should have "strong and maximal support"
> 
> The second class would favor creating a representative set over one of
> maximal support by voters. It could lead to a more "colorfull" result
> with seats filled by people representing minortes and lacking broad support.
> 
> The results likely will differ in reallity as well.
> 
> We dont have to write this down in "this" document but we should
> write it down in some document if what is considered "reasonable"
> is "Proportional representation" or not.
> 
> What i can say is that if we want a 
> * "Proportional representation" system
>  then schulze STV is a "beatifull" system free of ugly discrete choices like 
> STV 
>  and its also condorcet
> * non "Proportional representation".
>  then normal repeatly applying the normal schulze method is the obvious choice
> 
> IIUC CIVS supports repeatly applying the normal schulze method and thilo added
> support for schulze STV

AFAIU we used schulze STV for the votes so far, right? So it seems
reasonable to continue with that unless there are significant objections.

Generally I agree with your points, but IMO this should be in a separate
document that describes the "technicalities" of the development process.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/bitstream: Consistently treat symbol as VLC_TYPE

Andreas Rheinhardt:
> If a static VLC table gets initialized a second time (or concurrently by
> two threads) and if said VLC table uses symbols that have the sign bit
> of VLC_TYPE (a typedef for int16_t) set, initializing the VLC fails. The
> reason is that the type of the symbol in the temporary array is an
> uint16_t and so comparing it to the symbol read from the VLC table will
> fail, because only the lower 16bits coincide. Said failure triggers an
> assert.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> Found when playing around a bit with making the ClearVideo VLCs static.
> 
>  libavcodec/bitstream.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c
> index 39749c6092..a908c10980 100644
> --- a/libavcodec/bitstream.c
> +++ b/libavcodec/bitstream.c
> @@ -129,7 +129,7 @@ static int alloc_table(VLC *vlc, int size, int use_static)
>  
>  typedef struct VLCcode {
>  uint8_t bits;
> -uint16_t symbol;
> +VLC_TYPE symbol;
>  /** codeword, with the first bit-to-be-read in the msb
>   * (even if intended for a little-endian bitstream reader) */
>  uint32_t code;
> 
LGTM'ed by Lynne via email and pushed.

- 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 v3 4/6] ffmpeg: pass decoded or filtered AVFrame to output stream initialization

Quoting Jan Ekström (2020-10-16 15:16:47)
> Additionally, reap the first rewards by being able to set the
> color related encoding values based on the passed AVFrame.
> 
> The only tests that seem to have changed their results with this
> change seem to be the MXF tests. There, the muxer writes the
> limited/full range flag to the output container if the encoder
> is not set to "unspecified".
> ---
>  fftools/ffmpeg.c| 42 +++--
>  tests/ref/lavf/mxf_d10  |  2 +-
>  tests/ref/lavf/mxf_dv25 |  2 +-
>  tests/ref/lavf/mxf_dvcpro50 |  2 +-
>  tests/ref/lavf/mxf_opatom   |  2 +-
>  5 files changed, 35 insertions(+), 15 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 08db67a6ab..b2e210c814 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -941,9 +941,11 @@ early_exit:
>  return float_pts;
>  }
>  
> -static int init_output_stream(OutputStream *ost, char *error, int error_len);
> +static int init_output_stream(OutputStream *ost, AVFrame *frame,
> +  char *error, int error_len);
>  
> -static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal)
> +static int init_output_stream_wrapper(OutputStream *ost, AVFrame *frame,
> +  unsigned int fatal)
>  {
>  int ret = AVERROR_BUG;
>  char error[1024] = {0};
> @@ -951,7 +953,7 @@ static int init_output_stream_wrapper(OutputStream *ost, 
> unsigned int fatal)
>  if (ost->initialized)
>  return 0;
>  
> -ret = init_output_stream(ost, error, sizeof(error));
> +ret = init_output_stream(ost, frame, error, sizeof(error));
>  if (ret < 0) {
>  av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d 
> -- %s\n",
> ost->file_index, ost->index, error);
> @@ -1125,7 +1127,7 @@ static void do_video_out(OutputFile *of,
>  InputStream *ist = NULL;
>  AVFilterContext *filter = ost->filter->filter;
>  
> -init_output_stream_wrapper(ost, 1);
> +init_output_stream_wrapper(ost, next_picture, 1);
>  sync_ipts = adjust_frame_pts_to_encoder_tb(of, ost, next_picture);
>  
>  if (ost->source_index >= 0)
> @@ -1507,7 +1509,7 @@ static int reap_filters(int flush)
>   * the encoder earlier than receiving the first AVFrame.
>   */
>  if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_AUDIO)
> -init_output_stream_wrapper(ost, 1);
> +init_output_stream_wrapper(ost, NULL, 1);
>  
>  if (!ost->filtered_frame && !(ost->filtered_frame = 
> av_frame_alloc())) {
>  return AVERROR(ENOMEM);
> @@ -1930,7 +1932,7 @@ static void flush_encoders(void)
>  finish_output_stream(ost);
>  }
>  
> -init_output_stream_wrapper(ost, 1);
> +init_output_stream_wrapper(ost, NULL, 1);
>  }
>  
>  if (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type != 
> AVMEDIA_TYPE_AUDIO)
> @@ -3302,7 +3304,7 @@ static void init_encoder_time_base(OutputStream *ost, 
> AVRational default_time_ba
>  enc_ctx->time_base = default_time_base;
>  }
>  
> -static int init_output_stream_encode(OutputStream *ost)
> +static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
>  {
>  InputStream *ist = get_input_stream(ost);
>  AVCodecContext *enc_ctx = ost->enc_ctx;
> @@ -3399,6 +3401,23 @@ static int init_output_stream_encode(OutputStream *ost)
>  enc_ctx->bits_per_raw_sample = 
> FFMIN(dec_ctx->bits_per_raw_sample,
>   
> av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth);
>  
> +if (frame) {
> +if (!av_dict_get(ost->encoder_opts, "color_range", NULL, 0))

It doesn't seem to me the checks are needed, since encoder_opts are
applied AFTER this block, so they'd override whatever you set here
anyway.

Beyond that, looks very good.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 5/6] ffmpeg: move field order decision making to encoder initialization

Quoting Jan Ekström (2020-10-16 15:16:48)
> We now have the possibility of getting AVFrames here, and we should
> not touch the muxer's codecpar after writing the header.
> ---
>  fftools/ffmpeg.c  | 27 ++-
>  .../fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
>  .../fate/concat-demuxer-simple1-lavf-mxf_d10  |  2 +-
>  tests/ref/fate/rgb24-mkv  |  4 +--
>  tests/ref/lavf/mxf_d10|  2 +-
>  5 files changed, 19 insertions(+), 18 deletions(-)

Why do the tests change?

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/bitstream: Consistently treat symbol as VLC_TYPE

Quoting Andreas Rheinhardt (2020-10-27 11:30:32)
> Andreas Rheinhardt:
> > If a static VLC table gets initialized a second time (or concurrently by
> > two threads) and if said VLC table uses symbols that have the sign bit
> > of VLC_TYPE (a typedef for int16_t) set, initializing the VLC fails. The
> > reason is that the type of the symbol in the temporary array is an
> > uint16_t and so comparing it to the symbol read from the VLC table will
> > fail, because only the lower 16bits coincide. Said failure triggers an
> > assert.
> > 
> > Signed-off-by: Andreas Rheinhardt 
> > ---
> > Found when playing around a bit with making the ClearVideo VLCs static.
> > 
> >  libavcodec/bitstream.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c
> > index 39749c6092..a908c10980 100644
> > --- a/libavcodec/bitstream.c
> > +++ b/libavcodec/bitstream.c
> > @@ -129,7 +129,7 @@ static int alloc_table(VLC *vlc, int size, int 
> > use_static)
> >  
> >  typedef struct VLCcode {
> >  uint8_t bits;
> > -uint16_t symbol;
> > +VLC_TYPE symbol;
> >  /** codeword, with the first bit-to-be-read in the msb
> >   * (even if intended for a little-endian bitstream reader) */
> >  uint32_t code;
> > 
> LGTM'ed by Lynne via email and pushed.

This is the second time I see this, why does that not go through the ML?

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/bitstream: Consistently treat symbol as VLC_TYPE

Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-10-27 11:30:32)
>> Andreas Rheinhardt:
>>> If a static VLC table gets initialized a second time (or concurrently by
>>> two threads) and if said VLC table uses symbols that have the sign bit
>>> of VLC_TYPE (a typedef for int16_t) set, initializing the VLC fails. The
>>> reason is that the type of the symbol in the temporary array is an
>>> uint16_t and so comparing it to the symbol read from the VLC table will
>>> fail, because only the lower 16bits coincide. Said failure triggers an
>>> assert.
>>>
>>> Signed-off-by: Andreas Rheinhardt 
>>> ---
>>> Found when playing around a bit with making the ClearVideo VLCs static.
>>>
>>>  libavcodec/bitstream.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c
>>> index 39749c6092..a908c10980 100644
>>> --- a/libavcodec/bitstream.c
>>> +++ b/libavcodec/bitstream.c
>>> @@ -129,7 +129,7 @@ static int alloc_table(VLC *vlc, int size, int 
>>> use_static)
>>>  
>>>  typedef struct VLCcode {
>>>  uint8_t bits;
>>> -uint16_t symbol;
>>> +VLC_TYPE symbol;
>>>  /** codeword, with the first bit-to-be-read in the msb
>>>   * (even if intended for a little-endian bitstream reader) */
>>>  uint32_t code;
>>>
>> LGTM'ed by Lynne via email and pushed.
> 
> This is the second time I see this, why does that not go through the ML?
> 
I don't know. Seems like Lynne has some trouble with her MUA.

- 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] Moves yuv2yuvX_sse3 to yasm, unrolls main loop and other small optimizations for ~20% speedup.

Thanks for the feedback Anton.

The second patch incorporates changes suggested by James Almer:
avx2 instructions are wrapped in if cpuflag(avx2) and movddup restored
mm1 is replaced by m1 on x86_32



On Tue, Oct 27, 2020 at 10:40 AM Anton Khirnov  wrote:

> Hi,
> Quoting Alan Kelly (2020-10-27 10:10:14)
> > ---
> >  libswscale/x86/Makefile |   1 +
> >  libswscale/x86/swscale.c|  75 -
> >  libswscale/x86/yuv2yuvX.asm | 109 
> >  3 files changed, 120 insertions(+), 65 deletions(-)
> >  create mode 100644 libswscale/x86/yuv2yuvX.asm
> >
>
> No comments on the code itself (yet?), but as for your submission:
> - when you send multiple iterations of the same patch, it is helpful to
>   mention what changed, e.g. with git send-email --annotate
> - the commit message should follow the standard format of:
> * swscale: short summary of the change
>
>   Extended description of the commit, if needed.
>
> --
> Anton Khirnov
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel 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/cbs_av1: add a range check to tg_end

Section 6.10.1 of the AV1 spec states:

It is a requirement of bitstream conformance that the value of tg_end is 
greater than or equal to tg_start.

Signed-off-by: James Almer 
---
 libavcodec/cbs_av1_syntax_template.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index 2df5619279..63b4db7853 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -1785,7 +1785,7 @@ static int FUNC(tile_group_obu)(CodedBitstreamContext 
*ctx, RWContext *rw,
 tile_bits = cbs_av1_tile_log2(1, priv->tile_cols) +
 cbs_av1_tile_log2(1, priv->tile_rows);
 fb(tile_bits, tg_start);
-fb(tile_bits, tg_end);
+fc(tile_bits, tg_end, current->tg_start, MAX_UINT_BITS(tile_bits));
 }
 
 CHECK(FUNC(byte_alignment)(ctx, rw));
-- 
2.28.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/3] avcodec/codec2utils: remove avpriv prefix from inline functions

Signed-off-by: James Almer 
---
 libavcodec/codec2utils.c |  8 
 libavcodec/codec2utils.h | 14 +++---
 libavcodec/libcodec2.c   | 16 
 libavformat/codec2.c | 32 
 4 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/libavcodec/codec2utils.c b/libavcodec/codec2utils.c
index 931478f22a..f1b6e56526 100644
--- a/libavcodec/codec2utils.c
+++ b/libavcodec/codec2utils.c
@@ -37,7 +37,7 @@ int avpriv_codec2_mode_bit_rate(void *logctx, int mode)
 
 int avpriv_codec2_mode_frame_size(void *logctx, int mode)
 {
-int frame_size_table[AVPRIV_CODEC2_MODE_MAX+1] = {
+int frame_size_table[CODEC2_MODE_MAX+1] = {
 160,// 3200
 160,// 2400
 320,// 1600
@@ -49,7 +49,7 @@ int avpriv_codec2_mode_frame_size(void *logctx, int mode)
 320,// 700C
 };
 
-if (mode < 0 || mode > AVPRIV_CODEC2_MODE_MAX) {
+if (mode < 0 || mode > CODEC2_MODE_MAX) {
 av_log(logctx, AV_LOG_ERROR, "unknown codec2 mode %i, can't find 
frame_size\n", mode);
 return 0;
 } else {
@@ -59,7 +59,7 @@ int avpriv_codec2_mode_frame_size(void *logctx, int mode)
 
 int avpriv_codec2_mode_block_align(void *logctx, int mode)
 {
-int block_align_table[AVPRIV_CODEC2_MODE_MAX+1] = {
+int block_align_table[CODEC2_MODE_MAX+1] = {
 8,  // 3200
 6,  // 2400
 8,  // 1600
@@ -71,7 +71,7 @@ int avpriv_codec2_mode_block_align(void *logctx, int mode)
 4,  // 700C
 };
 
-if (mode < 0 || mode > AVPRIV_CODEC2_MODE_MAX) {
+if (mode < 0 || mode > CODEC2_MODE_MAX) {
 av_log(logctx, AV_LOG_ERROR, "unknown codec2 mode %i, can't find 
block_align\n", mode);
 return 0;
 } else {
diff --git a/libavcodec/codec2utils.h b/libavcodec/codec2utils.h
index 6def4d4aa3..e9b1f84d84 100644
--- a/libavcodec/codec2utils.h
+++ b/libavcodec/codec2utils.h
@@ -27,14 +27,14 @@
 //Highest mode we're willing to use.
 //Don't want to let users accidentally produce files that can't be decoded in 
the future.
 //CODEC2_MODE_WB (9) is experimental/unstable as of 2017-11-23.
-#define AVPRIV_CODEC2_MODE_MAX 8 //CODEC2_MODE_700C
+#define CODEC2_MODE_MAX 8 //CODEC2_MODE_700C
 
 //Used by both codec2raw demuxer and libcodec2 encoder.
 //The integers match the values in codec2.h, so "3200" -> CODEC2_MODE_3000 = 0 
and so on.
 //It is possible that we're linked to a version of libcodec2 that lacks some 
of these modes.
 //For example Debian stretch ships with libcodec2.so.0.4 which lacks 
CODEC2_MODE_700C.
-#define AVPRIV_CODEC2_AVOPTIONS(desc, classname, min_val, default_val, 
option_flags) \
-{ "mode", desc, offsetof(classname, mode), AV_OPT_TYPE_INT, {.i64 = 
default_val}, min_val, AVPRIV_CODEC2_MODE_MAX, .flags=option_flags, 
.unit="codec2_mode"},\
+#define CODEC2_AVOPTIONS(desc, classname, min_val, default_val, option_flags) \
+{ "mode", desc, offsetof(classname, mode), AV_OPT_TYPE_INT, {.i64 = 
default_val}, min_val, CODEC2_MODE_MAX, .flags=option_flags, 
.unit="codec2_mode"},\
 { "3200", "3200", 0, AV_OPT_TYPE_CONST, {.i64 = 0}, .flags=option_flags, 
.unit="codec2_mode"},\
 { "2400", "2400", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, .flags=option_flags, 
.unit="codec2_mode"},\
 { "1600", "1600", 0, AV_OPT_TYPE_CONST, {.i64 = 2}, .flags=option_flags, 
.unit="codec2_mode"},\
@@ -59,10 +59,10 @@ int avpriv_codec2_mode_frame_size(void *logctx, int mode);
 //Mimics (codec2_bits_per_frame()+7)/8
 int avpriv_codec2_mode_block_align(void *logctx, int mode);
 
-#define AVPRIV_CODEC2_EXTRADATA_SIZE 4
+#define CODEC2_EXTRADATA_SIZE 4
 
 //Used in codec2raw demuxer and libcodec2 encoder
-static inline void avpriv_codec2_make_extradata(uint8_t *ptr, int mode) {
+static inline void codec2_make_extradata(uint8_t *ptr, int mode) {
 //version 0.8 as of 2017-12-23 (r3386)
 ptr[0] = 0; //major
 ptr[1] = 8; //minor
@@ -71,11 +71,11 @@ static inline void avpriv_codec2_make_extradata(uint8_t 
*ptr, int mode) {
 }
 
 //Returns version as a 16-bit value. 0.8 -> 0x0008
-static inline uint16_t avpriv_codec2_version_from_extradata(uint8_t *ptr) {
+static inline uint16_t codec2_version_from_extradata(uint8_t *ptr) {
 return (ptr[0] << 8) + ptr[1];
 }
 
-static inline uint8_t avpriv_codec2_mode_from_extradata(uint8_t *ptr) {
+static inline uint8_t codec2_mode_from_extradata(uint8_t *ptr) {
 return ptr[2];
 }
 
diff --git a/libavcodec/libcodec2.c b/libavcodec/libcodec2.c
index 1d6bed0383..8421f5b261 100644
--- a/libavcodec/libcodec2.c
+++ b/libavcodec/libcodec2.c
@@ -34,7 +34,7 @@ typedef struct {
 static const AVOption options[] = {
 //not AV_OPT_FLAG_DECODING_PARAM since mode should come from the demuxer
 //1300 (aka FreeDV 1600) is the most common mode on-the-air, default to it 
here as well
-AVPRIV_CODEC2_AVOPTIONS("codec2 mode", LibCodec2Context, 0, 4 
/*CODEC2_MODE_1300*/, AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_ENCODING_PARAM),
+CODEC2_AV

[FFmpeg-devel] [PATCH 2/3] avcodec/codec2utils: move codec2_version_from_extradata to lavf

It's only used by the codec2 demuxers

Signed-off-by: James Almer 
---
 libavcodec/codec2utils.h | 5 -
 libavformat/codec2.c | 5 +
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavcodec/codec2utils.h b/libavcodec/codec2utils.h
index e9b1f84d84..2ee7a592a1 100644
--- a/libavcodec/codec2utils.h
+++ b/libavcodec/codec2utils.h
@@ -70,11 +70,6 @@ static inline void codec2_make_extradata(uint8_t *ptr, int 
mode) {
 ptr[3] = 0; //flags
 }
 
-//Returns version as a 16-bit value. 0.8 -> 0x0008
-static inline uint16_t codec2_version_from_extradata(uint8_t *ptr) {
-return (ptr[0] << 8) + ptr[1];
-}
-
 static inline uint8_t codec2_mode_from_extradata(uint8_t *ptr) {
 return ptr[2];
 }
diff --git a/libavformat/codec2.c b/libavformat/codec2.c
index 1f7f16a106..edd450716f 100644
--- a/libavformat/codec2.c
+++ b/libavformat/codec2.c
@@ -86,6 +86,11 @@ static int codec2_read_header_common(AVFormatContext *s, 
AVStream *st)
 return 0;
 }
 
+//Returns version as a 16-bit value. 0.8 -> 0x0008
+static uint16_t codec2_version_from_extradata(uint8_t *ptr) {
+return (ptr[0] << 8) + ptr[1];
+}
+
 static int codec2_read_header(AVFormatContext *s)
 {
 AVStream *st = avformat_new_stream(s, NULL);
-- 
2.28.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/3] avcodec/codec2utils: move the remaining avpriv functions to lavf

They are only used by the codec2 demuxers.

The symbols are kept around until the next major bump.

Signed-off-by: James Almer 
---
 libavcodec/codec2utils.c |  2 ++
 libavcodec/codec2utils.h |  4 +++
 libavformat/codec2.c | 66 ++--
 3 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/libavcodec/codec2utils.c b/libavcodec/codec2utils.c
index f1b6e56526..a942662403 100644
--- a/libavcodec/codec2utils.c
+++ b/libavcodec/codec2utils.c
@@ -23,6 +23,7 @@
 #include "internal.h"
 #include "libavcodec/codec2utils.h"
 
+#if LIBAVCODEC_VERSION_MAJOR < 59
 int avpriv_codec2_mode_bit_rate(void *logctx, int mode)
 {
 int frame_size  = avpriv_codec2_mode_frame_size(logctx, mode);
@@ -78,3 +79,4 @@ int avpriv_codec2_mode_block_align(void *logctx, int mode)
 return block_align_table[mode];
 }
 }
+#endif
diff --git a/libavcodec/codec2utils.h b/libavcodec/codec2utils.h
index 2ee7a592a1..37f0693dc7 100644
--- a/libavcodec/codec2utils.h
+++ b/libavcodec/codec2utils.h
@@ -24,6 +24,8 @@
 
 #include 
 
+#include "version.h"
+
 //Highest mode we're willing to use.
 //Don't want to let users accidentally produce files that can't be decoded in 
the future.
 //CODEC2_MODE_WB (9) is experimental/unstable as of 2017-11-23.
@@ -45,6 +47,7 @@
 { "700B", "700B", 0, AV_OPT_TYPE_CONST, {.i64 = 7}, .flags=option_flags, 
.unit="codec2_mode"},\
 { "700C", "700C", 0, AV_OPT_TYPE_CONST, {.i64 = 8}, .flags=option_flags, 
.unit="codec2_mode"}
 
+#if LIBAVCODEC_VERSION_MAJOR < 59
 //The three following functions are here to avoid needing libavformat/codec2.c 
to depend on libcodec2
 
 //Computes bitrate from mode, with frames rounded up to the nearest octet.
@@ -58,6 +61,7 @@ int avpriv_codec2_mode_frame_size(void *logctx, int mode);
 
 //Mimics (codec2_bits_per_frame()+7)/8
 int avpriv_codec2_mode_block_align(void *logctx, int mode);
+#endif
 
 #define CODEC2_EXTRADATA_SIZE 4
 
diff --git a/libavformat/codec2.c b/libavformat/codec2.c
index edd450716f..a64160283c 100644
--- a/libavformat/codec2.c
+++ b/libavformat/codec2.c
@@ -61,6 +61,66 @@ static int codec2_probe(const AVProbeData *p)
 return AVPROBE_SCORE_EXTENSION + 1;
 }
 
+//Mimics codec2_samples_per_frame()
+static int codec2_mode_frame_size(AVFormatContext *s, int mode)
+{
+int frame_size_table[CODEC2_MODE_MAX+1] = {
+160,// 3200
+160,// 2400
+320,// 1600
+320,// 1400
+320,// 1300
+320,// 1200
+320,// 700
+320,// 700B
+320,// 700C
+};
+
+if (mode < 0 || mode > CODEC2_MODE_MAX) {
+av_log(s, AV_LOG_ERROR, "unknown codec2 mode %i, can't find 
frame_size\n", mode);
+return 0;
+} else {
+return frame_size_table[mode];
+}
+}
+
+//Mimics (codec2_bits_per_frame()+7)/8
+static int codec2_mode_block_align(AVFormatContext *s, int mode)
+{
+int block_align_table[CODEC2_MODE_MAX+1] = {
+8,  // 3200
+6,  // 2400
+8,  // 1600
+7,  // 1400
+7,  // 1300
+6,  // 1200
+4,  // 700
+4,  // 700B
+4,  // 700C
+};
+
+if (mode < 0 || mode > CODEC2_MODE_MAX) {
+av_log(s, AV_LOG_ERROR, "unknown codec2 mode %i, can't find 
block_align\n", mode);
+return 0;
+} else {
+return block_align_table[mode];
+}
+}
+
+//Computes bitrate from mode, with frames rounded up to the nearest octet.
+//So 700 bit/s (28 bits/frame) becomes 800 bits/s (32 bits/frame).
+static int codec2_mode_bit_rate(AVFormatContext *s, int mode)
+{
+int frame_size  = codec2_mode_frame_size(s, mode);
+int block_align = codec2_mode_block_align(s, mode);
+
+if (frame_size <= 0 || block_align <= 0) {
+return 0;
+}
+
+return 8 * 8000 * block_align / frame_size;
+}
+
 static int codec2_read_header_common(AVFormatContext *s, AVStream *st)
 {
 int mode = codec2_mode_from_extradata(st->codecpar->extradata);
@@ -71,9 +131,9 @@ static int codec2_read_header_common(AVFormatContext *s, 
AVStream *st)
 st->codecpar->channels  = 1;
 st->codecpar->format= AV_SAMPLE_FMT_S16;
 st->codecpar->channel_layout= AV_CH_LAYOUT_MONO;
-st->codecpar->bit_rate  = avpriv_codec2_mode_bit_rate(s, mode);
-st->codecpar->frame_size= avpriv_codec2_mode_frame_size(s, mode);
-st->codecpar->block_align   = avpriv_codec2_mode_block_align(s, mode);
+st->codecpar->bit_rate  = codec2_mode_bit_rate(s, mode);
+st->codecpar->frame_size= codec2_mode_frame_size(s, mode);
+st->codecpar->block_align   = codec2_mode_block_align(s, mode);
 
 if (st->codecpar->bit_rate <= 0 ||
 st->codecpar->frame_size <= 0 ||
-- 
2.28.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit

Re: [FFmpeg-devel] [PATCH v3 4/6] ffmpeg: pass decoded or filtered AVFrame to output stream initialization

On Tue, Oct 27, 2020 at 1:03 PM Anton Khirnov  wrote:
>
> Quoting Jan Ekström (2020-10-16 15:16:47)
> > Additionally, reap the first rewards by being able to set the
> > color related encoding values based on the passed AVFrame.
> >
> > The only tests that seem to have changed their results with this
> > change seem to be the MXF tests. There, the muxer writes the
> > limited/full range flag to the output container if the encoder
> > is not set to "unspecified".
> > ---
> >  fftools/ffmpeg.c| 42 +++--
> >  tests/ref/lavf/mxf_d10  |  2 +-
> >  tests/ref/lavf/mxf_dv25 |  2 +-
> >  tests/ref/lavf/mxf_dvcpro50 |  2 +-
> >  tests/ref/lavf/mxf_opatom   |  2 +-
> >  5 files changed, 35 insertions(+), 15 deletions(-)
> >
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 08db67a6ab..b2e210c814 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -941,9 +941,11 @@ early_exit:
> >  return float_pts;
> >  }
> >
> > -static int init_output_stream(OutputStream *ost, char *error, int 
> > error_len);
> > +static int init_output_stream(OutputStream *ost, AVFrame *frame,
> > +  char *error, int error_len);
> >
> > -static int init_output_stream_wrapper(OutputStream *ost, unsigned int 
> > fatal)
> > +static int init_output_stream_wrapper(OutputStream *ost, AVFrame *frame,
> > +  unsigned int fatal)
> >  {
> >  int ret = AVERROR_BUG;
> >  char error[1024] = {0};
> > @@ -951,7 +953,7 @@ static int init_output_stream_wrapper(OutputStream 
> > *ost, unsigned int fatal)
> >  if (ost->initialized)
> >  return 0;
> >
> > -ret = init_output_stream(ost, error, sizeof(error));
> > +ret = init_output_stream(ost, frame, error, sizeof(error));
> >  if (ret < 0) {
> >  av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d 
> > -- %s\n",
> > ost->file_index, ost->index, error);
> > @@ -1125,7 +1127,7 @@ static void do_video_out(OutputFile *of,
> >  InputStream *ist = NULL;
> >  AVFilterContext *filter = ost->filter->filter;
> >
> > -init_output_stream_wrapper(ost, 1);
> > +init_output_stream_wrapper(ost, next_picture, 1);
> >  sync_ipts = adjust_frame_pts_to_encoder_tb(of, ost, next_picture);
> >
> >  if (ost->source_index >= 0)
> > @@ -1507,7 +1509,7 @@ static int reap_filters(int flush)
> >   * the encoder earlier than receiving the first AVFrame.
> >   */
> >  if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_AUDIO)
> > -init_output_stream_wrapper(ost, 1);
> > +init_output_stream_wrapper(ost, NULL, 1);
> >
> >  if (!ost->filtered_frame && !(ost->filtered_frame = 
> > av_frame_alloc())) {
> >  return AVERROR(ENOMEM);
> > @@ -1930,7 +1932,7 @@ static void flush_encoders(void)
> >  finish_output_stream(ost);
> >  }
> >
> > -init_output_stream_wrapper(ost, 1);
> > +init_output_stream_wrapper(ost, NULL, 1);
> >  }
> >
> >  if (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type != 
> > AVMEDIA_TYPE_AUDIO)
> > @@ -3302,7 +3304,7 @@ static void init_encoder_time_base(OutputStream *ost, 
> > AVRational default_time_ba
> >  enc_ctx->time_base = default_time_base;
> >  }
> >
> > -static int init_output_stream_encode(OutputStream *ost)
> > +static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
> >  {
> >  InputStream *ist = get_input_stream(ost);
> >  AVCodecContext *enc_ctx = ost->enc_ctx;
> > @@ -3399,6 +3401,23 @@ static int init_output_stream_encode(OutputStream 
> > *ost)
> >  enc_ctx->bits_per_raw_sample = 
> > FFMIN(dec_ctx->bits_per_raw_sample,
> >   
> > av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth);
> >
> > +if (frame) {
> > +if (!av_dict_get(ost->encoder_opts, "color_range", NULL, 0))
>
> It doesn't seem to me the checks are needed, since encoder_opts are
> applied AFTER this block, so they'd override whatever you set here
> anyway.

Ah, I kind of missed that being focused on the task at hand. I was so
happy that the options were in a dict so they could be checked for
(since, you know, unknown is still something the user could set). Oh
the happiness :D . So I will then just simplify the code and remove
the checks in that case.

>
> Beyond that, looks very good.

Thanks. I think late encoder init is what API users should in general
be doing unless their in/out is very constrained and they already know
what we currently have as the lavfi black box will output.

Jan
___
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 5/6] ffmpeg: move field order decision making to encoder initialization

On Tue, Oct 27, 2020, 13:13 Anton Khirnov  wrote:
>
> Quoting Jan Ekström (2020-10-16 15:16:48)
> > We now have the possibility of getting AVFrames here, and we should
> > not touch the muxer's codecpar after writing the header.
> > ---
> >  fftools/ffmpeg.c  | 27 ++-
> >  .../fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
> >  .../fate/concat-demuxer-simple1-lavf-mxf_d10  |  2 +-
> >  tests/ref/fate/rgb24-mkv  |  4 +--
> >  tests/ref/lavf/mxf_d10|  2 +-
> >  5 files changed, 19 insertions(+), 18 deletions(-)
>
> Why do the tests change?

1. Matroska and mxf write interlaced/progressive field
2. Before the codecpar was prodded after init/write_header was
executed for muxer.
3. Now the codecpar is properly prodded before init/write_header is
executed for muxer and the encoder itself is initialized.

So basically I verified that all the changes seemed to make sense.
Will add a remark about it to the commit message This logic already
existed, and now it is just done at the proper spot.

Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3] hlsenc: expand hls_fmp4_init_filename with strftime()

2020-10-27 Thread Nikola Pajkovsky

init.mp4 can be expanded with strftime() the same way as
hls_segment_filename.

Signed-off-by: Nikola Pajkovsky 
---
 v2: fix memleak on strftime failure
 v3: use av_free() insted of free()

 doc/muxers.texi  |  7 ++
 libavformat/hlsenc.c | 54 +++-
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 813b4678f409..179b9239517b 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -859,6 +859,13 @@ fmp4 files may be used in HLS version 7 and above.
 @item hls_fmp4_init_filename @var{filename}
 Set filename to the fragment files header file, default filename is 
@file{init.mp4}.
 
+Use @code{-strftime 1} on @var{filename} to expand the segment filename with 
localtime.
+@example
+ffmpeg -i in.nut  -hls_segment_type fmp4 -strftime 1 -hls_fmp4_init_filename 
"%s_init.mp4" out.m3u8
+@end example
+This will produce init like this
+@file{1602678741_init.mp4}
+
 @item hls_fmp4_init_resend
 Resend init file after m3u8 file refresh every time, default is @var{0}.
 
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index cbfd8f7c0d41..3457ed5201bf 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -259,6 +259,29 @@ typedef struct HLSContext {
 int has_video_m3u8; /* has video stream m3u8 list */
 } HLSContext;
 
+static int strftime_expand(const char *fmt, char **dest)
+{
+int r = 1;
+time_t now0;
+struct tm *tm, tmpbuf;
+char *buf;
+
+buf = av_mallocz(MAX_URL_SIZE);
+if (!buf)
+return AVERROR(ENOMEM);
+
+time(&now0);
+tm = localtime_r(&now0, &tmpbuf);
+r = strftime(buf, MAX_URL_SIZE, fmt, tm);
+if (!r) {
+av_free(buf);
+return AVERROR(EINVAL);
+}
+*dest = buf;
+
+return r;
+}
+
 static int hlsenc_io_open(AVFormatContext *s, AVIOContext **pb, char *filename,
   AVDictionary **options)
 {
@@ -1660,19 +1683,15 @@ static int hls_start(AVFormatContext *s, VariantStream 
*vs)
 ff_format_set_url(oc, filename);
 } else {
 if (c->use_localtime) {
-time_t now0;
-struct tm *tm, tmpbuf;
-int bufsize = strlen(vs->basename) + MAX_URL_SIZE;
-char *buf = av_mallocz(bufsize);
-if (!buf)
-return AVERROR(ENOMEM);
-time(&now0);
-tm = localtime_r(&now0, &tmpbuf);
-ff_format_set_url(oc, buf);
-if (!strftime(oc->url, bufsize, vs->basename, tm)) {
+int r;
+char *expanded = NULL;
+
+r = strftime_expand(vs->basename, &expanded);
+if (r < 0) {
 av_log(oc, AV_LOG_ERROR, "Could not get segment filename with 
strftime\n");
-return AVERROR(EINVAL);
+return r;
 }
+ff_format_set_url(oc, expanded);
 
 err = sls_flag_use_localtime_filename(oc, c, vs);
 if (err < 0) {
@@ -2980,6 +2999,19 @@ static int hls_init(AVFormatContext *s)
 return ret;
 }
 
+if (hls->use_localtime) {
+int r;
+char *expanded = NULL;
+
+r = strftime_expand(vs->fmp4_init_filename, &expanded);
+if (r < 0) {
+  av_log(s, AV_LOG_ERROR, "Could not get segment filename 
with strftime\n");
+  return r;
+   }
+   av_free(vs->fmp4_init_filename);
+   vs->fmp4_init_filename = expanded;
+   }
+
 p = strrchr(vs->m3u8_name, '/');
 if (p) {
 char tmp = *(++p);
-- 
2.28.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/codec2utils: move codec2_version_from_extradata to lavf

James Almer:
> It's only used by the codec2 demuxers
> 
> Signed-off-by: James Almer 
> ---
>  libavcodec/codec2utils.h | 5 -
>  libavformat/codec2.c | 5 +
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/codec2utils.h b/libavcodec/codec2utils.h
> index e9b1f84d84..2ee7a592a1 100644
> --- a/libavcodec/codec2utils.h
> +++ b/libavcodec/codec2utils.h
> @@ -70,11 +70,6 @@ static inline void codec2_make_extradata(uint8_t *ptr, int 
> mode) {
>  ptr[3] = 0; //flags
>  }
>  
> -//Returns version as a 16-bit value. 0.8 -> 0x0008
> -static inline uint16_t codec2_version_from_extradata(uint8_t *ptr) {
> -return (ptr[0] << 8) + ptr[1];
> -}
> -
>  static inline uint8_t codec2_mode_from_extradata(uint8_t *ptr) {
>  return ptr[2];
>  }
> diff --git a/libavformat/codec2.c b/libavformat/codec2.c
> index 1f7f16a106..edd450716f 100644
> --- a/libavformat/codec2.c
> +++ b/libavformat/codec2.c
> @@ -86,6 +86,11 @@ static int codec2_read_header_common(AVFormatContext *s, 
> AVStream *st)
>  return 0;
>  }
>  
> +//Returns version as a 16-bit value. 0.8 -> 0x0008
> +static uint16_t codec2_version_from_extradata(uint8_t *ptr) {
> +return (ptr[0] << 8) + ptr[1];
> +}
> +
>  static int codec2_read_header(AVFormatContext *s)
>  {
>  AVStream *st = avformat_new_stream(s, NULL);
> 
Why not just remove this and directly use AV_RB16()?

- 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 2/4] avformat/gxf: Check pkt_len

Fixes: Infinite loop
Fixes: 
26576/clusterfuzz-testcase-minimized-ffmpeg_dem_GXF_fuzzer-4823080360476672

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavformat/gxf.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavformat/gxf.c b/libavformat/gxf.c
index 49364b7205..bbad47c240 100644
--- a/libavformat/gxf.c
+++ b/libavformat/gxf.c
@@ -285,9 +285,12 @@ static void gxf_track_tags(AVIOContext *pb, int *len, 
struct gxf_stream_info *si
 static void gxf_read_index(AVFormatContext *s, int pkt_len) {
 AVIOContext *pb = s->pb;
 AVStream *st;
-uint32_t fields_per_map = avio_rl32(pb);
-uint32_t map_cnt = avio_rl32(pb);
+uint32_t fields_per_map, map_cnt;
 int i;
+if (pkt_len < 8)
+return;
+fields_per_map = avio_rl32(pb);
+map_cnt = avio_rl32(pb);
 pkt_len -= 8;
 if ((s->flags & AVFMT_FLAG_IGNIDX) || !s->streams) {
 avio_skip(pb, pkt_len);
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/4] avcodec/utils: Check sample rate before use for AV_CODEC_ID_BINKAUDIO_DCT in get_audio_frame_duration()

Fixes: shift exponent 95 is too large for 32-bit type 'int'
Fixes: 
26590/clusterfuzz-testcase-minimized-ffmpeg_dem_SMACKER_fuzzer-5120609937522688

Signed-off-by: Michael Niedermayer 
---
 libavcodec/utils.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 93ac1cd9f0..3d978b390e 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1633,8 +1633,11 @@ static int get_audio_frame_duration(enum AVCodecID id, 
int sr, int ch, int ba,
 
 if (ch > 0) {
 /* calc from sample rate and channels */
-if (id == AV_CODEC_ID_BINKAUDIO_DCT)
+if (id == AV_CODEC_ID_BINKAUDIO_DCT) {
+if (sr / 22050 > 22)
+return 0;
 return (480 << (sr / 22050)) / ch;
+}
 }
 
 if (id == AV_CODEC_ID_MP3)
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/4] avformat/aiffdec: Check packet size

Fixes: Fixes infinite loop
Fixes: 
26575/clusterfuzz-testcase-minimized-ffmpeg_dem_AIFF_fuzzer-5727522236661760

Signed-off-by: Michael Niedermayer 
---
 libavformat/aiffdec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c
index c650e9074d..15733478e1 100644
--- a/libavformat/aiffdec.c
+++ b/libavformat/aiffdec.c
@@ -406,6 +406,8 @@ static int aiff_read_packet(AVFormatContext *s,
 break;
 default:
 size = st->codecpar->block_align ? (MAX_SIZE / 
st->codecpar->block_align) * st->codecpar->block_align : MAX_SIZE;
+if (!size)
+return AVERROR_INVALIDDATA;
 }
 size = FFMIN(max_size, size);
 res = av_get_packet(s->pb, pkt, size);
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/4] avcodec/pixlet: Avoid signed integer overflow in scaling in filterfn()

Fixes: signed integer overflow: 11494 * 107374182400 cannot be represented 
in type 'long'
Fixes: 
26586/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_PIXLET_fuzzer-5752633970917376

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/pixlet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/pixlet.c b/libavcodec/pixlet.c
index 42acd683fa..fd5e2ef804 100644
--- a/libavcodec/pixlet.c
+++ b/libavcodec/pixlet.c
@@ -405,7 +405,7 @@ static void filterfn(int16_t *dest, int16_t *tmp, unsigned 
size, int64_t scale)
 (int64_t) low [i - 1] * -INT64_C(325392907)  +
 (int64_t) high[i + 0] *  INT64_C(1518500249) +
 (int64_t) high[i - 1] *  INT64_C(1518500249);
-dest[i * 2] = av_clip_int16(((value >> 32) * scale) >> 32);
+dest[i * 2] = av_clip_int16(((value >> 32) * (uint64_t)scale) >> 32);
 }
 
 for (i = 0; i < hsize; i++) {
@@ -416,7 +416,7 @@ static void filterfn(int16_t *dest, int16_t *tmp, unsigned 
size, int64_t scale)
 (int64_t) high[i + 1] *  INT64_C(303700064)  +
 (int64_t) high[i + 0] * -INT64_C(3644400640) +
 (int64_t) high[i - 1] *  INT64_C(303700064);
-dest[i * 2 + 1] = av_clip_int16(((value >> 32) * scale) >> 32);
+dest[i * 2 + 1] = av_clip_int16(((value >> 32) * (uint64_t)scale) >> 
32);
 }
 }
 
-- 
2.17.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] [PATCHv2] Document community process

On Tue, Oct 27, 2020 at 11:07:23AM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-10-26 19:01:47)
> > On Sun, Oct 25, 2020 at 01:55:11PM +0100, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2020-10-19 23:57:31)
> > > > On Mon, Oct 19, 2020 at 07:22:48PM +0200, Jean-Baptiste Kempf wrote:
> > > > > Yo,
> > > > > 
> > > > > On Mon, 19 Oct 2020, at 19:02, Michael Niedermayer wrote:
> > > > > > > +## Voting
> > > > > > > +
> > > > > > 
> > > > > > > +Voting is done using a ranked voting system, currently running 
> > > > > > > on https://vote.ffmpeg.org/ .
> > > > > > 
> > > > > > I think Voting should be defined more precissely
> > > > > 
> > > > > That's a good point. What would like to see here? The algo used? The 
> > > > > software used?
> > > > 
> > > > I dont know what is best.
> > > > 
> > > > What is the goal having this information there serves ?
> > > > I think there are 3 or 4 levels/classes of information that could be 
> > > > provided
> > > > at highest level, listing the properties of the vote system
> > > 
> > > In my view, this documented is intended to serve mainly as a statement
> > > of intent rather than a strict legalistic definition of everything, so
> > > it would be sufficient to mention that we are using a ranked Condorcet
> > > method. I would think very few developers know or care what the exact
> > > differences between the methods are, as long as they are in some sense
> > > "reasonable".
> > 
> > The problem is elections with multiple winners, That is elections of seats
> > in a committee or other group.
> > Consider a 5 seat comittee
> > and lets consider that there are blue and pink candidates
> > if you have 100 people voting and 51 of them vote only for pink candidates 
> > and
> > 49 only for blue candidates.
> > repeated application of a Condorcet method will give you 5 pink candidates
> > 
> > OTOH something like schulze STV, also a Condorcet method should in this case
> > give you 3 pink candidates and 2 blue ones.
> > 
> > The above is a bit oversimplified but basically there are 2 classes of 
> > voting
> > systems. The first class is applying single winner election methods 
> > repeatedly
> > to fill all seats. 
> > The other is trying to fill seats so they are representing the set of 
> > voters.
> > 
> > The first class can skip over minorities even when they are quite large,
> > but the people choosen should have "strong and maximal support"
> > 
> > The second class would favor creating a representative set over one of
> > maximal support by voters. It could lead to a more "colorfull" result
> > with seats filled by people representing minortes and lacking broad support.
> > 
> > The results likely will differ in reallity as well.
> > 
> > We dont have to write this down in "this" document but we should
> > write it down in some document if what is considered "reasonable"
> > is "Proportional representation" or not.
> > 
> > What i can say is that if we want a 
> > * "Proportional representation" system
> >  then schulze STV is a "beatifull" system free of ugly discrete choices 
> > like STV 
> >  and its also condorcet
> > * non "Proportional representation".
> >  then normal repeatly applying the normal schulze method is the obvious 
> > choice
> > 
> > IIUC CIVS supports repeatly applying the normal schulze method and thilo 
> > added
> > support for schulze STV
> 

> AFAIU we used schulze STV for the votes so far, right? So it seems

do you think this ? hope it ? or did you check it ?
 

> reasonable to continue with that unless there are significant objections.
> 
> Generally I agree with your points, but IMO this should be in a separate
> document that describes the "technicalities" of the development process.

sure, i have no oppinion on where to put it.

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


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] [PATCHv2] Document community process

2020-10-27 Thread Jean-Baptiste Kempf

On Tue, 27 Oct 2020, at 18:02, Michael Niedermayer wrote:
> > AFAIU we used schulze STV for the votes so far, right? So it seems
> 
> do you think this ? hope it ? or did you check it ?

All the methods gave the same results.

--
Jean-Baptiste Kempf -  President
+33 672 704 734
 

___
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 v4 0/5] ffmpeg: late A/V encoder init, AVFrame metadata usage

This patch set started with a very simple wish to not have to set color
related values manually each time when utilizing ffmpeg.c.

As of the fourth iteration, the following changes were done since the third:
1. The data size threshold patch was moved to be the first one, thus meaning
   that there is no case of it not being applied, and the encoder initialization
   being moved later.
2. As noted by Anton, as the encoder options are applied after the
   AVFrame-based configuration, that code can be simplified to just
   simple passing of values instead of first checking if the option is
   set in the dictionary.
3. Interlaced/progressive and field order decision making commit has been
   reworded to include an explanation of the FATE test changes.

Unfortunately, audio still needs two locations where the encoder is
initialized, due to how avfilter_graph_request_oldest peeks and already puts
one AVFrame to be available from the filter graph (which is then utilized
as-is as an early return inside both av_buffersink_get_frame_flags and
av_buffersink_get_samples). If this would be improved in lavfi (or the call
to avfilter_graph_request_oldest removed), we could at least remove one of
these.

Currently limited to using values for video and started with the basic values,
more can be added later if needed.

This probably fixes some trac issues, but with a quick look I couldn't find
anything that explicitly was due to lack of video color metadata passthrough.


Jan


Example 1:

I have an RGB 3-D render, which I would like to encode into BT.709 YCbCr.
The video filter I'm generally using for this (zscale) does flag the matrix in
the output AVFrame.

Yet to have the video encoder have the correct metadata set, I have to
set the value(s) manually.

With this patch set, the value(s) from the first AVFrame fed to do_video_out
will be utilized.

Example 2:

I have an input video that sets one or more of the following:
matrix/primaries/transfer function/range/chroma location.

I just want to re-encode it. All of this metadata gets stripped.

With this patch set, the value(s) from the first AVFrame fed to do_video_out
will be utilized.

Example 3:

I have a video which has incorrect metadata tagged. Before, I had to set
the correct data data manually.

With this patch set, since ffmpeg.c takes color related options as dictionary
keys, the AVFrame values will only be utilized if the user has not set the
option for a given stream. Thus, this use case still works.

Jan Ekström (5):
  ffmpeg: add a data size threshold for muxing queue size
  ffmpeg: move AVFrame time base adjustment into a function
  ffmpeg: move A/V non-streamcopy initialization to a later point
  ffmpeg: pass decoded or filtered AVFrame to output stream
initialization
  ffmpeg: move field order decision making to encoder initialization

 doc/ffmpeg.texi   |   5 +
 fftools/ffmpeg.c  | 237 --
 fftools/ffmpeg.h  |  11 +
 fftools/ffmpeg_opt.c  |   8 +
 .../fate/concat-demuxer-extended-lavf-mxf_d10 |   2 +-
 .../fate/concat-demuxer-simple1-lavf-mxf_d10  |   2 +-
 tests/ref/fate/rgb24-mkv  |   4 +-
 tests/ref/lavf/mxf_d10|   2 +-
 tests/ref/lavf/mxf_dv25   |   2 +-
 tests/ref/lavf/mxf_dvcpro50   |   2 +-
 tests/ref/lavf/mxf_opatom |   2 +-
 11 files changed, 192 insertions(+), 85 deletions(-)

-- 
2.28.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 1/5] ffmpeg: add a data size threshold for muxing queue size

This way the old max queue size limit based behavior for streams
where each individual packet is large is kept, while for smaller
streams more packets can be buffered (current default is at 50
megabytes per stream).

For some explanation, by default ffmpeg copies packets from before
the appointed seek point/start time and puts them into the local
muxing queue. Before, it getting utilized was much less likely
since as soon as the filter chain was initialized, the encoder
(and thus output stream) was also initialized.

Now, since we will be pushing the encoder initialization to when the
first AVFrame is decoded and filtered - which only happens after
the exact seek point is hit as packets are ignored until then -
this queue will be seeing much more usage.

In more layman's terms, this attempts to fix cases such as where:
- seek point ends up being 5 seconds before requested time.
- audio is set to copy, and thus immediately begins filling the
  muxing queue.
- video is being encoded, and thus all received packets are skipped
  until the requested time is hit.
---
 doc/ffmpeg.texi  |  5 +
 fftools/ffmpeg.c | 11 +--
 fftools/ffmpeg.h | 11 +++
 fftools/ffmpeg_opt.c |  8 
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
index 96b3257e79..95d6463685 100644
--- a/doc/ffmpeg.texi
+++ b/doc/ffmpeg.texi
@@ -1746,6 +1746,11 @@ this buffer, in packets, for the matching output stream.
 The default value of this option should be high enough for most uses, so only
 touch this option if you are sure that you need it.
 
+@item -muxing_queue_data_threshold @var{bytes} (@emph{output,per-stream})
+This is a minimum threshold until which the muxing queue size is not taken into
+account. Defaults to 50 megabytes per stream, and is based on the overall size
+of packets passed to the muxer.
+
 @item -auto_conversion_filters (@emph{global})
 Enable automatically inserting format conversion filters in all filter
 graphs, including those defined by @option{-vf}, @option{-af},
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index cb7644de6a..3af189e7f2 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -725,8 +725,13 @@ static void write_packet(OutputFile *of, AVPacket *pkt, 
OutputStream *ost, int u
 AVPacket tmp_pkt = {0};
 /* the muxer is not initialized yet, buffer the packet */
 if (!av_fifo_space(ost->muxing_queue)) {
-int new_size = FFMIN(2 * av_fifo_size(ost->muxing_queue),
- ost->max_muxing_queue_size);
+unsigned int are_we_over_size =
+(ost->muxing_queue_data_size + pkt->size) > 
ost->muxing_queue_data_threshold;
+int new_size = are_we_over_size ?
+   FFMIN(2 * av_fifo_size(ost->muxing_queue),
+ ost->max_muxing_queue_size) :
+   2 * av_fifo_size(ost->muxing_queue);
+
 if (new_size <= av_fifo_size(ost->muxing_queue)) {
 av_log(NULL, AV_LOG_ERROR,
"Too many packets buffered for output stream %d:%d.\n",
@@ -741,6 +746,7 @@ static void write_packet(OutputFile *of, AVPacket *pkt, 
OutputStream *ost, int u
 if (ret < 0)
 exit_program(1);
 av_packet_move_ref(&tmp_pkt, pkt);
+ost->muxing_queue_data_size += tmp_pkt.size;
 av_fifo_generic_write(ost->muxing_queue, &tmp_pkt, sizeof(tmp_pkt), 
NULL);
 return;
 }
@@ -2991,6 +2997,7 @@ static int check_init_output_file(OutputFile *of, int 
file_index)
 while (av_fifo_size(ost->muxing_queue)) {
 AVPacket pkt;
 av_fifo_generic_read(ost->muxing_queue, &pkt, sizeof(pkt), NULL);
+ost->muxing_queue_data_size -= pkt.size;
 write_packet(of, &pkt, ost, 1);
 }
 }
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 8665218dcf..3b54dab7fc 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -215,6 +215,8 @@ typedef struct OptionsContext {
 intnb_passlogfiles;
 SpecifierOpt *max_muxing_queue_size;
 intnb_max_muxing_queue_size;
+SpecifierOpt *muxing_queue_data_threshold;
+intnb_muxing_queue_data_threshold;
 SpecifierOpt *guess_layout_max;
 intnb_guess_layout_max;
 SpecifierOpt *apad;
@@ -547,6 +549,15 @@ typedef struct OutputStream {
 /* the packets are buffered here until the muxer is ready to be 
initialized */
 AVFifoBuffer *muxing_queue;
 
+/*
+ * The size of the AVPackets' buffers in queue.
+ * Updated when a packet is either pushed or pulled from the queue.
+ */
+size_t muxing_queue_data_size;
+
+/* Threshold after which max_muxing_queue_size will be in effect */
+size_t muxing_queue_data_threshold;
+
 /* packet picture type */
 int pict_type;
 
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 19f719e3ff..39e0a31ea2 1006

[FFmpeg-devel] [PATCH v4 3/5] ffmpeg: move A/V non-streamcopy initialization to a later point

- For video, this means a single initialization point in do_video_out.
- For audio we unfortunately need to do it in two places just
  before the buffer sink is utilized (if av_buffersink_get_samples
  would still work according to its specification after a call to
  avfilter_graph_request_oldest was made, we could at least remove
  the one in transcode_step).

Other adjustments to make things work:
- As the AVFrame PTS adjustment to encoder time base needs the encoder
  to be initialized, so it is now moved to do_{video,audio}_out,
  right after the encoder has been initialized. Due to this,
  the additional parameter in do_video_out is removed as it is no
  longer necessary.
---
 fftools/ffmpeg.c | 112 ---
 1 file changed, 77 insertions(+), 35 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 498e5f08a6..52d1b09c78 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -947,6 +947,28 @@ early_exit:
 return float_pts;
 }
 
+static int init_output_stream(OutputStream *ost, char *error, int error_len);
+
+static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal)
+{
+int ret = AVERROR_BUG;
+char error[1024] = {0};
+
+if (ost->initialized)
+return 0;
+
+ret = init_output_stream(ost, error, sizeof(error));
+if (ret < 0) {
+av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- 
%s\n",
+   ost->file_index, ost->index, error);
+
+if (fatal)
+exit_program(1);
+}
+
+return ret;
+}
+
 static void do_audio_out(OutputFile *of, OutputStream *ost,
  AVFrame *frame)
 {
@@ -958,6 +980,8 @@ static void do_audio_out(OutputFile *of, OutputStream *ost,
 pkt.data = NULL;
 pkt.size = 0;
 
+adjust_frame_pts_to_encoder_tb(of, ost, frame);
+
 if (!check_recording_time(ost))
 return;
 
@@ -1092,8 +1116,7 @@ static void do_subtitle_out(OutputFile *of,
 
 static void do_video_out(OutputFile *of,
  OutputStream *ost,
- AVFrame *next_picture,
- double sync_ipts)
+ AVFrame *next_picture)
 {
 int ret, format_video_sync;
 AVPacket pkt;
@@ -1103,10 +1126,14 @@ static void do_video_out(OutputFile *of,
 int nb_frames, nb0_frames, i;
 double delta, delta0;
 double duration = 0;
+double sync_ipts = AV_NOPTS_VALUE;
 int frame_size = 0;
 InputStream *ist = NULL;
 AVFilterContext *filter = ost->filter->filter;
 
+init_output_stream_wrapper(ost, 1);
+sync_ipts = adjust_frame_pts_to_encoder_tb(of, ost, next_picture);
+
 if (ost->source_index >= 0)
 ist = input_streams[ost->source_index];
 
@@ -1440,28 +1467,6 @@ static void do_video_stats(OutputStream *ost, int 
frame_size)
 }
 }
 
-static int init_output_stream(OutputStream *ost, char *error, int error_len);
-
-static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal)
-{
-int ret = AVERROR_BUG;
-char error[1024] = {0};
-
-if (ost->initialized)
-return 0;
-
-ret = init_output_stream(ost, error, sizeof(error));
-if (ret < 0) {
-av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- 
%s\n",
-   ost->file_index, ost->index, error);
-
-if (fatal)
-exit_program(1);
-}
-
-return ret;
-}
-
 static void finish_output_stream(OutputStream *ost)
 {
 OutputFile *of = output_files[ost->file_index];
@@ -1498,7 +1503,17 @@ static int reap_filters(int flush)
 continue;
 filter = ost->filter->filter;
 
-init_output_stream_wrapper(ost, 1);
+/*
+ * Unlike video, with audio the audio frame size matters.
+ * Currently we are fully reliant on the lavfi filter chain to
+ * do the buffering deed for us, and thus the frame size parameter
+ * needs to be set accordingly. Where does one get the required
+ * frame size? From the initialized AVCodecContext of an audio
+ * encoder. Thus, if we have gotten to an audio stream, initialize
+ * the encoder earlier than receiving the first AVFrame.
+ */
+if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_AUDIO)
+init_output_stream_wrapper(ost, 1);
 
 if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) 
{
 return AVERROR(ENOMEM);
@@ -1506,7 +1521,6 @@ static int reap_filters(int flush)
 filtered_frame = ost->filtered_frame;
 
 while (1) {
-double float_pts = AV_NOPTS_VALUE; // this is identical to 
filtered_frame.pts but with higher precision
 ret = av_buffersink_get_frame_flags(filter, filtered_frame,
AV_BUFFERSINK_FLAG_NO_REQUEST);
 if (ret < 0) {
@@ -1515,7 +1529,7 @@ static int reap_filters(int flush)
"Error in

[FFmpeg-devel] [PATCH v4 2/5] ffmpeg: move AVFrame time base adjustment into a function

This will have to be called later for video down the line.
---
 fftools/ffmpeg.c | 72 +++-
 1 file changed, 47 insertions(+), 25 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 3af189e7f2..498e5f08a6 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -903,6 +903,50 @@ static int check_recording_time(OutputStream *ost)
 return 1;
 }
 
+static double adjust_frame_pts_to_encoder_tb(OutputFile *of, OutputStream *ost,
+ AVFrame *frame)
+{
+double float_pts = AV_NOPTS_VALUE; // this is identical to frame.pts but 
with higher precision
+AVCodecContext *enc = ost->enc_ctx;
+if (!frame || frame->pts == AV_NOPTS_VALUE ||
+!enc || !ost->filter || !ost->filter->graph->graph)
+goto early_exit;
+
+{
+AVFilterContext *filter = ost->filter->filter;
+
+int64_t start_time = (of->start_time == AV_NOPTS_VALUE) ? 0 : 
of->start_time;
+AVRational filter_tb = av_buffersink_get_time_base(filter);
+AVRational tb = enc->time_base;
+int extra_bits = av_clip(29 - av_log2(tb.den), 0, 16);
+
+tb.den <<= extra_bits;
+float_pts =
+av_rescale_q(frame->pts, filter_tb, tb) -
+av_rescale_q(start_time, AV_TIME_BASE_Q, tb);
+float_pts /= 1 << extra_bits;
+// avoid exact midoints to reduce the chance of rounding differences, 
this can be removed in case the fps code is changed to work with integers
+float_pts += FFSIGN(float_pts) * 1.0 / (1<<17);
+
+frame->pts =
+av_rescale_q(frame->pts, filter_tb, enc->time_base) -
+av_rescale_q(start_time, AV_TIME_BASE_Q, enc->time_base);
+}
+
+early_exit:
+
+if (debug_ts) {
+av_log(NULL, AV_LOG_INFO, "filter -> pts:%s pts_time:%s exact:%f 
time_base:%d/%d\n",
+   frame ? av_ts2str(frame->pts) : "NULL",
+   frame ? av_ts2timestr(frame->pts, &enc->time_base) : "NULL",
+   float_pts,
+   enc ? enc->time_base.num : -1,
+   enc ? enc->time_base.den : -1);
+}
+
+return float_pts;
+}
+
 static void do_audio_out(OutputFile *of, OutputStream *ost,
  AVFrame *frame)
 {
@@ -1479,37 +1523,15 @@ static int reap_filters(int flush)
 av_frame_unref(filtered_frame);
 continue;
 }
-if (filtered_frame->pts != AV_NOPTS_VALUE) {
-int64_t start_time = (of->start_time == AV_NOPTS_VALUE) ? 0 : 
of->start_time;
-AVRational filter_tb = av_buffersink_get_time_base(filter);
-AVRational tb = enc->time_base;
-int extra_bits = av_clip(29 - av_log2(tb.den), 0, 16);
-
-tb.den <<= extra_bits;
-float_pts =
-av_rescale_q(filtered_frame->pts, filter_tb, tb) -
-av_rescale_q(start_time, AV_TIME_BASE_Q, tb);
-float_pts /= 1 << extra_bits;
-// avoid exact midoints to reduce the chance of rounding 
differences, this can be removed in case the fps code is changed to work with 
integers
-float_pts += FFSIGN(float_pts) * 1.0 / (1<<17);
-
-filtered_frame->pts =
-av_rescale_q(filtered_frame->pts, filter_tb, 
enc->time_base) -
-av_rescale_q(start_time, AV_TIME_BASE_Q, enc->time_base);
-}
+
+float_pts = adjust_frame_pts_to_encoder_tb(of, ost,
+   filtered_frame);
 
 switch (av_buffersink_get_type(filter)) {
 case AVMEDIA_TYPE_VIDEO:
 if (!ost->frame_aspect_ratio.num)
 enc->sample_aspect_ratio = 
filtered_frame->sample_aspect_ratio;
 
-if (debug_ts) {
-av_log(NULL, AV_LOG_INFO, "filter -> pts:%s pts_time:%s 
exact:%f time_base:%d/%d\n",
-av_ts2str(filtered_frame->pts), 
av_ts2timestr(filtered_frame->pts, &enc->time_base),
-float_pts,
-enc->time_base.num, enc->time_base.den);
-}
-
 do_video_out(of, ost, filtered_frame, float_pts);
 break;
 case AVMEDIA_TYPE_AUDIO:
-- 
2.28.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 5/5] ffmpeg: move field order decision making to encoder initialization

We now have the possibility of getting AVFrames here, and we should
not touch the muxer's codecpar after writing the header.

Results of FATE tests change as the MXF and Matroska muxers actually
write down the field/frame coding type of a stream in their
respective headers. Before this change, these values in codecpar
would only be set after the muxer was initialized. Now, the
information is also available for encoder and muxer initialization.
---
 fftools/ffmpeg.c  | 27 ++-
 .../fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
 .../fate/concat-demuxer-simple1-lavf-mxf_d10  |  2 +-
 tests/ref/fate/rgb24-mkv  |  4 +--
 tests/ref/lavf/mxf_d10|  2 +-
 5 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 20d191bcf6..788cf9c665 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1123,7 +1123,6 @@ static void do_video_out(OutputFile *of,
 int ret, format_video_sync;
 AVPacket pkt;
 AVCodecContext *enc = ost->enc_ctx;
-AVCodecParameters *mux_par = ost->st->codecpar;
 AVRational frame_rate;
 int nb_frames, nb0_frames, i;
 double delta, delta0;
@@ -1285,18 +1284,6 @@ static void do_video_out(OutputFile *of,
 if (!check_recording_time(ost))
 return;
 
-if (enc->flags & (AV_CODEC_FLAG_INTERLACED_DCT | 
AV_CODEC_FLAG_INTERLACED_ME) &&
-ost->top_field_first >= 0)
-in_picture->top_field_first = !!ost->top_field_first;
-
-if (in_picture->interlaced_frame) {
-if (enc->codec->id == AV_CODEC_ID_MJPEG)
-mux_par->field_order = in_picture->top_field_first ? 
AV_FIELD_TT:AV_FIELD_BB;
-else
-mux_par->field_order = in_picture->top_field_first ? 
AV_FIELD_TB:AV_FIELD_BT;
-} else
-mux_par->field_order = AV_FIELD_PROGRESSIVE;
-
 in_picture->quality = enc->global_quality;
 in_picture->pict_type = 0;
 
@@ -3433,6 +3420,20 @@ static int init_output_stream_encode(OutputStream *ost, 
AVFrame *frame)
 enc_ctx->field_order = AV_FIELD_TT;
 }
 
+if (frame) {
+if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | 
AV_CODEC_FLAG_INTERLACED_ME) &&
+ost->top_field_first >= 0)
+frame->top_field_first = !!ost->top_field_first;
+
+if (frame->interlaced_frame) {
+if (enc_ctx->codec->id == AV_CODEC_ID_MJPEG)
+enc_ctx->field_order = frame->top_field_first ? 
AV_FIELD_TT:AV_FIELD_BB;
+else
+enc_ctx->field_order = frame->top_field_first ? 
AV_FIELD_TB:AV_FIELD_BT;
+} else
+enc_ctx->field_order = AV_FIELD_PROGRESSIVE;
+}
+
 if (ost->forced_keyframes) {
 if (!strncmp(ost->forced_keyframes, "expr:", 5)) {
 ret = av_expr_parse(&ost->forced_keyframes_pexpr, 
ost->forced_keyframes+5,
diff --git a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 
b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
index e3e76f217a..f6efc00ca4 100644
--- a/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
+++ b/tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10
@@ -1 +1 @@
-d66177ea3922692bc91cd0f8aa907650 
*tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
+84496cfe2d668db395280ea67e5c6fbe 
*tests/data/fate/concat-demuxer-extended-lavf-mxf_d10.ffprobe
diff --git a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10 
b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
index 79ce1e2306..8f3f2e5265 100644
--- a/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
+++ b/tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10
@@ -78,5 +78,5 @@ 
video|0|34|1.36|34|1.36|1|0.04|N/A|N/A|15|1924096|K_|1
 Strings Metadata
 audio|1|65280|1.36|65280|1.36|1920|0.04|N/A|N/A|7680|2074624|K_|1
 Strings Metadata
-0|mpeg2video|0|video|1/25|[0][0][0][0]|0x|720|608|0|0|0|0|1:1|45:38|yuv422p|5|tv|unknown|unknown|unknown|topleft|tt|N/A|1|N/A|25/1|25/1|1/25|0|0.00|N/A|N/A|3000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301
+0|mpeg2video|0|video|1/25|[0][0][0][0]|0x|720|608|0|0|0|0|1:1|45:38|yuv422p|5|tv|unknown|unknown|unknown|topleft|tb|N/A|1|N/A|25/1|25/1|1/25|0|0.00|N/A|N/A|3000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301
 
1|pcm_s16le|unknown|audio|1/48000|[0][0][0][0]|0x|s16|48000|2|unknown|16|N/A|0/0|0/0|1/48000|0|0.00|N/A|N/A|1536000|N/A|N/A|N/A|N/A|35|0|0|0|0|0|0|0|0|0|0|0|0|0x060A2B340101010501010D001301
diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv
index 34d028cbfd..3b14cd0ef0 100644
--- a/tests/ref/fate/rgb24-mkv
+++ b/tests/ref/fate/rgb24-mkv
@@ -1,5 +1,5 @@
-fdc02d700dbe99315a9f0d928a9b935e *

[FFmpeg-devel] [PATCH v4 4/5] ffmpeg: pass decoded or filtered AVFrame to output stream initialization

Additionally, reap the first rewards by being able to set the
color related encoding values based on the passed AVFrame.

The only tests that seem to have changed their results with this
change seem to be the MXF tests. There, the muxer writes the
limited/full range flag to the output container if the encoder
is not set to "unspecified".
---
 fftools/ffmpeg.c| 33 ++---
 tests/ref/lavf/mxf_d10  |  2 +-
 tests/ref/lavf/mxf_dv25 |  2 +-
 tests/ref/lavf/mxf_dvcpro50 |  2 +-
 tests/ref/lavf/mxf_opatom   |  2 +-
 5 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 52d1b09c78..20d191bcf6 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -947,9 +947,11 @@ early_exit:
 return float_pts;
 }
 
-static int init_output_stream(OutputStream *ost, char *error, int error_len);
+static int init_output_stream(OutputStream *ost, AVFrame *frame,
+  char *error, int error_len);
 
-static int init_output_stream_wrapper(OutputStream *ost, unsigned int fatal)
+static int init_output_stream_wrapper(OutputStream *ost, AVFrame *frame,
+  unsigned int fatal)
 {
 int ret = AVERROR_BUG;
 char error[1024] = {0};
@@ -957,7 +959,7 @@ static int init_output_stream_wrapper(OutputStream *ost, 
unsigned int fatal)
 if (ost->initialized)
 return 0;
 
-ret = init_output_stream(ost, error, sizeof(error));
+ret = init_output_stream(ost, frame, error, sizeof(error));
 if (ret < 0) {
 av_log(NULL, AV_LOG_ERROR, "Error initializing output stream %d:%d -- 
%s\n",
ost->file_index, ost->index, error);
@@ -1131,7 +1133,7 @@ static void do_video_out(OutputFile *of,
 InputStream *ist = NULL;
 AVFilterContext *filter = ost->filter->filter;
 
-init_output_stream_wrapper(ost, 1);
+init_output_stream_wrapper(ost, next_picture, 1);
 sync_ipts = adjust_frame_pts_to_encoder_tb(of, ost, next_picture);
 
 if (ost->source_index >= 0)
@@ -1513,7 +1515,7 @@ static int reap_filters(int flush)
  * the encoder earlier than receiving the first AVFrame.
  */
 if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_AUDIO)
-init_output_stream_wrapper(ost, 1);
+init_output_stream_wrapper(ost, NULL, 1);
 
 if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) 
{
 return AVERROR(ENOMEM);
@@ -1936,7 +1938,7 @@ static void flush_encoders(void)
 finish_output_stream(ost);
 }
 
-init_output_stream_wrapper(ost, 1);
+init_output_stream_wrapper(ost, NULL, 1);
 }
 
 if (enc->codec_type != AVMEDIA_TYPE_VIDEO && enc->codec_type != 
AVMEDIA_TYPE_AUDIO)
@@ -3309,7 +3311,7 @@ static void init_encoder_time_base(OutputStream *ost, 
AVRational default_time_ba
 enc_ctx->time_base = default_time_base;
 }
 
-static int init_output_stream_encode(OutputStream *ost)
+static int init_output_stream_encode(OutputStream *ost, AVFrame *frame)
 {
 InputStream *ist = get_input_stream(ost);
 AVCodecContext *enc_ctx = ost->enc_ctx;
@@ -3406,6 +3408,14 @@ static int init_output_stream_encode(OutputStream *ost)
 enc_ctx->bits_per_raw_sample = FFMIN(dec_ctx->bits_per_raw_sample,
  
av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth);
 
+if (frame) {
+enc_ctx->color_range= frame->color_range;
+enc_ctx->color_primaries= frame->color_primaries;
+enc_ctx->color_trc  = frame->color_trc;
+enc_ctx->colorspace = frame->colorspace;
+enc_ctx->chroma_sample_location = frame->chroma_location;
+}
+
 enc_ctx->framerate = ost->frame_rate;
 
 ost->st->avg_frame_rate = ost->frame_rate;
@@ -3463,7 +3473,8 @@ static int init_output_stream_encode(OutputStream *ost)
 return 0;
 }
 
-static int init_output_stream(OutputStream *ost, char *error, int error_len)
+static int init_output_stream(OutputStream *ost, AVFrame *frame,
+  char *error, int error_len)
 {
 int ret = 0;
 
@@ -3472,7 +3483,7 @@ static int init_output_stream(OutputStream *ost, char 
*error, int error_len)
 AVCodecContext *dec = NULL;
 InputStream *ist;
 
-ret = init_output_stream_encode(ost);
+ret = init_output_stream_encode(ost, frame);
 if (ret < 0)
 return ret;
 
@@ -3724,7 +3735,7 @@ static int transcode_init(void)
  output_streams[i]->enc_ctx->codec_type == AVMEDIA_TYPE_AUDIO))
 continue;
 
-ret = init_output_stream_wrapper(output_streams[i], 0);
+ret = init_output_stream_wrapper(output_streams[i], NULL, 0);
 if (ret < 0)
 goto dump_format;
 }
@@ -4657,7 +4668,7 @@ static int transcode_step(void)
  * e

Re: [FFmpeg-devel] [PATCHv2] Document community process

Quoting Michael Niedermayer (2020-10-27 18:02:46)
> 
> > AFAIU we used schulze STV for the votes so far, right? So it seems
> 
> do you think this ? hope it ? or did you check it ?

It was my impression, because as I recall the vote was postponed so that
this method could be added to the software.
In any case, I don't think we are in disagreement on anything here.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Add enable_keyframe_filtering option for libaom-av1 encoder.


On 26/10/2020 22:04, Bohan Li wrote:

Add the option to use -enable-keyframe-filtering with libaom-av1
codec.  The option controls the encoder behavior on performing
temporal filtering on keyframes.

Signed-off-by: Bohan Li 
---
  doc/encoders.texi  | 13 +
  libavcodec/libaomenc.c |  5 +
  2 files changed, 18 insertions(+)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 0b1c69e982..8914546694 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1685,6 +1685,19 @@ Enable interintra compound. Default is true.
  @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0)
  Enable smooth interintra mode. Default is true.
  
+@item enable-keyframe-filtering (Requires libaom >= v2.0.0)

+Filtering type for key frames. Possible values:
+@table @samp
+@item @emph{-1}
+Use the default in libaom (default).
+@item @emph{0}
+Do not filter key frames.
+@item @emph{1}
+Filter key frames but do not apply overlays.
+@item @emph{2}
+Filter key frames and apply overlays to them (experimental).
+@end table


This documentation does not seem helpful.  Suppose I am a normal end-user; what 
effect does this option have on the output and under what circumstances would I 
set it?  If never, why is the option in the user manual?

(I would like to ask the same question of some of the options above this one as 
well.)


+
  @end table
  
  @section libsvtav1

diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 2b0581b15a..77c25770a4 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
  int enable_diff_wtd_comp;
  int enable_dist_wtd_comp;
  int enable_dual_filter;
+int enable_keyframe_filtering;
  } AOMContext;
  
  static const char *const ctlidstr[] = {

@@ -192,6 +193,7 @@ static const char *const ctlidstr[] = {
  [AV1E_SET_REDUCED_REFERENCE_SET]= "AV1E_SET_REDUCED_REFERENCE_SET",
  [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA",
  [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS",
+[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = "AV1E_SET_ENABLE_KEYFRAME_FILTERING"
  #endif
  };
  
@@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx,

  codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, 
ctx->enable_onesided_comp);
  if (ctx->enable_smooth_interintra >= 0)
  codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, 
ctx->enable_smooth_interintra);
+if (ctx->enable_keyframe_filtering >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, 
ctx->enable_keyframe_filtering);
  #endif
  
  codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh);

@@ -1261,6 +1265,7 @@ static const AVOption options[] = {
  { "enable-masked-comp",   "Enable masked compound",   
 OFFSET(enable_masked_comp),   AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
  { "enable-interintra-comp",   "Enable interintra compound",   
 OFFSET(enable_interintra_comp),   AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
  { "enable-smooth-interintra", "Enable smooth interintra mode",
 OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
+{ "enable-keyframe-filtering","Keyframe filtering type",   
OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT,  {.i64 = -1}, -1, 3, VE},
  { NULL },
  };
  



- 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] Add enable_keyframe_filtering option for libaom-av1 encoder.

2020-10-27 Thread Bohan Li

Thanks for the comment, Mark. Indeed this may not be helpful for people who
did not know the background of such parameters. I added more details to it
so people could understand the trade-off behind it better. Will re-submit
the patch soon.

Best,
Bohan

On Tue, Oct 27, 2020 at 1:26 PM Mark Thompson  wrote:

> On 26/10/2020 22:04, Bohan Li wrote:
> > Add the option to use -enable-keyframe-filtering with libaom-av1
> > codec.  The option controls the encoder behavior on performing
> > temporal filtering on keyframes.
> >
> > Signed-off-by: Bohan Li 
> > ---
> >   doc/encoders.texi  | 13 +
> >   libavcodec/libaomenc.c |  5 +
> >   2 files changed, 18 insertions(+)
> >
> > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > index 0b1c69e982..8914546694 100644
> > --- a/doc/encoders.texi
> > +++ b/doc/encoders.texi
> > @@ -1685,6 +1685,19 @@ Enable interintra compound. Default is true.
> >   @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >=
> v2.0.0)
> >   Enable smooth interintra mode. Default is true.
> >
> > +@item enable-keyframe-filtering (Requires libaom >= v2.0.0)
> > +Filtering type for key frames. Possible values:
> > +@table @samp
> > +@item @emph{-1}
> > +Use the default in libaom (default).
> > +@item @emph{0}
> > +Do not filter key frames.
> > +@item @emph{1}
> > +Filter key frames but do not apply overlays.
> > +@item @emph{2}
> > +Filter key frames and apply overlays to them (experimental).
> > +@end table
>
> This documentation does not seem helpful.  Suppose I am a normal end-user;
> what effect does this option have on the output and under what
> circumstances would I set it?  If never, why is the option in the user
> manual?
>
> (I would like to ask the same question of some of the options above this
> one as well.)
>
> > +
> >   @end table
> >
> >   @section libsvtav1
> > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> > index 2b0581b15a..77c25770a4 100644
> > --- a/libavcodec/libaomenc.c
> > +++ b/libavcodec/libaomenc.c
> > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
> >   int enable_diff_wtd_comp;
> >   int enable_dist_wtd_comp;
> >   int enable_dual_filter;
> > +int enable_keyframe_filtering;
> >   } AOMContext;
> >
> >   static const char *const ctlidstr[] = {
> > @@ -192,6 +193,7 @@ static const char *const ctlidstr[] = {
> >   [AV1E_SET_REDUCED_REFERENCE_SET]=
> "AV1E_SET_REDUCED_REFERENCE_SET",
> >   [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] =
> "AV1E_SET_ENABLE_SMOOTH_INTERINTRA",
> >   [AV1E_SET_ENABLE_REF_FRAME_MVS] =
> "AV1E_SET_ENABLE_REF_FRAME_MVS",
> > +[AV1E_SET_ENABLE_KEYFRAME_FILTERING] =
> "AV1E_SET_ENABLE_KEYFRAME_FILTERING"
> >   #endif
> >   };
> >
> > @@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx,
> >   codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP,
> ctx->enable_onesided_comp);
> >   if (ctx->enable_smooth_interintra >= 0)
> >   codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA,
> ctx->enable_smooth_interintra);
> > +if (ctx->enable_keyframe_filtering >= 0)
> > +codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING,
> ctx->enable_keyframe_filtering);
> >   #endif
> >
> >   codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh);
> > @@ -1261,6 +1265,7 @@ static const AVOption options[] = {
> >   { "enable-masked-comp",   "Enable masked compound",
> OFFSET(enable_masked_comp),   AV_OPT_TYPE_BOOL,
> {.i64 = -1}, -1, 1, VE},
> >   { "enable-interintra-comp",   "Enable interintra compound",
> OFFSET(enable_interintra_comp),   AV_OPT_TYPE_BOOL,
> {.i64 = -1}, -1, 1, VE},
> >   { "enable-smooth-interintra", "Enable smooth interintra
> mode", OFFSET(enable_smooth_interintra),
>  AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
> > +{ "enable-keyframe-filtering","Keyframe filtering type",
>OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT,
> {.i64 = -1}, -1, 3, VE},
> >   { NULL },
> >   };
> >
> >
>
> - 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".
___
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] Add enable_keyframe_filtering option for libaom-av1 encoder.

2020-10-27 Thread Bohan Li

Add the option to use -enable-keyframe-filtering with libaom-av1
codec.  The option controls the encoder behavior on performing
temporal filtering on keyframes.

Signed-off-by: Bohan Li 
---
 doc/encoders.texi  | 17 +
 libavcodec/libaomenc.c |  5 +
 2 files changed, 22 insertions(+)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 0b1c69e982..c80a520a20 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1685,6 +1685,23 @@ Enable interintra compound. Default is true.
 @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0)
 Enable smooth interintra mode. Default is true.
 
+@item enable-keyframe-filtering (Requires libaom >= v2.0.0)
+Filtering type for key frames. Temporal filtering of key frames improves
+compression efficiency, but may introduce key frames that are blurred at times.
+Adding an overlay to filtered frames mitigates the blurring issue, but could
+potentially confuse video players when performing random access.
+Possible values:
+@table @samp
+@item @emph{-1}
+Use the default in libaom (default).
+@item @emph{0}
+Do not filter key frames. 
+@item @emph{1}
+Filter key frames but do not apply overlays. 
+@item @emph{2}
+Filter key frames and apply overlays to them (experimental).
+@end table
+
 @end table
 
 @section libsvtav1
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 2b0581b15a..77c25770a4 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
 int enable_diff_wtd_comp;
 int enable_dist_wtd_comp;
 int enable_dual_filter;
+int enable_keyframe_filtering;
 } AOMContext;
 
 static const char *const ctlidstr[] = {
@@ -192,6 +193,7 @@ static const char *const ctlidstr[] = {
 [AV1E_SET_REDUCED_REFERENCE_SET]= "AV1E_SET_REDUCED_REFERENCE_SET",
 [AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA",
 [AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS",
+[AV1E_SET_ENABLE_KEYFRAME_FILTERING] = "AV1E_SET_ENABLE_KEYFRAME_FILTERING"
 #endif
 };
 
@@ -812,6 +814,8 @@ static av_cold int aom_init(AVCodecContext *avctx,
 codecctl_int(avctx, AV1E_SET_ENABLE_ONESIDED_COMP, 
ctx->enable_onesided_comp);
 if (ctx->enable_smooth_interintra >= 0)
 codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTERINTRA, 
ctx->enable_smooth_interintra);
+if (ctx->enable_keyframe_filtering >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_KEYFRAME_FILTERING, 
ctx->enable_keyframe_filtering);
 #endif
 
 codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh);
@@ -1261,6 +1265,7 @@ static const AVOption options[] = {
 { "enable-masked-comp",   "Enable masked compound",
OFFSET(enable_masked_comp),   AV_OPT_TYPE_BOOL, {.i64 = 
-1}, -1, 1, VE},
 { "enable-interintra-comp",   "Enable interintra compound",
OFFSET(enable_interintra_comp),   AV_OPT_TYPE_BOOL, {.i64 = 
-1}, -1, 1, VE},
 { "enable-smooth-interintra", "Enable smooth interintra mode", 
OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = 
-1}, -1, 1, VE},
+{ "enable-keyframe-filtering","Keyframe filtering type",   
OFFSET(enable_keyframe_filtering),AV_OPT_TYPE_INT,  {.i64 = 
-1}, -1, 3, VE},
 { NULL },
 };
 
-- 
2.29.0.rc2.309.g374f81d7ae-goog

___
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/3] avcodec/cbs_av1: infer loop filter delta parameters from reference frames


On 21/10/2020 01:11, James Almer wrote:

Partially implements of setup_past_independence() and load_previous().
These ensures they are always set, even if the values were not coded
in the input bitstream and will not be coded in the output bitstream.

Signed-off-by: James Almer 
---
  libavcodec/cbs_av1.h |  3 +++
  libavcodec/cbs_av1_syntax_template.c | 40 +---
  2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index 7a0c08c596..97aeee9795 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState {
  int subsampling_y;  // RefSubsamplingY
  int bit_depth;  // RefBitDepth
  int order_hint; // RefOrderHint
+
+int8_t  loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
+int8_t  loop_filter_mode_deltas[2];
  } AV1ReferenceFrameState;
  
  typedef struct CodedBitstreamAV1Context {

diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index bcaa334134..4edf4fd47c 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -837,6 +837,9 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext 
*ctx, RWContext *rw,
  AV1RawFrameHeader *current)
  {
  CodedBitstreamAV1Context *priv = ctx->priv_data;
+static const int8_t 
default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] =
+{ 1, 0, 0, 0, -1, 0, -1, -1 };
+static const int8_t default_loop_filter_mode_deltas[2] = { 0 };


I realise it's the same, but the single zero there looks like an error so I 
think put two of them.


  int i, err;
  
  if (priv->coded_lossless || current->allow_intrabc) {

@@ -870,19 +873,44 @@ static int FUNC(loop_filter_params)(CodedBitstreamContext 
*ctx, RWContext *rw,
  
  flag(loop_filter_delta_enabled);

  if (current->loop_filter_delta_enabled) {
+const int8_t *loop_filter_ref_deltas, *loop_filter_mode_deltas;


Maybe call these ref_* to make the below a little clearer?  (foo[n] is inferred 
from ref_foo[n].)


+
+if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) {
+loop_filter_ref_deltas = default_loop_filter_ref_deltas;
+loop_filter_mode_deltas = default_loop_filter_mode_deltas;
+} else {
+loop_filter_ref_deltas =
+
priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas;
+loop_filter_mode_deltas =
+
priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas;
+}
+
  flag(loop_filter_delta_update);
-if (current->loop_filter_delta_update) {
  for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) {
-flags(update_ref_delta[i], 1, i);
+if (current->loop_filter_delta_update)
+flags(update_ref_delta[i], 1, i);
+else
+infer(update_ref_delta[i], 0);
  if (current->update_ref_delta[i])
  sus(1 + 6, loop_filter_ref_deltas[i], 1, i);
+else
+infer(loop_filter_ref_deltas[i], 
loop_filter_ref_deltas[i]);
  }
  for (i = 0; i < 2; i++) {
-flags(update_mode_delta[i], 1, i);
+if (current->loop_filter_delta_update)
+flags(update_mode_delta[i], 1, i);
+else
+infer(update_mode_delta[i], 0);
  if (current->update_mode_delta[i])
  sus(1 + 6, loop_filter_mode_deltas[i], 1, i);
+else
+infer(loop_filter_mode_deltas[i], 
loop_filter_mode_deltas[i]);
  }
-}
+} else {
+for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++)
+infer(loop_filter_ref_deltas[i], 
default_loop_filter_ref_deltas[i]);
+for (i = 0; i < 2; i++)
+infer(loop_filter_mode_deltas[i], 
default_loop_filter_mode_deltas[i]);
  }
  
  return 0;

@@ -1613,6 +1641,10 @@ update_refs:
  .bit_depth  = priv->bit_depth,
  .order_hint = priv->order_hint,
  };
+memcpy(priv->ref[i].loop_filter_ref_deltas, 
current->loop_filter_ref_deltas,
+   sizeof(current->loop_filter_ref_deltas));
+memcpy(priv->ref[i].loop_filter_mode_deltas, 
current->loop_filter_mode_deltas,
+   sizeof(current->loop_filter_mode_deltas));
  }
  }
  



Looks sensible.

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] avcodec/cbs_av1: add a range check to tg_end


On 27/10/2020 13:49, James Almer wrote:

Section 6.10.1 of the AV1 spec states:

It is a requirement of bitstream conformance that the value of tg_end is 
greater than or equal to tg_start.

Signed-off-by: James Almer 
---
  libavcodec/cbs_av1_syntax_template.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index 2df5619279..63b4db7853 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -1785,7 +1785,7 @@ static int FUNC(tile_group_obu)(CodedBitstreamContext 
*ctx, RWContext *rw,
  tile_bits = cbs_av1_tile_log2(1, priv->tile_cols) +
  cbs_av1_tile_log2(1, priv->tile_rows);
  fb(tile_bits, tg_start);
-fb(tile_bits, tg_end);
+fc(tile_bits, tg_end, current->tg_start, MAX_UINT_BITS(tile_bits));
  }
  
  CHECK(FUNC(byte_alignment)(ctx, rw));




This looks good.

I think the upper bound on tg_end can be num_tiles - 1 rather than accepting 
any value of tile_bits.  Can we sensibly bound tg_start as well?

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] avcodec/cbs_av1: add a range check to tg_end

On 10/27/2020 5:09 PM, Mark Thompson wrote:
> On 27/10/2020 13:49, James Almer wrote:
>> Section 6.10.1 of the AV1 spec states:
>>
>> It is a requirement of bitstream conformance that the value of tg_end
>> is greater than or equal to tg_start.
>>
>> Signed-off-by: James Almer 
>> ---
>>   libavcodec/cbs_av1_syntax_template.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/cbs_av1_syntax_template.c
>> b/libavcodec/cbs_av1_syntax_template.c
>> index 2df5619279..63b4db7853 100644
>> --- a/libavcodec/cbs_av1_syntax_template.c
>> +++ b/libavcodec/cbs_av1_syntax_template.c
>> @@ -1785,7 +1785,7 @@ static int
>> FUNC(tile_group_obu)(CodedBitstreamContext *ctx, RWContext *rw,
>>   tile_bits = cbs_av1_tile_log2(1, priv->tile_cols) +
>>   cbs_av1_tile_log2(1, priv->tile_rows);
>>   fb(tile_bits, tg_start);
>> -    fb(tile_bits, tg_end);
>> +    fc(tile_bits, tg_end, current->tg_start,
>> MAX_UINT_BITS(tile_bits));
>>   }
>>     CHECK(FUNC(byte_alignment)(ctx, rw));
>>
> 
> This looks good.
> 
> I think the upper bound on tg_end can be num_tiles - 1 rather than
> accepting any value of tile_bits.  Can we sensibly bound tg_start as well?

We would need to keep track of the amount of tiles previous groups
contained. A field in CodedBitstreamAV1Context could work for that.

I'll try that and resend.

> 
> 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".

___
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/3] avcodec/cbs_av1: infer segmentation parameters from reference frames


On 21/10/2020 01:11, James Almer wrote:

Partially implements of setup_past_independence() and load_previous().
These ensures they are always set, even if the values were not coded
in the input bitstream and will not be coded in the output bitstream.

Signed-off-by: James Almer 
---
  libavcodec/cbs_av1.h |  2 ++
  libavcodec/cbs_av1_syntax_template.c | 43 +---
  2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index 97aeee9795..a2d78e736f 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -416,6 +416,8 @@ typedef struct AV1ReferenceFrameState {
  
  int8_t  loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];

  int8_t  loop_filter_mode_deltas[2];
+uint8_t feature_enabled[AV1_MAX_SEGMENTS][AV1_SEG_LVL_MAX];
+int16_t feature_value[AV1_MAX_SEGMENTS][AV1_SEG_LVL_MAX];
  } AV1ReferenceFrameState;
  
  typedef struct CodedBitstreamAV1Context {

diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index 4edf4fd47c..2df5619279 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -743,8 +743,11 @@ static int FUNC(quantization_params)(CodedBitstreamContext 
*ctx, RWContext *rw,
  static int FUNC(segmentation_params)(CodedBitstreamContext *ctx, RWContext 
*rw,
   AV1RawFrameHeader *current)
  {
+CodedBitstreamAV1Context  *priv = ctx->priv_data;
  static const uint8_t bits[AV1_SEG_LVL_MAX] = { 8, 6, 6, 6, 6, 3, 0, 0 };
  static const uint8_t sign[AV1_SEG_LVL_MAX] = { 1, 1, 1, 1, 1, 0, 0, 0 };
+static const uint8_t default_feature_enabled[AV1_SEG_LVL_MAX] = { 0 };
+static const int16_t default_feature_value[AV1_SEG_LVL_MAX] = { 0 };
  int i, j, err;
  
  flag(segmentation_enabled);

@@ -763,22 +766,38 @@ static int 
FUNC(segmentation_params)(CodedBitstreamContext *ctx, RWContext *rw,
  flag(segmentation_update_data);
  }
  
-if (current->segmentation_update_data) {

  for (i = 0; i < AV1_MAX_SEGMENTS; i++) {
-for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
-flags(feature_enabled[i][j], 2, i, j);
+const uint8_t *feature_enabled;
+const int16_t *feature_value;


Same comment as previous about ref_feature_enabled etc.


+
+if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) {
+feature_enabled = default_feature_enabled;
+feature_value = default_feature_value;
+} else {
+feature_enabled =
+
priv->ref[current->ref_frame_idx[current->primary_ref_frame]].feature_enabled[i];
+feature_value =
+
priv->ref[current->ref_frame_idx[current->primary_ref_frame]].feature_value[i];
+}
  
-if (current->feature_enabled[i][j] && bits[j] > 0) {

-if (sign[j])
-sus(1 + bits[j], feature_value[i][j], 2, i, j);
-else
-fbs(bits[j], feature_value[i][j], 2, i, j);
+for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
+if (current->segmentation_update_data) {
+flags(feature_enabled[i][j], 2, i, j);
+
+if (current->feature_enabled[i][j] && bits[j] > 0) {
+if (sign[j])
+sus(1 + bits[j], feature_value[i][j], 2, i, j);
+else
+fbs(bits[j], feature_value[i][j], 2, i, j);
+} else {
+infer(feature_value[i][j], 0);
+}
  } else {
-infer(feature_value[i][j], 0);
+infer(feature_enabled[i][j], feature_enabled[j]);
+infer(feature_value[i][j], feature_value[j]);
  }
  }
  }
-}
  } else {
  for (i = 0; i < AV1_MAX_SEGMENTS; i++) {
  for (j = 0; j < AV1_SEG_LVL_MAX; j++) {
@@ -1645,6 +1664,10 @@ update_refs:
 sizeof(current->loop_filter_ref_deltas));
  memcpy(priv->ref[i].loop_filter_mode_deltas, 
current->loop_filter_mode_deltas,
 sizeof(current->loop_filter_mode_deltas));
+memcpy(priv->ref[i].feature_enabled, current->feature_enabled,
+   sizeof(current->feature_enabled));
+memcpy(priv->ref[i].feature_value, current->feature_value,
+   sizeof(current->feature_value));
  }
  }
  



Also sensible.  (Needs the reindent as well, not much value in it being 
separate given that this diff has most lines changed anyway.)

Thanks,

- Mark
_

Re: [FFmpeg-devel] [PATCH 1/3] avcodec/cbs_av1: infer loop filter delta parameters from reference frames

On 10/27/2020 5:38 PM, Mark Thompson wrote:
> On 21/10/2020 01:11, James Almer wrote:
>> Partially implements of setup_past_independence() and load_previous().
>> These ensures they are always set, even if the values were not coded
>> in the input bitstream and will not be coded in the output bitstream.
>>
>> Signed-off-by: James Almer 
>> ---
>>   libavcodec/cbs_av1.h |  3 +++
>>   libavcodec/cbs_av1_syntax_template.c | 40 +---
>>   2 files changed, 39 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
>> index 7a0c08c596..97aeee9795 100644
>> --- a/libavcodec/cbs_av1.h
>> +++ b/libavcodec/cbs_av1.h
>> @@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState {
>>   int subsampling_y;  // RefSubsamplingY
>>   int bit_depth;  // RefBitDepth
>>   int order_hint; // RefOrderHint
>> +
>> +    int8_t  loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
>> +    int8_t  loop_filter_mode_deltas[2];
>>   } AV1ReferenceFrameState;
>>     typedef struct CodedBitstreamAV1Context {
>> diff --git a/libavcodec/cbs_av1_syntax_template.c
>> b/libavcodec/cbs_av1_syntax_template.c
>> index bcaa334134..4edf4fd47c 100644
>> --- a/libavcodec/cbs_av1_syntax_template.c
>> +++ b/libavcodec/cbs_av1_syntax_template.c
>> @@ -837,6 +837,9 @@ static int
>> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>>   AV1RawFrameHeader *current)
>>   {
>>   CodedBitstreamAV1Context *priv = ctx->priv_data;
>> +    static const int8_t
>> default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] =
>> +    { 1, 0, 0, 0, -1, 0, -1, -1 };
>> +    static const int8_t default_loop_filter_mode_deltas[2] = { 0 };
> 
> I realise it's the same, but the single zero there looks like an error
> so I think put two of them.
> 
>>   int i, err;
>>     if (priv->coded_lossless || current->allow_intrabc) {
>> @@ -870,19 +873,44 @@ static int
>> FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
>>     flag(loop_filter_delta_enabled);
>>   if (current->loop_filter_delta_enabled) {
>> +    const int8_t *loop_filter_ref_deltas, *loop_filter_mode_deltas;
> 
> Maybe call these ref_* to make the below a little clearer?  (foo[n] is
> inferred from ref_foo[n].)
> 
>> +
>> +    if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) {
>> +    loop_filter_ref_deltas = default_loop_filter_ref_deltas;
>> +    loop_filter_mode_deltas = default_loop_filter_mode_deltas;
>> +    } else {
>> +    loop_filter_ref_deltas =
>> +   
>> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas;
>>
>> +    loop_filter_mode_deltas =
>> +   
>> priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas;
>>
>> +    }
>> +
>>   flag(loop_filter_delta_update);
>> -    if (current->loop_filter_delta_update) {
>>   for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) {
>> -    flags(update_ref_delta[i], 1, i);
>> +    if (current->loop_filter_delta_update)
>> +    flags(update_ref_delta[i], 1, i);
>> +    else
>> +    infer(update_ref_delta[i], 0);
>>   if (current->update_ref_delta[i])
>>   sus(1 + 6, loop_filter_ref_deltas[i], 1, i);
>> +    else
>> +    infer(loop_filter_ref_deltas[i],
>> loop_filter_ref_deltas[i]);
>>   }
>>   for (i = 0; i < 2; i++) {
>> -    flags(update_mode_delta[i], 1, i);
>> +    if (current->loop_filter_delta_update)
>> +    flags(update_mode_delta[i], 1, i);
>> +    else
>> +    infer(update_mode_delta[i], 0);
>>   if (current->update_mode_delta[i])
>>   sus(1 + 6, loop_filter_mode_deltas[i], 1, i);
>> +    else
>> +    infer(loop_filter_mode_deltas[i],
>> loop_filter_mode_deltas[i]);
>>   }
>> -    }
>> +    } else {
>> +    for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++)
>> +    infer(loop_filter_ref_deltas[i],
>> default_loop_filter_ref_deltas[i]);
>> +    for (i = 0; i < 2; i++)
>> +    infer(loop_filter_mode_deltas[i],
>> default_loop_filter_mode_deltas[i]);
>>   }
>>     return 0;
>> @@ -1613,6 +1641,10 @@ update_refs:
>>   .bit_depth  = priv->bit_depth,
>>   .order_hint = priv->order_hint,
>>   };
>> +    memcpy(priv->ref[i].loop_filter_ref_deltas,
>> current->loop_filter_ref_deltas,
>> +   sizeof(current->loop_filter_ref_deltas));
>> +    memcpy(priv->ref[i].loop_filter_mode_deltas,
>> current->loop_filter_mode_deltas,
>> +   sizeof(current->loop_filter_mode_deltas));
>>   }
>>   }
>>

Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: fix loading PrevGmParams for frames with primary_ref_frame none


On 17/10/2020 20:30, James Almer wrote:

Signed-off-by: James Almer 
---
  libavcodec/av1dec.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 54aeba1812..04aaf5d148 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -109,13 +109,18 @@ static void read_global_param(AV1DecContext *s, int type, 
int ref, int idx)
  {
  uint8_t primary_frame, prev_frame;
  uint32_t abs_bits, prec_bits, round, prec_diff, sub, mx;
-int32_t r;
+int32_t r, prev_gm_param;
  
  primary_frame = s->raw_frame_header->primary_ref_frame;

  prev_frame = s->raw_frame_header->ref_frame_idx[primary_frame];
  abs_bits = AV1_GM_ABS_ALPHA_BITS;
  prec_bits = AV1_GM_ALPHA_PREC_BITS;
  
+if (s->raw_frame_header->primary_ref_frame == AV1_PRIMARY_REF_NONE)

+prev_gm_param = s->cur_frame.gm_params[ref][idx];


To clarify that I'm reading the standard correctly here, this is the value set 
for PrevGmParams by setup_past_independance(), which then matches the default 
for gm_params at the top of global_motion_params() so you've reused it here?


+else
+prev_gm_param = s->ref[prev_frame].gm_params[ref][idx];
+
  if (idx < 2) {
  if (type == AV1_WARP_MODEL_TRANSLATION) {
  abs_bits = AV1_GM_ABS_TRANS_ONLY_BITS -
@@ -131,7 +136,7 @@ static void read_global_param(AV1DecContext *s, int type, 
int ref, int idx)
  prec_diff = AV1_WARPEDMODEL_PREC_BITS - prec_bits;
  sub = (idx % 3) == 2 ? (1 << prec_bits) : 0;
  mx = 1 << abs_bits;
-r = (s->ref[prev_frame].gm_params[ref][idx] >> prec_diff) - sub;
+r = (prev_gm_param >> prec_diff) - sub;
  
  s->cur_frame.gm_params[ref][idx] =

  
(decode_signed_subexp_with_ref(s->raw_frame_header->gm_params[ref][idx],



- 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] avcodec/av1dec: fix loading PrevGmParams for frames with primary_ref_frame none

On 10/27/2020 5:57 PM, Mark Thompson wrote:
> On 17/10/2020 20:30, James Almer wrote:
>> Signed-off-by: James Almer 
>> ---
>>   libavcodec/av1dec.c | 9 +++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>> index 54aeba1812..04aaf5d148 100644
>> --- a/libavcodec/av1dec.c
>> +++ b/libavcodec/av1dec.c
>> @@ -109,13 +109,18 @@ static void read_global_param(AV1DecContext *s,
>> int type, int ref, int idx)
>>   {
>>   uint8_t primary_frame, prev_frame;
>>   uint32_t abs_bits, prec_bits, round, prec_diff, sub, mx;
>> -    int32_t r;
>> +    int32_t r, prev_gm_param;
>>     primary_frame = s->raw_frame_header->primary_ref_frame;
>>   prev_frame = s->raw_frame_header->ref_frame_idx[primary_frame];
>>   abs_bits = AV1_GM_ABS_ALPHA_BITS;
>>   prec_bits = AV1_GM_ALPHA_PREC_BITS;
>>   +    if (s->raw_frame_header->primary_ref_frame ==
>> AV1_PRIMARY_REF_NONE)
>> +    prev_gm_param = s->cur_frame.gm_params[ref][idx];
> 
> To clarify that I'm reading the standard correctly here, this is the
> value set for PrevGmParams by setup_past_independance(), which then
> matches the default for gm_params at the top of global_motion_params()
> so you've reused it here?

Correct. Assuming s->cur_frame.gm_params will contain the default values
when primary_ref_frame == AV1_PRIMARY_REF_NONE (as it happens in
global_motion_params()) simplifies the code quite a bit. Otherwise I'd
have to add some array with said defaults somewhere.

> 
>> +    else
>> +    prev_gm_param = s->ref[prev_frame].gm_params[ref][idx];
>> +
>>   if (idx < 2) {
>>   if (type == AV1_WARP_MODEL_TRANSLATION) {
>>   abs_bits = AV1_GM_ABS_TRANS_ONLY_BITS -
>> @@ -131,7 +136,7 @@ static void read_global_param(AV1DecContext *s,
>> int type, int ref, int idx)
>>   prec_diff = AV1_WARPEDMODEL_PREC_BITS - prec_bits;
>>   sub = (idx % 3) == 2 ? (1 << prec_bits) : 0;
>>   mx = 1 << abs_bits;
>> -    r = (s->ref[prev_frame].gm_params[ref][idx] >> prec_diff) - sub;
>> +    r = (prev_gm_param >> prec_diff) - sub;
>>     s->cur_frame.gm_params[ref][idx] =
>>  
>> (decode_signed_subexp_with_ref(s->raw_frame_header->gm_params[ref][idx],
>>
> 
> - 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".

___
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: fix loading PrevGmParams for frames with primary_ref_frame none


On 27/10/2020 21:01, James Almer wrote:

On 10/27/2020 5:57 PM, Mark Thompson wrote:

On 17/10/2020 20:30, James Almer wrote:

Signed-off-by: James Almer 
---
   libavcodec/av1dec.c | 9 +++--
   1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 54aeba1812..04aaf5d148 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -109,13 +109,18 @@ static void read_global_param(AV1DecContext *s,
int type, int ref, int idx)
   {
   uint8_t primary_frame, prev_frame;
   uint32_t abs_bits, prec_bits, round, prec_diff, sub, mx;
-    int32_t r;
+    int32_t r, prev_gm_param;
     primary_frame = s->raw_frame_header->primary_ref_frame;
   prev_frame = s->raw_frame_header->ref_frame_idx[primary_frame];
   abs_bits = AV1_GM_ABS_ALPHA_BITS;
   prec_bits = AV1_GM_ALPHA_PREC_BITS;
   +    if (s->raw_frame_header->primary_ref_frame ==
AV1_PRIMARY_REF_NONE)
+    prev_gm_param = s->cur_frame.gm_params[ref][idx];


To clarify that I'm reading the standard correctly here, this is the
value set for PrevGmParams by setup_past_independance(), which then
matches the default for gm_params at the top of global_motion_params()
so you've reused it here?


Correct. Assuming s->cur_frame.gm_params will contain the default values
when primary_ref_frame == AV1_PRIMARY_REF_NONE (as it happens in
global_motion_params()) simplifies the code quite a bit. Otherwise I'd
have to add some array with said defaults somewhere.


Do you mind adding a little comment saying that?  It took me quite a few 
minutes of staring at it thinking it was wrong to realise what was going on.




+    else
+    prev_gm_param = s->ref[prev_frame].gm_params[ref][idx];
+
   if (idx < 2) {
   if (type == AV1_WARP_MODEL_TRANSLATION) {
   abs_bits = AV1_GM_ABS_TRANS_ONLY_BITS -
@@ -131,7 +136,7 @@ static void read_global_param(AV1DecContext *s,
int type, int ref, int idx)
   prec_diff = AV1_WARPEDMODEL_PREC_BITS - prec_bits;
   sub = (idx % 3) == 2 ? (1 << prec_bits) : 0;
   mx = 1 << abs_bits;
-    r = (s->ref[prev_frame].gm_params[ref][idx] >> prec_diff) - sub;
+    r = (prev_gm_param >> prec_diff) - sub;
     s->cur_frame.gm_params[ref][idx] =
  
(decode_signed_subexp_with_ref(s->raw_frame_header->gm_params[ref][idx],




LGTM with that.

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] avcodec/av1dec: fix loading PrevGmParams for frames with primary_ref_frame none

On 10/27/2020 6:04 PM, Mark Thompson wrote:
> On 27/10/2020 21:01, James Almer wrote:
>> On 10/27/2020 5:57 PM, Mark Thompson wrote:
>>> On 17/10/2020 20:30, James Almer wrote:
 Signed-off-by: James Almer 
 ---
    libavcodec/av1dec.c | 9 +++--
    1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
 index 54aeba1812..04aaf5d148 100644
 --- a/libavcodec/av1dec.c
 +++ b/libavcodec/av1dec.c
 @@ -109,13 +109,18 @@ static void read_global_param(AV1DecContext *s,
 int type, int ref, int idx)
    {
    uint8_t primary_frame, prev_frame;
    uint32_t abs_bits, prec_bits, round, prec_diff, sub, mx;
 -    int32_t r;
 +    int32_t r, prev_gm_param;
      primary_frame = s->raw_frame_header->primary_ref_frame;
    prev_frame = s->raw_frame_header->ref_frame_idx[primary_frame];
    abs_bits = AV1_GM_ABS_ALPHA_BITS;
    prec_bits = AV1_GM_ALPHA_PREC_BITS;
    +    if (s->raw_frame_header->primary_ref_frame ==
 AV1_PRIMARY_REF_NONE)
 +    prev_gm_param = s->cur_frame.gm_params[ref][idx];
>>>
>>> To clarify that I'm reading the standard correctly here, this is the
>>> value set for PrevGmParams by setup_past_independance(), which then
>>> matches the default for gm_params at the top of global_motion_params()
>>> so you've reused it here?
>>
>> Correct. Assuming s->cur_frame.gm_params will contain the default values
>> when primary_ref_frame == AV1_PRIMARY_REF_NONE (as it happens in
>> global_motion_params()) simplifies the code quite a bit. Otherwise I'd
>> have to add some array with said defaults somewhere.
> 
> Do you mind adding a little comment saying that?  It took me quite a few
> minutes of staring at it thinking it was wrong to realise what was going
> on.

Sure. And sorry.

> 
>>>
 +    else
 +    prev_gm_param = s->ref[prev_frame].gm_params[ref][idx];
 +
    if (idx < 2) {
    if (type == AV1_WARP_MODEL_TRANSLATION) {
    abs_bits = AV1_GM_ABS_TRANS_ONLY_BITS -
 @@ -131,7 +136,7 @@ static void read_global_param(AV1DecContext *s,
 int type, int ref, int idx)
    prec_diff = AV1_WARPEDMODEL_PREC_BITS - prec_bits;
    sub = (idx % 3) == 2 ? (1 << prec_bits) : 0;
    mx = 1 << abs_bits;
 -    r = (s->ref[prev_frame].gm_params[ref][idx] >> prec_diff) - sub;
 +    r = (prev_gm_param >> prec_diff) - sub;
      s->cur_frame.gm_params[ref][idx] =
  
 (decode_signed_subexp_with_ref(s->raw_frame_header->gm_params[ref][idx],


> 
> LGTM with that.
> 
> Thanks,

Will apply, 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".

___
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] Add enable_keyframe_filtering option for libaom-av1 encoder.

On Tue, Oct 27, 2020 at 10:38 PM Bohan Li
 wrote:
>
> Add the option to use -enable-keyframe-filtering with libaom-av1
> codec.  The option controls the encoder behavior on performing
> temporal filtering on keyframes.
>

Hi,

Looking at the amount of options etc which you want to expose to
FFmpeg users (or just utilize yourselves via FFmpeg), and by the
feeling that we're duplicating a lot of option parsing that aomenc CLI
is already doing, I wonder if it would be more useful for libaom to
provide a key=value API? This then could be exposed to FFmpeg users
with an option similar to {x264,x265,rav1e}-params.

This way each time libaom adds new options, they don't explicitly need
to be added to the wrapper. They can just be utilized.

Best regards,
Jan
___
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/3] avcodec/cbs_av1: infer loop filter delta parameters from reference frames


On 27/10/2020 20:53, James Almer wrote:

On 10/27/2020 5:38 PM, Mark Thompson wrote:

On 21/10/2020 01:11, James Almer wrote:

Partially implements of setup_past_independence() and load_previous().
These ensures they are always set, even if the values were not coded
in the input bitstream and will not be coded in the output bitstream.

Signed-off-by: James Almer 
---
   libavcodec/cbs_av1.h |  3 +++
   libavcodec/cbs_av1_syntax_template.c | 40 +---
   2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index 7a0c08c596..97aeee9795 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -413,6 +413,9 @@ typedef struct AV1ReferenceFrameState {
   int subsampling_y;  // RefSubsamplingY
   int bit_depth;  // RefBitDepth
   int order_hint; // RefOrderHint
+
+    int8_t  loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME];
+    int8_t  loop_filter_mode_deltas[2];
   } AV1ReferenceFrameState;
     typedef struct CodedBitstreamAV1Context {
diff --git a/libavcodec/cbs_av1_syntax_template.c
b/libavcodec/cbs_av1_syntax_template.c
index bcaa334134..4edf4fd47c 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -837,6 +837,9 @@ static int
FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
   AV1RawFrameHeader *current)
   {
   CodedBitstreamAV1Context *priv = ctx->priv_data;
+    static const int8_t
default_loop_filter_ref_deltas[AV1_TOTAL_REFS_PER_FRAME] =
+    { 1, 0, 0, 0, -1, 0, -1, -1 };
+    static const int8_t default_loop_filter_mode_deltas[2] = { 0 };


I realise it's the same, but the single zero there looks like an error
so I think put two of them.


   int i, err;
     if (priv->coded_lossless || current->allow_intrabc) {
@@ -870,19 +873,44 @@ static int
FUNC(loop_filter_params)(CodedBitstreamContext *ctx, RWContext *rw,
     flag(loop_filter_delta_enabled);
   if (current->loop_filter_delta_enabled) {
+    const int8_t *loop_filter_ref_deltas, *loop_filter_mode_deltas;


Maybe call these ref_* to make the below a little clearer?  (foo[n] is
inferred from ref_foo[n].)


+
+    if (current->primary_ref_frame == AV1_PRIMARY_REF_NONE) {
+    loop_filter_ref_deltas = default_loop_filter_ref_deltas;
+    loop_filter_mode_deltas = default_loop_filter_mode_deltas;
+    } else {
+    loop_filter_ref_deltas =
+
priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_ref_deltas;

+    loop_filter_mode_deltas =
+
priv->ref[current->ref_frame_idx[current->primary_ref_frame]].loop_filter_mode_deltas;

+    }
+
   flag(loop_filter_delta_update);
-    if (current->loop_filter_delta_update) {
   for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++) {
-    flags(update_ref_delta[i], 1, i);
+    if (current->loop_filter_delta_update)
+    flags(update_ref_delta[i], 1, i);
+    else
+    infer(update_ref_delta[i], 0);
   if (current->update_ref_delta[i])
   sus(1 + 6, loop_filter_ref_deltas[i], 1, i);
+    else
+    infer(loop_filter_ref_deltas[i],
loop_filter_ref_deltas[i]);
   }
   for (i = 0; i < 2; i++) {
-    flags(update_mode_delta[i], 1, i);
+    if (current->loop_filter_delta_update)
+    flags(update_mode_delta[i], 1, i);
+    else
+    infer(update_mode_delta[i], 0);
   if (current->update_mode_delta[i])
   sus(1 + 6, loop_filter_mode_deltas[i], 1, i);
+    else
+    infer(loop_filter_mode_deltas[i],
loop_filter_mode_deltas[i]);
   }
-    }
+    } else {
+    for (i = 0; i < AV1_TOTAL_REFS_PER_FRAME; i++)
+    infer(loop_filter_ref_deltas[i],
default_loop_filter_ref_deltas[i]);
+    for (i = 0; i < 2; i++)
+    infer(loop_filter_mode_deltas[i],
default_loop_filter_mode_deltas[i]);
   }
     return 0;
@@ -1613,6 +1641,10 @@ update_refs:
   .bit_depth  = priv->bit_depth,
   .order_hint = priv->order_hint,
   };
+    memcpy(priv->ref[i].loop_filter_ref_deltas,
current->loop_filter_ref_deltas,
+   sizeof(current->loop_filter_ref_deltas));
+    memcpy(priv->ref[i].loop_filter_mode_deltas,
current->loop_filter_mode_deltas,
+   sizeof(current->loop_filter_mode_deltas));
   }
   }
  


Looks sensible.


If you'd rather let the caller derive these values (because atm, none of
the bsfs care, only av1dec), then that's fine by me too.
See
https://github.com/jamrial/FFmpeg/commit/0803491e2e794a8d6cf409432b4d970da68a717b
for how it would be done there (replacing patch 3

[FFmpeg-devel] [PATCH] avcodec/cbs_av1: add a range check to tg_end

Section 6.10.1 of the AV1 spec states:

It is a requirement of bitstream conformance that the value of tg_start is
equal to the value of TileNum at the point that tile_group_obu is invoked.
It is a requirement of bitstream conformance that the value of tg_end is
greater than or equal to tg_start.

Signed-off-by: James Almer 
---
Not sure if we should clear priv->tile_num in other places, too.

 libavcodec/cbs_av1.h | 1 +
 libavcodec/cbs_av1_syntax_template.c | 8 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
index a2d78e736f..386774750a 100644
--- a/libavcodec/cbs_av1.h
+++ b/libavcodec/cbs_av1.h
@@ -446,6 +446,7 @@ typedef struct CodedBitstreamAV1Context {
 int all_lossless;
 int tile_cols;
 int tile_rows;
+int tile_num;
 
 AV1ReferenceFrameState ref[AV1_NUM_REF_FRAMES];
 } CodedBitstreamAV1Context;
diff --git a/libavcodec/cbs_av1_syntax_template.c 
b/libavcodec/cbs_av1_syntax_template.c
index 2df5619279..f0be7aab59 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -1719,6 +1719,8 @@ static int FUNC(frame_header_obu)(CodedBitstreamContext 
*ctx, RWContext *rw,
 
 CHECK(FUNC(uncompressed_header)(ctx, rw, current));
 
+priv->tile_num = 0;
+
 if (current->show_existing_frame) {
 priv->seen_frame_header = 0;
 } else {
@@ -1784,10 +1786,12 @@ static int FUNC(tile_group_obu)(CodedBitstreamContext 
*ctx, RWContext *rw,
 } else {
 tile_bits = cbs_av1_tile_log2(1, priv->tile_cols) +
 cbs_av1_tile_log2(1, priv->tile_rows);
-fb(tile_bits, tg_start);
-fb(tile_bits, tg_end);
+fc(tile_bits, tg_start, priv->tile_num, priv->tile_num);
+fc(tile_bits, tg_end, current->tg_start, num_tiles - 1);
 }
 
+priv->tile_num += current->tg_end - current->tg_start + 1;
+
 CHECK(FUNC(byte_alignment)(ctx, rw));
 
 // Reset header for next frame.
-- 
2.28.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/5] lavc/amfenc: HWConfig for amf encoder.

2020-10-27 Thread Дмитрий Овчинников

Hi! could you please review and accept this patch? It fixes a broken AMF
hwaccel pipeline.
___
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] Add enable_keyframe_filtering option for libaom-av1 encoder.

2020-10-27 Thread Bohan Li

Hi Jan,

Thanks for the suggestion! I believe that is a good idea. I am not super
familiar with this part, though, so will probably need to propose this to
the Aomedia issue tracker.
Meanwhile I think it is also useful to expose useful options (like this
one, enable-keyframe-filtering, which is discussed quite a few times in the
community as far as I know) to the users as well. So I think we should
still apply this patch, and if once the mentioned API is working, we could
maybe re-consider which ones are more needed.

Best,
Bohan

On Tue, Oct 27, 2020 at 2:19 PM Jan Ekström  wrote:

> On Tue, Oct 27, 2020 at 10:38 PM Bohan Li
>  wrote:
> >
> > Add the option to use -enable-keyframe-filtering with libaom-av1
> > codec.  The option controls the encoder behavior on performing
> > temporal filtering on keyframes.
> >
>
> Hi,
>
> Looking at the amount of options etc which you want to expose to
> FFmpeg users (or just utilize yourselves via FFmpeg), and by the
> feeling that we're duplicating a lot of option parsing that aomenc CLI
> is already doing, I wonder if it would be more useful for libaom to
> provide a key=value API? This then could be exposed to FFmpeg users
> with an option similar to {x264,x265,rav1e}-params.
>
> This way each time libaom adds new options, they don't explicitly need
> to be added to the wrapper. They can just be utilized.
>
> Best regards,
> Jan
> ___
> 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/3] avcodec/codec2utils: move codec2_version_from_extradata to lavf

On 10/27/2020 12:47 PM, Andreas Rheinhardt wrote:
> James Almer:
>> It's only used by the codec2 demuxers
>>
>> Signed-off-by: James Almer 
>> ---
>>  libavcodec/codec2utils.h | 5 -
>>  libavformat/codec2.c | 5 +
>>  2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/codec2utils.h b/libavcodec/codec2utils.h
>> index e9b1f84d84..2ee7a592a1 100644
>> --- a/libavcodec/codec2utils.h
>> +++ b/libavcodec/codec2utils.h
>> @@ -70,11 +70,6 @@ static inline void codec2_make_extradata(uint8_t *ptr, 
>> int mode) {
>>  ptr[3] = 0; //flags
>>  }
>>  
>> -//Returns version as a 16-bit value. 0.8 -> 0x0008
>> -static inline uint16_t codec2_version_from_extradata(uint8_t *ptr) {
>> -return (ptr[0] << 8) + ptr[1];
>> -}
>> -
>>  static inline uint8_t codec2_mode_from_extradata(uint8_t *ptr) {
>>  return ptr[2];
>>  }
>> diff --git a/libavformat/codec2.c b/libavformat/codec2.c
>> index 1f7f16a106..edd450716f 100644
>> --- a/libavformat/codec2.c
>> +++ b/libavformat/codec2.c
>> @@ -86,6 +86,11 @@ static int codec2_read_header_common(AVFormatContext *s, 
>> AVStream *st)
>>  return 0;
>>  }
>>  
>> +//Returns version as a 16-bit value. 0.8 -> 0x0008
>> +static uint16_t codec2_version_from_extradata(uint8_t *ptr) {
>> +return (ptr[0] << 8) + ptr[1];
>> +}
>> +
>>  static int codec2_read_header(AVFormatContext *s)
>>  {
>>  AVStream *st = avformat_new_stream(s, NULL);
>>
> Why not just remove this and directly use AV_RB16()?

Ok, will do that.

> 
> - Andreas
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Preventing Buffer Overflow for RTSP Links Increasing the buffer size of control uri, used when storing the input argument RTSP link. Following the Web URI standards, lengths

2020-10-27 Thread git

Hi Anton,

Thanks for responding!

- For more information on URI length standards for the web domain, this
link provides a good explanation:

https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

Simply speaking there are no set standards for the maximum limit on a
HTTPs or RTSPs URI, but a de-facto one from browsers such as Chrome and
Firefox start clipping URIs at 2k characters. In line with this, many
infrastructure preparing, serving and consuming these URIs also
generally try to stay under the limit of 2k characters.

In our case (Google), the infrastructure preparing these URIs is also
targeting 2k characters. In the RTSPs (secured RTSP) case, with
authentication tokens 512 or 1024 character long attached to the URI, it
is very easy to come close to this limit.

As we open up several million cameras around the world to individual
developers and companies, as part of our Device Access program
(https://developers.google.com/nest/device-access), we started to get
questions from people that wanted to use FFmpeg to stream their video.

Your application handles RTSPs URIs up to an arbitrary 1k limit fine,
but falls short on URIs that goes up to 2k characters. This patch
addresses that issue.

- How to run:
ffmpeg

To test, if you run this command with an RTSPs URI longer than 1024
characters, you'll see the playback fails.

With this patch applied, those URIs will start to work too.

Please let me know if there are any non-acceptable blockers on your side
to ingest this patch. Otherwise, please help me to get this patch into
the next ffmpeg release in a timely manner, so people can start using
it.

Best,

- Yiğit
___
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/4] avcodec/utils: Check sample rate before use for AV_CODEC_ID_BINKAUDIO_DCT in get_audio_frame_duration()

2020-10-27 Thread Peter Ross

On Tue, Oct 27, 2020 at 05:21:19PM +0100, Michael Niedermayer wrote:
> Fixes: shift exponent 95 is too large for 32-bit type 'int'
> Fixes: 
> 26590/clusterfuzz-testcase-minimized-ffmpeg_dem_SMACKER_fuzzer-5120609937522688
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/utils.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 93ac1cd9f0..3d978b390e 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1633,8 +1633,11 @@ static int get_audio_frame_duration(enum AVCodecID id, 
> int sr, int ch, int ba,
>  
>  if (ch > 0) {
>  /* calc from sample rate and channels */
> -if (id == AV_CODEC_ID_BINKAUDIO_DCT)
> +if (id == AV_CODEC_ID_BINKAUDIO_DCT) {
> +if (sr / 22050 > 22)
> +return 0;
>  return (480 << (sr / 22050)) / ch;
> +}
>  }
>  
>  if (id == AV_CODEC_ID_MP3)

looks good

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/2] avformat/mpegtsenc: first_pts_check -> first_pts_checked

2020-10-27 Thread lance . lmwang

From: Limin Wang 

change to first_pts_checked and reverse the logic. 

Signed-off-by: Limin Wang 
---
 libavformat/mpegtsenc.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 29dcaf6..383181d 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -232,7 +232,7 @@ typedef struct MpegTSWriteStream {
 int cc;
 int discontinuity;
 int payload_size;
-int first_pts_check; ///< first pts check needed
+int first_pts_checked; ///< first pts check needed
 int prev_payload_key;
 int64_t payload_pts;
 int64_t payload_dts;
@@ -1101,7 +1101,6 @@ static int mpegts_init(AVFormatContext *s)
 }
 ts_st->payload_pts = AV_NOPTS_VALUE;
 ts_st->payload_dts = AV_NOPTS_VALUE;
-ts_st->first_pts_check = 1;
 ts_st->cc  = 15;
 ts_st->discontinuity   = ts->flags & MPEGTS_FLAG_DISCONT;
 if (st->codecpar->codec_id == AV_CODEC_ID_AAC &&
@@ -1700,11 +1699,11 @@ static int mpegts_write_packet_internal(AVFormatContext 
*s, AVPacket *pkt)
 dts += delay;
 }
 
-if (ts_st->first_pts_check && pts == AV_NOPTS_VALUE) {
+if (!ts_st->first_pts_checked && pts == AV_NOPTS_VALUE) {
 av_log(s, AV_LOG_ERROR, "first pts value must be set\n");
 return AVERROR_INVALIDDATA;
 }
-ts_st->first_pts_check = 0;
+ts_st->first_pts_checked = 1;
 
 if (st->codecpar->codec_id == AV_CODEC_ID_H264) {
 const uint8_t *p = buf, *buf_end = p + size;
-- 
1.8.3.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] avformat/mpegtsenc: first dts check needed

2020-10-27 Thread lance . lmwang

From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavformat/mpegtsenc.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 383181d..acc8c32 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -233,6 +233,7 @@ typedef struct MpegTSWriteStream {
 int discontinuity;
 int payload_size;
 int first_pts_checked; ///< first pts check needed
+int first_dts_checked; ///< first dts check needed
 int prev_payload_key;
 int64_t payload_pts;
 int64_t payload_dts;
@@ -1705,6 +1706,12 @@ static int mpegts_write_packet_internal(AVFormatContext 
*s, AVPacket *pkt)
 }
 ts_st->first_pts_checked = 1;
 
+if (!ts_st->first_dts_checked && dts == AV_NOPTS_VALUE) {
+av_log(s, AV_LOG_ERROR, "first dts value must be set\n");
+return AVERROR_INVALIDDATA;
+}
+ts_st->first_dts_checked = 1;
+
 if (st->codecpar->codec_id == AV_CODEC_ID_H264) {
 const uint8_t *p = buf, *buf_end = p + size;
 uint32_t state = -1;
-- 
1.8.3.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/clearvideo: Don't check for errors for complete VLC