Re: [FFmpeg-devel] [PATCH 1/4] swscale/input: add support for XV36LE

2022-09-05 Thread Xiang, Haihao
On Sun, 2022-09-04 at 22:14 -0700, Philip Langdale wrote:
> Signed-off-by: Philip Langdale 
> ---
>  libswscale/input.c | 25 +
>  libswscale/utils.c |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/libswscale/input.c b/libswscale/input.c
> index 92681c9c53..8032360907 100644
> --- a/libswscale/input.c
> +++ b/libswscale/input.c
> @@ -685,6 +685,25 @@ static void read_vuya_A_c(uint8_t *dst, const uint8_t
> *src, const uint8_t *unuse
>  dst[i] = src[i * 4 + 3];
>  }
>  
> +static void read_xv36le_Y_c(uint8_t *dst, const uint8_t *src, const uint8_t
> *unused0, const uint8_t *unused1, int width,
> +   uint32_t *unused2, void *opq)
> +{
> +int i;
> +for (i = 0; i < width; i++)
> +AV_WN16(dst + i * 2, AV_RL16(src + i * 8 + 2) >> 4);
> +}
> +
> +
> +static void read_xv36le_UV_c(uint8_t *dstU, uint8_t *dstV, const uint8_t
> *unused0, const uint8_t *src,
> +   const uint8_t *unused1, int width, uint32_t
> *unused2, void *opq)
> +{
> +int i;
> +for (i = 0; i < width; i++) {
> +AV_WN16(dstU + i * 2, AV_RL16(src + i * 8 + 0) >> 4);
> +AV_WN16(dstV + i * 2, AV_RL16(src + i * 8 + 4) >> 4);
> +}
> +}
> +
>  /* This is almost identical to the previous, end exists only because
>   * yuy2ToY/UV)(dst, src + 1, ...) would have 100% unaligned accesses. */
>  static void uyvyToY_c(uint8_t *dst, const uint8_t *src, const uint8_t
> *unused1, const uint8_t *unused2,  int width,
> @@ -1381,6 +1400,9 @@ av_cold void ff_sws_init_input_funcs(SwsContext *c)
>  case AV_PIX_FMT_AYUV64LE:
>  c->chrToYV12 = read_ayuv64le_UV_c;
>  break;
> +case AV_PIX_FMT_XV36LE:
> +c->chrToYV12 = read_xv36le_UV_c;
> +break;
>  case AV_PIX_FMT_P010LE:
>  case AV_PIX_FMT_P210LE:
>  case AV_PIX_FMT_P410LE:
> @@ -1759,6 +1781,9 @@ av_cold void ff_sws_init_input_funcs(SwsContext *c)
>  case AV_PIX_FMT_AYUV64LE:
>  c->lumToYV12 = read_ayuv64le_Y_c;
>  break;
> +case AV_PIX_FMT_XV36LE:
> +c->lumToYV12 = read_xv36le_Y_c;
> +break;
>  case AV_PIX_FMT_YUYV422:
>  case AV_PIX_FMT_YVYU422:
>  case AV_PIX_FMT_YA8:
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index a621a35862..a67e07b612 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -262,6 +262,7 @@ static const FormatEntry format_entries[] = {
>  [AV_PIX_FMT_VUYX]= { 1, 1 },
>  [AV_PIX_FMT_RGBAF16BE]   = { 1, 0 },
>  [AV_PIX_FMT_RGBAF16LE]   = { 1, 0 },
> +[AV_PIX_FMT_XV36LE]  = { 1, 0 },
>  };
>  
>  int ff_shuffle_filter_coefficients(SwsContext *c, int *filterPos,

Patchset LGTM. You have another patchset for output, right?

Thanks
Haihao

___
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] cpu: Limit the number of auto threads in 32 bit builds

2022-09-05 Thread Martin Storsjö
Limit the returned value from av_cpu_count to sensible amounts
in 32 bit builds.

This chosen limit, 64, is somewhat arbitrary - a 32 bit process
is capable of creating much more than 64 threads. But in many
cases, multiple parts of the encoding pipeline (decoder, filters,
encoders) all create a pool of threads, auto sized according to the
number of cores.

In one failing test, the process had managed to create 506 threads
before a pthread_create call failed.

In the current set of fate tests, the filter-lavd-scalenorm test
seems to be the limiting factor; in a 32 bit build (arm linux,
running on an aarch64 kernel), it starts failing with an auto
thread count somewhere around 85. Therefore, pick the maximum
with some margin below this.

This fixes running fate without any manually set number of threads
in 32 bit builds on machines with huge numbers of cores.
---
 libavutil/cpu.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavutil/cpu.c b/libavutil/cpu.c
index 0035e927a5..094bd71d3d 100644
--- a/libavutil/cpu.c
+++ b/libavutil/cpu.c
@@ -233,6 +233,12 @@ int av_cpu_count(void)
 nb_cpus = sysinfo.dwNumberOfProcessors;
 #endif
 
+#if SIZE_MAX <= UINT32_MAX
+// Avoid running out of memory/address space in 32 bit builds, by
+// limiting the number of auto threads.
+nb_cpus = FFMIN(nb_cpus, 64);
+#endif
+
 if (!atomic_exchange_explicit(&printed, 1, memory_order_relaxed))
 av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus);
 
-- 
2.25.1

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

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


Re: [FFmpeg-devel] [PATCH v3 2/3] lavf/dashdec: Multithreaded DASH initialization

2022-09-05 Thread Lukas Fellechner
> Gesendet: Montag, 05. September 2022 um 00:50 Uhr
> Von: "Andreas Rheinhardt" 
> An: ffmpeg-devel@ffmpeg.org
> Betreff: Re: [FFmpeg-devel] [PATCH v3 2/3] lavf/dashdec: Multithreaded DASH 
> initialization
> Lukas Fellechner:
> > Andreas Rheinhardt andreas.rheinhardt at outlook.com
> > Wed Aug 31 05:54:12 EEST 2022
> >>
> >
> >> 1. We actually have an API to process multiple tasks by different
> >> threads: Look at libavutil/slicethread.h. Why can't you reuse that?
> >> 2. In case initialization of one of the conditions/mutexes fails, you
> >> are nevertheless destroying them; you are even destroying completely
> >> uninitialized mutexes. This is undefined behaviour. Checking the result
> >> of it does not fix this.
> >>
> >> - Andreas
> >
> > 1. The slicethread implementation is pretty hard to understand.
> > I was not sure if it can be used for normal parallelization, because
> > it looked more like some kind of thread pool for continuous data
> > processing. But after taking a second look, I think I can use it here.
> > I will try and see if it works well.
> >
> > 2. I was not aware that this is undefined behavior. Will switch to
> > PTHREAD_MUTEX_INITIALIZER and PTHREAD_COND_INITIALIZER macros,
> > which return a safely initialized mutex/cond.
> >
> 
> "The behavior is undefined if the value specified by the mutex argument
> to pthread_mutex_destroy() does not refer to an initialized mutex."
> (From
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_destroy.html)
> 
> Furthermore: "In cases where default mutex attributes are appropriate,
> the macro PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes.
> The effect shall be equivalent to dynamic initialization by a call to
> pthread_mutex_init() with parameter attr specified as NULL, except that
> no error checks are performed." The last sentence sounds as if one would
> then have to check mutex locking.
> 
> Moreover, older pthread standards did not allow to use
> PTHREAD_MUTEX_INITIALIZER with non-static mutexes, so I don't know
> whether we can use that. Also our pthreads-wrapper on top of
> OS/2-threads does not provide PTHREAD_COND_INITIALIZER (which is used
> nowhere in the codebase).

I missed that detail about the initializer macro. Thank you for clearing
that up.

After looking more into threads implementation in ffmpeg, I wonder if I
really need to check any results of init/destroy or other functions.
In slicethreads.c, there is zero checking on any of the lock functions.
The pthreads-based implementation does internally check the results of all
function calls and calls abort() in case of errors ("strict_" wrappers).
The Win32 implementation uses SRW locks which cannot even return errors. 
And the OS2 implementation returns 0 on all calls as well.

So right now, I think that I should continue with normal _init() calls
(no macros) and drop all error checking, just like slicethread does.
Are you fine with that approach?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH 5/5] avcodec/cfhddata: Reduce stack usage

2022-09-05 Thread Paul B Mahol
On Sat, Sep 3, 2022 at 11:56 PM Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:

> Paul B Mahol:
> > The FATE does not cover 9 (old) codebook, so make sure it is still
> working.
> >
>
> It's CRC checksum didn't change in patches 2-5.
>

Set LGTM.


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

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


[FFmpeg-devel] [PATCH v3] fftools/ffplay: fix rotation incorrect when frame contains the displaymatrix

2022-09-05 Thread 1035567130
From: Wang Yaqiang 

For example, if the jpeg contains exif information
and the rotation direction is included in the exif,
the displaymatrix will be set on the side_data of the frame when decoding.
However, when ffplay is used to play the image,
only the side data in the stream will be determined.
It does not check whether the frame also contains rotation information,
causing it to play in the wrong direction

Signed-off-by: Wang Yaqiang 
---
 fftools/ffplay.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index 9242047f5c..bcc00afe31 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -1915,8 +1915,14 @@ static int configure_video_filters(AVFilterGraph *graph, 
VideoState *is, const c
 } while (0)
 
 if (autorotate) {
-int32_t *displaymatrix = (int32_t 
*)av_stream_get_side_data(is->video_st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
-double theta = get_rotation(displaymatrix);
+double theta = 0.0;
+int32_t *displaymatrix = NULL;
+AVFrameSideData *sd = av_frame_get_side_data(frame, 
AV_FRAME_DATA_DISPLAYMATRIX);
+if (sd)
+displaymatrix = (int32_t *)sd->data;
+if (!displaymatrix)
+displaymatrix = (int32_t *)av_stream_get_side_data(is->video_st, 
AV_PKT_DATA_DISPLAYMATRIX, NULL);
+theta = get_rotation(displaymatrix);
 
 if (fabs(theta - 90) < 1.0) {
 INSERT_FILT("transpose", "clock");
-- 
2.33.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 v3 2/3] lavf/dashdec: Multithreaded DASH initialization

2022-09-05 Thread Andreas Rheinhardt
Lukas Fellechner:
>> Gesendet: Montag, 05. September 2022 um 00:50 Uhr
>> Von: "Andreas Rheinhardt" 
>> An: ffmpeg-devel@ffmpeg.org
>> Betreff: Re: [FFmpeg-devel] [PATCH v3 2/3] lavf/dashdec: Multithreaded DASH 
>> initialization
>> Lukas Fellechner:
>>> Andreas Rheinhardt andreas.rheinhardt at outlook.com
>>> Wed Aug 31 05:54:12 EEST 2022

>>>
 1. We actually have an API to process multiple tasks by different
 threads: Look at libavutil/slicethread.h. Why can't you reuse that?
 2. In case initialization of one of the conditions/mutexes fails, you
 are nevertheless destroying them; you are even destroying completely
 uninitialized mutexes. This is undefined behaviour. Checking the result
 of it does not fix this.

 - Andreas
>>>
>>> 1. The slicethread implementation is pretty hard to understand.
>>> I was not sure if it can be used for normal parallelization, because
>>> it looked more like some kind of thread pool for continuous data
>>> processing. But after taking a second look, I think I can use it here.
>>> I will try and see if it works well.
>>>
>>> 2. I was not aware that this is undefined behavior. Will switch to
>>> PTHREAD_MUTEX_INITIALIZER and PTHREAD_COND_INITIALIZER macros,
>>> which return a safely initialized mutex/cond.
>>>
>>
>> "The behavior is undefined if the value specified by the mutex argument
>> to pthread_mutex_destroy() does not refer to an initialized mutex."
>> (From
>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_destroy.html)
>>
>> Furthermore: "In cases where default mutex attributes are appropriate,
>> the macro PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes.
>> The effect shall be equivalent to dynamic initialization by a call to
>> pthread_mutex_init() with parameter attr specified as NULL, except that
>> no error checks are performed." The last sentence sounds as if one would
>> then have to check mutex locking.
>>
>> Moreover, older pthread standards did not allow to use
>> PTHREAD_MUTEX_INITIALIZER with non-static mutexes, so I don't know
>> whether we can use that. Also our pthreads-wrapper on top of
>> OS/2-threads does not provide PTHREAD_COND_INITIALIZER (which is used
>> nowhere in the codebase).
> 
> I missed that detail about the initializer macro. Thank you for clearing
> that up.
> 
> After looking more into threads implementation in ffmpeg, I wonder if I
> really need to check any results of init/destroy or other functions.
> In slicethreads.c, there is zero checking on any of the lock functions.
> The pthreads-based implementation does internally check the results of all
> function calls and calls abort() in case of errors ("strict_" wrappers).
> The Win32 implementation uses SRW locks which cannot even return errors. 
> And the OS2 implementation returns 0 on all calls as well.
> 
> So right now, I think that I should continue with normal _init() calls
> (no macros) and drop all error checking, just like slicethread does.
> Are you fine with that approach?

Zero checking is our old approach; the new approach checks for errors
and ensures that only mutexes/condition variables that have been
properly initialized are destroyed. See ff_pthread_init/free in
libavcodec/pthread.c (you can't use this in libavformat, because these
functions are local to libavcodec).

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

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


Re: [FFmpeg-devel] [PATCH] avcodec: flac x86 asm fix

2022-09-05 Thread James Almer

On 9/3/2022 6:45 PM, Paul B Mahol wrote:

Patch attached.


[...]


From 0f220489eb6402c6be3bc1d897c95fa9bc10431c Mon Sep 17 00:00:00 2001
From: Paul B Mahol 
Date: Sat, 3 Sep 2022 23:41:38 +0200
Subject: [PATCH] avcodec/x86/flacdsp: fix bug in decorrelation

Fixes #9297

Signed-off-by: Paul B Mahol 
---
 libavcodec/x86/flacdsp.asm| 23 +++-
 libavcodec/x86/flacdsp_init.c | 41 ++-
 2 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/libavcodec/x86/flacdsp.asm b/libavcodec/x86/flacdsp.asm
index 7138611526..6d755f4972 100644
--- a/libavcodec/x86/flacdsp.asm
+++ b/libavcodec/x86/flacdsp.asm
@@ -23,6 +23,10 @@
 
 %include "libavutil/x86/x86util.asm"
 
+SECTION_RODATA

+
+vector:  db 
0,1,4,5,8,9,12,13,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,0,1,4,5,8,9,12,13,
+
 SECTION .text
 
 %macro PMACSDQL 5

@@ -89,6 +93,9 @@ LPC_32 sse4
 
;--
 %macro FLAC_DECORRELATE_16 3-4
 cglobal flac_decorrelate_%1_16, 2, 4, 4, out, in0, in1, len
+%ifidn %1, indep2
+VBROADCASTI128 m2, [vector]


You're loading 16 bytes, yet vector is 32 bytes. Are you sure this is 
ok? The upper 16 bytes look different, which makes me think you meant to 
use them.



+%endif
 %if ARCH_X86_32
 mov  lend, lenm
 %endif
@@ -112,11 +119,17 @@ align 16
 %endif
 %ifnidn %1, indep2
 p%4d   m2, m0, m1
+packssdw   m%2, m%2
+packssdw   m%3, m%3
+punpcklwd  m%2, m%3
+psllw  m%2, m3
+%else
+pslld  m%2, m3
+pslld  m%3, m3
+pshufb m%2, m%2, m2
+pshufb m%3, m%3, m2
+punpcklwd  m%2, m%3
 %endif
-packssdw  m%2, m%2
-packssdw  m%3, m%3
-punpcklwd m%2, m%3
-psllw m%2, m3
 mova [outq + lenq], m%2
 add  lenq, 16
 jl .loop
@@ -292,7 +305,7 @@ align 16
 REP_RET
 %endmacro
 
-INIT_XMM sse2

+INIT_XMM ssse3


Why are you turning all the unrelated indep functions into ssse3 when 
you only changed the FLAC_DECORRELATE_16 macro to fix the stereo 16bit 
indep path?


And did you try using FLAC_DECORRELATE_INDEP for it instead? I did 
mention below i purposely used a different macro for that specific 
scenario (probably to save some instructions), so the fix could have 
been using the proper indep macro.



 FLAC_DECORRELATE_16 indep2, 0, 1 ; Reuse stereo 16bits macro
 FLAC_DECORRELATE_INDEP 32, 2, 3, d
 FLAC_DECORRELATE_INDEP 16, 4, 3, w
diff --git a/libavcodec/x86/flacdsp_init.c b/libavcodec/x86/flacdsp_init.c
index 2deaf3117f..48e3e7c55c 100644
--- a/libavcodec/x86/flacdsp_init.c
+++ b/libavcodec/x86/flacdsp_init.c
@@ -34,7 +34,9 @@ void ff_flac_decorrelate_ls_##fmt##_##opt(uint8_t **out, 
int32_t **in, int chann
 void ff_flac_decorrelate_rs_##fmt##_##opt(uint8_t **out, int32_t **in, int 
channels, \
   int len, int shift); 
  \
 void ff_flac_decorrelate_ms_##fmt##_##opt(uint8_t **out, int32_t **in, int 
channels, \
-  int len, int shift); 
  \
+  int len, int shift);
+
+#define DECORRELATE_IFUNCS(fmt, opt)   
  \
 void ff_flac_decorrelate_indep2_##fmt##_##opt(uint8_t **out, int32_t **in, int 
channels, \
  int len, int shift);  
  \
 void ff_flac_decorrelate_indep4_##fmt##_##opt(uint8_t **out, int32_t **in, int 
channels, \
@@ -48,6 +50,10 @@ DECORRELATE_FUNCS(16, sse2);
 DECORRELATE_FUNCS(16,  avx);
 DECORRELATE_FUNCS(32, sse2);
 DECORRELATE_FUNCS(32,  avx);
+DECORRELATE_IFUNCS(16, ssse3);
+DECORRELATE_IFUNCS(16,  avx);
+DECORRELATE_IFUNCS(32, ssse3);
+DECORRELATE_IFUNCS(32,  avx);
 
 av_cold void ff_flacdsp_init_x86(FLACDSPContext *c, enum AVSampleFormat fmt, int channels)

 {
@@ -55,30 +61,35 @@ av_cold void ff_flacdsp_init_x86(FLACDSPContext *c, enum 
AVSampleFormat fmt, int
 int cpu_flags = av_get_cpu_flags();
 
 if (EXTERNAL_SSE2(cpu_flags)) {

+if (fmt == AV_SAMPLE_FMT_S16) {
+c->decorrelate[1] = ff_flac_decorrelate_ls_16_sse2;
+c->decorrelate[2] = ff_flac_decorrelate_rs_16_sse2;
+c->decorrelate[3] = ff_flac_decorrelate_ms_16_sse2;
+} else if (fmt == AV_SAMPLE_FMT_S32) {
+c->decorrelate[1] = ff_flac_decorrelate_ls_32_sse2;
+c->decorrelate[2] = ff_flac_decorrelate_rs_32_sse2;
+c->decorrelate[3] = ff_flac_decorrelate_ms_32_sse2;
+}
+}
+if (EXTERNAL_SSSE3(cpu_flags)) {
 if (fmt == AV_SAMPLE_FMT_S16) {
 if (channels == 2)
-c->decorrelate[0] = ff_flac_decorrelate_indep2_16_sse2;
+c->decorrelate[0] = ff_flac_decorrelate_indep2_16_ssse3;
 else if (channels == 4)
-c->decorrelate[0] = ff_flac_decorrelate_indep4_16_sse2;
+c->dec

Re: [FFmpeg-devel] [PATCH] cpu: Limit the number of auto threads in 32 bit builds

2022-09-05 Thread Andreas Rheinhardt
Martin Storsjö:
> Limit the returned value from av_cpu_count to sensible amounts
> in 32 bit builds.
> 
> This chosen limit, 64, is somewhat arbitrary - a 32 bit process
> is capable of creating much more than 64 threads. But in many
> cases, multiple parts of the encoding pipeline (decoder, filters,
> encoders) all create a pool of threads, auto sized according to the
> number of cores.
> 
> In one failing test, the process had managed to create 506 threads
> before a pthread_create call failed.
> 
> In the current set of fate tests, the filter-lavd-scalenorm test
> seems to be the limiting factor; in a 32 bit build (arm linux,
> running on an aarch64 kernel), it starts failing with an auto
> thread count somewhere around 85. Therefore, pick the maximum
> with some margin below this.
> 
> This fixes running fate without any manually set number of threads
> in 32 bit builds on machines with huge numbers of cores.
> ---
>  libavutil/cpu.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavutil/cpu.c b/libavutil/cpu.c
> index 0035e927a5..094bd71d3d 100644
> --- a/libavutil/cpu.c
> +++ b/libavutil/cpu.c
> @@ -233,6 +233,12 @@ int av_cpu_count(void)
>  nb_cpus = sysinfo.dwNumberOfProcessors;
>  #endif
>  
> +#if SIZE_MAX <= UINT32_MAX
> +// Avoid running out of memory/address space in 32 bit builds, by
> +// limiting the number of auto threads.
> +nb_cpus = FFMIN(nb_cpus, 64);
> +#endif
> +
>  if (!atomic_exchange_explicit(&printed, 1, memory_order_relaxed))
>  av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus);
>  

I don't think we should be lying in libavutil/cpu.c. We should instead
limit the number of threads in the functions that actually create
threads based upon the return value of av_cpu_count(); e.g. both
frame_thread_encoder.c (limit 64) as well as pthread_frame.c and
pthread_slice.c (limit 16) already limit these numbers.
lavu/slicethread.c doesn't seem to do so.

- 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] cpu: Limit the number of auto threads in 32 bit builds

2022-09-05 Thread Martin Storsjö

On Mon, 5 Sep 2022, Andreas Rheinhardt wrote:


Martin Storsjö:

Limit the returned value from av_cpu_count to sensible amounts
in 32 bit builds.

This chosen limit, 64, is somewhat arbitrary - a 32 bit process
is capable of creating much more than 64 threads. But in many
cases, multiple parts of the encoding pipeline (decoder, filters,
encoders) all create a pool of threads, auto sized according to the
number of cores.

In one failing test, the process had managed to create 506 threads
before a pthread_create call failed.

In the current set of fate tests, the filter-lavd-scalenorm test
seems to be the limiting factor; in a 32 bit build (arm linux,
running on an aarch64 kernel), it starts failing with an auto
thread count somewhere around 85. Therefore, pick the maximum
with some margin below this.

This fixes running fate without any manually set number of threads
in 32 bit builds on machines with huge numbers of cores.
---
 libavutil/cpu.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavutil/cpu.c b/libavutil/cpu.c
index 0035e927a5..094bd71d3d 100644
--- a/libavutil/cpu.c
+++ b/libavutil/cpu.c
@@ -233,6 +233,12 @@ int av_cpu_count(void)
 nb_cpus = sysinfo.dwNumberOfProcessors;
 #endif

+#if SIZE_MAX <= UINT32_MAX
+// Avoid running out of memory/address space in 32 bit builds, by
+// limiting the number of auto threads.
+nb_cpus = FFMIN(nb_cpus, 64);
+#endif
+
 if (!atomic_exchange_explicit(&printed, 1, memory_order_relaxed))
 av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n", nb_cpus);



I don't think we should be lying in libavutil/cpu.c.


We should instead limit the number of threads in the functions that 
actually create threads based upon the return value of av_cpu_count(); 
e.g. both frame_thread_encoder.c (limit 64) as well as pthread_frame.c 
and pthread_slice.c (limit 16) already limit these numbers. 
lavu/slicethread.c doesn't seem to do so.


Right, that's probably the more sensible thing to do.

For something like lavu/slicethread.c, it probably makes sense with a 
low-ish limit, like 16 or 32 - splitting up an individual frame in more 
slices than that probably doesn't make sense as automatic default, right?


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

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


Re: [FFmpeg-devel] [PATCH] cpu: Limit the number of auto threads in 32 bit builds

2022-09-05 Thread Andreas Rheinhardt
Martin Storsjö:
> On Mon, 5 Sep 2022, Andreas Rheinhardt wrote:
> 
>> Martin Storsjö:
>>> Limit the returned value from av_cpu_count to sensible amounts
>>> in 32 bit builds.
>>>
>>> This chosen limit, 64, is somewhat arbitrary - a 32 bit process
>>> is capable of creating much more than 64 threads. But in many
>>> cases, multiple parts of the encoding pipeline (decoder, filters,
>>> encoders) all create a pool of threads, auto sized according to the
>>> number of cores.
>>>
>>> In one failing test, the process had managed to create 506 threads
>>> before a pthread_create call failed.
>>>
>>> In the current set of fate tests, the filter-lavd-scalenorm test
>>> seems to be the limiting factor; in a 32 bit build (arm linux,
>>> running on an aarch64 kernel), it starts failing with an auto
>>> thread count somewhere around 85. Therefore, pick the maximum
>>> with some margin below this.
>>>
>>> This fixes running fate without any manually set number of threads
>>> in 32 bit builds on machines with huge numbers of cores.
>>> ---
>>>  libavutil/cpu.c | 6 ++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/libavutil/cpu.c b/libavutil/cpu.c
>>> index 0035e927a5..094bd71d3d 100644
>>> --- a/libavutil/cpu.c
>>> +++ b/libavutil/cpu.c
>>> @@ -233,6 +233,12 @@ int av_cpu_count(void)
>>>  nb_cpus = sysinfo.dwNumberOfProcessors;
>>>  #endif
>>>
>>> +#if SIZE_MAX <= UINT32_MAX
>>> +    // Avoid running out of memory/address space in 32 bit builds, by
>>> +    // limiting the number of auto threads.
>>> +    nb_cpus = FFMIN(nb_cpus, 64);
>>> +#endif
>>> +
>>>  if (!atomic_exchange_explicit(&printed, 1, memory_order_relaxed))
>>>  av_log(NULL, AV_LOG_DEBUG, "detected %d logical cores\n",
>>> nb_cpus);
>>>
>>
>> I don't think we should be lying in libavutil/cpu.c.
> 
>> We should instead limit the number of threads in the functions that
>> actually create threads based upon the return value of av_cpu_count();
>> e.g. both frame_thread_encoder.c (limit 64) as well as pthread_frame.c
>> and pthread_slice.c (limit 16) already limit these numbers.
>> lavu/slicethread.c doesn't seem to do so.
> 
> Right, that's probably the more sensible thing to do.
> 
> For something like lavu/slicethread.c, it probably makes sense with a
> low-ish limit, like 16 or 32 - splitting up an individual frame in more
> slices than that probably doesn't make sense as automatic default, right?
> 

Yes. E.g. MAX_AUTO_THREADS in pthread_slice.c is 16.

- 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] slicethread: Limit the automatic number of threads to 16

2022-09-05 Thread Martin Storsjö
This matches a similar cap on the number of automatic threads
in libavcodec/pthread_slice.c.

On systems with lots of cores, this does speed things up in
general (measurable on the level of the runtime of running
"make fate"), and fixes a couple fate failures in 32 bit mode on
such machines (where spawning a huge number of threads runs
out of address space).
---
 libavutil/slicethread.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavutil/slicethread.c b/libavutil/slicethread.c
index ea1c9c8311..115b099736 100644
--- a/libavutil/slicethread.c
+++ b/libavutil/slicethread.c
@@ -24,6 +24,8 @@
 #include "thread.h"
 #include "avassert.h"
 
+#define MAX_AUTO_THREADS 16
+
 #if HAVE_PTHREADS || HAVE_W32THREADS || HAVE_OS2THREADS
 
 typedef struct WorkerContext {
@@ -105,7 +107,7 @@ int avpriv_slicethread_create(AVSliceThread **pctx, void 
*priv,
 if (!nb_threads) {
 int nb_cpus = av_cpu_count();
 if (nb_cpus > 1)
-nb_threads = nb_cpus + 1;
+nb_threads = FFMIN(nb_cpus + 1, MAX_AUTO_THREADS);
 else
 nb_threads = 1;
 }
-- 
2.25.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] avfilter/avfilter: Don't use AVFrame.channel_layout

2022-09-05 Thread James Almer

On 9/4/2022 8:30 PM, Andreas Rheinhardt wrote:

Signed-off-by: Andreas Rheinhardt 
---
  libavfilter/avfilter.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 965f5d0f63..6740339808 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -62,7 +62,7 @@ static void tlog_ref(void *ctx, AVFrame *ref, int end)
  }
  if (ref->nb_samples) {
  ff_tlog(ctx, " cl:%"PRId64"d n:%d r:%d",
-ref->channel_layout,
+ref->ch_layout.order == AV_CHANNEL_ORDER_NATIVE ? 
ref->ch_layout.u.mask : 0,


Should be ok, but i expect that eventually filters will support and 
start propagating custom layouts (after the old API is gone), so maybe 
using the layout name helpers is a better idea.



  ref->nb_samples,
  ref->sample_rate);
  }

___
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 2/3] lavf/dashdec: Multithreaded DASH initialization

2022-09-05 Thread Lukas Fellechner
>Gesendet: Montag, 05. September 2022 um 12:45 Uhr
>Von: "Andreas Rheinhardt" 
>An: ffmpeg-devel@ffmpeg.org
>Betreff: Re: [FFmpeg-devel] [PATCH v3 2/3] lavf/dashdec: Multithreaded DASH 
>initialization
>Lukas Fellechner:
>>> Gesendet: Montag, 05. September 2022 um 00:50 Uhr
>>> Von: "Andreas Rheinhardt" 
>>> An: ffmpeg-devel@ffmpeg.org
>>> Betreff: Re: [FFmpeg-devel] [PATCH v3 2/3] lavf/dashdec: Multithreaded DASH 
>>> initialization
>>> Lukas Fellechner:
 Andreas Rheinhardt andreas.rheinhardt at outlook.com
 Wed Aug 31 05:54:12 EEST 2022
>

> 1. We actually have an API to process multiple tasks by different
> threads: Look at libavutil/slicethread.h. Why can't you reuse that?
> 2. In case initialization of one of the conditions/mutexes fails, you
> are nevertheless destroying them; you are even destroying completely
> uninitialized mutexes. This is undefined behaviour. Checking the result
> of it does not fix this.
>
> - Andreas

 1. The slicethread implementation is pretty hard to understand.
 I was not sure if it can be used for normal parallelization, because
 it looked more like some kind of thread pool for continuous data
 processing. But after taking a second look, I think I can use it here.
 I will try and see if it works well.

 2. I was not aware that this is undefined behavior. Will switch to
 PTHREAD_MUTEX_INITIALIZER and PTHREAD_COND_INITIALIZER macros,
 which return a safely initialized mutex/cond.

>>>
>>> "The behavior is undefined if the value specified by the mutex argument
>>> to pthread_mutex_destroy() does not refer to an initialized mutex."
>>> (From
>>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_destroy.html)
>>>
>>> Furthermore: "In cases where default mutex attributes are appropriate,
>>> the macro PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes.
>>> The effect shall be equivalent to dynamic initialization by a call to
>>> pthread_mutex_init() with parameter attr specified as NULL, except that
>>> no error checks are performed." The last sentence sounds as if one would
>>> then have to check mutex locking.
>>>
>>> Moreover, older pthread standards did not allow to use
>>> PTHREAD_MUTEX_INITIALIZER with non-static mutexes, so I don't know
>>> whether we can use that. Also our pthreads-wrapper on top of
>>> OS/2-threads does not provide PTHREAD_COND_INITIALIZER (which is used
>>> nowhere in the codebase).
>>
>> I missed that detail about the initializer macro. Thank you for clearing
>> that up.
>>
>> After looking more into threads implementation in ffmpeg, I wonder if I
>> really need to check any results of init/destroy or other functions.
>> In slicethreads.c, there is zero checking on any of the lock functions.
>> The pthreads-based implementation does internally check the results of all
>> function calls and calls abort() in case of errors ("strict_" wrappers).
>> The Win32 implementation uses SRW locks which cannot even return errors.
>> And the OS2 implementation returns 0 on all calls as well.
>>
>> So right now, I think that I should continue with normal _init() calls
>> (no macros) and drop all error checking, just like slicethread does.
>> Are you fine with that approach?
>
> Zero checking is our old approach; the new approach checks for errors
> and ensures that only mutexes/condition variables that have been
> properly initialized are destroyed. See ff_pthread_init/free in
> libavcodec/pthread.c (you can't use this in libavformat, because these
> functions are local to libavcodec).
>
> - Andreas

I see. I will try to do clean error checking then.
___
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] lavu/pixfmt: Add P012, Y212, XV30, and XV36 formats

2022-09-05 Thread James Almer

On 8/25/2022 11:17 PM, Philip Langdale wrote:

  static const char * const color_range_names[] = {
@@ -2778,7 +2871,7 @@ void ff_check_pixfmt_descriptors(void){
  
  if (!d->name && !d->nb_components && !d->log2_chroma_w && !d->log2_chroma_h && !d->flags)

  continue;
-// av_log(NULL, AV_LOG_DEBUG, "Checking: %s\n", d->name);
+av_log(NULL, AV_LOG_INFO, "Checking: %s\n", d->name);
  av_assert0(d->log2_chroma_w <= 3);
  av_assert0(d->log2_chroma_h <= 3);
  av_assert0(d->nb_components <= 4);


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

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


Re: [FFmpeg-devel] [RFC] d3dva security hw+threads

2022-09-05 Thread Lukas Fellechner
> Gesendet: Freitag, 02. September 2022 um 01:46 Uhr
> Von: "Timo Rothenpieler" 
> An: ffmpeg-devel@ffmpeg.org
> Betreff: Re: [FFmpeg-devel] [RFC] d3dva security hw+threads
> On 02.09.2022 01:32, Michael Niedermayer wrote:
>> Hi all
>>
>> Theres a use after free issue in H.264 Decoding on d3d11va with multiple 
>> threads
>> I dont have the hardware/platform nor do i know the hw decoding code so i 
>> made
>> no attempt to fix this beyond asking others to ...
>
> hwaccel with multiple threads being broken is not exactly a surprise.
> So we could just disable that, and always have it be one single thread?

I am using FFmpeg as decoder library in a video player, either with sw decoding
or d3d11va. Originally, I had threads set to auto in all cases. While it worked
for some codecs such as H.264, it produced garbage output for other codecs.
I think VP8/VP9 are severly broken with threading+d3d11va. So I had to manually
set threads to 1, if using hwaccel. Only then I had stable output for all 
codecs.

Using multithreading together with hwaccel really does not make any sense.
All the work is done by the GPU, not by the CPU. And the GPU will parallelize
internally where possible. Adding CPU multithreading will not help at all.

IMHO the best would be to completely disable threading when hwaccel is in use.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [RFC] d3dva security hw+threads

2022-09-05 Thread Anton Khirnov
Quoting Lukas Fellechner (2022-09-05 17:50:26)
> > Gesendet: Freitag, 02. September 2022 um 01:46 Uhr
> > Von: "Timo Rothenpieler" 
> > An: ffmpeg-devel@ffmpeg.org
> > Betreff: Re: [FFmpeg-devel] [RFC] d3dva security hw+threads
> > On 02.09.2022 01:32, Michael Niedermayer wrote:
> >> Hi all
> >>
> >> Theres a use after free issue in H.264 Decoding on d3d11va with multiple 
> >> threads
> >> I dont have the hardware/platform nor do i know the hw decoding code so i 
> >> made
> >> no attempt to fix this beyond asking others to ...
> >
> > hwaccel with multiple threads being broken is not exactly a surprise.
> > So we could just disable that, and always have it be one single thread?
> 
> I am using FFmpeg as decoder library in a video player, either with sw 
> decoding
> or d3d11va. Originally, I had threads set to auto in all cases. While it 
> worked
> for some codecs such as H.264, it produced garbage output for other codecs.
> I think VP8/VP9 are severly broken with threading+d3d11va. So I had to 
> manually
> set threads to 1, if using hwaccel. Only then I had stable output for all 
> codecs.

Does this still happen with recent code? Using frame threading with
hwaccel is a supported use case and should work, any breakage is a bug
that should be fixed in libavcodec.

> Using multithreading together with hwaccel really does not make any sense.
> All the work is done by the GPU, not by the CPU. And the GPU will parallelize
> internally where possible. Adding CPU multithreading will not help at all.
> 
> IMHO the best would be to completely disable threading when hwaccel is in use.

The problem is that in general you cannot know beforehand whether
hwaccel will be usable. What's more, since stream parameters can change
dynamically, you can have a stream where some segments are decodable
with hwaccel, but some are not.

In this case you typically want lavc to use hwaccel wherever possible
and frame-threaded software decoding otherwise, switching back and forth
as needed. This is what is actually meant by "hwaccel with multiple
threads" --- actual hwaccel decoding is already serialized in
libavcodec.

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

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


[FFmpeg-devel] [PATCH] avcodec/flac_parser: ensure there are more headers for scoring

2022-09-05 Thread Paul B Mahol
Patch attached.
From 81656532bdf649b43e3ac777637433b033b56d03 Mon Sep 17 00:00:00 2001
From: Paul B Mahol 
Date: Mon, 5 Sep 2022 18:12:10 +0200
Subject: [PATCH] avcodec/flac_parser: ensure there are more headers for
 scoring

Previously invalid frame may be returned, happened when seeking.

Fixes #7684

Signed-off-by: Paul B Mahol 
---
 libavcodec/flac_parser.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
index a1d9cd7f29..11cd5540cf 100644
--- a/libavcodec/flac_parser.c
+++ b/libavcodec/flac_parser.c
@@ -600,7 +600,7 @@ static int score_header(FLACParseContext *fpc, FLACHeaderMarker *header)
 static void score_sequences(FLACParseContext *fpc)
 {
 FLACHeaderMarker *curr;
-int best_score = 0;//FLAC_HEADER_NOT_SCORED_YET;
+int best_score = FLAC_HEADER_NOT_SCORED_YET;
 /* First pass to clear all old scores. */
 for (curr = fpc->headers; curr; curr = curr->next)
 curr->max_score = FLAC_HEADER_NOT_SCORED_YET;
@@ -682,7 +682,7 @@ static int flac_parse(AVCodecParserContext *s, AVCodecContext *avctx,
 }
 
 fpc->avctx = avctx;
-if (fpc->best_header_valid)
+if (fpc->best_header_valid && fpc->nb_headers_buffered >= FLAC_MIN_HEADERS)
 return get_best_header(fpc, poutbuf, poutbuf_size);
 
 /* If a best_header was found last call remove it with the buffer data. */
-- 
2.37.2

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

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


[FFmpeg-devel] [PATCH] lavu/riscv: cycle counter for AV_READ_TIME

2022-09-05 Thread remi
From: Rémi Denis-Courmont 

This uses the architected RISC-V 64-bit cycle counter from the
RISC-V unprivileged instruction set.

In 64-bit and 128-bit, this is a straightforward CSR read.
In 32-bit mode, the 64-bit value is exposed as two CSRs, which
cannot be read atomically, so a loop is necessary to detect and fix up
the race condition where the bottom half wraps exactly between the two
reads.

---
Tested on VisionFive SBC courtesy of Shanghai StarFive Technology.
---
 libavutil/riscv/timer.h | 54 +
 libavutil/timer.h   |  2 ++
 2 files changed, 56 insertions(+)
 create mode 100644 libavutil/riscv/timer.h

diff --git a/libavutil/riscv/timer.h b/libavutil/riscv/timer.h
new file mode 100644
index 00..02a4b3962e
--- /dev/null
+++ b/libavutil/riscv/timer.h
@@ -0,0 +1,54 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVUTIL_RISCV_TIMER_H
+#define AVUTIL_RISCV_TIMER_H
+
+#include "config.h"
+
+#if HAVE_INLINE_ASM
+#include 
+
+static inline uint64_t rdcycle64(void)
+{
+#if (__riscv_xlen >= 64)
+uintptr_t cycles;
+
+__asm__ volatile ("rdcycle %0" : "=r"(cycles));
+
+#else
+uint64_t cycles;
+uint32_t hi, lo, check;
+
+do {
+__asm__ volatile (
+"rdcycleh %0\n"
+"rdcycle  %1\n"
+"rdcycleh %2\n" : "=r" (hi), "=r" (lo), "=r" (check));
+} while (hi != check);
+
+cycles = (((uint64_t)hi) << 32) | lo;
+
+#endif
+return cycles;
+}
+
+#define AV_READ_TIME rdcycle64
+
+#endif
+#endif /* AVUTIL_RISCV_TIMER_H */
diff --git a/libavutil/timer.h b/libavutil/timer.h
index 48e576739f..0b3460c682 100644
--- a/libavutil/timer.h
+++ b/libavutil/timer.h
@@ -55,6 +55,8 @@
 #   include "aarch64/timer.h"
 #elif ARCH_ARM
 #   include "arm/timer.h"
+#elif ARCH_RISCV
+#   include "riscv/timer.h"
 #elif ARCH_PPC
 #   include "ppc/timer.h"
 #elif ARCH_X86
-- 
2.37.2

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

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


Re: [FFmpeg-devel] [PATCH v1] avformat/mxf: set stream frame rates for ST 422 essence containers

2022-09-05 Thread Pierre-Anthony Lemieux
On Sat, Sep 3, 2022 at 11:26 AM Tomas Härdin  wrote:
>
> sön 2022-08-28 klockan 08:31 -0700 skrev p...@sandflow.com:
> > From: Pierre-Anthony Lemieux 
> >
> > The MXF demuxer does not currently set AVStream::avg_frame_rate and
> > ::r_frame_rate
> > when J2K essence is wrapped according to SMPTE ST 422.
> >
> > ---
> >  libavformat/mxfdec.c | 25 +
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > index feebff67aa..043e2e06ec 100644
> > --- a/libavformat/mxfdec.c
> > +++ b/libavformat/mxfdec.c
> > @@ -2140,6 +2140,13 @@ finish_decoding_index:
> >  return ret;
> >  }
> >
> > +static int mxf_is_st_422(const UID *essence_container_ul) {
> > +static const uint8_t st_422_essence_container_ul[] = {
> > 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c
> > };
> > +
> > +return essence_container_ul &&
> > mxf_match_uid(*essence_container_ul, st_422_essence_container_ul,
> > +
> > sizeof(st_422_essence_container_ul));
> > +}
>
> Seems unnecessary to have a function for this

Ok. I was trying to avoid a long if() line but I am happy to move the
logic to mxf_parse_structural_metadata().

Would you like me to update the patch?

>
> > +
> >  static int mxf_is_intra_only(MXFDescriptor *descriptor)
> >  {
> >  return mxf_get_codec_ul(mxf_intra_only_essence_container_uls,
> > @@ -2892,6 +2899,24 @@ static int
> > mxf_parse_structural_metadata(MXFContext *mxf)
> >  av_log(mxf->fc, AV_LOG_INFO, "Unknown frame
> > layout type: %d\n", descriptor->frame_layout);
> >  }
> >
> > +if (mxf_is_st_422(essence_container_ul)) {
> > +switch ((*essence_container_ul)[14]) {
> > +case 2: /* Cn: Clip- wrapped Picture Element */
> > +case 3: /* I1: Interlaced Frame, 1 field/KLV */
> > +case 4: /* I2: Interlaced Frame, 2 fields/KLV */
> > +case 6: /* P1: Frame- wrapped Picture Element */
> > +st->avg_frame_rate = source_track->edit_rate;
> > +st->r_frame_rate = st->avg_frame_rate;
> > +break;
> > +case 5: /* F1: Field-wrapped Picture Element */
> > +st->avg_frame_rate = av_mul_q(av_make_q(2, 1),
> > source_track->edit_rate);
> > +st->r_frame_rate = st->avg_frame_rate;
> > +break;
> > +default:
> > +break;
> > +}
> > +}
>
> Looks OK, but we should have a test for this as well I think

Ok. imf/countdown/countdown-small.mxf contained ST 422-wrapped essence
and ffprobe reports `24 fps`. Should this patchset include the test?

>
> At some point we'll probably need to implement proper timecode
> support..

+1

Happy to collaborate.

>
> /Tomas
>
> ___
> 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 v1 1/2] avformat/imfdec: use CPL start timecode if available

2022-09-05 Thread Pierre-Anthony Lemieux
Just a quick ping.

Looking forward to feedback.

This patchset is intended to address https://trac.ffmpeg.org/ticket/9842.


On Mon, Aug 22, 2022 at 10:11 PM  wrote:
>
> From: Pierre-Anthony Lemieux 
>
> The IMF CPL contains an optional timecode start address. This patch reads the
> latter, if present, into the context's timecode metadata parameter.
> This addresses https://trac.ffmpeg.org/ticket/9842.
>
> ---
>  libavformat/imf.h |   2 +
>  libavformat/imf_cpl.c | 109 ++
>  libavformat/imfdec.c  |  11 +
>  3 files changed, 122 insertions(+)
>
> diff --git a/libavformat/imf.h b/libavformat/imf.h
> index 4271cd9582..70ed007312 100644
> --- a/libavformat/imf.h
> +++ b/libavformat/imf.h
> @@ -59,6 +59,7 @@
>  #include "libavformat/avio.h"
>  #include "libavutil/rational.h"
>  #include "libavutil/uuid.h"
> +#include "libavutil/timecode.h"
>  #include 
>
>  /**
> @@ -130,6 +131,7 @@ typedef struct FFIMFCPL {
>  AVUUID id_uuid;   /**< 
> CompositionPlaylist/Id element */
>  xmlChar *content_title_utf8; /**< 
> CompositionPlaylist/ContentTitle element */
>  AVRational edit_rate;/**< 
> CompositionPlaylist/EditRate element */
> +AVTimecode *tc;  /**< 
> CompositionPlaylist/CompositionTimecode element */
>  FFIMFMarkerVirtualTrack *main_markers_track; /**< Main Marker 
> Virtual Track */
>  FFIMFTrackFileVirtualTrack *main_image_2d_track; /**< Main Image Virtual 
> Track */
>  uint32_t main_audio_track_count; /**< Number of Main 
> Audio Virtual Tracks */
> diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c
> index 4acc20feee..1868d7db45 100644
> --- a/libavformat/imf_cpl.c
> +++ b/libavformat/imf_cpl.c
> @@ -116,6 +116,25 @@ int ff_imf_xml_read_uint32(xmlNodePtr element, uint32_t 
> *number)
>  return ret;
>  }
>
> +static int ff_imf_xml_read_boolean(xmlNodePtr element, int *value)
> +{
> +xmlChar *element_text = NULL;
> +int ret = 0;
> +
> +element_text = xmlNodeListGetString(element->doc, 
> element->xmlChildrenNode, 1);
> +
> +if (xmlStrcmp(element_text, "true") == 0 || xmlStrcmp(element_text, "1") 
> == 0)
> +*value = 1;
> +else if (xmlStrcmp(element_text, "false") == 0 || 
> xmlStrcmp(element_text, "0") == 0)
> +*value = 0;
> +else
> +ret = 1;
> +
> +xmlFree(element_text);
> +
> +return ret;
> +}
> +
>  static void imf_base_virtual_track_init(FFIMFBaseVirtualTrack *track)
>  {
>  memset(track->id_uuid, 0, sizeof(track->id_uuid));
> @@ -179,6 +198,90 @@ static int fill_content_title(xmlNodePtr cpl_element, 
> FFIMFCPL *cpl)
>  return 0;
>  }
>
> +static int digit_to_int(char digit)
> +{
> +if (digit >= '0' && digit <= '9')
> +return digit - '0';
> +return -1;
> +}
> +
> +/**
> + * Parses a string that conform to the TimecodeType used in IMF CPL and 
> defined
> + * in SMPTE ST 2067-3.
> + * @param[in] s string to parse
> + * @param[out] tc_comps pointer to an array of 4 integers where the parsed 
> HH,
> + *  MM, SS and FF fields of the timecode are returned.
> + * @return 0 on success, < 0 AVERROR code on error.
> + */
> +static int parse_cpl_tc_type(const char *s, int *tc_comps)
> +{
> +if (av_strnlen(s, 11) != 11)
> +return AVERROR(EINVAL);
> +
> +for (int i = 0; i < 4; i++) {
> +int hi;
> +int lo;
> +
> +hi = digit_to_int(s[i * 3]);
> +lo = digit_to_int(s[i * 3 + 1]);
> +
> +if (hi == -1 || lo == -1)
> +return AVERROR(EINVAL);
> +
> +tc_comps[i] = 10 * hi + lo;
> +}
> +
> +return 0;
> +}
> +
> +static int fill_timecode(xmlNodePtr cpl_element, FFIMFCPL *cpl)
> +{
> +xmlNodePtr tc_element = NULL;
> +xmlNodePtr element = NULL;
> +xmlChar *tc_str = NULL;
> +int df = 0;
> +int comps[4];
> +int ret = 0;
> +
> +tc_element = ff_imf_xml_get_child_element_by_name(cpl_element, 
> "CompositionTimecode");
> +if (!tc_element)
> +   return 0;
> +
> +element = ff_imf_xml_get_child_element_by_name(tc_element, 
> "TimecodeDropFrame");
> +if (!element) {
> +av_log(NULL, AV_LOG_ERROR, "CompositionTimecode element is missing\
> +a TimecodeDropFrame child element\n");
> +return AVERROR_INVALIDDATA;
> +}
> +
> +if (ff_imf_xml_read_boolean(element, &df)) {
> +av_log(NULL, AV_LOG_ERROR, "TimecodeDropFrame element is invalid\n");
> +return AVERROR_INVALIDDATA;
> +}
> +element = ff_imf_xml_get_child_element_by_name(tc_element, 
> "TimecodeStartAddress");
> +if (!element) {
> +av_log(NULL, AV_LOG_ERROR, "CompositionTimecode element is missing\
> +a TimecodeStartAddress child element\n");
> +return AVERROR_INVALIDDATA;
> +}
> +
> +tc_str = xmlNodeListGe

[FFmpeg-devel] [PATCH] avocdec/flac_parser: another fix

2022-09-05 Thread Paul B Mahol
Patch attached.
From 01e931d2c7f50727d8893268cdc3ae0dbe75c250 Mon Sep 17 00:00:00 2001
From: Paul B Mahol 
Date: Mon, 5 Sep 2022 20:16:13 +0200
Subject: [PATCH] avcodec/flac_parser: add missed opportunity to check crc

Fixes #9621

Signed-off-by: Paul B Mahol 
---
 libavcodec/flac_parser.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
index 11cd5540cf..dd70721696 100644
--- a/libavcodec/flac_parser.c
+++ b/libavcodec/flac_parser.c
@@ -455,7 +455,7 @@ static int check_header_mismatch(FLACParseContext  *fpc,
  intlog_level_offset)
 {
 FLACFrameInfo  *header_fi = &header->fi, *child_fi = &child->fi;
-int deduction, deduction_expected = 0, i;
+int check_crc, deduction, deduction_expected = 0, i;
 deduction = check_header_fi_mismatch(fpc, header_fi, child_fi,
  log_level_offset);
 /* Check sample and frame numbers. */
@@ -491,8 +491,15 @@ static int check_header_mismatch(FLACParseContext  *fpc,
"sample/frame number mismatch in adjacent frames\n");
 }
 
+if ((fpc->last_fi.frame_or_sample_num + 1 == header_fi->frame_or_sample_num) ||
+(fpc->last_fi.frame_or_sample_num + fpc->last_fi.blocksize == header_fi->frame_or_sample_num)) {
+check_crc = 0;
+} else {
+check_crc = !deduction && !deduction_expected;
+}
+
 /* If we have suspicious headers, check the CRC between them */
-if (deduction && !deduction_expected) {
+if (check_crc || (deduction && !deduction_expected)) {
 FLACHeaderMarker *curr;
 int read_len;
 uint8_t *buf;
-- 
2.37.2

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

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


[FFmpeg-devel] [PATCH v2 1/2] avformat/mxf: set stream frame rates for ST 422 essence containers

2022-09-05 Thread pal
From: Pierre-Anthony Lemieux 

The MXF demuxer does not currently set AVStream::avg_frame_rate and 
::r_frame_rate
when J2K essence is wrapped according to SMPTE ST 422.

---
 libavformat/mxfdec.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index feebff67aa..043e2e06ec 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2140,6 +2140,13 @@ finish_decoding_index:
 return ret;
 }
 
+static int mxf_is_st_422(const UID *essence_container_ul) {
+static const uint8_t st_422_essence_container_ul[] = { 
0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x0c };
+
+return essence_container_ul && mxf_match_uid(*essence_container_ul, 
st_422_essence_container_ul,
+ 
sizeof(st_422_essence_container_ul));
+}
+
 static int mxf_is_intra_only(MXFDescriptor *descriptor)
 {
 return mxf_get_codec_ul(mxf_intra_only_essence_container_uls,
@@ -2892,6 +2899,24 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
 av_log(mxf->fc, AV_LOG_INFO, "Unknown frame layout type: 
%d\n", descriptor->frame_layout);
 }
 
+if (mxf_is_st_422(essence_container_ul)) {
+switch ((*essence_container_ul)[14]) {
+case 2: /* Cn: Clip- wrapped Picture Element */
+case 3: /* I1: Interlaced Frame, 1 field/KLV */
+case 4: /* I2: Interlaced Frame, 2 fields/KLV */
+case 6: /* P1: Frame- wrapped Picture Element */
+st->avg_frame_rate = source_track->edit_rate;
+st->r_frame_rate = st->avg_frame_rate;
+break;
+case 5: /* F1: Field-wrapped Picture Element */
+st->avg_frame_rate = av_mul_q(av_make_q(2, 1), 
source_track->edit_rate);
+st->r_frame_rate = st->avg_frame_rate;
+break;
+default:
+break;
+}
+}
+
 if (st->codecpar->codec_id == AV_CODEC_ID_PRORES) {
 switch (descriptor->essence_codec_ul[14]) {
 case 1: st->codecpar->codec_tag = MKTAG('a','p','c','o'); 
break;
-- 
2.25.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 v2 2/2] fate/mxf: add JPEG 2000 test

2022-09-05 Thread pal
From: Pierre-Anthony Lemieux 

---
 tests/fate/mxf.mak   |  4 ++
 tests/ref/fate/mxf-probe-j2k | 78 
 2 files changed, 82 insertions(+)
 create mode 100644 tests/ref/fate/mxf-probe-j2k

diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak
index 3ab936b5de..38d1c2ef38 100644
--- a/tests/fate/mxf.mak
+++ b/tests/fate/mxf.mak
@@ -29,6 +29,10 @@ FATE_MXF_PROBE-$(call ENCDEC, DNXHD, MXF) += 
fate-mxf-probe-dnxhd
 fate-mxf-probe-dnxhd: SRC = $(TARGET_SAMPLES)/mxf/multiple_components.mxf
 fate-mxf-probe-dnxhd: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i "$(SRC)"
 
+FATE_MXF_PROBE-$(call ENCDEC, JPEG2000, MXF) += fate-mxf-probe-j2k
+fate-mxf-probe-j2k: SRC = $(TARGET_SAMPLES)/imf/countdown/countdown-small.mxf
+fate-mxf-probe-j2k: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i "$(SRC)"
+
 FATE_MXF_PROBE-$(call ENCDEC2, DVVIDEO, PCM_S16LE, MXF) += fate-mxf-probe-dv25
 fate-mxf-probe-dv25: SRC = $(TARGET_SAMPLES)/mxf/Avid-5.mxf
 fate-mxf-probe-dv25: CMD = run $(PROBE_FORMAT_STREAMS_COMMAND) -i "$(SRC)"
diff --git a/tests/ref/fate/mxf-probe-j2k b/tests/ref/fate/mxf-probe-j2k
new file mode 100644
index 00..2dbf2ac37d
--- /dev/null
+++ b/tests/ref/fate/mxf-probe-j2k
@@ -0,0 +1,78 @@
+[STREAM]
+index=0
+codec_name=jpeg2000
+profile=1798
+codec_type=video
+codec_tag_string=[0][0][0][0]
+codec_tag=0x
+width=640
+height=360
+coded_width=640
+coded_height=360
+closed_captions=0
+film_grain=0
+has_b_frames=0
+sample_aspect_ratio=1:1
+display_aspect_ratio=16:9
+pix_fmt=rgb48le
+level=-99
+color_range=unknown
+color_space=unknown
+color_transfer=bt709
+color_primaries=bt709
+chroma_location=unspecified
+field_order=progressive
+refs=1
+id=N/A
+r_frame_rate=24/1
+avg_frame_rate=24/1
+time_base=1/24
+start_pts=0
+start_time=0.00
+duration_ts=24
+duration=1.00
+bit_rate=N/A
+max_bit_rate=N/A
+bits_per_raw_sample=16
+nb_frames=N/A
+nb_read_frames=N/A
+nb_read_packets=N/A
+DISPOSITION:default=0
+DISPOSITION:dub=0
+DISPOSITION:original=0
+DISPOSITION:comment=0
+DISPOSITION:lyrics=0
+DISPOSITION:karaoke=0
+DISPOSITION:forced=0
+DISPOSITION:hearing_impaired=0
+DISPOSITION:visual_impaired=0
+DISPOSITION:clean_effects=0
+DISPOSITION:attached_pic=0
+DISPOSITION:timed_thumbnails=0
+DISPOSITION:captions=0
+DISPOSITION:descriptions=0
+DISPOSITION:metadata=0
+DISPOSITION:dependent=0
+DISPOSITION:still_image=0
+TAG:file_package_umid=0x060A2B340101010501010F20130035E05073878E4B2FB69D2369F25ADFC9
+TAG:file_package_name=File Package: SMPTE ST 422 / ST 2067-5 frame wrapping of 
JPEG 2000 codestreams
+TAG:track_name=Image Track
+[/STREAM]
+[FORMAT]
+format_name=mxf
+duration=1.00
+bit_rate=577792
+TAG:operational_pattern_ul=060e2b34.04010101.0d010201.01010100
+TAG:uid=f1994e51-a844-49e4-9459-1ddd622eb65d
+TAG:generation_uid=1be151ac-cc95-4314-b09f-7420eda9932b
+TAG:company_name=Sandflow Consulting LLC
+TAG:product_name=dcdm2imf
+TAG:product_version_num=0.0.0.0.0
+TAG:product_version=1.0-beta1
+TAG:product_uid=927fc4d1-89a3-4f88-88bb-d363ed33084a
+TAG:modification_date=2022-01-07T22:05:01.00Z
+TAG:toolkit_version_num=2.10.38.27240.1
+TAG:application_platform=win32
+TAG:material_package_umid=0x060A2B340101010501010F20130072BAF0557DA749308C14738BCD4FA116
+TAG:material_package_name=Material Package
+[/FORMAT]
-- 
2.25.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] avcodec/x86/flacdsp_init: Remove double '; '

2022-09-05 Thread Andreas Rheinhardt
Inside a function, the second ';' in ";;" is just a null statement,
but it is actually illegal outside of functions. Compilers
nevertheless accept it without warning, except when in -pedantic
mode when e.g. Clang emits a -Wextra-semi warning. Therefore
remove the unnecessary ';'.

Signed-off-by: Andreas Rheinhardt 
---
Will apply this tomorrow unless there are objections.

 libavcodec/x86/flacdsp_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/x86/flacdsp_init.c b/libavcodec/x86/flacdsp_init.c
index 48e3e7c55c..87daed7005 100644
--- a/libavcodec/x86/flacdsp_init.c
+++ b/libavcodec/x86/flacdsp_init.c
@@ -34,7 +34,7 @@ void ff_flac_decorrelate_ls_##fmt##_##opt(uint8_t **out, 
int32_t **in, int chann
 void ff_flac_decorrelate_rs_##fmt##_##opt(uint8_t **out, int32_t **in, int 
channels, \
   int len, int shift); 
  \
 void ff_flac_decorrelate_ms_##fmt##_##opt(uint8_t **out, int32_t **in, int 
channels, \
-  int len, int shift);
+  int len, int shift)
 
 #define DECORRELATE_IFUNCS(fmt, opt)   
  \
 void ff_flac_decorrelate_indep2_##fmt##_##opt(uint8_t **out, int32_t **in, int 
channels, \
-- 
2.34.1

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

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


Re: [FFmpeg-devel] [PATCH] lavc/pthread_frame: avoid leaving stale hwaccel state in worker threads

2022-09-05 Thread Michael Niedermayer
On Mon, Sep 05, 2022 at 07:42:17AM +0200, Steve Lhomme wrote:
> Hi Anton,
> 
> On 2022-09-02 22:59, Anton Khirnov wrote:
> > This state is not refcounted, so make sure it always has a well-defined
> > owner.
> > ---
> > Steve, could you please test this?
> 
> I can confirm it doesn't leak the context and plays correctly. It also
> doesn't crash ;)

just wanted to say a big thanks to both you and anton for working on
this !

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

Elect your leaders based on what they did after the last election, not
based on what they say before an election.



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

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


Re: [FFmpeg-devel] [RFC] d3dva security hw+threads

2022-09-05 Thread Soft Works



> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Anton Khirnov
> Sent: Monday, September 5, 2022 7:59 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] d3dva security hw+threads
> 
> Quoting Soft Works (2022-09-04 09:43:36)
> >
> >
> > > -Original Message-
> > > From: ffmpeg-devel  On Behalf Of
> > > Anton Khirnov
> > > Sent: Sunday, September 4, 2022 8:58 AM
> > > To: FFmpeg development discussions and patches  > > de...@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [RFC] d3dva security hw+threads
> > >
> > > Quoting Timo Rothenpieler (2022-09-02 01:46:59)
> > > > On 02.09.2022 01:32, Michael Niedermayer wrote:
> > > > > Hi all
> > > > >
> > > > > Theres a use after free issue in H.264 Decoding on d3d11va
> with
> > > multiple threads
> > > > > I dont have the hardware/platform nor do i know the hw
> decoding
> > > code so i made
> > > > > no attempt to fix this beyond asking others to ...
> > > >
> > > > hwaccel with multiple threads being broken is not exactly a
> > > surprise.
> > > > So we could just disable that, and always have it be one single
> > > thread?
> > >
> > > We are already disabling it in a way - the frame threading code
> > > ensures
> > > that threads run one at a time when hwaccel is being used.
> >
> >
> > Is there a described way to repro? I would try whether it still
> > happens after removing the lock code in hwcontext_d3d11va.c.
> > Those locks are not really needed and might prevent release
> > of dx11 resources in proper order. It's a guess only but
> > easy to try.
> 
> The problem is not in d3d11 locking code, but in the generic code
> that
> does not have clear enough ownership rules. Steve already tested that
> my
> patch from Friday fixes this.

Oh I see. I was missing the context. The patch makes sense to me.

Thanks,
sw


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

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


Re: [FFmpeg-devel] [PATCH] slicethread: Limit the automatic number of threads to 16

2022-09-05 Thread Martin Storsjö

On Mon, 5 Sep 2022, Martin Storsjö wrote:


This matches a similar cap on the number of automatic threads
in libavcodec/pthread_slice.c.

On systems with lots of cores, this does speed things up in
general (measurable on the level of the runtime of running
"make fate"), and fixes a couple fate failures in 32 bit mode on
such machines (where spawning a huge number of threads runs
out of address space).
---


On second thought - this observation that it speeds up "make -j$(nproc) 
fate" isn't surprising at all; as long as there are jobs to saturate all 
cores with the make level parallelism anyway, any threading within each 
job just adds extra overhead, nothing more.


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

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


Re: [FFmpeg-devel] [PATCH v1] avformat/imfdec: check if Asset/Id exists before trying to read it

2022-09-05 Thread Pierre-Anthony Lemieux
Ping.

This fixes Coverity issue #1512406.

On Thu, Aug 25, 2022 at 8:22 PM  wrote:
>
> From: Pierre-Anthony Lemieux 
>
> Fixes Coverity issue #1512406
>
> ---
>  libavformat/imfdec.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> index 5bbe7a53f8..fde91a6419 100644
> --- a/libavformat/imfdec.c
> +++ b/libavformat/imfdec.c
> @@ -233,7 +233,12 @@ static int 
> parse_imf_asset_map_from_xml_dom(AVFormatContext *s,
>
>  asset = &(asset_map->assets[asset_map->asset_count]);
>
> -if 
> (ff_imf_xml_read_uuid(ff_imf_xml_get_child_element_by_name(asset_element, 
> "Id"), asset->uuid)) {
> +if (!(node = ff_imf_xml_get_child_element_by_name(asset_element, 
> "Id"))) {
> +av_log(s, AV_LOG_ERROR, "Unable to parse asset map XML - missing 
> Id node\n");
> +return AVERROR_INVALIDDATA;
> +}
> +
> +if (ff_imf_xml_read_uuid(node, asset->uuid)) {
>  av_log(s, AV_LOG_ERROR, "Could not parse UUID from asset in 
> asset map.\n");
>  return AVERROR_INVALIDDATA;
>  }
> --
> 2.25.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 v4 0/4] lavf/dashdec: Multithreaded DASH initialization

2022-09-05 Thread Lukas Fellechner
Initializing DASH streams is currently slow, because each individual
stream is opened and probed sequentially. With DASH streams often
having somewhere between 10-20 streams, this can easily take up to
half a minute on slow connections.

This patch adds an "init_threads" option, specifying the max number
of threads to use. Multiple worker threads are spun up to massively
bring down init times.
In-Reply-To: 
trinity-36a68f08-f239-4450-b893-af6bfa783181-1661031307501@3c-app-gmx-bs35


___
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 3/4] lavf/dashdec: Prevent cross-thread avio_opts modification

2022-09-05 Thread Lukas Fellechner
open_url modifies the shared avio_opts dict (update cookies).
This can cause problems during multithreaded initialization.
To prevent this, I take a copy of avio_opts, use that in open_url,
and copy the updated dict back afterwards.
---
 libavformat/dashdec.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 0532e2c918..19e657d836 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -156,6 +156,11 @@ typedef struct DASHContext {

 int init_threads;

+#if HAVE_THREADS
+/* Set during parallel initialization, to allow locking of avio_opts */
+pthread_mutex_t *init_mutex;
+#endif
+
 /* Flags for init section*/
 int is_init_section_common_video;
 int is_init_section_common_audio;
@@ -1699,7 +1704,32 @@ static int open_input(DASHContext *c, struct 
representation *pls, struct fragmen
 ff_make_absolute_url(url, c->max_url_size, c->base_url, seg->url);
 av_log(pls->parent, AV_LOG_VERBOSE, "DASH request for url '%s', offset 
%"PRId64"\n",
url, seg->url_offset);
-ret = open_url(pls->parent, &pls->input, url, &c->avio_opts, opts, NULL);
+
+AVDictionary *avio_opts = c->avio_opts;
+
+#if HAVE_THREADS
+// If we are doing parallel initialization, take a snapshot of the 
avio_opts,
+// and copy the modified dictionary ("cookies" updated) back, after the 
url is opened.
+if (c->init_mutex) {
+pthread_mutex_lock(c->init_mutex);
+avio_opts = NULL;
+ret = av_dict_copy(&avio_opts, c->avio_opts, 0);
+pthread_mutex_unlock(c->init_mutex);
+if (ret < 0)
+goto cleanup;
+}
+#endif
+
+ret = open_url(pls->parent, &pls->input, url, &avio_opts, opts, NULL);
+
+#if HAVE_THREADS
+if (c->init_mutex) {
+pthread_mutex_lock(c->init_mutex);
+av_dict_free(&c->avio_opts);
+c->avio_opts = avio_opts;
+pthread_mutex_unlock(c->init_mutex);
+}
+#endif

 cleanup:
 av_free(url);
@@ -2192,7 +,7 @@ static int init_streams_multithreaded(AVFormatContext *s, 
int nstreams, int thre
 if (!avpriv_slicethread_create(&slice_thread, (void*)work_pool, 
&thread_worker, NULL, threads)) {
 av_free(work_pool);
 return AVERROR(ENOMEM);
-}
+}

 // alloc mutex and conditions
 c->init_mutex = create_mutex();
--
2.28.0.windows.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 v4 2/4] lavf/dashdec: Multithreaded DASH initialization

2022-09-05 Thread Lukas Fellechner
This patch adds an "init_threads" option, specifying the max
number of threads to use. Multiple worker threads are spun up
to massively bring down init times.
---
 libavformat/dashdec.c | 286 +-
 1 file changed, 285 insertions(+), 1 deletion(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index e82da45e43..0532e2c918 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -24,6 +24,8 @@
 #include "libavutil/opt.h"
 #include "libavutil/time.h"
 #include "libavutil/parseutils.h"
+#include "libavutil/thread.h"
+#include "libavutil/slicethread.h"
 #include "internal.h"
 #include "avio_internal.h"
 #include "dash.h"
@@ -152,6 +154,8 @@ typedef struct DASHContext {
 int max_url_size;
 char *cenc_decryption_key;

+int init_threads;
+
 /* Flags for init section*/
 int is_init_section_common_video;
 int is_init_section_common_audio;
@@ -2033,6 +2037,265 @@ static void move_metadata(AVStream *st, const char 
*key, char **value)
 }
 }

+#if HAVE_THREADS
+
+typedef struct WorkPoolData
+{
+AVFormatContext *ctx;
+struct representation *pls;
+struct representation *common_pls;
+pthread_mutex_t *common_mutex;
+pthread_cond_t *common_condition;
+int is_common;
+int is_started;
+int result;
+} WorkPoolData;
+
+static void thread_worker(void *priv, int jobnr, int threadnr, int nb_jobs, 
int nb_threads)
+{
+WorkPoolData *work_pool = (WorkPoolData*)priv;
+WorkPoolData *data = work_pool + jobnr;
+int ret;
+
+// if we are common section provider, init and signal
+if (data->is_common) {
+data->pls->parent = data->ctx;
+ret = update_init_section(data->pls);
+if (ret < 0) {
+pthread_cond_signal(data->common_condition);
+goto end;
+}
+else
+ret = AVERROR(pthread_cond_signal(data->common_condition));
+}
+
+// if we depend on common section provider, wait for signal and copy
+if (data->common_pls) {
+ret = AVERROR(pthread_cond_wait(data->common_condition, 
data->common_mutex));
+if (ret < 0)
+goto end;
+
+if (!data->common_pls->init_sec_buf) {
+goto end;
+ret = AVERROR(EFAULT);
+}
+
+ret = copy_init_section(data->pls, data->common_pls);
+if (ret < 0)
+goto end;
+}
+
+ret = begin_open_demux_for_component(data->ctx, data->pls);
+if (ret < 0)
+goto end;
+
+end:
+data->result = ret;
+}
+
+static void create_work_pool_data(AVFormatContext *ctx, int *stream_index,
+struct representation **streams, int num_streams, int 
is_init_section_common,
+WorkPoolData *work_pool, pthread_mutex_t* common_mutex,
+pthread_cond_t* common_condition)
+{
+work_pool += *stream_index;
+
+for (int i = 0; i < num_streams; i++) {
+work_pool->ctx = ctx;
+work_pool->pls = streams[i];
+work_pool->pls->stream_index = *stream_index;
+work_pool->common_condition = common_condition;
+work_pool->common_mutex = common_mutex;
+work_pool->result = -1;
+
+if (is_init_section_common) {
+if (i == 0)
+work_pool->is_common = 1;
+else
+work_pool->common_pls = streams[0];
+}
+
+work_pool++;
+*stream_index = *stream_index + 1;
+}
+}
+
+static pthread_mutex_t* create_mutex()
+{
+pthread_mutex_t* mutex = 
(pthread_mutex_t*)av_malloc(sizeof(pthread_mutex_t));
+if (!mutex)
+return NULL;
+
+if (pthread_mutex_init(mutex, NULL)) {
+av_free(mutex);
+return NULL;
+}
+
+return mutex;
+}
+
+static int free_mutex(pthread_mutex_t **mutex)
+{
+int ret = 0;
+if (*mutex) {
+ret = pthread_mutex_destroy(*mutex);
+av_free(*mutex);
+*mutex = NULL;
+}
+return ret;
+}
+
+static pthread_cond_t* create_cond()
+{
+pthread_cond_t* cond = (pthread_cond_t*)av_malloc(sizeof(pthread_cond_t));
+if (!cond)
+return NULL;
+
+if (pthread_cond_init(cond, NULL)) {
+av_free(cond);
+return NULL;
+}
+
+return cond;
+}
+
+static int free_cond(pthread_cond_t **cond)
+{
+int ret = 0;
+if (*cond) {
+ret = pthread_cond_destroy(*cond);
+av_free(*cond);
+*cond = NULL;
+}
+return ret;
+}
+
+static int init_streams_multithreaded(AVFormatContext *s, int nstreams, int 
threads)
+{
+DASHContext *c = s->priv_data;
+int ret = 0;
+int stream_index = 0;
+AVSliceThread *slice_thread;
+
+// we need to cleanup even in case of errors,
+// so we need to store results of run and cleanup phase
+int initResult = 0;
+int runResult = 0;
+int cleanupResult = 0;
+
+// alloc data
+WorkPoolData *work_pool = (WorkPoolData*)av_mallocz(
+sizeof(WorkPoolData) * nstreams);
+if (!work_pool)
+return AVERROR(ENOMEM);
+
+if (

[FFmpeg-devel] [PATCH v4 1/4] lavf/dashdec: Prepare DASH decoder for multithreading

2022-09-05 Thread Lukas Fellechner
For adding multithreading to the DASH decoder initialization,
the open_demux_for_component() method must be split up into two parts:

begin_open_demux_for_component(): Opens the stream and does probing
and format detection. This can be run in parallel.

end_open_demux_for_component(): Creates the AVStreams and adds
them to the common parent AVFormatContext. This method must always be
run synchronously, after all threads are finished.
---
 libavformat/dashdec.c | 42 ++
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 63bf7e96a5..e82da45e43 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1918,10 +1918,9 @@ fail:
 return ret;
 }

-static int open_demux_for_component(AVFormatContext *s, struct representation 
*pls)
+static int begin_open_demux_for_component(AVFormatContext *s, struct 
representation *pls)
 {
 int ret = 0;
-int i;

 pls->parent = s;
 pls->cur_seq_no  = calc_cur_seg_no(s, pls);
@@ -1931,9 +1930,15 @@ static int open_demux_for_component(AVFormatContext *s, 
struct representation *p
 }

 ret = reopen_demux_for_component(s, pls);
-if (ret < 0) {
-goto fail;
-}
+
+return ret;
+}
+
+static int end_open_demux_for_component(AVFormatContext *s, struct 
representation *pls)
+{
+int ret = 0;
+int i;
+
 for (i = 0; i < pls->ctx->nb_streams; i++) {
 AVStream *st = avformat_new_stream(s, NULL);
 AVStream *ist = pls->ctx->streams[i];
@@ -1965,6 +1970,19 @@ fail:
 return ret;
 }

+static int open_demux_for_component(AVFormatContext* s, struct representation* 
pls)
+{
+int ret = 0;
+
+ret = begin_open_demux_for_component(s, pls);
+if (ret < 0)
+return ret;
+
+ret = end_open_demux_for_component(s, pls);
+
+return ret;
+}
+
 static int is_common_init_section_exist(struct representation **pls, int n_pls)
 {
 struct fragment *first_init_section = pls[0]->init_section;
@@ -2040,9 +2058,15 @@ static int dash_read_header(AVFormatContext *s)
 av_dict_set(&c->avio_opts, "seekable", "0", 0);
 }

-if(c->n_videos)
+if (c->n_videos)
 c->is_init_section_common_video = 
is_common_init_section_exist(c->videos, c->n_videos);

+if (c->n_audios)
+c->is_init_section_common_audio = 
is_common_init_section_exist(c->audios, c->n_audios);
+
+if (c->n_subtitles)
+c->is_init_section_common_subtitle = 
is_common_init_section_exist(c->subtitles, c->n_subtitles);
+
 /* Open the demuxer for video and audio components if available */
 for (i = 0; i < c->n_videos; i++) {
 rep = c->videos[i];
@@ -2059,9 +2083,6 @@ static int dash_read_header(AVFormatContext *s)
 ++stream_index;
 }

-if(c->n_audios)
-c->is_init_section_common_audio = 
is_common_init_section_exist(c->audios, c->n_audios);
-
 for (i = 0; i < c->n_audios; i++) {
 rep = c->audios[i];
 if (i > 0 && c->is_init_section_common_audio) {
@@ -2077,9 +2098,6 @@ static int dash_read_header(AVFormatContext *s)
 ++stream_index;
 }

-if (c->n_subtitles)
-c->is_init_section_common_subtitle = 
is_common_init_section_exist(c->subtitles, c->n_subtitles);
-
 for (i = 0; i < c->n_subtitles; i++) {
 rep = c->subtitles[i];
 if (i > 0 && c->is_init_section_common_subtitle) {
--
2.28.0.windows.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 v4 4/4] lavf/dashdec: Fix indentation after adding multithreading

2022-09-05 Thread Lukas Fellechner
Whitespace change only. No functional changes.
---
 libavformat/dashdec.c | 74 +--
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 19e657d836..22f727da3b 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -2377,54 +2377,54 @@ static int dash_read_header(AVFormatContext *s)
 }
 else
 {
-/* Open the demuxer for video and audio components if available */
-for (i = 0; i < c->n_videos; i++) {
-rep = c->videos[i];
-if (i > 0 && c->is_init_section_common_video) {
-ret = copy_init_section(rep, c->videos[0]);
-if (ret < 0)
+/* Open the demuxer for video and audio components if available */
+for (i = 0; i < c->n_videos; i++) {
+rep = c->videos[i];
+if (i > 0 && c->is_init_section_common_video) {
+ret = copy_init_section(rep, c->videos[0]);
+if (ret < 0)
+return ret;
+}
+ret = open_demux_for_component(s, rep);
+
+if (ret)
 return ret;
+rep->stream_index = stream_index;
+++stream_index;
 }
-ret = open_demux_for_component(s, rep);

-if (ret)
-return ret;
-rep->stream_index = stream_index;
-++stream_index;
-}
+for (i = 0; i < c->n_audios; i++) {
+rep = c->audios[i];
+if (i > 0 && c->is_init_section_common_audio) {
+ret = copy_init_section(rep, c->audios[0]);
+if (ret < 0)
+return ret;
+}
+ret = open_demux_for_component(s, rep);

-for (i = 0; i < c->n_audios; i++) {
-rep = c->audios[i];
-if (i > 0 && c->is_init_section_common_audio) {
-ret = copy_init_section(rep, c->audios[0]);
-if (ret < 0)
+if (ret)
 return ret;
+rep->stream_index = stream_index;
+++stream_index;
 }
-ret = open_demux_for_component(s, rep);

-if (ret)
-return ret;
-rep->stream_index = stream_index;
-++stream_index;
-}
+for (i = 0; i < c->n_subtitles; i++) {
+rep = c->subtitles[i];
+if (i > 0 && c->is_init_section_common_subtitle) {
+ret = copy_init_section(rep, c->subtitles[0]);
+if (ret < 0)
+return ret;
+}
+ret = open_demux_for_component(s, rep);

-for (i = 0; i < c->n_subtitles; i++) {
-rep = c->subtitles[i];
-if (i > 0 && c->is_init_section_common_subtitle) {
-ret = copy_init_section(rep, c->subtitles[0]);
-if (ret < 0)
+if (ret)
 return ret;
+rep->stream_index = stream_index;
+++stream_index;
 }
-ret = open_demux_for_component(s, rep);

-if (ret)
-return ret;
-rep->stream_index = stream_index;
-++stream_index;
-}
-
-if (!stream_index)
-return AVERROR_INVALIDDATA;
+if (!stream_index)
+return AVERROR_INVALIDDATA;
 }

 /* Create a program */
--
2.28.0.windows.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] ref/fate/ffprobe_xsd: Change ref file

2022-09-05 Thread Andreas Rheinhardt
Forgotten in 5c16df1b92c519238e10664eeab3adb3b9016edd,
because neither I nor patchwork ran fate with xmllint.

Signed-off-by: Andreas Rheinhardt 
---
Will apply this soon.

 tests/ref/fate/ffprobe_xsd | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/ref/fate/ffprobe_xsd b/tests/ref/fate/ffprobe_xsd
index f1fac9a87c..412fd9a180 100644
--- a/tests/ref/fate/ffprobe_xsd
+++ b/tests/ref/fate/ffprobe_xsd
@@ -32,7 +32,7 @@
 
 
 
-
+
 
 
 
-- 
2.34.1

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

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


Re: [FFmpeg-devel] [PATCH 1/9] avcodec/dvdata: Order code table by codes

2022-09-05 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Right now, it is nearly ordered by "left codes in the tree first";
> the only exception is the escape value which has been put at the
> end. This commit moves it to the place it should have according
> to the above order. This is in preparation for further commits.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/dv_tablegen.h |  2 +-
>  libavcodec/dvdata.c  | 12 
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/dv_tablegen.h b/libavcodec/dv_tablegen.h
> index 941b5572be..0dcfffc140 100644
> --- a/libavcodec/dv_tablegen.h
> +++ b/libavcodec/dv_tablegen.h
> @@ -51,7 +51,7 @@ static struct dv_vlc_pair 
> dv_vlc_map[DV_VLC_MAP_RUN_SIZE][DV_VLC_MAP_LEV_SIZE];
>  static av_cold void dv_vlc_map_tableinit(void)
>  {
>  int i, j;
> -for (i = 0; i < NB_DV_VLC - 1; i++) {
> +for (int i = 0; i < NB_DV_VLC; i++) {
>  if (ff_dv_vlc_run[i] >= DV_VLC_MAP_RUN_SIZE)
>  continue;
>  #if CONFIG_SMALL
> diff --git a/libavcodec/dvdata.c b/libavcodec/dvdata.c
> index 231569a328..1e48db591d 100644
> --- a/libavcodec/dvdata.c
> +++ b/libavcodec/dvdata.c
> @@ -75,7 +75,7 @@ const uint8_t ff_dv_quant_offset[4] = { 6, 3, 0, 1 };
>   * when building misc. tables. E.g. (1, 0) can be either 0x7cf or 0x1f82.
>   */
>  const uint16_t ff_dv_vlc_bits[NB_DV_VLC] = {
> -0x, 0x0002, 0x0007, 0x0008, 0x0009, 0x0014, 0x0015, 0x0016,
> +0x, 0x0002, 0x0006, 0x0007, 0x0008, 0x0009, 0x0014, 0x0015, 0x0016,
>  0x0017, 0x0030, 0x0031, 0x0032, 0x0033, 0x0068, 0x0069, 0x006a,
>  0x006b, 0x006c, 0x006d, 0x006e, 0x006f, 0x00e0, 0x00e1, 0x00e2,
>  0x00e3, 0x00e4, 0x00e5, 0x00e6, 0x00e7, 0x00e8, 0x00e9, 0x00ea,
> @@ -126,11 +126,10 @@ const uint16_t ff_dv_vlc_bits[NB_DV_VLC] = {
>  0x7fe8, 0x7fe9, 0x7fea, 0x7feb, 0x7fec, 0x7fed, 0x7fee, 0x7fef,
>  0x7ff0, 0x7ff1, 0x7ff2, 0x7ff3, 0x7ff4, 0x7ff5, 0x7ff6, 0x7ff7,
>  0x7ff8, 0x7ff9, 0x7ffa, 0x7ffb, 0x7ffc, 0x7ffd, 0x7ffe, 0x7fff,
> -0x0006,
>  };
>  
>  const uint8_t ff_dv_vlc_len[NB_DV_VLC] = {
> - 2,  3,  4,  4,  4,  5,  5,  5,
> + 2,  3,  4,  4,  4,  4,  5,  5,  5,
>   5,  6,  6,  6,  6,  7,  7,  7,
>   7,  7,  7,  7,  7,  8,  8,  8,
>   8,  8,  8,  8,  8,  8,  8,  8,
> @@ -181,11 +180,10 @@ const uint8_t ff_dv_vlc_len[NB_DV_VLC] = {
>  15, 15, 15, 15, 15, 15, 15, 15,
>  15, 15, 15, 15, 15, 15, 15, 15,
>  15, 15, 15, 15, 15, 15, 15, 15,
> - 4,
>  };
>  
>  const uint8_t ff_dv_vlc_run[NB_DV_VLC] = {
> - 0,  0,  1,  0,  0,  2,  1,  0,
> + 0,  0, 127, 1,  0,  0,  2,  1,  0,
>   0,  3,  4,  0,  0,  5,  6,  2,
>   1,  1,  0,  0,  0,  7,  8,  9,
>  10,  3,  4,  2,  1,  1,  1,  0,
> @@ -236,11 +234,10 @@ const uint8_t ff_dv_vlc_run[NB_DV_VLC] = {
>   0,  0,  0,  0,  0,  0,  0,  0,
>   0,  0,  0,  0,  0,  0,  0,  0,
>   0,  0,  0,  0,  0,  0,  0,  0,
> -   127,
>  };
>  
>  const uint8_t ff_dv_vlc_level[NB_DV_VLC] = {
> - 1,   2,   1,   3,   4,   1,   2,   5,
> + 1,   2,   0,   1,   3,   4,   1,   2,   5,
>   6,   1,   1,   7,   8,   1,   1,   2,
>   3,   4,   9,  10,  11,   1,   1,   1,
>   1,   2,   2,   3,   5,   6,   7,  12,
> @@ -291,5 +288,4 @@ const uint8_t ff_dv_vlc_level[NB_DV_VLC] = {
> 232, 233, 234, 235, 236, 237, 238, 239,
> 240, 241, 242, 243, 244, 245, 246, 247,
> 248, 249, 250, 251, 252, 253, 254, 255,
> - 0,
>  };

Will apply this tomorrow unless there are objections.

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

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


Re: [FFmpeg-devel] [PATCH] avcodec/ffv1: Only allocate ThreadFrames for the decoder

2022-09-05 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> The FFV1 decoder only uses the last frame's data to conceal
> errors. The encoder does not have this problem and therefore
> only uses the current frame and none of the ThreadFrames.
> So only allocate them for the decoder.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/ffv1.c| 13 -
>  libavcodec/ffv1dec.c | 23 ++-
>  2 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/libavcodec/ffv1.c b/libavcodec/ffv1.c
> index c8781cdaaa..b6204740ed 100644
> --- a/libavcodec/ffv1.c
> +++ b/libavcodec/ffv1.c
> @@ -43,11 +43,6 @@ av_cold int ff_ffv1_common_init(AVCodecContext *avctx)
>  s->avctx = avctx;
>  s->flags = avctx->flags;
>  
> -s->picture.f = av_frame_alloc();
> -s->last_picture.f = av_frame_alloc();
> -if (!s->picture.f || !s->last_picture.f)
> -return AVERROR(ENOMEM);
> -
>  s->width  = avctx->width;
>  s->height = avctx->height;
>  
> @@ -198,14 +193,6 @@ av_cold int ff_ffv1_close(AVCodecContext *avctx)
>  FFV1Context *s = avctx->priv_data;
>  int i, j;
>  
> -if (s->picture.f)
> -ff_thread_release_ext_buffer(avctx, &s->picture);
> -av_frame_free(&s->picture.f);
> -
> -if (s->last_picture.f)
> -ff_thread_release_ext_buffer(avctx, &s->last_picture);
> -av_frame_free(&s->last_picture.f);
> -
>  for (j = 0; j < s->max_slice_count; j++) {
>  FFV1Context *fs = s->slice_context[j];
>  for (i = 0; i < s->plane_count; i++) {
> diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
> index 794c58cc40..d4bc60a7da 100644
> --- a/libavcodec/ffv1dec.c
> +++ b/libavcodec/ffv1dec.c
> @@ -823,6 +823,11 @@ static av_cold int decode_init(AVCodecContext *avctx)
>  if ((ret = ff_ffv1_common_init(avctx)) < 0)
>  return ret;
>  
> +f->picture.f  = av_frame_alloc();
> +f->last_picture.f = av_frame_alloc();
> +if (!f->picture.f || !f->last_picture.f)
> +return AVERROR(ENOMEM);
> +
>  if (avctx->extradata_size > 0 && (ret = read_extra_header(f)) < 0)
>  return ret;
>  
> @@ -1068,6 +1073,22 @@ static int update_thread_context(AVCodecContext *dst, 
> const AVCodecContext *src)
>  }
>  #endif
>  
> +static av_cold int ffv1_decode_close(AVCodecContext *avctx)
> +{
> +FFV1Context *const s = avctx->priv_data;
> +
> +if (s->picture.f) {
> +ff_thread_release_ext_buffer(avctx, &s->picture);
> +av_frame_free(&s->picture.f);
> +}
> +
> +if (s->last_picture.f) {
> +ff_thread_release_ext_buffer(avctx, &s->last_picture);
> +av_frame_free(&s->last_picture.f);
> +}
> +return ff_ffv1_close(avctx);
> +}
> +
>  const FFCodec ff_ffv1_decoder = {
>  .p.name = "ffv1",
>  CODEC_LONG_NAME("FFmpeg video codec #1"),
> @@ -1075,7 +1096,7 @@ const FFCodec ff_ffv1_decoder = {
>  .p.id   = AV_CODEC_ID_FFV1,
>  .priv_data_size = sizeof(FFV1Context),
>  .init   = decode_init,
> -.close  = ff_ffv1_close,
> +.close  = ffv1_decode_close,
>  FF_CODEC_DECODE_CB(decode_frame),
>  UPDATE_THREAD_CONTEXT(update_thread_context),
>  .p.capabilities = AV_CODEC_CAP_DR1 /*| AV_CODEC_CAP_DRAW_HORIZ_BAND*/ |

Will apply this tomorrow unless there are objections.

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

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


Re: [FFmpeg-devel] [PATCH v3] libavcodec/cbs_av1: Add size check before parse obu

2022-09-05 Thread Chen, Wenbin
> > cbs_av1_write_unit() check pbc size after parsing obu frame, and return
> > AVERROR(ENOSPC) if pbc is small. pbc will be reallocated and this obu
> > frame will be parsed again, but this may cause error because
> > CodedBitstreamAV1Context has already been updated, for example
> > ref_order_hint is updated and will not match the same obu frame. Now
> size
> > check is added before parsing obu frame to avoid this error.
> >
> > Signed-off-by: Wenbin Chen 
> > ---
> >  libavcodec/cbs_av1.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> > index 154d9156cf..9c51a8c7c8 100644
> > --- a/libavcodec/cbs_av1.c
> > +++ b/libavcodec/cbs_av1.c
> > @@ -1075,6 +1075,9 @@ static int
> > cbs_av1_write_obu(CodedBitstreamContext *ctx,
> >  put_bits32(pbc, 0);
> >  }
> >
> > +if (8 * (unit->data_size + obu->obu_size) > put_bits_left(pbc))
> > +return AVERROR(ENOSPC);
> > +
> >  td = NULL;
> >  start_pos = put_bits_count(pbc);
> >
> > --
> > 2.32.0
> 
> Ping

Ping 

> 
> >
> > ___
> > 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 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/6] libavcodec/qsvenc: Add gop_size reset support to qsvenc

2022-09-05 Thread Xiang, Haihao
On Thu, 2022-08-18 at 14:59 +0800, Wenbin Chen wrote:
> Signed-off-by: Wenbin Chen 
> ---
>  doc/encoders.texi   |  3 +++
>  libavcodec/qsvenc.c | 18 +-
>  libavcodec/qsvenc.h |  2 ++
>  3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index 69fa46f3ea..9fb63856b1 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -3348,6 +3348,9 @@ Change these value to reset qsv codec's qp
> configuration.
>  @item @var{max_frame_size}
>  Supported in h264_qsv and hevc_qsv.
>  Change this value to reset qsv codec's MaxFrameSize configuration.
> +
> +@item @var{gop_size}
> +Change this value to reset qsv codec's gop configuration.
>  @end table
>  
>  @subsection H264 options
> diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> index c911b81e7d..b3820e4fe0 100644
> --- a/libavcodec/qsvenc.c
> +++ b/libavcodec/qsvenc.c
> @@ -635,6 +635,7 @@ static int init_video_param(AVCodecContext *avctx,
> QSVEncContext *q)
>  q->param.mfx.CodecProfile   = q->profile;
>  q->param.mfx.TargetUsage= avctx->compression_level;
>  q->param.mfx.GopPicSize = FFMAX(0, avctx->gop_size);
> +q->old_gop_size = avctx->gop_size;
>  q->param.mfx.GopRefDist = FFMAX(-1, avctx->max_b_frames) + 1;
>  q->param.mfx.GopOptFlag = avctx->flags & AV_CODEC_FLAG_CLOSED_GOP
> ?
>MFX_GOP_CLOSED : MFX_GOP_STRICT;
> @@ -1692,16 +1693,31 @@ static int update_max_frame_size(AVCodecContext
> *avctx, QSVEncContext *q)
>  return updated;
>  }
>  
> +static int update_gop_size(AVCodecContext *avctx, QSVEncContext *q)
> +{
> +int updated = 0;
> +UPDATE_PARAM(q->old_gop_size, avctx->gop_size);
> +if (!updated)
> +return 0;
> +
> +q->param.mfx.GopPicSize = FFMAX(0, avctx->gop_size);
> +av_log(avctx, AV_LOG_DEBUG, "reset GopPicSize to %d\n",
> +   q->param.mfx.GopPicSize);
> +
> +return updated;
> +}
> +
>  static int update_parameters(AVCodecContext *avctx, QSVEncContext *q,
>   const AVFrame *frame)
>  {
>  int needReset = 0, ret = 0;
>  
> -if (!frame)
> +if (!frame || avctx->codec_id == AV_CODEC_ID_MJPEG)
>  return 0;

Better to fix mjpeg_qsv in a separate patch if mjpeg_qsv is not able to 
resetany parameter. 

Thanks
Haihao

>  
>  needReset = update_qp(avctx, q);
>  needReset |= update_max_frame_size(avctx, q);
> +needReset |= update_gop_size(avctx, q);
>  if (!needReset)
>  return 0;
>  
> diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> index c452c5b806..fdedae28dd 100644
> --- a/libavcodec/qsvenc.h
> +++ b/libavcodec/qsvenc.h
> @@ -237,6 +237,8 @@ typedef struct QSVEncContext {
>  float old_b_quant_offset;
>  // This is used for max_frame_size reset
>  int old_max_frame_size;
> +// This is used for gop reset
> +int old_gop_size;
>  } QSVEncContext;
>  
>  int ff_qsv_enc_init(AVCodecContext *avctx, QSVEncContext *q);
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


[FFmpeg-devel] [PATCH v2 1/2] lavu/hwcontext_qsv: add support for AV_PIX_FMT_VUYX on Linux

2022-09-05 Thread Xiang, Haihao
From: Haihao Xiang 

AV_PIX_FMT_VUYX is used for 8bit 4:4:4 content in FFmpeg VAAPI, so
AV_PIX_FMT_VUYX should be used for 8bit 4:4:4 content in FFmpeg QSV too
because QSV is based on VAAPI on Linux. However the SDK only declares
support for AYUV and does nothing with the alpha, so this commit fudged
a mapping between AV_PIX_FMT_VUYX and MFX_FOURCC_AYUV.
---
 libavutil/hwcontext_qsv.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
index 510f422562..9fa0dfa1c0 100644
--- a/libavutil/hwcontext_qsv.c
+++ b/libavutil/hwcontext_qsv.c
@@ -119,6 +119,10 @@ static const struct {
MFX_FOURCC_YUY2 },
 { AV_PIX_FMT_Y210,
MFX_FOURCC_Y210 },
+// VUYX is used for VAAPI child device,
+// the SDK only delares support for AYUV
+{ AV_PIX_FMT_VUYX,
+   MFX_FOURCC_AYUV },
 #endif
 };
 
@@ -1502,6 +1506,14 @@ static int map_frame_to_surface(const AVFrame *frame, 
mfxFrameSurface1 *surface)
 surface->Data.U16 = (mfxU16 *)frame->data[0] + 1;
 surface->Data.V16 = (mfxU16 *)frame->data[0] + 3;
 break;
+case AV_PIX_FMT_VUYX:
+surface->Data.V = frame->data[0];
+surface->Data.U = frame->data[0] + 1;
+surface->Data.Y = frame->data[0] + 2;
+// Only set Data.A to a valid address, the SDK doesn't
+// use the value from the frame.
+surface->Data.A = frame->data[0] + 3;
+break;
 #endif
 default:
 return MFX_ERR_UNSUPPORTED;
-- 
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 v2 2/2] lavc/qsv: Add support for decoding & encoding 8bit 4:4:4 content

2022-09-05 Thread Xiang, Haihao
From: Haihao Xiang 

AV_PIX_FMT_VUYX is used in FFmpeg and MFX_FOURCC_AYUV is used in the SDK
---
 libavcodec/qsv.c | 14 ++
 libavcodec/qsvdec.c  |  2 ++
 libavcodec/qsvenc_hevc.c |  1 +
 libavcodec/qsvenc_vp9.c  |  1 +
 4 files changed, 18 insertions(+)

diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
index 3449789a2c..51aac16695 100644
--- a/libavcodec/qsv.c
+++ b/libavcodec/qsv.c
@@ -211,6 +211,7 @@ enum AVPixelFormat ff_qsv_map_fourcc(uint32_t fourcc)
 #if CONFIG_VAAPI
 case MFX_FOURCC_YUY2: return AV_PIX_FMT_YUYV422;
 case MFX_FOURCC_Y210: return AV_PIX_FMT_Y210;
+case MFX_FOURCC_AYUV: return AV_PIX_FMT_VUYX;
 #endif
 }
 return AV_PIX_FMT_NONE;
@@ -243,6 +244,9 @@ int ff_qsv_map_pixfmt(enum AVPixelFormat format, uint32_t 
*fourcc)
 case AV_PIX_FMT_Y210:
 *fourcc = MFX_FOURCC_Y210;
 return AV_PIX_FMT_Y210;
+case AV_PIX_FMT_VUYX:
+*fourcc = MFX_FOURCC_AYUV;
+return AV_PIX_FMT_VUYX;
 #endif
 default:
 return AVERROR(ENOSYS);
@@ -277,6 +281,16 @@ int ff_qsv_map_frame_to_surface(const AVFrame *frame, 
mfxFrameSurface1 *surface)
 surface->Data.U16 = (mfxU16 *)frame->data[0] + 1;
 surface->Data.V16 = (mfxU16 *)frame->data[0] + 3;
 break;
+
+case AV_PIX_FMT_VUYX:
+surface->Data.V = frame->data[0];
+surface->Data.U = frame->data[0] + 1;
+surface->Data.Y = frame->data[0] + 2;
+// Only set Data.A to a valid address, the SDK doesn't
+// use the value from the frame.
+surface->Data.A = frame->data[0] + 3;
+break;
+
 default:
 return AVERROR(ENOSYS);
 }
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index 0f0d719e23..0254a394bd 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -141,6 +141,7 @@ static int qsv_get_continuous_buffer(AVCodecContext *avctx, 
AVFrame *frame,
 frame->linesize[0] = 2 * FFALIGN(avctx->width, 128);
 break;
 case AV_PIX_FMT_Y210:
+case AV_PIX_FMT_VUYX:
 frame->linesize[0] = 4 * FFALIGN(avctx->width, 128);
 break;
 default:
@@ -1041,6 +1042,7 @@ const FFCodec ff_##x##_qsv_decoder = { \
 AV_PIX_FMT_P010, \
 AV_PIX_FMT_YUYV422, \
 AV_PIX_FMT_Y210, \
+AV_PIX_FMT_VUYX, \
 AV_PIX_FMT_QSV, \
 AV_PIX_FMT_NONE }, \
 .hw_configs = qsv_hw_configs, \
diff --git a/libavcodec/qsvenc_hevc.c b/libavcodec/qsvenc_hevc.c
index 5986c3f1a6..8072e676e1 100644
--- a/libavcodec/qsvenc_hevc.c
+++ b/libavcodec/qsvenc_hevc.c
@@ -314,6 +314,7 @@ const FFCodec ff_hevc_qsv_encoder = {
 AV_PIX_FMT_QSV,
 AV_PIX_FMT_BGRA,
 AV_PIX_FMT_X2RGB10,
+AV_PIX_FMT_VUYX,
 AV_PIX_FMT_NONE },
 .p.priv_class   = &class,
 .defaults   = qsv_enc_defaults,
diff --git a/libavcodec/qsvenc_vp9.c b/libavcodec/qsvenc_vp9.c
index 61d50156d3..044a882d1a 100644
--- a/libavcodec/qsvenc_vp9.c
+++ b/libavcodec/qsvenc_vp9.c
@@ -113,6 +113,7 @@ const FFCodec ff_vp9_qsv_encoder = {
 .p.capabilities = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_HYBRID,
 .p.pix_fmts = (const enum AVPixelFormat[]){ AV_PIX_FMT_NV12,
 AV_PIX_FMT_P010,
+AV_PIX_FMT_VUYX,
 AV_PIX_FMT_QSV,
 AV_PIX_FMT_NONE },
 .p.priv_class   = &class,
-- 
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] [PATCH v3] fftools/ffplay: fix rotation incorrect when frame contains the displaymatrix

2022-09-05 Thread Zhao Zhili
On Mon, 2022-09-05 at 18:40 +0800, 1035567...@qq.com wrote:
> From: Wang Yaqiang 
> 
> For example, if the jpeg contains exif information
> and the rotation direction is included in the exif,
> the displaymatrix will be set on the side_data of the frame when
> decoding.
> However, when ffplay is used to play the image,
> only the side data in the stream will be determined.
> It does not check whether the frame also contains rotation
> information,
> causing it to play in the wrong direction
> 
> Signed-off-by: Wang Yaqiang 
> ---
>  fftools/ffplay.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 9242047f5c..bcc00afe31 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -1915,8 +1915,14 @@ static int
> configure_video_filters(AVFilterGraph *graph, VideoState *is, const c
>  } while (0)
>  
>  if (autorotate) {
> -int32_t *displaymatrix = (int32_t
> *)av_stream_get_side_data(is->video_st, AV_PKT_DATA_DISPLAYMATRIX,
> NULL);
> -double theta = get_rotation(displaymatrix);
> +double theta = 0.0;
> +int32_t *displaymatrix = NULL;
> +AVFrameSideData *sd = av_frame_get_side_data(frame,
> AV_FRAME_DATA_DISPLAYMATRIX);
> +if (sd)
> +displaymatrix = (int32_t *)sd->data;
> +if (!displaymatrix)
> +displaymatrix = (int32_t *)av_stream_get_side_data(is-
> >video_st, AV_PKT_DATA_DISPLAYMATRIX, NULL);
> +theta = get_rotation(displaymatrix);
>  
>  if (fabs(theta - 90) < 1.0) {
>  INSERT_FILT("transpose", "clock");

LGTM.

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

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