[FFmpeg-devel] [PATCH] avcodec/hevcdec: fix the order of in-loop filters

2021-12-17 Thread Zhexiong Huang
As we know, in-loop filters in HEVC are applied by the following
ordered steps: firstly deblocking filter, then sample adaptive offset
filter if enabled. However, in the current version of FFmpeg, pixels
without being deblocking-filtered could be used by SAO filter when CTU
size is 16 and chroma format is 4:2:0 or 4:2:2, which could lead to the
wrong result in chroma components.

This patch changes the algorithm of deblocking filter, which ensures
that SAO filter is applied after deblocking filter for all the related
pixels. The new algorithm fixes this decoding problem when CTU size is
16, and shall not affect performance and correctness when CTU size is
32 or 64.

Signed-off-by: Zhexiong Huang 
---
 libavcodec/hevc_filter.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/libavcodec/hevc_filter.c b/libavcodec/hevc_filter.c
index 3c45b5a39e..c53875d85f 100644
--- a/libavcodec/hevc_filter.c
+++ b/libavcodec/hevc_filter.c
@@ -512,10 +512,10 @@ static void deblocking_filter_CTB(HEVCContext *s, int x0, 
int y0)
 
 x_end2 = x_end;
 if (x_end2 != s->ps.sps->width)
-x_end2 -= 8;
+x_end2 += 8;
 for (y = y0; y < y_end; y += 8) {
 // vertical filtering luma
-for (x = x0 ? x0 : 8; x < x_end; x += 8) {
+for (x = x0 + 8; x < x_end2; x += 8) {
 const int bs0 = s->vertical_bs[(x +  y  * s->bs_width) >> 2];
 const int bs1 = s->vertical_bs[(x + (y + 4) * s->bs_width) >> 2];
 if (bs0 || bs1) {
@@ -545,7 +545,7 @@ static void deblocking_filter_CTB(HEVCContext *s, int x0, 
int y0)
  continue;
 
 // horizontal filtering luma
-for (x = x0 ? x0 - 8 : 0; x < x_end2; x += 8) {
+for (x = x0; x < x_end; x += 8) {
 const int bs0 = s->horizontal_bs[( x  + y * s->bs_width) >> 2];
 const int bs1 = s->horizontal_bs[((x + 4) + y * s->bs_width) >> 2];
 if (bs0 || bs1) {
@@ -579,9 +579,13 @@ static void deblocking_filter_CTB(HEVCContext *s, int x0, 
int y0)
 int h = 1 << s->ps.sps->hshift[chroma];
 int v = 1 << s->ps.sps->vshift[chroma];
 
+x_end2 = x_end;
+if (x_end2 != s->ps.sps->width)
+x_end2 += 8 * h;
+
 // vertical filtering chroma
 for (y = y0; y < y_end; y += (8 * v)) {
-for (x = x0 ? x0 : 8 * h; x < x_end; x += (8 * h)) {
+for (x = x0 + 8 * h; x < x_end2; x += (8 * h)) {
 const int bs0 = s->vertical_bs[(x +  y* 
s->bs_width) >> 2];
 const int bs1 = s->vertical_bs[(x + (y + (4 * v)) * 
s->bs_width) >> 2];
 
@@ -612,10 +616,7 @@ static void deblocking_filter_CTB(HEVCContext *s, int x0, 
int y0)
 
 // horizontal filtering chroma
 tc_offset = x0 ? left_tc_offset : cur_tc_offset;
-x_end2 = x_end;
-if (x_end != s->ps.sps->width)
-x_end2 = x_end - 8 * h;
-for (x = x0 ? x0 - 8 * h : 0; x < x_end2; x += (8 * h)) {
+for (x = x0; x < x_end; x += (8 * h)) {
 const int bs0 = s->horizontal_bs[( x  + y * 
s->bs_width) >> 2];
 const int bs1 = s->horizontal_bs[((x + 4 * h) + y * 
s->bs_width) >> 2];
 if ((bs0 == 2) || (bs1 == 2)) {
-- 
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/5] build: detect Metal.framework and build .metal files

2021-12-17 Thread Martin Storsjö

On Thu, 16 Dec 2021, Aman Karmani wrote:


From: Aman Karmani 

Signed-off-by: Aman Karmani 
---
.gitignore | 3 +++
configure  | 8 +++-
ffbuild/common.mak | 9 +
3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 9ed24b542e..1a5bb29ad5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,6 +19,9 @@
*.swp
*.ver
*.version
+*.metal.air
+*.metallib
+*.metallib.c
*.ptx
*.ptx.c
*.ptx.gz
diff --git a/configure b/configure
index 5fffcb8afe..ab00b2d7cb 100755
--- a/configure
+++ b/configure
@@ -309,6 +309,7 @@ External library support:
   if openssl, gnutls or libtls is not used [no]
  --enable-mediacodec  enable Android MediaCodec support [no]
  --enable-mediafoundation enable encoding via MediaFoundation [auto]
+  --disable-metal  disable Apple Metal framework [autodetect]
  --enable-libmysofa   enable libmysofa, needed for sofalizer filter [no]
  --enable-openal  enable OpenAL 1.1 capture support [no]
  --enable-opencl  enable OpenCL processing [no]
@@ -382,6 +383,7 @@ Toolchain options:
  --dep-cc=DEPCC   use dependency generator DEPCC [$cc_default]
  --nvcc=NVCC  use Nvidia CUDA compiler NVCC or clang 
[$nvcc_default]
  --ld=LD  use linker LD [$ld_default]
+  --metalcc=METALCCuse metal compiler METALCC [$metalcc_default]
  --pkg-config=PKGCONFIG   use pkg-config tool PKGCONFIG [$pkg_config_default]
  --pkg-config-flags=FLAGS pass additional flags to pkgconf []
  --ranlib=RANLIB  use ranlib RANLIB [$ranlib_default]
@@ -2564,6 +2566,7 @@ CMDLINE_SET="
ln_s
logfile
malloc_prefix
+metalcc
nm
optflags
nvcc
@@ -3835,6 +3838,7 @@ host_cc_default="gcc"
doxygen_default="doxygen"
install="install"
ln_s_default="ln -s -f"
+metalcc_default="xcrun metal"
nm_default="nm -g"
pkg_config_default=pkg-config
ranlib_default="ranlib"
@@ -4435,7 +4439,7 @@ if enabled cuda_nvcc; then
fi

set_default arch cc cxx doxygen pkg_config ranlib strip sysinclude \
-target_exec x86asmexe
+target_exec x86asmexe metalcc
enabled cross_compile || host_cc_default=$cc
set_default host_cc

@@ -6326,6 +6330,7 @@ check_apple_framework CoreFoundation
check_apple_framework CoreMedia
check_apple_framework CoreVideo
check_apple_framework CoreAudio
+check_apple_framework Metal

enabled avfoundation && {
disable coregraphics applicationservices
@@ -7620,6 +7625,7 @@ ARFLAGS=$arflags
AR_O=$ar_o
AR_CMD=$ar
NM_CMD=$nm
+METALCC=$metalcc
RANLIB=$ranlib
STRIP=$strip
STRIPTYPE=$striptype
diff --git a/ffbuild/common.mak b/ffbuild/common.mak
index 0eb831d434..05440911f4 100644
--- a/ffbuild/common.mak
+++ b/ffbuild/common.mak
@@ -112,6 +112,15 @@ COMPILE_LASX = $(call COMPILE,CC,LASXFLAGS)
$(BIN2CEXE): ffbuild/bin2c_host.o
$(HOSTLD) $(HOSTLDFLAGS) $(HOSTLD_O) $^ $(HOSTEXTRALIBS)

+%.metal.air: %.metal
+   $(METALCC) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) -o $@
+
+%.metallib: %.metal.air
+   $(METALCC)lib --split-module-without-linking $(patsubst 
$(SRC_PATH)/%,$(SRC_LINK)/%,$<) -o $@


Hmm, so does this try to run "xcrun metallib" instead of "xcrun metal"? I 
think that can be kinda brittle, e.g. if someone wants to configure a 
custom build env, where METALCC expands to e.g. 
"my-wrapped-metal-compiler.sh".


I guess it feels a bit boring to need to define two separate variables, 
but if it really is two separate tools, then I think that'd be the best 
for clarity.


// 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] avcodec/hevcdec: fix the order of in-loop filters

2021-12-17 Thread 黄哲雄
To verify if this problem has been fixed, the following bitstream can
be used:
https://streams.videolan.org/ffmpeg/incoming/BlowingBubbles_ctu16.265
This bitstream is encoded by the following command line:
x265 --input BlowingBubbles_416x240_50.yuv --input-res 416x240 --fps 50 -s
16 -o bs.265
The MD5 of decoded yuv shall be 947723f7d8eeca7be6fcf5c91a071ada, not
18ca1e6fd9d8c47709a5bb156ee275a6 in the current version of FFmpeg.
It is verified by HM and libde265 decoder.
On Fri, Dec 17, 2021, 16:46  wrote:
As we know, in-loop filters in HEVC are applied by the following ordered
steps: firstly deblocking filter, then sample adaptive offset filter if
enabled. However, in the current version of FFmpeg, pixels without being
deblocking-filtered could be used by SAO filter when CTU size is 16 and
chroma format is 4:2:0 or 4:2:2, which could lead to the wrong result in
chroma components. This patch changes the algorithm of deblocking filter,
which ensures that SAO filter is applied after deblocking filter for all
the related pixels. The new algorithm fixes this decoding problem when CTU
size is 16, and shall not affect performance and correctness when CTU size
is 32 or 64. Signed-off-by: Zhexiong Huang --- libavcodec/hevc_filter.c |
17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff
--git a/libavcodec/hevc_filter.c b/libavcodec/hevc_filter.c index
3c45b5a39e..c53875d85f 100644 --- a/libavcodec/hevc_filter.c +++
b/libavcodec/hevc_filter.c @@ -512,10 +512,10 @@ static void
deblocking_filter_CTB(HEVCContext *s, int x0, int y0) x_end2 = x_end; if
(x_end2 != s->ps.sps->width) - x_end2 -= 8; + x_end2 += 8; for (y = y0; y <
y_end; y += 8) { // vertical filtering luma - for (x = x0 ? x0 : 8; x <
x_end; x += 8) { + for (x = x0 + 8; x < x_end2; x += 8) { const int bs0 =
s->vertical_bs[(x + y * s->bs_width) >> 2]; const int bs1 =
s->vertical_bs[(x + (y + 4) * s->bs_width) >> 2]; if (bs0 || bs1) { @@
-545,7 +545,7 @@ static void deblocking_filter_CTB(HEVCContext *s, int x0,
int y0) continue; // horizontal filtering luma - for (x = x0 ? x0 - 8 : 0;
x < x_end2; x += 8) { + for (x = x0; x < x_end; x += 8) { const int bs0 =
s->horizontal_bs[( x + y * s->bs_width) >> 2]; const int bs1 =
s->horizontal_bs[((x + 4) + y * s->bs_width) >> 2]; if (bs0 || bs1) { @@
-579,9 +579,13 @@ static void deblocking_filter_CTB(HEVCContext *s, int x0,
int y0) int h = 1 << s->ps.sps->hshift[chroma]; int v = 1 <<
s->ps.sps->vshift[chroma]; + x_end2 = x_end; + if (x_end2 !=
s->ps.sps->width) + x_end2 += 8 * h; + // vertical filtering chroma for (y
= y0; y < y_end; y += (8 * v)) { - for (x = x0 ? x0 : 8 * h; x < x_end; x
+= (8 * h)) { + for (x = x0 + 8 * h; x < x_end2; x += (8 * h)) { const int
bs0 = s->vertical_bs[(x + y * s->bs_width) >> 2]; const int bs1 =
s->vertical_bs[(x + (y + (4 * v)) * s->bs_width) >> 2]; @@ -612,10 +616,7
@@ static void deblocking_filter_CTB(HEVCContext *s, int x0, int y0) //
horizontal filtering chroma tc_offset = x0 ? left_tc_offset :
cur_tc_offset; - x_end2 = x_end; - if (x_end != s->ps.sps->width) - x_end2
= x_end - 8 * h; - for (x = x0 ? x0 - 8 * h : 0; x < x_end2; x += (8 * h))
{ + for (x = x0; x < x_end; x += (8 * h)) { const int bs0 =
s->horizontal_bs[( x + y * s->bs_width) >> 2]; const int bs1 =
s->horizontal_bs[((x + 4 * h) + y * s->bs_width) >> 2]; if ((bs0 == 2) ||
(bs1 == 2)) { -- 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 05/24] ffmpeg: move some muxing-related code into a separate file

2021-12-17 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2021-12-17 02:55:43)
> Anton Khirnov:
> > This is a first step towards making muxers more independent from the
> > rest of the code.
> > ---
> >  fftools/Makefile |  11 +-
> >  fftools/ffmpeg.c | 273 ++--
> >  fftools/ffmpeg.h |  10 ++
> >  fftools/ffmpeg_mux.c | 293 +++
> >  4 files changed, 320 insertions(+), 267 deletions(-)
> >  create mode 100644 fftools/ffmpeg_mux.c
> > 
> > diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> > new file mode 100644
> > index 00..e7c6ddd8f8
> > --- /dev/null
> > +++ b/fftools/ffmpeg_mux.c
> > @@ -0,0 +1,293 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
> > 02110-1301 USA
> > + */
> > +
> > +#include 
> > +#include 
> > +
> > +#include "libavformat/avformat.h"
> > +#include "libavformat/avio.h"
> > +
> > +#include "libavcodec/packet.h"
> > +
> > +#include "libavutil/fifo.h"
> > +#include "libavutil/intreadwrite.h"
> > +#include "libavutil/log.h"
> > +#include "libavutil/mem.h"
> > +#include "libavutil/timestamp.h"
> > +
> > +#include "ffmpeg.h"
> > +
> 
> These library headers are ordered reversely to our usual order. Is
> this intended?

I am not aware of there being a usual order. So no, it's not.

> (It has the advantage that e.g. missing lavu headers
> in the lavf headers could be uncovered.)

Won't make checkheaders also find that?

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

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


Re: [FFmpeg-devel] [PATCH 11/24] ffmpeg: set want_sdp when initializing the muxer

2021-12-17 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2021-12-16 22:40:31)
> Anton Khirnov:
> > Allows making the variable local to ffmpeg_mux.
> > ---
> >  fftools/ffmpeg.c | 9 +
> >  fftools/ffmpeg.h | 1 -
> >  fftools/ffmpeg_mux.c | 5 +
> >  3 files changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 3ed1201fda..f76e5df8d2 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -138,8 +138,6 @@ static int nb_frames_drop = 0;
> >  static int64_t decode_error_stat[2];
> >  unsigned nb_output_dumped = 0;
> >  
> > -int want_sdp = 1;
> > -
> >  static BenchmarkTimeStamps current_time;
> >  AVIOContext *progress_avio = NULL;
> >  
> > @@ -4557,7 +4555,7 @@ static void log_callback_null(void *ptr, int level, 
> > const char *fmt, va_list vl)
> >  
> >  int main(int argc, char **argv)
> >  {
> > -int i, ret;
> > +int ret;
> >  BenchmarkTimeStamps ti;
> >  
> >  init_dynload();
> > @@ -4600,11 +4598,6 @@ int main(int argc, char **argv)
> >  exit_program(1);
> >  }
> >  
> > -for (i = 0; i < nb_output_files; i++) {
> > -if (strcmp(output_files[i]->format->name, "rtp"))
> > -want_sdp = 0;
> > -}
> > -
> >  current_time = ti = get_benchmark_time_stamps();
> >  if (transcode() < 0)
> >  exit_program(1);
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > index 67ff391334..4aed24c2a7 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -646,7 +646,6 @@ extern char *qsv_device;
> >  #endif
> >  extern HWDevice *filter_hw_device;
> >  
> > -extern int want_sdp;
> >  extern unsigned nb_output_dumped;
> >  extern int main_return_code;
> >  
> > diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> > index aba4b563a4..e6417cdb8c 100644
> > --- a/fftools/ffmpeg_mux.c
> > +++ b/fftools/ffmpeg_mux.c
> > @@ -38,6 +38,8 @@ struct Muxer {
> >  int header_written;
> >  };
> >  
> > +static int want_sdp = 1;
> > +
> >  static void close_all_output_streams(OutputStream *ost, OSTFinished 
> > this_stream, OSTFinished others)
> >  {
> >  int i;
> > @@ -349,6 +351,9 @@ int of_muxer_init(OutputFile *of, uint64_t 
> > limit_filesize)
> >  
> >  mux->limit_filesize = limit_filesize;
> >  
> > +if (strcmp(of->format->name, "rtp"))
> > +want_sdp = 0;
> > +
> >  return 0;
> >  }
> >  
> > 
> 
> This variable could actually be completely local to
> check_init_output_file()/of_check_init().

If we are to have global variables at all (I'd prefer we didn't), I'd
rather have them visible, not hidden inside functions.

-- 
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] avformat/movenc: fix assert failure in get_cluster_duration()

2021-12-17 Thread Zhao Zhili
When editlist is disabled, the workaournd method of shift dts to
zero and increase the first sample duration doesn't work if the
timestamp is larger than mp4 spec restriction (e.g., sample_delta
in stts entry). Further more, it triggers get_cluster_duration()
assert failure. This patch will drop large offsets between
multiple tracks.
---
 libavformat/movenc.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0f912dd012..dd92c0f26d 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5917,7 +5917,18 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket 
*pkt)
  * to signal the difference in starting time without an edit list.
  * Thus move the timestamp for this first sample to 0, increasing
  * its duration instead. */
-trk->cluster[trk->entry].dts = trk->start_dts = 0;
+if (pkt->dts < INT32_MAX) {
+trk->cluster[trk->entry].dts = trk->start_dts = 0;
+} else {
+/* Impossible to write a sample duration >= UINT32_MAX.
+ * Use INT32_MAX as a tight restriction.
+ */
+trk->start_dts = pkt->dts;
+av_log(s, AV_LOG_WARNING,
+   "Track %d starts with a nonzero dts %" PRId64
+   " which will be shifted to zero\n",
+   pkt->stream_index, pkt->dts);
+}
 }
 if (trk->start_dts == AV_NOPTS_VALUE) {
 trk->start_dts = pkt->dts;
-- 
2.31.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 16/24] ffmpeg: access output file chapters through a wrapper

2021-12-17 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2021-12-17 00:08:07)
> Anton Khirnov:
> > Avoid accessing the muxer context directly, as this will become
> > forbidden in future commits.
> > ---
> >  fftools/ffmpeg.c | 15 +--
> >  fftools/ffmpeg.h |  2 ++
> >  fftools/ffmpeg_mux.c |  7 +++
> >  3 files changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> > index 902f190191..d69e4119ef 100644
> > --- a/fftools/ffmpeg.c
> > +++ b/fftools/ffmpeg.c
> > @@ -2909,12 +2909,15 @@ static void parse_forced_key_frames(char *kf, 
> > OutputStream *ost,
> >  *next++ = 0;
> >  
> >  if (!memcmp(p, "chapters", 8)) {
> > -
> > -AVFormatContext *avf = output_files[ost->file_index]->ctx;
> > +OutputFile *of = output_files[ost->file_index];
> > +AVChapter * const *ch;
> > +unsigned intnb_ch;
> >  int j;
> >  
> > -if (avf->nb_chapters > INT_MAX - size ||
> > -!(pts = av_realloc_f(pts, size += avf->nb_chapters - 1,
> > +ch = of_get_chapters(of, &nb_ch);
> > +
> > +if (nb_ch > INT_MAX - size ||
> > +!(pts = av_realloc_f(pts, size += nb_ch - 1,
> >   sizeof(*pts {
> >  av_log(NULL, AV_LOG_FATAL,
> > "Could not allocate forced key frames array.\n");
> > @@ -2923,8 +2926,8 @@ static void parse_forced_key_frames(char *kf, 
> > OutputStream *ost,
> >  t = p[8] ? parse_time_or_die("force_key_frames", p + 8, 1) : 0;
> >  t = av_rescale_q(t, AV_TIME_BASE_Q, avctx->time_base);
> >  
> > -for (j = 0; j < avf->nb_chapters; j++) {
> > -AVChapter *c = avf->chapters[j];
> > +for (j = 0; j < nb_ch; j++) {
> > +const AVChapter *c = ch[j];
> >  av_assert1(index < size);
> >  pts[index++] = av_rescale_q(c->start, c->time_base,
> >  avctx->time_base) + t;
> > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> > index 78c2295c8e..8119282aed 100644
> > --- a/fftools/ffmpeg.h
> > +++ b/fftools/ffmpeg.h
> > @@ -697,5 +697,7 @@ void of_write_packet(OutputFile *of, AVPacket *pkt, 
> > OutputStream *ost,
> >   int unqueue);
> >  int of_finished(OutputFile *of);
> >  int64_t of_bytes_written(OutputFile *of);
> > +AVChapter * const *
> > +of_get_chapters(OutputFile *of, unsigned int *nb_chapters);
> >  
> >  #endif /* FFTOOLS_FFMPEG_H */
> > diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> > index 3ee0fc0667..6c9f10db9c 100644
> > --- a/fftools/ffmpeg_mux.c
> > +++ b/fftools/ffmpeg_mux.c
> > @@ -390,3 +390,10 @@ int64_t of_bytes_written(OutputFile *of)
> >  return of->mux->final_filesize ? of->mux->final_filesize :
> >  pb ? pb->bytes_written : -1;
> >  }
> > +
> > +AVChapter * const *
> > +of_get_chapters(OutputFile *of, unsigned int *nb_chapters)
> > +{
> > +*nb_chapters = of->ctx->nb_chapters;
> > +return of->ctx->chapters;
> > +}
> > 
> 
> I don't see any benefit of factoring muxing out of OutputFile at all; to
> the contrary, it adds another layer of indirection, of allocations for
> your opaque structs, of prohibitions and of unnatural getters like this
> one here.

The benefit is in making it clear which objects can be accessed by what
code. ffmpeg.c currently has a big problem with lack of structure -
random bits of code change random bits of state without much coherence
to it. As a result it is very hard to reason about the code or add new
features cleanly (or even at all, without breaking 10 random unrelated
use cases). In that light I see the new indirections and prohibitions as
an improvement.

The ultimate goal I'm aiming at here is splitting each component
(demuxer, decoder, filtergraph, encoder, muxer) into a separate object
with clearly defined interfaces. This will allow implementing certain
features people want, such as
- multithreading invidividual components
- sending a single encoder's output to multiple muxers
- looping an encoder back to a decoder

I agree that this specific getter is not very pretty (better suggestions
are welcome). But it's hard enough to move forward here at all, if I had
to worry about every commit smelling extra sweet I would never get
anywhere at all.

> If your patchset were already applied and someone posted the reverse
> of patch #15, I'd LGTM it, because it makes checking for the bitexact
> flag local to the place where it is used and thereby avoids storing
> these variables in a context for only one usage.

One reason why the bitexact refactor is an improvement on its own
(disregarding the above considerations), is that currently
set_encoder_id() needs to know how exactly the bitexact flags can be set
(either through -f[f]lags bitexact or -bitexact -- that's why it
initializes codec_flags to ost->enc_ctx->flags). With 

Re: [FFmpeg-devel] [PATCH 21/24] ffmpeg_mux: split of_write_packet()

2021-12-17 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2021-12-17 00:42:25)
> Anton Khirnov:
> > diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> > index d4b674c9e2..e97ec8ab93 100644
> > --- a/fftools/ffmpeg_mux.c
> > +++ b/fftools/ffmpeg_mux.c
> > @@ -102,39 +102,12 @@ static int queue_packet(OutputFile *of, OutputStream 
> > *ost, AVPacket *pkt)
> >  return 0;
> >  }
> >  
> > -void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
> > - int unqueue)
> > +static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
> >  {
> >  AVFormatContext *s = of->ctx;
> >  AVStream *st = ost->st;
> >  int ret;
> >  
> > -/*
> > - * Audio encoders may split the packets --  #frames in != #packets out.
> > - * But there is no reordering, so we can limit the number of output 
> > packets
> > - * by simply dropping them here.
> > - * Counting encoded video frames needs to be done separately because of
> > - * reordering, see do_video_out().
> > - * Do not count the packet when unqueued because it has been counted 
> > when queued.
> > - */
> > -if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && 
> > ost->encoding_needed) && !unqueue) {
> > -if (ost->frame_number >= ost->max_frames) {
> > -av_packet_unref(pkt);
> > -return;
> > -}
> > -ost->frame_number++;
> > -}
> 
> Factoring this chunk out of write_packet() (effectively inlining
> unqueue) looks good (and I actually pondered it myself),
> 
> > -
> > -/* the muxer is not initialized yet, buffer the packet */
> > -if (!of->mux->header_written) {
> > -ret = queue_packet(of, ost, pkt);
> > -if (ret < 0) {
> > -av_packet_unref(pkt);
> > -exit_program(1);
> > -}
> > -return;
> > -}
> > -
> 
> but I could not prove that the header has already been written in case
> unqueue == 0. Can you guarantee this to be so and explain it to me?

I don't understand what you mean. unqueue == 0 in the current code tells
you nothing about whether the header has been written.
How does that relate to the patch?

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

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


Re: [FFmpeg-devel] [PATCH 000/279 v2] New channel layout API

2021-12-17 Thread Michael Niedermayer
On Fri, Dec 17, 2021 at 01:04:19AM +0100, Marton Balint wrote:
> 
> 
> On Thu, 16 Dec 2021, James Almer wrote:
> 
> > Resending the first two patches only, since this is meant to
> > show the implementation of one of the several suggestions made
> > in the previous set that need to be discussed and hopefully
> > resolved in a call.
> 
> Can you push the full branch somewhere?
> 
> > 
> > The proposals so far to extend the API to support either custom
> > labels for channels are, or some form of extra user information.
> > 
> > - Fixed array of bytes to hold a label. Simple solution, but
> >  the labels will have a hard limit that can only be extended
> >  with a major bump. This is what i implemented in this version.
> > - "char *name" per channel that the user may allocate and the
> >  API will manage, duplicate and free. Simple solution, and the
> >  name can be arbitrarily long, but inefficient (av_strdup() per
> >  channel with a custom label on layout copy).
> > - "const char *name" per channel for compile time constants, or
> >  that the user may allocate and free. Very efficient, but for
> >  non compile time strings ensuring they outlive the layout can
> >  be tricky.
> > - Refcounted AVChannelCustom with a dictionary. This can't be
> >  done with AVBufferRef, so it would require some other form
> >  of reference counting. And a dictionary may add quite a bit of
> >  complexity to the API, as you can set anything on them.
> 
> Until we have proper refcounting API we can make the AVBufferRef in
> AVChannelLayout a void *, and only allow channel_layout functions to
> dereference it as an AVBufferRef. This would mean adding some extra helper
> functions to channel layout, but overall it is not unsolvable.
> 
> The real question is that if you want to use refcounting and add helpers to
> query / replace per-channel metadata, or you find the idea too heavy weight
> and would like to stick to flat structs.

what is the advantage of refcounting for channel metadata ?
is it about the used memory, about the reduced need to copy ?

what kind of metadata and what size do you expect ?
bytes, kilobytes, megabytes, gigabytes per channel ?

what is the overhead for dynamic allocation and ref counting?
that is at which point does it even make sense ?

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Nations do behave wisely once they have exhausted all other alternatives. 
-- Abba Eban


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

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


Re: [FFmpeg-devel] [PATCH v1] avformat/aviobuf: ffio_copy_url_options

2021-12-17 Thread Zane van Iperen

Will apply tomorrow unless there are objections.



On 15/12/21 10:35, p...@sandflow.com wrote:

From: Pierre-Anthony Lemieux 

Signed-off-by: Pierre-Anthony Lemieux 
---

Notes:
 Refactors save_avio_options() from dashdec.c and hls.c
 into a common ffio_copy_url_options() in libavformat/aviobuf.c.
 
 Co-authored: Nicholas Vanderzwet 


  libavformat/avio_internal.h |  6 ++
  libavformat/aviobuf.c   | 24 
  libavformat/dashdec.c   | 27 +--
  libavformat/hls.c   | 24 +---
  4 files changed, 32 insertions(+), 49 deletions(-)

diff --git a/libavformat/avio_internal.h b/libavformat/avio_internal.h
index 187433f283..1f5e3d474b 100644
--- a/libavformat/avio_internal.h
+++ b/libavformat/avio_internal.h
@@ -206,6 +206,12 @@ int ffio_fdopen(AVIOContext **s, URLContext *h);
   */
  URLContext *ffio_geturlcontext(AVIOContext *s);
  
+

+/**
+ * Read url related dictionary options from the AVIOContext and write to the 
given dictionary
+ */
+int ffio_copy_url_options(AVIOContext* pb, AVDictionary** avio_opts);
+
  /**
   * Open a write-only fake memory stream. The written data is not stored
   * anywhere - this is only used for measuring the amount of data
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 969c127b23..096f37ae23 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -1022,6 +1022,30 @@ URLContext* ffio_geturlcontext(AVIOContext *s)
  return NULL;
  }
  
+int ffio_copy_url_options(AVIOContext* pb, AVDictionary** avio_opts)

+{
+const char *opts[] = {
+"headers", "user_agent", "cookies", "http_proxy", "referer", "rw_timeout", 
"icy", NULL };
+const char **opt = opts;
+uint8_t *buf = NULL;
+int ret = 0;
+
+while (*opt) {
+if (av_opt_get(pb, *opt, AV_OPT_SEARCH_CHILDREN, &buf) >= 0) {
+if (buf[0] != '\0') {
+ret = av_dict_set(avio_opts, *opt, buf, 
AV_DICT_DONT_STRDUP_VAL);
+if (ret < 0)
+return ret;
+} else {
+av_freep(&buf);
+}
+}
+opt++;
+}
+
+return ret;
+}
+
  static void update_checksum(AVIOContext *s)
  {
  if (s->update_checksum && s->buf_ptr > s->checksum_ptr) {
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 983dc85d65..797fe74157 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -1833,31 +1833,6 @@ end:
  return ret;
  }
  
-static int save_avio_options(AVFormatContext *s)

-{
-DASHContext *c = s->priv_data;
-const char *opts[] = {
-"headers", "user_agent", "cookies", "http_proxy", "referer", "rw_timeout", 
"icy", NULL };
-const char **opt = opts;
-uint8_t *buf = NULL;
-int ret = 0;
-
-while (*opt) {
-if (av_opt_get(s->pb, *opt, AV_OPT_SEARCH_CHILDREN, &buf) >= 0) {
-if (buf[0] != '\0') {
-ret = av_dict_set(&c->avio_opts, *opt, buf, 
AV_DICT_DONT_STRDUP_VAL);
-if (ret < 0)
-return ret;
-} else {
-av_freep(&buf);
-}
-}
-opt++;
-}
-
-return ret;
-}
-
  static int nested_io_open(AVFormatContext *s, AVIOContext **pb, const char 
*url,
int flags, AVDictionary **opts)
  {
@@ -2057,7 +2032,7 @@ static int dash_read_header(AVFormatContext *s)
  
  c->interrupt_callback = &s->interrupt_callback;
  
-if ((ret = save_avio_options(s)) < 0)

+if ((ret = ffio_copy_url_options(s->pb, &c->avio_opts)) < 0)
  return ret;
  
  if ((ret = parse_manifest(s, s->url, s->pb)) < 0)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 557faf8e8d..8c526f748f 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1717,28 +1717,6 @@ static int64_t select_cur_seq_no(HLSContext *c, struct 
playlist *pls)
  return pls->start_seq_no;
  }
  
-static int save_avio_options(AVFormatContext *s)

-{
-HLSContext *c = s->priv_data;
-static const char * const opts[] = {
-"headers", "http_proxy", "user_agent", "cookies", "referer", "rw_timeout", 
"icy", NULL };
-const char * const * opt = opts;
-uint8_t *buf;
-int ret = 0;
-
-while (*opt) {
-if (av_opt_get(s->pb, *opt, AV_OPT_SEARCH_CHILDREN | AV_OPT_ALLOW_NULL, 
&buf) >= 0) {
-ret = av_dict_set(&c->avio_opts, *opt, buf,
-  AV_DICT_DONT_STRDUP_VAL);
-if (ret < 0)
-return ret;
-}
-opt++;
-}
-
-return ret;
-}
-
  static int nested_io_open(AVFormatContext *s, AVIOContext **pb, const char 
*url,
int flags, AVDictionary **opts)
  {
@@ -1884,7 +1862,7 @@ static int hls_read_header(AVFormatContext *s)
  c->first_timestamp = AV_NOPTS_VALUE;
  c->cur_timestamp = AV_NOPTS_VALUE;
  
-if ((ret = save_avio_options(s)) < 0)

+

Re: [FFmpeg-devel] [PATCH] Add 32 bit-per-sample capability to FLAC encoder

2021-12-17 Thread Michael Niedermayer
On Thu, Dec 16, 2021 at 10:09:13PM +0100, Paul B Mahol wrote:
> use wavpack, this is bad patch.

the flac specification supports 32bit
"FLAC supports from 4 to 32 bits per sample"
so theres nothing fundamental wrong with adding 32bit support

thx

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

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus


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

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


Re: [FFmpeg-devel] [PATCH 05/24] ffmpeg: move some muxing-related code into a separate file

2021-12-17 Thread Andreas Rheinhardt
Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-12-17 02:55:43)
>> Anton Khirnov:
>>> This is a first step towards making muxers more independent from the
>>> rest of the code.
>>> ---
>>>  fftools/Makefile |  11 +-
>>>  fftools/ffmpeg.c | 273 ++--
>>>  fftools/ffmpeg.h |  10 ++
>>>  fftools/ffmpeg_mux.c | 293 +++
>>>  4 files changed, 320 insertions(+), 267 deletions(-)
>>>  create mode 100644 fftools/ffmpeg_mux.c
>>>
>>> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
>>> new file mode 100644
>>> index 00..e7c6ddd8f8
>>> --- /dev/null
>>> +++ b/fftools/ffmpeg_mux.c
>>> @@ -0,0 +1,293 @@
>>> +/*
>>> + * This file is part of FFmpeg.
>>> + *
>>> + * FFmpeg is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * FFmpeg is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with FFmpeg; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 
>>> 02110-1301 USA
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +
>>> +#include "libavformat/avformat.h"
>>> +#include "libavformat/avio.h"
>>> +
>>> +#include "libavcodec/packet.h"
>>> +
>>> +#include "libavutil/fifo.h"
>>> +#include "libavutil/intreadwrite.h"
>>> +#include "libavutil/log.h"
>>> +#include "libavutil/mem.h"
>>> +#include "libavutil/timestamp.h"
>>> +
>>> +#include "ffmpeg.h"
>>> +
>>
>> These library headers are ordered reversely to our usual order. Is
>> this intended?
> 
> I am not aware of there being a usual order. So no, it's not.
> 

The usual order is the inverse of linking order (i.e. lavu-lavc-lavf).

>> (It has the advantage that e.g. missing lavu headers
>> in the lavf headers could be uncovered.)
> 
> Won't make checkheaders also find that?
> 

Some of our headers behave differently when included internally (when
DHAVE_AV_CONFIG_H is defined) than when included externally; the fftools
belong to the latter category, I believe checkheaders belongs to the former.

- 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] Add 32 bit-per-sample capability to FLAC encoder

2021-12-17 Thread Michael Niedermayer
On Thu, Dec 16, 2021 at 08:43:21PM +0100, Martijn van Beurden wrote:

[...]

> @@ -972,7 +1007,25 @@ static int encode_residual_ch(FlacEncodeContext *s, int 
> ch)
>  for (i = 0; i < sub->order; i++)
>  sub->coefs[i] = coefs[sub->order-1][i];
>  
> -if (s->bps_code * 4 + s->options.lpc_coeff_precision + 
> av_log2(opt_order) <= 32) {
> +if (s->avctx->bits_per_raw_sample > 24) {
> +if (!ff_flacdsp_lpc_encode_c_32_overflow_detect(res, smp, n, 
> sub->order,
> +   sub->coefs, 
> sub->shift)) {
> +/* The found LPC coefficients produce predictions that overflow
> + * 32-bit signed integer or produce residuals that do not fall
> + * between -2^30 and 2^30. First try again with slightly smaller
> + * coefficients so that the prediction undershoots, if that
> + * doesn't help return a verbatim subframe instead */
> +for (i = 0; i < sub->order; i++) {
> +sub->coefs[i] = sub->coefs[i]*0.98;
 
This is ugly, the amount of actual overflow should be known at this point
so no arbitrary downscale should be needed here
 
 

> +if (!ff_flacdsp_lpc_encode_c_32_overflow_detect(res, smp, n, 
> sub->order,
> +sub->coefs, 
> sub->shift)) {
> +sub->type = sub->type_code = FLAC_SUBFRAME_VERBATIM;
> +memcpy(res, smp, n * sizeof(int32_t));
> +return subframe_count_exact(s, sub, 0);

How often does this occur ?

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato


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

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


Re: [FFmpeg-devel] [PATCH 21/24] ffmpeg_mux: split of_write_packet()

2021-12-17 Thread Andreas Rheinhardt
Anton Khirnov:
> Quoting Andreas Rheinhardt (2021-12-17 00:42:25)
>> Anton Khirnov:
>>> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
>>> index d4b674c9e2..e97ec8ab93 100644
>>> --- a/fftools/ffmpeg_mux.c
>>> +++ b/fftools/ffmpeg_mux.c
>>> @@ -102,39 +102,12 @@ static int queue_packet(OutputFile *of, OutputStream 
>>> *ost, AVPacket *pkt)
>>>  return 0;
>>>  }
>>>  
>>> -void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
>>> - int unqueue)
>>> +static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
>>>  {
>>>  AVFormatContext *s = of->ctx;
>>>  AVStream *st = ost->st;
>>>  int ret;
>>>  
>>> -/*
>>> - * Audio encoders may split the packets --  #frames in != #packets out.
>>> - * But there is no reordering, so we can limit the number of output 
>>> packets
>>> - * by simply dropping them here.
>>> - * Counting encoded video frames needs to be done separately because of
>>> - * reordering, see do_video_out().
>>> - * Do not count the packet when unqueued because it has been counted 
>>> when queued.
>>> - */
>>> -if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && 
>>> ost->encoding_needed) && !unqueue) {
>>> -if (ost->frame_number >= ost->max_frames) {
>>> -av_packet_unref(pkt);
>>> -return;
>>> -}
>>> -ost->frame_number++;
>>> -}
>>
>> Factoring this chunk out of write_packet() (effectively inlining
>> unqueue) looks good (and I actually pondered it myself),
>>
>>> -
>>> -/* the muxer is not initialized yet, buffer the packet */
>>> -if (!of->mux->header_written) {
>>> -ret = queue_packet(of, ost, pkt);
>>> -if (ret < 0) {
>>> -av_packet_unref(pkt);
>>> -exit_program(1);
>>> -}
>>> -return;
>>> -}
>>> -
>>
>> but I could not prove that the header has already been written in case
>> unqueue == 0. Can you guarantee this to be so and explain it to me?
> 
> I don't understand what you mean. unqueue == 0 in the current code tells
> you nothing about whether the header has been written.
> How does that relate to the patch?
> 

Wait, I misread this: You keep the header_written check for the two
callers in output_packet(), but remove it for the one caller for which
we know that initialization has already happened. This is good. Sorry
for the confusion.

- 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 001/279 v2] Add a new channel layout API

2021-12-17 Thread James Almer



On 12/16/2021 8:27 PM, Marton Balint wrote:



On Thu, 16 Dec 2021, James Almer wrote:


- Added a 16 byte fixed array to AVChannelCustom to give custom
 labels to channels in Custom order layouts. These labes will
 be used by the helpers when querying channels by name or
 describing the layout.


I don't think this is a good idea to use the labels directly in helpers 
instead of the channel designation names. See more below.


It is also not great that if the user wants to label a channel, he has 
to use a custom layout. So I'd rather remove the name field from 
AVChannelCustom, I find it limited anyway.


[..]


+enum AVChannel av_channel_from_string(const char *str)
+{
+    int i;
+    char *endptr = (char *)str;
+    enum AVChannel id = AV_CHAN_NONE;
+    for (i = 0; i < FF_ARRAY_ELEMS(channel_names); i++) {
+    if (channel_names[i].name && !strcmp(str, 
channel_names[i].name))

+    return i;
+    }
+    if (!strncmp(str, "USR", 3)) {
+    const char *p = str + 3;
+    id = strtol(p, &endptr, 0);
+    }
+    if (id > 0 && !*endptr)


id >= 0

[..]



+int av_channel_layout_from_mask(AVChannelLayout *channel_layout,
+    uint64_t mask)
+{
+    if (!mask)
+    return AVERROR(EINVAL);
+
+    channel_layout->order   = AV_CHANNEL_ORDER_NATIVE;
+    channel_layout->nb_channels = av_popcount64(mask);
+    channel_layout->u.mask  = mask;
+
+    return 0;
+}


Probably a constructor for custom layout would also make sense:

int av_channel_layout_custom(AVChannelLayout *channel_layout, int 
nb_channels)


Doesn't seem too useful to me. It will basically just call av_malloc for 
u.map for you. You still need to fill the array with the channels in the 
order you want it, unlike av_channel_layout_from_mask() which gives you 
a usable layout directly on return.


You can use av_channel_layout_from_string() for this instead, thanks to 
your suggestion to use generic USR%d designations for no-name ids. In 
the tests from patch 2 i have a couple examples of it.




[...]


+
+int av_channel_layout_copy(AVChannelLayout *dst, const 
AVChannelLayout *src)

+{
+    av_channel_layout_uninit(dst);
+    *dst = *src;
+    if (src->order == AV_CHANNEL_ORDER_CUSTOM) {
+    dst->u.map = av_malloc(src->nb_channels * sizeof(*dst->u.map));


av_malloc_array()

+int av_channel_layout_describe_bprint(const AVChannelLayout 
*channel_layout,

+  AVBPrint *bp)
+{
+    int i;
+
+    av_bprint_clear(bp);
+
+    switch (channel_layout->order) {
+    case AV_CHANNEL_ORDER_NATIVE:
+    for (i = 0; i < FF_ARRAY_ELEMS(channel_layout_map); i++)
+    if (channel_layout->u.mask == 
channel_layout_map[i].layout.u.mask) {

+    av_bprintf(bp, "%s", channel_layout_map[i].name);
+    return 0;
+    }
+    // fall-through
+    case AV_CHANNEL_ORDER_CUSTOM:
+    for (i = 0; i < channel_layout->nb_channels; i++) {
+    const char *ch_name = NULL;
+    enum AVChannel ch = AV_CHAN_NONE;
+
+    if (channel_layout->order == AV_CHANNEL_ORDER_CUSTOM &&
+    channel_layout->u.map[i].name[0])
+    ch_name = channel_layout->u.map[i].name;
+    if (channel_layout->order == AV_CHANNEL_ORDER_NATIVE || 
!ch_name) {
+    ch = 
av_channel_layout_channel_from_index(channel_layout, i);

+    ch_name = get_channel_name(ch);
+    }


You are mixing channel labels and channel designations, and it can be 
the source of confusion. I guess the main problem is that the label of 
the channel can be totally unrelated to the channel designation.


You could show it as some combination. E.g. FL@label. This way there 
will be no confusion, that part before @ is the designation, part after 
the @ is the channel label.


[..]


+enum AVChannel
+av_channel_layout_channel_from_string(const AVChannelLayout 
*channel_layout,

+  const char *name)
+{
+    int channel, ret;
+
+    switch (channel_layout->order) {
+    case AV_CHANNEL_ORDER_CUSTOM:
+    for (int i = 0; i < channel_layout->nb_channels; i++) {
+    if (channel_layout->u.map[i].name[0] && !strcmp(name, 
channel_layout->u.map[i].name))

+    return channel_layout->u.map[i].id;


This is the reverse case, I also find it confusing that name can be a 
label and a designation at the same time and it depends on the channel 
layout type which one is it... Yet again, some syntax could help. E.g. a 
string without @ is a designation, @label is a label without any filter 
for channel designation, designation@label filters both.


[..]

+int av_channel_layout_index_from_string(const AVChannelLayout 
*channel_layout,

+    const char *name)
+{
+    enum AVChannel ch;
+
+    switch (channel_layout->order) {
+    case AV_CHANNEL_ORDER_CUSTOM:
+    for (int i = 0; i < channel_layout->nb_channels; i++) {

[FFmpeg-devel] [PATCH] avformat/movenc: fix duration in mdhd box

2021-12-17 Thread Zhao Zhili
It's the duration of this media, should not take account of
editlist.
---
 libavformat/movenc.c  | 9 +++--
 tests/ref/fate/movenc | 2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0f912dd012..643beac6f2 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2970,8 +2970,13 @@ static int64_t calc_pts_duration(MOVMuxContext *mov, 
MOVTrack *track)
 static int mov_write_mdhd_tag(AVIOContext *pb, MOVMuxContext *mov,
   MOVTrack *track)
 {
-int64_t duration = calc_pts_duration(mov, track);
-int version = duration < INT32_MAX ? 0 : 1;
+int64_t start, end;
+int64_t duration;
+int version;
+
+get_pts_range(mov, track, &start, &end);
+duration = end - start;
+version = duration < INT32_MAX ? 0 : 1;
 
 if (track->mode == MODE_ISM)
 version = 1;
diff --git a/tests/ref/fate/movenc b/tests/ref/fate/movenc
index 81ea75f372..19e4e291b8 100644
--- a/tests/ref/fate/movenc
+++ b/tests/ref/fate/movenc
@@ -7,7 +7,7 @@ write_data len 36, time nopts, type header atom ftyp
 write_data len 2761, time nopts, type header atom -
 write_data len 908, time 97, type sync atom moof
 write_data len 110, time nopts, type trailer atom -
-caf0876986b5f033efc0958c338289cc 3815 non-empty-moov-elst
+9d260d424e9de4626163fd25ccce5bab 3815 non-empty-moov-elst
 write_data len 36, time nopts, type header atom ftyp
 write_data len 2669, time nopts, type header atom -
 write_data len 908, time 100, type sync atom moof
-- 
2.31.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] avformat/movenc: fix duration in mdhd box

2021-12-17 Thread Martin Storsjö

On Fri, 17 Dec 2021, Zhao Zhili wrote:


It's the duration of this media, should not take account of
editlist.
---
libavformat/movenc.c  | 9 +++--
tests/ref/fate/movenc | 2 +-
2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0f912dd012..643beac6f2 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2970,8 +2970,13 @@ static int64_t calc_pts_duration(MOVMuxContext *mov, 
MOVTrack *track)
static int mov_write_mdhd_tag(AVIOContext *pb, MOVMuxContext *mov,
  MOVTrack *track)
{
-int64_t duration = calc_pts_duration(mov, track);
-int version = duration < INT32_MAX ? 0 : 1;
+int64_t start, end;
+int64_t duration;
+int version;
+
+get_pts_range(mov, track, &start, &end);
+duration = end - start;
+version = duration < INT32_MAX ? 0 : 1;


Isn't this equal to what calc_samples_pts_duration() returns?

It'd be good to point out in the commit message, that 
c2424b1f35a1c6c06f1f9fe5f77a7157ed84e1cd was incorrect in this 
aspect. It'd also be good to really spell it out clearly, that (if I 
understand it correctly), mvhd and tkhd should present the post-editlist 
duration, while mdhd should have the pre-editlist duration?


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


[FFmpeg-devel] [PATCH v2] avformat/movenc: fix duration in mdhd box

2021-12-17 Thread Zhao Zhili
It's the duration of this media, should not take account of
editlist.
---
 libavformat/movenc.c  | 2 +-
 tests/ref/fate/movenc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0f912dd012..f76ef430cf 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2970,7 +2970,7 @@ static int64_t calc_pts_duration(MOVMuxContext *mov, 
MOVTrack *track)
 static int mov_write_mdhd_tag(AVIOContext *pb, MOVMuxContext *mov,
   MOVTrack *track)
 {
-int64_t duration = calc_pts_duration(mov, track);
+int64_t duration = calc_samples_pts_duration(mov, track);
 int version = duration < INT32_MAX ? 0 : 1;
 
 if (track->mode == MODE_ISM)
diff --git a/tests/ref/fate/movenc b/tests/ref/fate/movenc
index 81ea75f372..19e4e291b8 100644
--- a/tests/ref/fate/movenc
+++ b/tests/ref/fate/movenc
@@ -7,7 +7,7 @@ write_data len 36, time nopts, type header atom ftyp
 write_data len 2761, time nopts, type header atom -
 write_data len 908, time 97, type sync atom moof
 write_data len 110, time nopts, type trailer atom -
-caf0876986b5f033efc0958c338289cc 3815 non-empty-moov-elst
+9d260d424e9de4626163fd25ccce5bab 3815 non-empty-moov-elst
 write_data len 36, time nopts, type header atom ftyp
 write_data len 2669, time nopts, type header atom -
 write_data len 908, time 100, type sync atom moof
-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH v3] avformat/movenc: fix duration in mdhd box

2021-12-17 Thread Zhao Zhili
mvhd and tkhd present the post-editlist duration, while mdhd should
have the pre-editlist duration. Regression since c2424b1f3.
---
 libavformat/movenc.c  | 2 +-
 tests/ref/fate/movenc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 0f912dd012..f76ef430cf 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2970,7 +2970,7 @@ static int64_t calc_pts_duration(MOVMuxContext *mov, 
MOVTrack *track)
 static int mov_write_mdhd_tag(AVIOContext *pb, MOVMuxContext *mov,
   MOVTrack *track)
 {
-int64_t duration = calc_pts_duration(mov, track);
+int64_t duration = calc_samples_pts_duration(mov, track);
 int version = duration < INT32_MAX ? 0 : 1;
 
 if (track->mode == MODE_ISM)
diff --git a/tests/ref/fate/movenc b/tests/ref/fate/movenc
index 81ea75f372..19e4e291b8 100644
--- a/tests/ref/fate/movenc
+++ b/tests/ref/fate/movenc
@@ -7,7 +7,7 @@ write_data len 36, time nopts, type header atom ftyp
 write_data len 2761, time nopts, type header atom -
 write_data len 908, time 97, type sync atom moof
 write_data len 110, time nopts, type trailer atom -
-caf0876986b5f033efc0958c338289cc 3815 non-empty-moov-elst
+9d260d424e9de4626163fd25ccce5bab 3815 non-empty-moov-elst
 write_data len 36, time nopts, type header atom ftyp
 write_data len 2669, time nopts, type header atom -
 write_data len 908, time 100, type sync atom moof
-- 
2.31.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 v10 1/2] avformat/imf: Demuxer

2021-12-17 Thread Anton Khirnov
Quoting Pierre-Anthony Lemieux (2021-12-15 21:41:25)
> On Wed, Dec 15, 2021 at 12:20 PM Anton Khirnov  wrote:
> >
> > Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
> > > >
> > > > Now the question is whether a malicious attacker can craft those two
> > > > files to get access to anything they shouldn't. I suppose at the very
> > > > least the attacker can get information that the user opened the file (by
> > > > adding an asset on an attacker's server) but that will be a danger with
> > > > any playlists allowing network resources and can be controlled with
> > > > io_open(). Can you think of any other possible issues?
> > > >
> > >
> > > Some security considerations:
> > >
> > > - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
> > > distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
> > > does not contain paths/hyperlinks and (b) only those resources
> > > referenced by the CPL are fetched using the ASSETMAP.
> > > - the CPL uses XML, which has its own security considerations. For
> > > example, XML parsing can result in entities being fetched over the
> > > network, but this is disabled by default in libxml AFAIK.
> >
> > This is concerning. From a brief glance at libxml2, it seems that you
> > need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
> > actually disabling network fetching.
> > But it is possible I'm misreading the code, so if you or anyone else
> > understands this better then clarifications are welcome.
> 
> I was referring to entity expansion and the loading of DTDs being
> disabled by default -- see XML_PARSE_NOENT and XML_PARSE_DTDLOAD at
> [1-2].

Okay then. If nobody has further comments, I will push your latest patch
in a few days.

-- 
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 v4 1/3] libavdevice/avfoundation.m: use AudioConvert, extend supported formats

2021-12-17 Thread Romain Beauxis
This is the first patch of a series of 3 that cleanup and enhance the 
avfoundation implementation for libavdevice.


Changes:
* v2: None
* v3: None
* v4: None

* Implement support for AudioConverter
* Switch to AudioConverter's API to convert unsupported PCM
  formats (non-interleaved, non-packed) to supported formats
* Minimize data copy.

This fixes: https://trac.ffmpeg.org/ticket/9502

API ref: 
https://developer.apple.com/documentation/audiotoolbox/audio_converter_services


Signed-off-by: Romain Beauxis 
---
 libavdevice/avfoundation.m | 250 +
 1 file changed, 144 insertions(+), 106 deletions(-)

diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index 0cd6e646d5..79c9207cfa 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -111,16 +111,10 @@
  int num_video_devices;
 -int audio_channels;
-int audio_bits_per_sample;
-int audio_float;
-int audio_be;
-int audio_signed_integer;
-int audio_packed;
-int audio_non_interleaved;
-
-int32_t *audio_buffer;
-int audio_buffer_size;
+UInt32audio_buffers;
+UInt32audio_channels;
+UInt32bytes_per_sample;
+AudioConverterRef audio_converter;
  enum AVPixelFormat pixel_format;
 @@ -299,7 +293,10 @@ static void destroy_context(AVFContext* ctx)
 ctx->avf_delegate= NULL;
 ctx->avf_audio_delegate = NULL;
 -av_freep(&ctx->audio_buffer);
+if (ctx->audio_converter) {
+  AudioConverterDispose(ctx->audio_converter);
+  ctx->audio_converter = NULL;
+}
  pthread_mutex_destroy(&ctx->frame_lock);
 @@ -673,6 +670,10 @@ static int get_audio_config(AVFormatContext *s)
 AVFContext *ctx = (AVFContext*)s->priv_data;
 CMFormatDescriptionRef format_desc;
 AVStream* stream = avformat_new_stream(s, NULL);
+AudioStreamBasicDescription output_format = {0};
+int audio_bits_per_sample, audio_float, audio_be;
+int audio_signed_integer, audio_packed, audio_non_interleaved;
+int must_convert = 0;
  if (!stream) {
 return 1;
@@ -690,60 +691,95 @@ static int get_audio_config(AVFormatContext *s)
 avpriv_set_pts_info(stream, 64, 1, avf_time_base);
  format_desc = 
CMSampleBufferGetFormatDescription(ctx->current_audio_frame);
-const AudioStreamBasicDescription *basic_desc = 
CMAudioFormatDescriptionGetStreamBasicDescription(format_desc);
+const AudioStreamBasicDescription *input_format = 
CMAudioFormatDescriptionGetStreamBasicDescription(format_desc);

 -if (!basic_desc) {
+if (!input_format) {
 unlock_frames(ctx);
 av_log(s, AV_LOG_ERROR, "audio format not available\n");
 return 1;
 }
 +if (input_format->mFormatID != kAudioFormatLinearPCM) {
+unlock_frames(ctx);
+av_log(s, AV_LOG_ERROR, "only PCM audio format are supported at 
the moment\n");

+return 1;
+}
+
 stream->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
-stream->codecpar->sample_rate= basic_desc->mSampleRate;
-stream->codecpar->channels   = basic_desc->mChannelsPerFrame;
+stream->codecpar->sample_rate= input_format->mSampleRate;
+stream->codecpar->channels   = input_format->mChannelsPerFrame;
 stream->codecpar->channel_layout = 
av_get_default_channel_layout(stream->codecpar->channels);

 -ctx->audio_channels= basic_desc->mChannelsPerFrame;
-ctx->audio_bits_per_sample = basic_desc->mBitsPerChannel;
-ctx->audio_float   = basic_desc->mFormatFlags & 
kAudioFormatFlagIsFloat;
-ctx->audio_be  = basic_desc->mFormatFlags & 
kAudioFormatFlagIsBigEndian;
-ctx->audio_signed_integer  = basic_desc->mFormatFlags & 
kAudioFormatFlagIsSignedInteger;
-ctx->audio_packed  = basic_desc->mFormatFlags & 
kAudioFormatFlagIsPacked;
-ctx->audio_non_interleaved = basic_desc->mFormatFlags & 
kAudioFormatFlagIsNonInterleaved;

-
-if (basic_desc->mFormatID == kAudioFormatLinearPCM &&
-ctx->audio_float &&
-ctx->audio_bits_per_sample == 32 &&
-ctx->audio_packed) {
-stream->codecpar->codec_id = ctx->audio_be ? 
AV_CODEC_ID_PCM_F32BE : AV_CODEC_ID_PCM_F32LE;

-} else if (basic_desc->mFormatID == kAudioFormatLinearPCM &&
-ctx->audio_signed_integer &&
-ctx->audio_bits_per_sample == 16 &&
-ctx->audio_packed) {
-stream->codecpar->codec_id = ctx->audio_be ? 
AV_CODEC_ID_PCM_S16BE : AV_CODEC_ID_PCM_S16LE;

-} else if (basic_desc->mFormatID == kAudioFormatLinearPCM &&
-ctx->audio_signed_integer &&
-ctx->audio_bits_per_sample == 24 &&
-ctx->audio_packed) {
-stream->codecpar->codec_id = ctx->audio_be ? 
AV_CODEC_ID_PCM_S24BE : AV_CODEC_ID_PCM_S24LE;

-} else if (basic_desc->mFormatID == kAudioFormatLinearPCM &&
-ctx->a

[FFmpeg-devel] [PATCH v4 2/3] libavdevice/avfoundation.m: Replace mutex-based concurrency handling in avfoundation.m by a thread-safe fifo queue with maximum length.

2021-12-17 Thread Romain Beauxis

This is the second patch of a series of 3 that cleanup and enhance the
avfoundation implementation for libavdevice.

Changes:
v2: None
v3: Switched queue implementation to CMSimpleQueue
v4: None

This patch fixes the concurrency model. Avfoundation runs its own
producing thread to send produced frames and ffmpeg runs its own thread
to consume them.

The existing implementation stores the last transmitted frame and uses a
mutex to avoid concurrent access. However, this leads to situations
where upcoming frames can be dropped if the ffmpeg thread is acessing
the latest frame. This happens even when the thread would otherwise
catch up and process frames fast enought.

This patches changes this implementation to use a buffer queue with a
max queue length and encapsulated thread-safety. This greatly simplifies
the logic of the calling code and gives the consuming thread a chance to
process all frames concurrently to the producing thread while avoiding
memory leaks.

Signed-off-by: Romain Beauxis 
---
 libavdevice/avfoundation.m | 169 +
 1 file changed, 76 insertions(+), 93 deletions(-)

diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index 79c9207cfa..b602cfbe95 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -26,7 +26,7 @@
  */
  #import 
-#include 
+#import 
  #include "libavutil/channel_layout.h"
 #include "libavutil/pixdesc.h"
@@ -80,13 +80,12 @@
 { AV_PIX_FMT_NONE, 0 }
 };
 +#define MAX_QUEUED_FRAMES 10
+
 typedef struct
 {
 AVClass*class;
 -int frames_captured;
-int audio_frames_captured;
-pthread_mutex_t frame_lock;
 id  avf_delegate;
 id  avf_audio_delegate;
 @@ -121,8 +120,8 @@
 AVCaptureSession *capture_session;
 AVCaptureVideoDataOutput *video_output;
 AVCaptureAudioDataOutput *audio_output;
-CMSampleBufferRef current_frame;
-CMSampleBufferRef current_audio_frame;
+CMSimpleQueueRef  audio_frames_queue;
+CMSimpleQueueRef  video_frames_queue;
  AVCaptureDevice  *observed_device;
 #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
@@ -131,16 +130,6 @@
 int  observed_quit;
 } AVFContext;
 -static void lock_frames(AVFContext* ctx)
-{
-pthread_mutex_lock(&ctx->frame_lock);
-}
-
-static void unlock_frames(AVFContext* ctx)
-{
-pthread_mutex_unlock(&ctx->frame_lock);
-}
-
 /** FrameReciever class - delegate for AVCaptureSession
  */
 @interface AVFFrameReceiver : NSObject
@@ -218,17 +207,8 @@ - (void)  captureOutput:(AVCaptureOutput 
*)captureOutput

   didOutputSampleBuffer:(CMSampleBufferRef)videoFrame
  fromConnection:(AVCaptureConnection *)connection
 {
-lock_frames(_context);
-
-if (_context->current_frame != nil) {
-CFRelease(_context->current_frame);
-}
-
-_context->current_frame = (CMSampleBufferRef)CFRetain(videoFrame);
-
-unlock_frames(_context);
-
-++_context->frames_captured;
+CFRetain(videoFrame);
+CMSimpleQueueEnqueue(_context->video_frames_queue, videoFrame);
 }
  @end
@@ -262,17 +242,8 @@ - (void)  captureOutput:(AVCaptureOutput 
*)captureOutput

   didOutputSampleBuffer:(CMSampleBufferRef)audioFrame
  fromConnection:(AVCaptureConnection *)connection
 {
-lock_frames(_context);
-
-if (_context->current_audio_frame != nil) {
-CFRelease(_context->current_audio_frame);
-}
-
-_context->current_audio_frame = 
(CMSampleBufferRef)CFRetain(audioFrame);

-
-unlock_frames(_context);
-
-++_context->audio_frames_captured;
+CFRetain(audioFrame);
+CMSimpleQueueEnqueue(_context->audio_frames_queue, audioFrame);
 }
  @end
@@ -287,6 +258,30 @@ static void destroy_context(AVFContext* ctx)
 [ctx->avf_delegaterelease];
 [ctx->avf_audio_delegate release];
 +CMSampleBufferRef frame;
+
+if (ctx->video_frames_queue) {
+frame = 
(CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue);

+do {
+  CFRelease(frame);
+  frame = 
(CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue);

+} while (frame);
+
+CFRelease(ctx->video_frames_queue);
+ctx->video_frames_queue = NULL;
+}
+
+if (ctx->audio_frames_queue) {
+frame = 
(CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue);

+do {
+  CFRelease(frame);
+  frame = 
(CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue);

+} while (frame);
+
+CFRelease(ctx->audio_frames_queue);
+ctx->audio_frames_queue = NULL;
+}
+
 ctx->capture_session = NULL;
 ctx->video_output= NULL;
 ctx->audio_output= NULL;
@@ -297,12 +292,6 @@ static void destroy_context(AVFContext* ctx)
   AudioConverterDispose(ctx->audio_converter);
   ctx->audio_converter = NULL;
 }
-
-pthread_mutex_destroy(&ctx->frame

[FFmpeg-devel] [PATCH v4 3/3] libavdevice/avfoundation.m: Allow to select devices by unique ID.

2021-12-17 Thread Romain Beauxis



This is the third patch of a series of 3 that cleanup and enhance the
avfoundation implementation for libavdevice.

Changes:
v2: None
v3:
  * Switched unique ID to use system-prodvided unique ID
  * Implemented unique IDs for screen capture
v4: Cleanup

This patch adds a unique ID to avfoundation devices. This is needed
because device index can change while the machine is running when
devices are plugged or unplugged and device names can be tricky to use
with localization and etc.

Example of output:
./ffmpeg -f avfoundation -list_devices true -i ""
[...]
[AVFoundation indev @ 0x158705230] AVFoundation video devices:
[AVFoundation indev @ 0x158705230] [0] FaceTime HD Camera (ID:
47B4B64B70674B9CAD2BAE273A71F4B5)
[AVFoundation indev @ 0x158705230] [1] Capture screen 0 (ID:
AvfilterAvfoundationCaptureScreen1)
[AVFoundation indev @ 0x158705230] AVFoundation audio devices:
[AVFoundation indev @ 0x158705230] [0] Loopback Audio (ID:
com.rogueamoeba.Loopback.A5668B36-711E-4DF5-8A8D-7148508C735B)
[AVFoundation indev @ 0x158705230] [1] MacBook Pro Microphone (ID:
BuiltInMicrophoneDevice)

Notes:
* Unique names do not seem to follow any specific pattern. I have used
one similar to the builtin microphone for screen capture
* The : substitution is actually required. The loopback device above did
have it in its name.

Signed-off-by: Romain Beauxis 
---
 doc/indevs.texi|  6 ++--
 libavdevice/avfoundation.m | 72 +-
 2 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 5be647f70a..2b55399c8c 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -114,7 +114,7 @@ The input filename has to be given in the following 
syntax:

 -i "[[VIDEO]:[AUDIO]]"
 @end example
 The first entry selects the video input while the latter selects the 
audio input.
-The stream has to be specified by the device name or the device index 
as shown by the device list.
+The stream has to be specified by the device name, index or ID as shown 
by the device list.
 Alternatively, the video and/or audio input device can be chosen by 
index using the

 @option{
 -video_device_index 
@@ -127,7 +127,9 @@ and/or
 device name or index given in the input filename.
  All available devices can be enumerated by using 
@option{-list_devices true}, listing

-all device names and corresponding indices.
+all device names, corresponding indices and IDs, when available. Device 
name can be +tricky to use when localized and device index can change 
when devices are plugged or unplugged. A device
+hash, when available, uniquely identifies a device and should not 
change over time.

  There are two device name aliases:
 @table @code
diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index b602cfbe95..25286507d6 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -39,6 +39,8 @@
 #include "libavutil/imgutils.h"
 #include "avdevice.h"
 +#define CLEANUP_DEVICE_ID(s) [[s 
stringByReplacingOccurrencesOfString:@":" withString:@"."] UTF8String]

+
 static const int avf_time_base = 100;
  static const AVRational avf_time_base_q = {
@@ -797,21 +799,23 @@ static int avf_read_header(AVFormatContext *s)
 int index = 0;
 av_log(ctx, AV_LOG_INFO, "AVFoundation video devices:\n");
 for (AVCaptureDevice *device in devices) {
-const char *name = [[device localizedName] UTF8String];
-index= [devices indexOfObject:device];
-av_log(ctx, AV_LOG_INFO, "[%d] %s\n", index, name);
+const char *name = [[device localizedName] UTF8String];
+const char *uniqueId = CLEANUP_DEVICE_ID([device uniqueID]);
+index= [devices indexOfObject:device];
+av_log(ctx, AV_LOG_INFO, "[%d] %s (ID: %s)\n", index, name, 
uniqueId);

 }
 for (AVCaptureDevice *device in devices_muxed) {
-const char *name = [[device localizedName] UTF8String];
-index= [devices count] + [devices_muxed 
indexOfObject:device];

-av_log(ctx, AV_LOG_INFO, "[%d] %s\n", index, name);
+const char *name = [[device localizedName] UTF8String];
+const char *uniqueId = CLEANUP_DEVICE_ID([device uniqueID]);
+index= [devices count] + [devices_muxed 
indexOfObject:device];
+av_log(ctx, AV_LOG_INFO, "[%d] %s (ID: %s)\n", index, name, 
uniqueId);

 }
 #if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
 if (num_screens > 0) {
 CGDirectDisplayID screens[num_screens];
 CGGetActiveDisplayList(num_screens, screens, &num_screens);
 for (int i = 0; i < num_screens; i++) {
-av_log(ctx, AV_LOG_INFO, "[%d] Capture screen %d\n", 
ctx->num_video_devices + i, i);
+av_log(ctx, AV_LOG_INFO, "[%d] Capture screen %d (ID: 
AvfilterAvfoundationCaptureScreen%d)\n",

Re: [FFmpeg-devel] [PATCH 3/4] avcodec/alacdsp: fix integer overflow in decorrelate_stereo()

2021-12-17 Thread Michael Niedermayer
On Fri, Dec 03, 2021 at 06:19:55PM +0100, Michael Niedermayer wrote:
> Fixes: signed integer overflow: -16777216 * 131 cannot be represented in type 
> 'int'
> Fixes: 
> 23835/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALAC_fuzzer-5669943160078336
> Fixes: 
> 41101/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALAC_fuzzer-4636330705944576
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/alacdsp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

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


[FFmpeg-devel] [PATCH v5 1/3] libavdevice/avfoundation.m: use AudioConvert, extend supported formats

2021-12-17 Thread Romain Beauxis
(Apologies for the previously mangled message, I’m still honing my tools here!)

This is the first patch of a series of 3 that cleanup and enhance the
avfoundation implementation for libavdevice.

Changes:
* v2: None
* v3: None
* v4: None
* v5: Fix indentation/wrapping

* Implement support for AudioConverter
* Switch to AudioConverter's API to convert unsupported PCM
  formats (non-interleaved, non-packed) to supported formats
* Minimize data copy.

This fixes: https://trac.ffmpeg.org/ticket/9502

API ref:
https://developer.apple.com/documentation/audiotoolbox/audio_converter_services

Signed-off-by: Romain Beauxis 
---
libavdevice/avfoundation.m | 250 +
1 file changed, 144 insertions(+), 106 deletions(-)

diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index 0cd6e646d5..79c9207cfa 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -111,16 +111,10 @@

int num_video_devices;

-int audio_channels;
-int audio_bits_per_sample;
-int audio_float;
-int audio_be;
-int audio_signed_integer;
-int audio_packed;
-int audio_non_interleaved;
-
-int32_t *audio_buffer;
-int audio_buffer_size;
+UInt32audio_buffers;
+UInt32audio_channels;
+UInt32bytes_per_sample;
+AudioConverterRef audio_converter;

enum AVPixelFormat pixel_format;

@@ -299,7 +293,10 @@ static void destroy_context(AVFContext* ctx)
ctx->avf_delegate= NULL;
ctx->avf_audio_delegate = NULL;

-av_freep(&ctx->audio_buffer);
+if (ctx->audio_converter) {
+  AudioConverterDispose(ctx->audio_converter);
+  ctx->audio_converter = NULL;
+}

pthread_mutex_destroy(&ctx->frame_lock);

@@ -673,6 +670,10 @@ static int get_audio_config(AVFormatContext *s)
AVFContext *ctx = (AVFContext*)s->priv_data;
CMFormatDescriptionRef format_desc;
AVStream* stream = avformat_new_stream(s, NULL);
+AudioStreamBasicDescription output_format = {0};
+int audio_bits_per_sample, audio_float, audio_be;
+int audio_signed_integer, audio_packed, audio_non_interleaved;
+int must_convert = 0;

if (!stream) {
return 1;
@@ -690,60 +691,95 @@ static int get_audio_config(AVFormatContext *s)
avpriv_set_pts_info(stream, 64, 1, avf_time_base);

format_desc = CMSampleBufferGetFormatDescription(ctx->current_audio_frame);
-const AudioStreamBasicDescription *basic_desc = 
CMAudioFormatDescriptionGetStreamBasicDescription(format_desc);
+const AudioStreamBasicDescription *input_format = 
CMAudioFormatDescriptionGetStreamBasicDescription(format_desc);

-if (!basic_desc) {
+if (!input_format) {
unlock_frames(ctx);
av_log(s, AV_LOG_ERROR, "audio format not available\n");
return 1;
}

+if (input_format->mFormatID != kAudioFormatLinearPCM) {
+unlock_frames(ctx);
+av_log(s, AV_LOG_ERROR, "only PCM audio format are supported at the 
moment\n");
+return 1;
+}
+
stream->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
-stream->codecpar->sample_rate= basic_desc->mSampleRate;
-stream->codecpar->channels   = basic_desc->mChannelsPerFrame;
+stream->codecpar->sample_rate= input_format->mSampleRate;
+stream->codecpar->channels   = input_format->mChannelsPerFrame;
stream->codecpar->channel_layout = 
av_get_default_channel_layout(stream->codecpar->channels);

-ctx->audio_channels= basic_desc->mChannelsPerFrame;
-ctx->audio_bits_per_sample = basic_desc->mBitsPerChannel;
-ctx->audio_float   = basic_desc->mFormatFlags & 
kAudioFormatFlagIsFloat;
-ctx->audio_be  = basic_desc->mFormatFlags & 
kAudioFormatFlagIsBigEndian;
-ctx->audio_signed_integer  = basic_desc->mFormatFlags & 
kAudioFormatFlagIsSignedInteger;
-ctx->audio_packed  = basic_desc->mFormatFlags & 
kAudioFormatFlagIsPacked;
-ctx->audio_non_interleaved = basic_desc->mFormatFlags & 
kAudioFormatFlagIsNonInterleaved;
-
-if (basic_desc->mFormatID == kAudioFormatLinearPCM &&
-ctx->audio_float &&
-ctx->audio_bits_per_sample == 32 &&
-ctx->audio_packed) {
-stream->codecpar->codec_id = ctx->audio_be ? AV_CODEC_ID_PCM_F32BE : 
AV_CODEC_ID_PCM_F32LE;
-} else if (basic_desc->mFormatID == kAudioFormatLinearPCM &&
-ctx->audio_signed_integer &&
-ctx->audio_bits_per_sample == 16 &&
-ctx->audio_packed) {
-stream->codecpar->codec_id = ctx->audio_be ? AV_CODEC_ID_PCM_S16BE : 
AV_CODEC_ID_PCM_S16LE;
-} else if (basic_desc->mFormatID == kAudioFormatLinearPCM &&
-ctx->audio_signed_integer &&
-ctx->audio_bits_per_sample == 24 &&
-ctx->audio_packed) {
-stream->codecpar->codec_id = ctx->audio_be ? AV_CODEC_ID_PCM_S24BE : 
AV_CODEC_ID_PCM_S24LE;
-

[FFmpeg-devel] [PATCH v5 2/3] libavdevice/avfoundation.m: Replace mutex-based concurrency handling in avfoundation.m by a thread-safe fifo queue with maximum length

2021-12-17 Thread Romain Beauxis
(Apologies for the previously mangled message, I’m still honing my tools here!)

This is the second patch of a series of 3 that cleanup and enhance the
avfoundation implementation for libavdevice.

Changes:
v2: None
v3: Switched queue implementation to CMSimpleQueue
v4: None
v5: Fix indentation/wrapping

This patch fixes the concurrency model. Avfoundation runs its own
producing thread to send produced frames and ffmpeg runs its own thread
to consume them.

The existing implementation stores the last transmitted frame and uses a
mutex to avoid concurrent access. However, this leads to situations
where upcoming frames can be dropped if the ffmpeg thread is acessing
the latest frame. This happens even when the thread would otherwise
catch up and process frames fast enought.

This patches changes this implementation to use a buffer queue with a
max queue length and encapsulated thread-safety. This greatly simplifies
the logic of the calling code and gives the consuming thread a chance to
process all frames concurrently to the producing thread while avoiding
memory leaks.

Signed-off-by: Romain Beauxis 
---
libavdevice/avfoundation.m | 169 +
1 file changed, 76 insertions(+), 93 deletions(-)

diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index 79c9207cfa..b602cfbe95 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -26,7 +26,7 @@
 */

#import 
-#include 
+#import 

#include "libavutil/channel_layout.h"
#include "libavutil/pixdesc.h"
@@ -80,13 +80,12 @@
{ AV_PIX_FMT_NONE, 0 }
};

+#define MAX_QUEUED_FRAMES 10
+
typedef struct
{
AVClass*class;

-int frames_captured;
-int audio_frames_captured;
-pthread_mutex_t frame_lock;
id  avf_delegate;
id  avf_audio_delegate;

@@ -121,8 +120,8 @@
AVCaptureSession *capture_session;
AVCaptureVideoDataOutput *video_output;
AVCaptureAudioDataOutput *audio_output;
-CMSampleBufferRef current_frame;
-CMSampleBufferRef current_audio_frame;
+CMSimpleQueueRef  audio_frames_queue;
+CMSimpleQueueRef  video_frames_queue;

AVCaptureDevice  *observed_device;
#if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
@@ -131,16 +130,6 @@
int  observed_quit;
} AVFContext;

-static void lock_frames(AVFContext* ctx)
-{
-pthread_mutex_lock(&ctx->frame_lock);
-}
-
-static void unlock_frames(AVFContext* ctx)
-{
-pthread_mutex_unlock(&ctx->frame_lock);
-}
-
/** FrameReciever class - delegate for AVCaptureSession
 */
@interface AVFFrameReceiver : NSObject
@@ -218,17 +207,8 @@ - (void)  captureOutput:(AVCaptureOutput *)captureOutput
  didOutputSampleBuffer:(CMSampleBufferRef)videoFrame
 fromConnection:(AVCaptureConnection *)connection
{
-lock_frames(_context);
-
-if (_context->current_frame != nil) {
-CFRelease(_context->current_frame);
-}
-
-_context->current_frame = (CMSampleBufferRef)CFRetain(videoFrame);
-
-unlock_frames(_context);
-
-++_context->frames_captured;
+CFRetain(videoFrame);
+CMSimpleQueueEnqueue(_context->video_frames_queue, videoFrame);
}

@end
@@ -262,17 +242,8 @@ - (void)  captureOutput:(AVCaptureOutput *)captureOutput
  didOutputSampleBuffer:(CMSampleBufferRef)audioFrame
 fromConnection:(AVCaptureConnection *)connection
{
-lock_frames(_context);
-
-if (_context->current_audio_frame != nil) {
-CFRelease(_context->current_audio_frame);
-}
-
-_context->current_audio_frame = (CMSampleBufferRef)CFRetain(audioFrame);
-
-unlock_frames(_context);
-
-++_context->audio_frames_captured;
+CFRetain(audioFrame);
+CMSimpleQueueEnqueue(_context->audio_frames_queue, audioFrame);
}

@end
@@ -287,6 +258,30 @@ static void destroy_context(AVFContext* ctx)
[ctx->avf_delegaterelease];
[ctx->avf_audio_delegate release];

+CMSampleBufferRef frame;
+
+if (ctx->video_frames_queue) {
+frame = 
(CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue);
+do {
+  CFRelease(frame);
+  frame = 
(CMSampleBufferRef)CMSimpleQueueDequeue(ctx->video_frames_queue);
+} while (frame);
+
+CFRelease(ctx->video_frames_queue);
+ctx->video_frames_queue = NULL;
+}
+
+if (ctx->audio_frames_queue) {
+frame = 
(CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue);
+do {
+  CFRelease(frame);
+  frame = 
(CMSampleBufferRef)CMSimpleQueueDequeue(ctx->audio_frames_queue);
+} while (frame);
+
+CFRelease(ctx->audio_frames_queue);
+ctx->audio_frames_queue = NULL;
+}
+
ctx->capture_session = NULL;
ctx->video_output= NULL;
ctx->audio_output= NULL;
@@ -297,12 +292,6 @@ static void destroy_context(AVFContext* ctx)
  AudioConverterDispose(ctx->audio_converter);
  ctx->audio_conver

[FFmpeg-devel] [PATCH v5 3/3] libavdevice/avfoundation.m: Allow to select devices by unique ID

2021-12-17 Thread Romain Beauxis
(Apologies for the previously mangled message, I’m still honing my tools here!)

This is the third patch of a series of 3 that cleanup and enhance the
avfoundation implementation for libavdevice.

Changes:
v2: None
v3:
  * Switched unique ID to use system-prodvided unique ID
  * Implemented unique IDs for screen capture
v4: Cleanup
v5: Fix indentation/wrapping

This patch adds a unique ID to avfoundation devices. This is needed
because device index can change while the machine is running when
devices are plugged or unplugged and device names can be tricky to use
with localization and etc.

Example of output:
./ffmpeg -f avfoundation -list_devices true -i ""
[...]
[AVFoundation indev @ 0x158705230] AVFoundation video devices:
[AVFoundation indev @ 0x158705230] [0] FaceTime HD Camera (ID: 
47B4B64B70674B9CAD2BAE273A71F4B5)
[AVFoundation indev @ 0x158705230] [1] Capture screen 0 (ID: 
AvfilterAvfoundationCaptureScreen1)
[AVFoundation indev @ 0x158705230] AVFoundation audio devices:
[AVFoundation indev @ 0x158705230] [0] Loopback Audio (ID: 
com.rogueamoeba.Loopback.A5668B36-711E-4DF5-8A8D-7148508C735B)
[AVFoundation indev @ 0x158705230] [1] MacBook Pro Microphone 
(ID:BuiltInMicrophoneDevice)

Notes:
* Unique names do not seem to follow any specific pattern. I have used
one similar to the builtin microphone for screen capture
* The : substitution is actually required. The loopback device above did
have it in its name.

Signed-off-by: Romain Beauxis 
---
doc/indevs.texi|  6 ++--
libavdevice/avfoundation.m | 72 +-
2 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 5be647f70a..2b55399c8c 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -114,7 +114,7 @@ The input filename has to be given in the following syntax:
-i "[[VIDEO]:[AUDIO]]"
@end example
The first entry selects the video input while the latter selects the audio 
input.
-The stream has to be specified by the device name or the device index as shown 
by the device list.
+The stream has to be specified by the device name, index or ID as shown by the 
device list.
Alternatively, the video and/or audio input device can be chosen by index using 
the
@option{
-video_device_index 
@@ -127,7 +127,9 @@ and/or
device name or index given in the input filename.

All available devices can be enumerated by using @option{-list_devices true}, 
listing
-all device names and corresponding indices.
+all device names, corresponding indices and IDs, when available. Device name 
can be 
+tricky to use when localized and device index can change when devices are 
plugged or unplugged. A device
+hash, when available, uniquely identifies a device and should not change over 
time.

There are two device name aliases:
@table @code
diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index b602cfbe95..25286507d6 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -39,6 +39,8 @@
#include "libavutil/imgutils.h"
#include "avdevice.h"

+#define CLEANUP_DEVICE_ID(s) [[s stringByReplacingOccurrencesOfString:@":" 
withString:@"."] UTF8String]
+
static const int avf_time_base = 100;

static const AVRational avf_time_base_q = {
@@ -797,21 +799,23 @@ static int avf_read_header(AVFormatContext *s)
int index = 0;
av_log(ctx, AV_LOG_INFO, "AVFoundation video devices:\n");
for (AVCaptureDevice *device in devices) {
-const char *name = [[device localizedName] UTF8String];
-index= [devices indexOfObject:device];
-av_log(ctx, AV_LOG_INFO, "[%d] %s\n", index, name);
+const char *name = [[device localizedName] UTF8String];
+const char *uniqueId = CLEANUP_DEVICE_ID([device uniqueID]);
+index= [devices indexOfObject:device];
+av_log(ctx, AV_LOG_INFO, "[%d] %s (ID: %s)\n", index, name, 
uniqueId);
}
for (AVCaptureDevice *device in devices_muxed) {
-const char *name = [[device localizedName] UTF8String];
-index= [devices count] + [devices_muxed 
indexOfObject:device];
-av_log(ctx, AV_LOG_INFO, "[%d] %s\n", index, name);
+const char *name = [[device localizedName] UTF8String];
+const char *uniqueId = CLEANUP_DEVICE_ID([device uniqueID]);
+index= [devices count] + [devices_muxed 
indexOfObject:device];
+av_log(ctx, AV_LOG_INFO, "[%d] %s (ID: %s)\n", index, name, 
uniqueId);
}
#if !TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 1070
if (num_screens > 0) {
CGDirectDisplayID screens[num_screens];
CGGetActiveDisplayList(num_screens, screens, &num_screens);
for (int i = 0; i < num_screens; i++) {
-av_log(ctx, AV_LOG_INFO, "[%d] Capture screen %d\n", 
ctx->num_video_devices + i, i);
+av_log(ctx, AV_LOG_IN

Re: [FFmpeg-devel] [PATCH v5 1/3] libavdevice/avfoundation.m: use AudioConvert, extend supported formats

2021-12-17 Thread Andreas Rheinhardt
Romain Beauxis:
> (Apologies for the previously mangled message, I’m still honing my tools 
> here!)
> 

Things like this and the changelog do not really belong into a commit
message to be committed; the proper place for such comments is between
the "---" and the "libavdevice/avfoundation.m | 250
+" below.

> This is the first patch of a series of 3 that cleanup and enhance the
> avfoundation implementation for libavdevice.
> 
> Changes:
> * v2: None
> * v3: None
> * v4: None
> * v5: Fix indentation/wrapping
> 
> * Implement support for AudioConverter
> * Switch to AudioConverter's API to convert unsupported PCM
>   formats (non-interleaved, non-packed) to supported formats
> * Minimize data copy.
> 
> This fixes: https://trac.ffmpeg.org/ticket/9502
> 
> API ref:
> https://developer.apple.com/documentation/audiotoolbox/audio_converter_services
> 
> Signed-off-by: Romain Beauxis 
> ---
> libavdevice/avfoundation.m | 250 +
> 1 file changed, 144 insertions(+), 106 deletions(-)
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxf: support MCA audio information

2021-12-17 Thread Marc-Antoine ARNAUD
Hi all,

Can I have an update on this patch submission ?
Is something required to be done before it can be merged ?

Thanks you,
Marc-Antoine


Le ven. 3 déc. 2021 à 10:57, Marc-Antoine Arnaud
 a écrit :
>
> ---
>  doc/demuxers.texi |  10 ++
>  libavformat/mxf.h |   3 +
>  libavformat/mxfdec.c  | 335 +-
>  libavformat/version.h |   2 +-
>  4 files changed, 343 insertions(+), 7 deletions(-)
>
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index cab8a7072c..23b6753602 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -770,6 +770,16 @@ MJPEG stream. Turning this option on by setting it to 1 
> will result in a stricte
>  of the boundary value.
>  @end table
>
> +@section mxf
> +
> +MXF demuxer.
> +
> +@table @option
> +
> +@item -skip_audio_reordering @var{bool}
> +This option will disable the audio reordering based on Multi-Channel Audio 
> (MCA) labelling (SMPTE ST-377-4).
> +@end table
> +
>  @section rawvideo
>
>  Raw video demuxer.
> diff --git a/libavformat/mxf.h b/libavformat/mxf.h
> index fe9c52732c..d53a16df51 100644
> --- a/libavformat/mxf.h
> +++ b/libavformat/mxf.h
> @@ -50,6 +50,9 @@ enum MXFMetadataSetType {
>  TaggedValue,
>  TapeDescriptor,
>  AVCSubDescriptor,
> +AudioChannelLabelSubDescriptor,
> +SoundfieldGroupLabelSubDescriptor,
> +GroupOfSoundfieldGroupsLabelSubDescriptor,
>  };
>
>  enum MXFFrameLayout {
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index af9d33f796..6e1da75542 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -51,11 +51,14 @@
>  #include "libavutil/mastering_display_metadata.h"
>  #include "libavutil/mathematics.h"
>  #include "libavcodec/bytestream.h"
> +#include "libavcodec/internal.h"
> +#include "libavutil/channel_layout.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/parseutils.h"
>  #include "libavutil/timecode.h"
>  #include "libavutil/opt.h"
>  #include "avformat.h"
> +#include "avlanguage.h"
>  #include "internal.h"
>  #include "mxf.h"
>
> @@ -177,6 +180,8 @@ typedef struct {
>  int body_sid;
>  MXFWrappingScheme wrapping;
>  int edit_units_per_packet; /* how many edit units to read at a time 
> (PCM, ClipWrapped) */
> +int require_reordering;
> +int channel_ordering[FF_SANE_NB_CHANNELS];
>  } MXFTrack;
>
>  typedef struct MXFDescriptor {
> @@ -205,6 +210,8 @@ typedef struct MXFDescriptor {
>  unsigned int vert_subsampling;
>  UID *file_descriptors_refs;
>  int file_descriptors_count;
> +UID *sub_descriptors_refs;
> +int sub_descriptors_count;
>  int linked_track_id;
>  uint8_t *extradata;
>  int extradata_size;
> @@ -217,6 +224,18 @@ typedef struct MXFDescriptor {
>  size_t coll_size;
>  } MXFDescriptor;
>
> +typedef struct MXFMCASubDescriptor {
> +MXFMetadataSet meta;
> +UID uid;
> +UID mca_link_id;
> +UID soundfield_group_link_id;
> +UID *group_of_soundfield_groups_link_id_refs;
> +int group_of_soundfield_groups_link_id_count;
> +UID mca_label_dictionary_id;
> +int mca_channel_id;
> +char *language;
> +} MXFMCASubDescriptor;
> +
>  typedef struct MXFIndexTableSegment {
>  MXFMetadataSet meta;
>  int edit_unit_byte_count;
> @@ -290,6 +309,7 @@ typedef struct MXFContext {
>  int nb_index_tables;
>  MXFIndexTable *index_tables;
>  int eia608_extract;
> +int skip_audio_reordering;
>  } MXFContext;
>
>  /* NOTE: klv_offset is not set (-1) for local keys */
> @@ -311,6 +331,7 @@ static const uint8_t mxf_system_item_key_cp[] 
>  = { 0x06,0x0e,0x2b,0x
>  static const uint8_t mxf_system_item_key_gc[]  = { 
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x03,0x01,0x14 };
>  static const uint8_t mxf_klv_key[] = { 
> 0x06,0x0e,0x2b,0x34 };
>  static const uint8_t mxf_apple_coll_prefix[]   = { 
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x03,0x01 };
> +
>  /* complete keys to match */
>  static const uint8_t mxf_crypto_source_container_ul[]  = { 
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x09,0x06,0x01,0x01,0x02,0x02,0x00,0x00,0x00
>  };
>  static const uint8_t mxf_encrypted_triplet_key[]   = { 
> 0x06,0x0e,0x2b,0x34,0x02,0x04,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x7e,0x01,0x00
>  };
> @@ -323,6 +344,17 @@ static const uint8_t mxf_indirect_value_utf16be[]
>   = { 0x42,0x01,0x10,0x
>  static const uint8_t mxf_apple_coll_max_cll[]  = { 
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x03,0x01,0x01
>  };
>  static const uint8_t mxf_apple_coll_max_fall[] = { 
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x03,0x01,0x02
>  };
>
> +static const uint8_t mxf_mca_label_dictionary_id[] = { 
> 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x01,0x03,0x07,0x01,0x01,0x00,0x00,0x00
>  };
> +static const uint8_t mxf_mca_tag_symbol[]  = { 
> 0x06,0x

Re: [FFmpeg-devel] [PATCH 000/279 v2] New channel layout API

2021-12-17 Thread Marton Balint




On Fri, 17 Dec 2021, Michael Niedermayer wrote:


On Fri, Dec 17, 2021 at 01:04:19AM +0100, Marton Balint wrote:



On Thu, 16 Dec 2021, James Almer wrote:


Resending the first two patches only, since this is meant to
show the implementation of one of the several suggestions made
in the previous set that need to be discussed and hopefully
resolved in a call.


Can you push the full branch somewhere?



The proposals so far to extend the API to support either custom
labels for channels are, or some form of extra user information.

- Fixed array of bytes to hold a label. Simple solution, but
 the labels will have a hard limit that can only be extended
 with a major bump. This is what i implemented in this version.
- "char *name" per channel that the user may allocate and the
 API will manage, duplicate and free. Simple solution, and the
 name can be arbitrarily long, but inefficient (av_strdup() per
 channel with a custom label on layout copy).
- "const char *name" per channel for compile time constants, or
 that the user may allocate and free. Very efficient, but for
 non compile time strings ensuring they outlive the layout can
 be tricky.
- Refcounted AVChannelCustom with a dictionary. This can't be
 done with AVBufferRef, so it would require some other form
 of reference counting. And a dictionary may add quite a bit of
 complexity to the API, as you can set anything on them.


Until we have proper refcounting API we can make the AVBufferRef in
AVChannelLayout a void *, and only allow channel_layout functions to
dereference it as an AVBufferRef. This would mean adding some extra helper
functions to channel layout, but overall it is not unsolvable.

The real question is that if you want to use refcounting and add helpers to
query / replace per-channel metadata, or you find the idea too heavy weight
and would like to stick to flat structs.


what is the advantage of refcounting for channel metadata ?
is it about the used memory, about the reduced need to copy ?


Basicly it is the ability to store per-channel metadata in avdictionary, 
because otherwise it would have to be copyed, and avdictionary is very 
ineffective at copying because of many mallocs.




what kind of metadata and what size do you expect ?
bytes, kilobytes, megabytes, gigabytes per channel ?


Usually, nothing, because most format don't have support for per-channel 
metadata. In some cases it is going to be a couple of textual metadata 
key-value pairs, such as language, label, group, speaker, positon, so 4-5 
dynamically allocated string pairs, plus the AVDictionary itself, 
multiplied by the number of channels in a layout.




what is the overhead for dynamic allocation and ref counting?
that is at which point does it even make sense ?


I don't have exact measurements. It is generally felt that copying 
AVDictionary per-channel is a huge overhead for something as lightweight 
as an audio frame which is a 2-4 kB per channel at most and only a couple 
of allocs usually not dependant on the number of channels. That's why 
refcounting was proposed.


Also some people simply don't want to store extendable channel metadata in 
channel layout, and want to keep it simple.


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

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


Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxf: support MCA audio information

2021-12-17 Thread Marton Balint



On Fri, 17 Dec 2021, Marc-Antoine ARNAUD wrote:


Hi all,

Can I have an update on this patch submission ?
Is something required to be done before it can be merged ?


New channel layout API is on its way, which makes in-demuxer channel 
reordering uneeded. Therefore the reordering option should not be added 
as it is in this patch. I can rework the patch after the channel layout 
API is in. (should happen in a couple of weeks at most).


Regards,
Marton



Thanks you,
Marc-Antoine


Le ven. 3 déc. 2021 à 10:57, Marc-Antoine Arnaud
 a écrit :


---
 doc/demuxers.texi |  10 ++
 libavformat/mxf.h |   3 +
 libavformat/mxfdec.c  | 335 +-
 libavformat/version.h |   2 +-
 4 files changed, 343 insertions(+), 7 deletions(-)

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index cab8a7072c..23b6753602 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -770,6 +770,16 @@ MJPEG stream. Turning this option on by setting it to 1 
will result in a stricte
 of the boundary value.
 @end table

+@section mxf
+
+MXF demuxer.
+
+@table @option
+
+@item -skip_audio_reordering @var{bool}
+This option will disable the audio reordering based on Multi-Channel Audio 
(MCA) labelling (SMPTE ST-377-4).
+@end table
+
 @section rawvideo

 Raw video demuxer.
diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index fe9c52732c..d53a16df51 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -50,6 +50,9 @@ enum MXFMetadataSetType {
 TaggedValue,
 TapeDescriptor,
 AVCSubDescriptor,
+AudioChannelLabelSubDescriptor,
+SoundfieldGroupLabelSubDescriptor,
+GroupOfSoundfieldGroupsLabelSubDescriptor,
 };

 enum MXFFrameLayout {
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index af9d33f796..6e1da75542 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -51,11 +51,14 @@
 #include "libavutil/mastering_display_metadata.h"
 #include "libavutil/mathematics.h"
 #include "libavcodec/bytestream.h"
+#include "libavcodec/internal.h"
+#include "libavutil/channel_layout.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/parseutils.h"
 #include "libavutil/timecode.h"
 #include "libavutil/opt.h"
 #include "avformat.h"
+#include "avlanguage.h"
 #include "internal.h"
 #include "mxf.h"

@@ -177,6 +180,8 @@ typedef struct {
 int body_sid;
 MXFWrappingScheme wrapping;
 int edit_units_per_packet; /* how many edit units to read at a time (PCM, 
ClipWrapped) */
+int require_reordering;
+int channel_ordering[FF_SANE_NB_CHANNELS];
 } MXFTrack;

 typedef struct MXFDescriptor {
@@ -205,6 +210,8 @@ typedef struct MXFDescriptor {
 unsigned int vert_subsampling;
 UID *file_descriptors_refs;
 int file_descriptors_count;
+UID *sub_descriptors_refs;
+int sub_descriptors_count;
 int linked_track_id;
 uint8_t *extradata;
 int extradata_size;
@@ -217,6 +224,18 @@ typedef struct MXFDescriptor {
 size_t coll_size;
 } MXFDescriptor;

+typedef struct MXFMCASubDescriptor {
+MXFMetadataSet meta;
+UID uid;
+UID mca_link_id;
+UID soundfield_group_link_id;
+UID *group_of_soundfield_groups_link_id_refs;
+int group_of_soundfield_groups_link_id_count;
+UID mca_label_dictionary_id;
+int mca_channel_id;
+char *language;
+} MXFMCASubDescriptor;
+
 typedef struct MXFIndexTableSegment {
 MXFMetadataSet meta;
 int edit_unit_byte_count;
@@ -290,6 +309,7 @@ typedef struct MXFContext {
 int nb_index_tables;
 MXFIndexTable *index_tables;
 int eia608_extract;
+int skip_audio_reordering;
 } MXFContext;

 /* NOTE: klv_offset is not set (-1) for local keys */
@@ -311,6 +331,7 @@ static const uint8_t mxf_system_item_key_cp[]  
= { 0x06,0x0e,0x2b,0x
 static const uint8_t mxf_system_item_key_gc[]  = { 
0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x03,0x01,0x14 };
 static const uint8_t mxf_klv_key[] = { 
0x06,0x0e,0x2b,0x34 };
 static const uint8_t mxf_apple_coll_prefix[]   = { 
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x03,0x01 };
+
 /* complete keys to match */
 static const uint8_t mxf_crypto_source_container_ul[]  = { 
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x09,0x06,0x01,0x01,0x02,0x02,0x00,0x00,0x00 
};
 static const uint8_t mxf_encrypted_triplet_key[]   = { 
0x06,0x0e,0x2b,0x34,0x02,0x04,0x01,0x07,0x0d,0x01,0x03,0x01,0x02,0x7e,0x01,0x00 
};
@@ -323,6 +344,17 @@ static const uint8_t mxf_indirect_value_utf16be[]  
= { 0x42,0x01,0x10,0x
 static const uint8_t mxf_apple_coll_max_cll[]  = { 
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x03,0x01,0x01 
};
 static const uint8_t mxf_apple_coll_max_fall[] = { 
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x03,0x01,0x02 
};

+static const uint8_t mxf_mca_label_dictionary_id[] = { 
0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x01,0x03,0x07,0x01,0x01,0x00,0x00,0x

Re: [FFmpeg-devel] [PATCH v3] avformat/movenc: fix duration in mdhd box

2021-12-17 Thread Martin Storsjö

On Fri, 17 Dec 2021, Zhao Zhili wrote:


mvhd and tkhd present the post-editlist duration, while mdhd should
have the pre-editlist duration. Regression since c2424b1f3.
---
libavformat/movenc.c  | 2 +-
tests/ref/fate/movenc | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)


Thanks, this looks sensible to me. I haven't verified these details with 
the spec, but it sounds reasonable.


// 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 000/279 v2] New channel layout API

2021-12-17 Thread Marton Balint




On Thu, 16 Dec 2021, James Almer wrote:




On 12/16/2021 9:04 PM, Marton Balint wrote:



 On Thu, 16 Dec 2021, James Almer wrote:


 Resending the first two patches only, since this is meant to
 show the implementation of one of the several suggestions made
 in the previous set that need to be discussed and hopefully
 resolved in a call.


 Can you push the full branch somewhere?


Just force pushed the latest version to the same repo as last time in 
https://github.com/jamrial/FFmpeg/commits/channel_layout


Can you check the libfdk-aac patch? The decoder has some garbage text 
before the drc_boost context variable, the encoder does not compile with 
gcc because of the deprecation warning pragmas at the very end of the 
file.


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

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


Re: [FFmpeg-devel] [PATCH 000/279 v2] New channel layout API

2021-12-17 Thread James Almer



On 12/17/2021 4:20 PM, Marton Balint wrote:



On Thu, 16 Dec 2021, James Almer wrote:




On 12/16/2021 9:04 PM, Marton Balint wrote:



 On Thu, 16 Dec 2021, James Almer wrote:


 Resending the first two patches only, since this is meant to
 show the implementation of one of the several suggestions made
 in the previous set that need to be discussed and hopefully
 resolved in a call.


 Can you push the full branch somewhere?


Just force pushed the latest version to the same repo as last time in 
https://github.com/jamrial/FFmpeg/commits/channel_layout


Can you check the libfdk-aac patch? The decoder has some garbage text 
before the drc_boost context variable, the encoder does not compile with 
gcc because of the deprecation warning pragmas at the very end of the file.


Thanks,
Marton


Should be fixed, sorry about that.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH v3 2/5] build: detect Metal.framework and build .metal files

2021-12-17 Thread Aman Karmani
On Fri, Dec 17, 2021 at 12:54 AM Martin Storsjö  wrote:

> On Thu, 16 Dec 2021, Aman Karmani wrote:
>
> > From: Aman Karmani 
> >
> > Signed-off-by: Aman Karmani 
> > ---
> > .gitignore | 3 +++
> > configure  | 8 +++-
> > ffbuild/common.mak | 9 +
> > 3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/.gitignore b/.gitignore
> > index 9ed24b542e..1a5bb29ad5 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -19,6 +19,9 @@
> > *.swp
> > *.ver
> > *.version
> > +*.metal.air
> > +*.metallib
> > +*.metallib.c
> > *.ptx
> > *.ptx.c
> > *.ptx.gz
> > diff --git a/configure b/configure
> > index 5fffcb8afe..ab00b2d7cb 100755
> > --- a/configure
> > +++ b/configure
> > @@ -309,6 +309,7 @@ External library support:
> >if openssl, gnutls or libtls is not used [no]
> >   --enable-mediacodec  enable Android MediaCodec support [no]
> >   --enable-mediafoundation enable encoding via MediaFoundation [auto]
> > +  --disable-metal  disable Apple Metal framework [autodetect]
> >   --enable-libmysofa   enable libmysofa, needed for sofalizer filter
> [no]
> >   --enable-openal  enable OpenAL 1.1 capture support [no]
> >   --enable-opencl  enable OpenCL processing [no]
> > @@ -382,6 +383,7 @@ Toolchain options:
> >   --dep-cc=DEPCC   use dependency generator DEPCC [$cc_default]
> >   --nvcc=NVCC  use Nvidia CUDA compiler NVCC or clang
> [$nvcc_default]
> >   --ld=LD  use linker LD [$ld_default]
> > +  --metalcc=METALCCuse metal compiler METALCC [$metalcc_default]
> >   --pkg-config=PKGCONFIG   use pkg-config tool PKGCONFIG
> [$pkg_config_default]
> >   --pkg-config-flags=FLAGS pass additional flags to pkgconf []
> >   --ranlib=RANLIB  use ranlib RANLIB [$ranlib_default]
> > @@ -2564,6 +2566,7 @@ CMDLINE_SET="
> > ln_s
> > logfile
> > malloc_prefix
> > +metalcc
> > nm
> > optflags
> > nvcc
> > @@ -3835,6 +3838,7 @@ host_cc_default="gcc"
> > doxygen_default="doxygen"
> > install="install"
> > ln_s_default="ln -s -f"
> > +metalcc_default="xcrun metal"
> > nm_default="nm -g"
> > pkg_config_default=pkg-config
> > ranlib_default="ranlib"
> > @@ -4435,7 +4439,7 @@ if enabled cuda_nvcc; then
> > fi
> >
> > set_default arch cc cxx doxygen pkg_config ranlib strip sysinclude \
> > -target_exec x86asmexe
> > +target_exec x86asmexe metalcc
> > enabled cross_compile || host_cc_default=$cc
> > set_default host_cc
> >
> > @@ -6326,6 +6330,7 @@ check_apple_framework CoreFoundation
> > check_apple_framework CoreMedia
> > check_apple_framework CoreVideo
> > check_apple_framework CoreAudio
> > +check_apple_framework Metal
> >
> > enabled avfoundation && {
> > disable coregraphics applicationservices
> > @@ -7620,6 +7625,7 @@ ARFLAGS=$arflags
> > AR_O=$ar_o
> > AR_CMD=$ar
> > NM_CMD=$nm
> > +METALCC=$metalcc
> > RANLIB=$ranlib
> > STRIP=$strip
> > STRIPTYPE=$striptype
> > diff --git a/ffbuild/common.mak b/ffbuild/common.mak
> > index 0eb831d434..05440911f4 100644
> > --- a/ffbuild/common.mak
> > +++ b/ffbuild/common.mak
> > @@ -112,6 +112,15 @@ COMPILE_LASX = $(call COMPILE,CC,LASXFLAGS)
> > $(BIN2CEXE): ffbuild/bin2c_host.o
> >   $(HOSTLD) $(HOSTLDFLAGS) $(HOSTLD_O) $^ $(HOSTEXTRALIBS)
> >
> > +%.metal.air: %.metal
> > + $(METALCC) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) -o $@
> > +
> > +%.metallib: %.metal.air
> > + $(METALCC)lib --split-module-without-linking $(patsubst
> $(SRC_PATH)/%,$(SRC_LINK)/%,$<) -o $@
>
> Hmm, so does this try to run "xcrun metallib" instead of "xcrun metal"? I
> think that can be kinda brittle, e.g. if someone wants to configure a
> custom build env, where METALCC expands to e.g.
> "my-wrapped-metal-compiler.sh".


> I guess it feels a bit boring to need to define two separate variables,
> but if it really is two separate tools, then I think that'd be the best
> for clarity.
>

Good catch, I forgot about this little hack.

I agree and will split the variables.


>
> // 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".
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


[FFmpeg-devel] [PATCH v4 1/5] avfilter/vf_yadif_cuda: simplify filter definition

2021-12-17 Thread Aman Karmani
From: Aman Karmani 

Signed-off-by: Aman Karmani 
Signed-off-by: Philip Langdale 
---
 libavfilter/vf_yadif_cuda.c | 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/libavfilter/vf_yadif_cuda.c b/libavfilter/vf_yadif_cuda.c
index da1ab5a8ff..685b8a2035 100644
--- a/libavfilter/vf_yadif_cuda.c
+++ b/libavfilter/vf_yadif_cuda.c
@@ -212,23 +212,6 @@ static av_cold void deint_cuda_uninit(AVFilterContext *ctx)
 s->input_frames = NULL;
 }
 
-static int deint_cuda_query_formats(AVFilterContext *ctx)
-{
-enum AVPixelFormat pix_fmts[] = {
-AV_PIX_FMT_CUDA, AV_PIX_FMT_NONE,
-};
-int ret;
-
-if ((ret = ff_formats_ref(ff_make_format_list(pix_fmts),
-  &ctx->inputs[0]->outcfg.formats)) < 0)
-return ret;
-if ((ret = ff_formats_ref(ff_make_format_list(pix_fmts),
-  &ctx->outputs[0]->incfg.formats)) < 0)
-return ret;
-
-return 0;
-}
-
 static int config_input(AVFilterLink *inlink)
 {
 AVFilterContext *ctx = inlink->dst;
@@ -380,9 +363,9 @@ const AVFilter ff_vf_yadif_cuda = {
 .priv_size  = sizeof(DeintCUDAContext),
 .priv_class = &yadif_cuda_class,
 .uninit = deint_cuda_uninit,
+FILTER_SINGLE_PIXFMT(AV_PIX_FMT_CUDA),
 FILTER_INPUTS(deint_cuda_inputs),
 FILTER_OUTPUTS(deint_cuda_outputs),
-FILTER_QUERY_FUNC(deint_cuda_query_formats),
 .flags  = AVFILTER_FLAG_SUPPORT_TIMELINE_INTERNAL,
 .flags_internal = FF_FILTER_FLAG_HWFRAME_AWARE,
 };
-- 
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".


[FFmpeg-devel] [PATCH v4 2/5] build: detect Metal.framework and build .metal files

2021-12-17 Thread Aman Karmani
From: Aman Karmani 

Reviewed-by: Ridley Combs 
Signed-off-by: Aman Karmani 
---
 .gitignore |  3 +++
 configure  | 12 +++-
 ffbuild/common.mak |  9 +
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 9ed24b542e..1a5bb29ad5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,6 +19,9 @@
 *.swp
 *.ver
 *.version
+*.metal.air
+*.metallib
+*.metallib.c
 *.ptx
 *.ptx.c
 *.ptx.gz
diff --git a/configure b/configure
index 5fffcb8afe..32a39f5f5b 100755
--- a/configure
+++ b/configure
@@ -309,6 +309,7 @@ External library support:
if openssl, gnutls or libtls is not used [no]
   --enable-mediacodec  enable Android MediaCodec support [no]
   --enable-mediafoundation enable encoding via MediaFoundation [auto]
+  --disable-metal  disable Apple Metal framework [autodetect]
   --enable-libmysofa   enable libmysofa, needed for sofalizer filter [no]
   --enable-openal  enable OpenAL 1.1 capture support [no]
   --enable-opencl  enable OpenCL processing [no]
@@ -382,6 +383,8 @@ Toolchain options:
   --dep-cc=DEPCC   use dependency generator DEPCC [$cc_default]
   --nvcc=NVCC  use Nvidia CUDA compiler NVCC or clang 
[$nvcc_default]
   --ld=LD  use linker LD [$ld_default]
+  --metalcc=METALCCuse metal compiler METALCC [$metalcc_default]
+  --metallib=METALLIB  use metal linker METALLIB [$metallib_default]
   --pkg-config=PKGCONFIG   use pkg-config tool PKGCONFIG [$pkg_config_default]
   --pkg-config-flags=FLAGS pass additional flags to pkgconf []
   --ranlib=RANLIB  use ranlib RANLIB [$ranlib_default]
@@ -2564,6 +2567,8 @@ CMDLINE_SET="
 ln_s
 logfile
 malloc_prefix
+metalcc
+metallib
 nm
 optflags
 nvcc
@@ -3835,6 +3840,8 @@ host_cc_default="gcc"
 doxygen_default="doxygen"
 install="install"
 ln_s_default="ln -s -f"
+metalcc_default="xcrun metal"
+metallib_default="xcrun metallib"
 nm_default="nm -g"
 pkg_config_default=pkg-config
 ranlib_default="ranlib"
@@ -4435,7 +4442,7 @@ if enabled cuda_nvcc; then
 fi
 
 set_default arch cc cxx doxygen pkg_config ranlib strip sysinclude \
-target_exec x86asmexe
+target_exec x86asmexe metalcc metallib
 enabled cross_compile || host_cc_default=$cc
 set_default host_cc
 
@@ -6326,6 +6333,7 @@ check_apple_framework CoreFoundation
 check_apple_framework CoreMedia
 check_apple_framework CoreVideo
 check_apple_framework CoreAudio
+check_apple_framework Metal
 
 enabled avfoundation && {
 disable coregraphics applicationservices
@@ -7620,6 +7628,8 @@ ARFLAGS=$arflags
 AR_O=$ar_o
 AR_CMD=$ar
 NM_CMD=$nm
+METALCC=$metalcc
+METALLIB=$metallib
 RANLIB=$ranlib
 STRIP=$strip
 STRIPTYPE=$striptype
diff --git a/ffbuild/common.mak b/ffbuild/common.mak
index 0eb831d434..e79b509425 100644
--- a/ffbuild/common.mak
+++ b/ffbuild/common.mak
@@ -112,6 +112,15 @@ COMPILE_LASX = $(call COMPILE,CC,LASXFLAGS)
 $(BIN2CEXE): ffbuild/bin2c_host.o
$(HOSTLD) $(HOSTLDFLAGS) $(HOSTLD_O) $^ $(HOSTEXTRALIBS)
 
+%.metal.air: %.metal
+   $(METALCC) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) -o $@
+
+%.metallib: %.metal.air
+   $(METALLIB) --split-module-without-linking $(patsubst 
$(SRC_PATH)/%,$(SRC_LINK)/%,$<) -o $@
+
+%.metallib.c: %.metallib $(BIN2CEXE)
+   $(BIN2C) $(patsubst $(SRC_PATH)/%,$(SRC_LINK)/%,$<) $@ $(subst 
.,_,$(basename $(notdir $@)))
+
 %.ptx: %.cu $(SRC_PATH)/compat/cuda/cuda_runtime.h
$(COMPILE_NVCC)
 
-- 
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".


[FFmpeg-devel] [PATCH v4 3/5] avutil: add obj-c helpers into header-only include

2021-12-17 Thread Aman Karmani
From: Aman Karmani 

Reviewed-by: Ridley Combs 
Signed-off-by: Aman Karmani 
---
 libavutil/objc.h | 32 
 1 file changed, 32 insertions(+)
 create mode 100644 libavutil/objc.h

diff --git a/libavutil/objc.h b/libavutil/objc.h
new file mode 100644
index 00..0db993f716
--- /dev/null
+++ b/libavutil/objc.h
@@ -0,0 +1,32 @@
+/*
+ * 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_OBJC_H
+#define AVUTIL_OBJC_H
+
+#include 
+
+static inline void ff_objc_release(NSObject **obj)
+{
+if (*obj) {
+[*obj release];
+*obj = nil;
+}
+}
+
+#endif /* AVUTIL_OBJC_H */
-- 
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".


[FFmpeg-devel] [PATCH v4 4/5] avfilter: add metal utilities

2021-12-17 Thread Aman Karmani
From: Aman Karmani 

Reviewed-by: Ridley Combs 
Signed-off-by: Aman Karmani 
---
 libavfilter/metal/utils.h | 35 +++
 libavfilter/metal/utils.m | 73 +++
 2 files changed, 108 insertions(+)
 create mode 100644 libavfilter/metal/utils.h
 create mode 100644 libavfilter/metal/utils.m

diff --git a/libavfilter/metal/utils.h b/libavfilter/metal/utils.h
new file mode 100644
index 00..bd0319f63c
--- /dev/null
+++ b/libavfilter/metal/utils.h
@@ -0,0 +1,35 @@
+/*
+ * 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 AVFILTER_METAL_UTILS_H
+#define AVFILTER_METAL_UTILS_H
+
+#include 
+#include 
+
+void ff_metal_compute_encoder_dispatch(id device,
+   id pipeline,
+   id encoder,
+   NSUInteger width, NSUInteger height);
+
+CVMetalTextureRef ff_metal_texture_from_pixbuf(void *avclass,
+   CVMetalTextureCacheRef 
textureCache,
+   CVPixelBufferRef pixbuf,
+   int plane,
+   MTLPixelFormat format);
+#endif /* AVFILTER_METAL_UTILS_H */
diff --git a/libavfilter/metal/utils.m b/libavfilter/metal/utils.m
new file mode 100644
index 00..759ebedfba
--- /dev/null
+++ b/libavfilter/metal/utils.m
@@ -0,0 +1,73 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/log.h"
+#include 
+
+void ff_metal_compute_encoder_dispatch(id device,
+   id pipeline,
+   id encoder,
+   NSUInteger width, NSUInteger height)
+{
+[encoder setComputePipelineState:pipeline];
+NSUInteger w = pipeline.threadExecutionWidth;
+NSUInteger h = pipeline.maxTotalThreadsPerThreadgroup / w;
+MTLSize threadsPerThreadgroup = MTLSizeMake(w, h, 1);
+BOOL fallback = YES;
+if (@available(macOS 10.15, iOS 11, tvOS 14.5, *)) {
+if ([device supportsFamily:MTLGPUFamilyCommon3]) {
+MTLSize threadsPerGrid = MTLSizeMake(width, height, 1);
+[encoder dispatchThreads:threadsPerGrid 
threadsPerThreadgroup:threadsPerThreadgroup];
+fallback = NO;
+}
+}
+if (fallback) {
+MTLSize threadgroups = MTLSizeMake((width + w - 1) / w,
+   (height + h - 1) / h,
+   1);
+[encoder dispatchThreadgroups:threadgroups 
threadsPerThreadgroup:threadsPerThreadgroup];
+}
+}
+
+CVMetalTextureRef ff_metal_texture_from_pixbuf(void *ctx,
+   CVMetalTextureCacheRef 
textureCache,
+   CVPixelBufferRef pixbuf,
+   int plane,
+   MTLPixelFormat format)
+{
+CVMetalTextureRef tex = NULL;
+CVReturn ret;
+
+ret = CVMetalTextureCacheCreateTextureFromImage(
+NULL,
+textureCache,
+pixbuf,
+NULL,
+format,
+CVPixelBufferGetWidthOfPlane(pixbuf, plane),
+CVPixelBufferGetHeightOfPlane(pixbuf, plane),
+plane,
+&tex
+);
+if (ret != kCVReturnSuccess) {
+av_log(ctx, AV_LOG_ERROR, "Failed to create CVMetalTexture from image: 
%d\n", ret);
+return NULL;
+}
+

[FFmpeg-devel] [PATCH v4 5/5] avfilter: add vf_yadif_videotoolbox

2021-12-17 Thread Aman Karmani
From: Aman Karmani 

deinterlaces CVPixelBuffers, i.e. AV_PIX_FMT_VIDEOTOOLBOX frames

for example, an interlaced mpeg2 video can be decoded by avcodec,
uploaded into a CVPixelBuffer, deinterlaced by Metal, and then
encoded to h264 by VideoToolbox as follows:

ffmpeg \
   -init_hw_device videotoolbox \
   -i interlaced.ts \
   -vf hwupload,yadif_videotoolbox \
   -c:v h264_videotoolbox \
   -b:v 2000k \
   -c:a copy \
   -y progressive.ts

(note that uploading AVFrame into CVPixelBuffer via hwupload
 requires 504c60660d3194758823ddd45ceddb86e35d806f)

this work is sponsored by Fancy Bits LLC

Reviewed-by: Ridley Combs 
Signed-off-by: Aman Karmani 
---
 configure |   1 +
 libavfilter/Makefile  |   4 +
 libavfilter/allfilters.c  |   1 +
 libavfilter/metal/vf_yadif_videotoolbox.metal | 269 
 libavfilter/vf_yadif_videotoolbox.m   | 406 ++
 5 files changed, 681 insertions(+)
 create mode 100644 libavfilter/metal/vf_yadif_videotoolbox.metal
 create mode 100644 libavfilter/vf_yadif_videotoolbox.m

diff --git a/configure b/configure
index 32a39f5f5b..d8b07c8e00 100755
--- a/configure
+++ b/configure
@@ -3748,6 +3748,7 @@ vpp_qsv_filter_select="qsvvpp"
 xfade_opencl_filter_deps="opencl"
 yadif_cuda_filter_deps="ffnvcodec"
 yadif_cuda_filter_deps_any="cuda_nvcc cuda_llvm"
+yadif_videotoolbox_filter_deps="metal corevideo videotoolbox"
 
 # examples
 avio_list_dir_deps="avformat avutil"
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 2fe495df28..9a061ba3c8 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -519,6 +519,10 @@ OBJS-$(CONFIG_XSTACK_FILTER) += vf_stack.o 
framesync.o
 OBJS-$(CONFIG_YADIF_FILTER)  += vf_yadif.o yadif_common.o
 OBJS-$(CONFIG_YADIF_CUDA_FILTER) += vf_yadif_cuda.o 
vf_yadif_cuda.ptx.o \
 yadif_common.o 
cuda/load_helper.o
+OBJS-$(CONFIG_YADIF_VIDEOTOOLBOX_FILTER) += vf_yadif_videotoolbox.o \
+
metal/vf_yadif_videotoolbox.metallib.o \
+metal/utils.o \
+yadif_common.o
 OBJS-$(CONFIG_YAEPBLUR_FILTER)   += vf_yaepblur.o
 OBJS-$(CONFIG_ZMQ_FILTER)+= f_zmq.o
 OBJS-$(CONFIG_ZOOMPAN_FILTER)+= vf_zoompan.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index ec57a2c49c..26f1c73505 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -496,6 +496,7 @@ extern const AVFilter ff_vf_xmedian;
 extern const AVFilter ff_vf_xstack;
 extern const AVFilter ff_vf_yadif;
 extern const AVFilter ff_vf_yadif_cuda;
+extern const AVFilter ff_vf_yadif_videotoolbox;
 extern const AVFilter ff_vf_yaepblur;
 extern const AVFilter ff_vf_zmq;
 extern const AVFilter ff_vf_zoompan;
diff --git a/libavfilter/metal/vf_yadif_videotoolbox.metal 
b/libavfilter/metal/vf_yadif_videotoolbox.metal
new file mode 100644
index 00..50783f2ffe
--- /dev/null
+++ b/libavfilter/metal/vf_yadif_videotoolbox.metal
@@ -0,0 +1,269 @@
+/*
+ * Copyright (C) 2018 Philip Langdale 
+ *   2020 Aman Karmani 
+ *   2020 Stefan Dyulgerov 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include 
+#include 
+#include 
+
+using namespace metal;
+
+/*
+ * Parameters
+ */
+
+struct deintParams {
+uint channels;
+uint parity;
+uint tff;
+bool is_second_field;
+bool skip_spatial_check;
+int field_mode;
+};
+
+/*
+ * Texture access helpers
+ */
+
+#define accesstype access::sample
+const sampler s(coord::pixel);
+
+template 
+T tex2D(texture2d tex, uint x, uint y)
+{
+return tex.sample(s, float2(x, y)).x;
+}
+
+template <>
+float2 tex2D(texture2d tex, uint x, uint y)
+{
+return tex.sample(s, float2(x, y)).xy;
+}
+
+template 
+T tex2D(texture2d tex, uint x, uint y)
+{
+return tex.read(uint2(x, y)).x;
+}
+
+template <>
+float2 tex2D(texture2d tex, uint x, uint y)
+{
+return tex.read(uint2(x, y)).xy;
+}
+
+/*
+ * YADIF helpers
+ */
+
+template
+T spatial_predic

Re: [FFmpeg-devel] [PATCH] Add 32 bit-per-sample capability to FLAC encoder

2021-12-17 Thread Martijn van Beurden
Op vr 17 dec. 2021 om 12:43 schreef Michael Niedermayer
:
> > +sub->coefs[i] = sub->coefs[i]*0.98;
>  
> This is ugly, the amount of actual overflow should be known at this point
> so no arbitrary downscale should be needed here


Many thanks for the suggestion, I didn't think of using a more
intelligent approach. I'll work something out.


> > +if (!ff_flacdsp_lpc_encode_c_32_overflow_detect(res, smp, 
> > n, sub->order,
> > +
> > sub->coefs, sub->shift)) {
> > +sub->type = sub->type_code = FLAC_SUBFRAME_VERBATIM;
> > +memcpy(res, smp, n * sizeof(int32_t));
> > +return subframe_count_exact(s, sub, 0);
>
> How often does this occur ?


Depends on the content. On low dynamic range, distorted, overdriven
sounds like certain kinds of metal, this happens on about half of the
subframes. On most popular music, up to about 5%. On classical music
never. For the intended use (archiving of tape recordings with ample
headroom) probably never as well. A more intelligent approach as
suggested could bring these numbers down.
___
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 v10 1/2] avformat/imf: Demuxer

2021-12-17 Thread Lynne
Dec 17, 2021, 3:25 PM by an...@khirnov.net:

> Quoting Pierre-Anthony Lemieux (2021-12-15 21:41:25)
>
>> On Wed, Dec 15, 2021 at 12:20 PM Anton Khirnov  wrote:
>> >
>> > Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
>> > > >
>> > > > Now the question is whether a malicious attacker can craft those two
>> > > > files to get access to anything they shouldn't. I suppose at the very
>> > > > least the attacker can get information that the user opened the file 
>> > > > (by
>> > > > adding an asset on an attacker's server) but that will be a danger with
>> > > > any playlists allowing network resources and can be controlled with
>> > > > io_open(). Can you think of any other possible issues?
>> > > >
>> > >
>> > > Some security considerations:
>> > >
>> > > - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
>> > > distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
>> > > does not contain paths/hyperlinks and (b) only those resources
>> > > referenced by the CPL are fetched using the ASSETMAP.
>> > > - the CPL uses XML, which has its own security considerations. For
>> > > example, XML parsing can result in entities being fetched over the
>> > > network, but this is disabled by default in libxml AFAIK.
>> >
>> > This is concerning. From a brief glance at libxml2, it seems that you
>> > need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
>> > actually disabling network fetching.
>> > But it is possible I'm misreading the code, so if you or anyone else
>> > understands this better then clarifications are welcome.
>>
>> I was referring to entity expansion and the loading of DTDs being
>> disabled by default -- see XML_PARSE_NOENT and XML_PARSE_DTDLOAD at
>> [1-2].
>>
>
> Okay then. If nobody has further comments, I will push your latest patch
> in a few days.
>

I think this shouldn't get merged into 5.0. It would get minimal amount
of fuzzing if it does now, so let's leave it for a later release?
I'd still like to see libuuid being used, we have several uses for it already.

___
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 v10 1/2] avformat/imf: Demuxer

2021-12-17 Thread Pierre-Anthony Lemieux
On Fri, Dec 17, 2021 at 12:54 PM Lynne  wrote:
>
> Dec 17, 2021, 3:25 PM by an...@khirnov.net:
>
> > Quoting Pierre-Anthony Lemieux (2021-12-15 21:41:25)
> >
> >> On Wed, Dec 15, 2021 at 12:20 PM Anton Khirnov  wrote:
> >> >
> >> > Quoting Pierre-Anthony Lemieux (2021-12-15 01:17:26)
> >> > > >
> >> > > > Now the question is whether a malicious attacker can craft those two
> >> > > > files to get access to anything they shouldn't. I suppose at the very
> >> > > > least the attacker can get information that the user opened the file 
> >> > > > (by
> >> > > > adding an asset on an attacker's server) but that will be a danger 
> >> > > > with
> >> > > > any playlists allowing network resources and can be controlled with
> >> > > > io_open(). Can you think of any other possible issues?
> >> > > >
> >> > >
> >> > > Some security considerations:
> >> > >
> >> > > - a DDoS can conceivably occur if a malicious CPL+ASSETMAP is widely
> >> > > distributed. Both an ASSETMAP and a CPL are required since (a) the CPL
> >> > > does not contain paths/hyperlinks and (b) only those resources
> >> > > referenced by the CPL are fetched using the ASSETMAP.
> >> > > - the CPL uses XML, which has its own security considerations. For
> >> > > example, XML parsing can result in entities being fetched over the
> >> > > network, but this is disabled by default in libxml AFAIK.
> >> >
> >> > This is concerning. From a brief glance at libxml2, it seems that you
> >> > need to pass XML_PARSE_NONET as the last parameter to xmlReadMemory() to
> >> > actually disabling network fetching.
> >> > But it is possible I'm misreading the code, so if you or anyone else
> >> > understands this better then clarifications are welcome.
> >>
> >> I was referring to entity expansion and the loading of DTDs being
> >> disabled by default -- see XML_PARSE_NOENT and XML_PARSE_DTDLOAD at
> >> [1-2].
> >>
> >
> > Okay then. If nobody has further comments, I will push your latest patch
> > in a few days.
> >
>
> I think this shouldn't get merged into 5.0. It would get minimal amount
> of fuzzing if it does now, so let's leave it for a later release?

What amount of fuzzing are you looking for? Is there an HLS and/or
DASH equivalent? Happy to put in the work to make it happen.

> I'd still like to see libuuid being used, we have several uses for it already.

libuuid is not an obvious match with the IMF demuxer -- see details in
previous thread. I am happy to work on a UUID refactoring across
libavformat, but this is a separable task with impact on several other
modules.

>
> ___
> 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 v11 1/2] avformat/imf: Demuxer

2021-12-17 Thread Andreas Rheinhardt
p...@sandflow.com:
> From: Pierre-Anthony Lemieux 
> 
> Signed-off-by: Pierre-Anthony Lemieux 
> ---
> 
> Notes:
> The IMF demuxer accepts as input an IMF CPL. The assets referenced by the 
> CPL can be
> contained in multiple deliveries, each defined by an ASSETMAP file:
> 
> ffmpeg -assetmaps ,,... -i  CPL>
> 
> If -assetmaps is not specified, FFMPEG looks for a file called 
> ASSETMAP.xml in the same directory as the CPL.
> 
> EXAMPLE:
> ffmpeg -i 
> http://ffmpeg-imf-samples-public.s3-website-us-west-1.amazonaws.com/countdown/CPL_f5095caa-f204-4e1c-8a84-7af48c7ae16b.xml
>  out.mp4
> 
> The Interoperable Master Format (IMF) is a file-based media format for the
> delivery and storage of professional audio-visual masters.
> An IMF Composition consists of an XML playlist (the Composition Playlist)
> and a collection of MXF files (the Track Files). The Composition Playlist 
> (CPL)
> assembles the Track Files onto a timeline, which consists of multiple 
> tracks.
> The location of the Track Files referenced by the Composition Playlist is 
> stored
> in one or more XML documents called Asset Maps. More details at 
> https://www.imfug.com/explainer.
> The IMF standard was first introduced in 2013 and is managed by the SMPTE.
> 
> CHANGE NOTES:
> 
> - limit Track Files to MXF
> - remove stealth variable assignment
> - remove unused function parameter
> 
>  MAINTAINERS  |   1 +
>  configure|   3 +-
>  doc/demuxers.texi|   6 +
>  libavformat/Makefile |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/imf.h| 207 +
>  libavformat/imf_cpl.c| 783 +
>  libavformat/imfdec.c | 905 +++
>  8 files changed, 1906 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/imf.h
>  create mode 100644 libavformat/imf_cpl.c
>  create mode 100644 libavformat/imfdec.c
> 
> diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> new file mode 100644
> index 00..e7c094ebd7
> --- /dev/null
> +++ b/libavformat/imfdec.c
> @@ -0,0 +1,905 @@
> +/*
> + * 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
> + */
> +
> +/*
> + *
> + * Copyright (c) Sandflow Consulting LLC
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + *
> + * * Redistributions of source code must retain the above copyright notice, 
> this
> + *   list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright 
> notice,
> + *   this list of conditions and the following disclaimer in the 
> documentation
> + *   and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +/**
> + * Demuxes an IMF Composition
> + *
> + * References
> + * OV 2067-0:2018 - SMPTE Overview Document - Interoperable Master Format
> + * ST 2067-2:2020 - SMPTE Standard - Interoperable Master Format — Core 
> Constraints
> + * ST 2067-3:2020 - SMPTE Standard - Interoperable Master Format — 
> Composition Playlist
> + * ST 2067-5:2020 - SMPTE Standard - Interoperable Master Format — Essence 
> Component
> + * ST 2067-20:2016 - SMPTE Standard - Interoperable Master Format — 
> Application #2
> + * ST 2067-21:2020 - SMPT

Re: [FFmpeg-devel] [PATCH v4 3/3] libavdevice/avfoundation.m: Allow to select devices by unique ID.

2021-12-17 Thread Marvin Scholz




On 17 Dec 2021, at 16:12, Romain Beauxis wrote:


This is the third patch of a series of 3 that cleanup and enhance the
avfoundation implementation for libavdevice.

Changes:
v2: None
v3:
  * Switched unique ID to use system-prodvided unique ID
  * Implemented unique IDs for screen capture
v4: Cleanup

This patch adds a unique ID to avfoundation devices. This is needed
because device index can change while the machine is running when
devices are plugged or unplugged and device names can be tricky to use
with localization and etc.

Example of output:
./ffmpeg -f avfoundation -list_devices true -i ""
[...]
[AVFoundation indev @ 0x158705230] AVFoundation video devices:
[AVFoundation indev @ 0x158705230] [0] FaceTime HD Camera (ID:
47B4B64B70674B9CAD2BAE273A71F4B5)
[AVFoundation indev @ 0x158705230] [1] Capture screen 0 (ID:
AvfilterAvfoundationCaptureScreen1)
[AVFoundation indev @ 0x158705230] AVFoundation audio devices:
[AVFoundation indev @ 0x158705230] [0] Loopback Audio (ID:
com.rogueamoeba.Loopback.A5668B36-711E-4DF5-8A8D-7148508C735B)
[AVFoundation indev @ 0x158705230] [1] MacBook Pro Microphone (ID:
BuiltInMicrophoneDevice)

Notes:
* Unique names do not seem to follow any specific pattern. I have used
one similar to the builtin microphone for screen capture
* The : substitution is actually required. The loopback device above 
did

have it in its name.



Is there no way to escape the : in the command so that we would not
need to mess with the ID the system gives us?
And if we need to, it would be ideal to have a fully reversible way of
doing so, as then you could just reverse the mangling and use
`deviceWithUniqueID:` instead of iterating all devices.

That said, if thats not easily doable I am fine with the patch as-is,
aside from the minor comments below, thanks for your work on this.


Signed-off-by: Romain Beauxis 
---
 doc/indevs.texi|  6 ++--
 libavdevice/avfoundation.m | 72 
+-

 2 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 5be647f70a..2b55399c8c 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -114,7 +114,7 @@ The input filename has to be given in the 
following syntax:

 -i "[[VIDEO]:[AUDIO]]"
 @end example
 The first entry selects the video input while the latter selects the 
audio input.
-The stream has to be specified by the device name or the device index 
as shown by the device list.
+The stream has to be specified by the device name, index or ID as 
shown by the device list.
 Alternatively, the video and/or audio input device can be chosen by 
index using the

 @option{
 -video_device_index 
@@ -127,7 +127,9 @@ and/or
 device name or index given in the input filename.
  All available devices can be enumerated by using 
@option{-list_devices true}, listing

-all device names and corresponding indices.
+all device names, corresponding indices and IDs, when available. 
Device name can be +tricky to use when localized and device index can 
change when devices are plugged or unplugged. A device
+hash, when available, uniquely identifies a device and should not 
change over time.


This should say ID I think, as hash was never mentioned before.


  There are two device name aliases:
 @table @code
diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index b602cfbe95..25286507d6 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -39,6 +39,8 @@
 #include "libavutil/imgutils.h"
 #include "avdevice.h"
 +#define CLEANUP_DEVICE_ID(s) [[s 
stringByReplacingOccurrencesOfString:@":" withString:@"."] UTF8String]

+
 static const int avf_time_base = 100;
  static const AVRational avf_time_base_q = {
@@ -797,21 +799,23 @@ static int avf_read_header(AVFormatContext *s)
 int index = 0;
 av_log(ctx, AV_LOG_INFO, "AVFoundation video devices:\n");
 for (AVCaptureDevice *device in devices) {
-const char *name = [[device localizedName] UTF8String];
-index= [devices indexOfObject:device];
-av_log(ctx, AV_LOG_INFO, "[%d] %s\n", index, name);
+const char *name = [[device localizedName] 
UTF8String];
+const char *uniqueId = CLEANUP_DEVICE_ID([device 
uniqueID]);

+index= [devices indexOfObject:device];
+av_log(ctx, AV_LOG_INFO, "[%d] %s (ID: %s)\n", index, 
name, uniqueId);

 }
 for (AVCaptureDevice *device in devices_muxed) {
-const char *name = [[device localizedName] UTF8String];
-index= [devices count] + [devices_muxed 
indexOfObject:device];

-av_log(ctx, AV_LOG_INFO, "[%d] %s\n", index, name);
+const char *name = [[device localizedName] 
UTF8String];
+const char *uniqueId = CLEANUP_DEVICE_ID([device 
uniqueID]);
+index= [devices count] + [devices_muxed 
indexOfObject:device];
+av_l

[FFmpeg-devel] [PATCH 3/4] avcodec/tiff: Pass max_pixels to mjpeg context

2021-12-17 Thread Michael Niedermayer
Signed-off-by: Michael Niedermayer 
---
 libavcodec/tiff.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index 870e0666aa3..9af602eef70 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -2155,6 +2155,7 @@ static av_cold int tiff_init(AVCodecContext *avctx)
 s->avctx_mjpeg->flags2 = avctx->flags2;
 s->avctx_mjpeg->dct_algo = avctx->dct_algo;
 s->avctx_mjpeg->idct_algo = avctx->idct_algo;
+s->avctx_mjpeg->max_pixels = avctx->max_pixels;
 ret = avcodec_open2(s->avctx_mjpeg, codec, NULL);
 if (ret < 0) {
 return ret;
-- 
2.17.1

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

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


[FFmpeg-devel] [PATCH 4/4] avcodec/tiff: Use ff_set_dimensions() for setting up mjpeg context dimensions

2021-12-17 Thread Michael Niedermayer
Fixes: OOM
Fixes: 
42263/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-565619113984

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

diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index 9af602eef70..60773d59ed0 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -724,13 +724,14 @@ static int dng_decode_jpeg(AVCodecContext *avctx, AVFrame 
*frame,
 static int dng_decode_strip(AVCodecContext *avctx, AVFrame *frame)
 {
 TiffContext *s = avctx->priv_data;
+int ret = ff_set_dimensions(s->avctx_mjpeg, s->width, s->height);
+
+if (ret < 0)
+return ret;
 
 s->jpgframe->width  = s->width;
 s->jpgframe->height = s->height;
 
-s->avctx_mjpeg->width = s->width;
-s->avctx_mjpeg->height = s->height;
-
 return dng_decode_jpeg(avctx, frame, s->stripsize, 0, 0, s->width, 
s->height);
 }
 
@@ -971,14 +972,14 @@ static int dng_decode_tiles(AVCodecContext *avctx, 
AVFrame *frame,
 int has_width_leftover, has_height_leftover;
 int tile_x = 0, tile_y = 0;
 int pos_x = 0, pos_y = 0;
-int ret;
+int ret = ff_set_dimensions(s->avctx_mjpeg, s->tile_width, s->tile_length);
+
+if (ret < 0)
+return ret;
 
 s->jpgframe->width  = s->tile_width;
 s->jpgframe->height = s->tile_length;
 
-s->avctx_mjpeg->width = s->tile_width;
-s->avctx_mjpeg->height = s->tile_length;
-
 has_width_leftover = (s->width % s->tile_width != 0);
 has_height_leftover = (s->height % s->tile_length != 0);
 
-- 
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 2/4] avcodec/ccaption_dec: Use ff_ass_add_rect2()

2021-12-17 Thread Michael Niedermayer
Fixes: Timeout
Fixes: 
42258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CCAPTION_fuzzer-5540144118104064

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

diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index 27c61527f6e..2cd5b0aef0d 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -850,6 +850,7 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
 int len = avpkt->size;
 int ret = 0;
 int i;
+size_t nb_rect_allocated = 0;
 
 for (i = 0; i < len; i += 3) {
 uint8_t hi, cc_type = bptr[i] & 1;
@@ -886,7 +887,7 @@ static int decode(AVCodecContext *avctx, void *data, int 
*got_sub, AVPacket *avp
  AV_TIME_BASE_Q, ms_tb);
 else
 sub->end_display_time = -1;
-ret = ff_ass_add_rect(sub, ctx->buffer[bidx].str, 
ctx->readorder++, 0, NULL, NULL);
+ret = ff_ass_add_rect2(sub, ctx->buffer[bidx].str, 
ctx->readorder++, 0, NULL, NULL, &nb_rect_allocated);
 if (ret < 0)
 return ret;
 ctx->last_real_time = sub->pts;
-- 
2.17.1

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

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


[FFmpeg-devel] [PATCH 1/4] avcodec/ass: Faster ff_ass_add_rect()

2021-12-17 Thread Michael Niedermayer
Signed-off-by: Michael Niedermayer 
---
 libavcodec/ass.c | 32 ++--
 libavcodec/ass.h |  7 +++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/libavcodec/ass.c b/libavcodec/ass.c
index 725e4d42ba1..d0a4d678bb4 100644
--- a/libavcodec/ass.c
+++ b/libavcodec/ass.c
@@ -114,17 +114,30 @@ char *ff_ass_get_dialog(int readorder, int layer, const 
char *style,
speaker ? speaker : "", text);
 }
 
-int ff_ass_add_rect(AVSubtitle *sub, const char *dialog,
+int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog,
 int readorder, int layer, const char *style,
-const char *speaker)
+const char *speaker, size_t *nb_rect_allocated)
 {
-AVSubtitleRect **rects, *rect;
+AVSubtitleRect **rects = sub->rects, *rect;
 char *ass_str;
+uint64_t new_nb = 0;
 
-rects = av_realloc_array(sub->rects, sub->num_rects+1, 
sizeof(*sub->rects));
-if (!rects)
+if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) {
+new_nb = sub->num_rects + sub->num_rects/16LL + 1;
+} else if (!nb_rect_allocated)
+new_nb = sub->num_rects + 1LL;
+if (new_nb > SIZE_MAX)
 return AVERROR(ENOMEM);
-sub->rects = rects;
+
+if (new_nb) {
+rects = av_realloc_array(rects, new_nb, sizeof(*sub->rects));
+if (!rects)
+return AVERROR(ENOMEM);
+if (nb_rect_allocated)
+*nb_rect_allocated = new_nb;
+sub->rects = rects;
+}
+
 rect   = av_mallocz(sizeof(*rect));
 if (!rect)
 return AVERROR(ENOMEM);
@@ -137,6 +150,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog,
 return 0;
 }
 
+int ff_ass_add_rect(AVSubtitle *sub, const char *dialog,
+int readorder, int layer, const char *style,
+const char *speaker)
+{
+return ff_ass_add_rect2(sub, dialog, readorder, layer, style, speaker, 
NULL);
+}
+
 void ff_ass_decoder_flush(AVCodecContext *avctx)
 {
 FFASSDecoderContext *s = avctx->priv_data;
diff --git a/libavcodec/ass.h b/libavcodec/ass.h
index 2c260e4e785..784f46c42c5 100644
--- a/libavcodec/ass.h
+++ b/libavcodec/ass.h
@@ -118,6 +118,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog,
 int readorder, int layer, const char *style,
 const char *speaker);
 
+/**
+ * Add an ASS dialog to a subtitle.
+ */
+int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog,
+ int readorder, int layer, const char *style,
+ const char *speaker, size_t *nb_rect_allocated);
+
 /**
  * Helper to flush a text subtitles decoder making use of the
  * FFASSDecoderContext.
-- 
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 1/4] avcodec/ass: Faster ff_ass_add_rect()

2021-12-17 Thread Andreas Rheinhardt
Michael Niedermayer:
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/ass.c | 32 ++--
>  libavcodec/ass.h |  7 +++
>  2 files changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/ass.c b/libavcodec/ass.c
> index 725e4d42ba1..d0a4d678bb4 100644
> --- a/libavcodec/ass.c
> +++ b/libavcodec/ass.c
> @@ -114,17 +114,30 @@ char *ff_ass_get_dialog(int readorder, int layer, const 
> char *style,
> speaker ? speaker : "", text);
>  }
>  
> -int ff_ass_add_rect(AVSubtitle *sub, const char *dialog,
> +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog,
>  int readorder, int layer, const char *style,
> -const char *speaker)
> +const char *speaker, size_t *nb_rect_allocated)
>  {
> -AVSubtitleRect **rects, *rect;
> +AVSubtitleRect **rects = sub->rects, *rect;
>  char *ass_str;
> +uint64_t new_nb = 0;
>  
> -rects = av_realloc_array(sub->rects, sub->num_rects+1, 
> sizeof(*sub->rects));
> -if (!rects)
> +if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) {
> +new_nb = sub->num_rects + sub->num_rects/16LL + 1;
> +} else if (!nb_rect_allocated)
> +new_nb = sub->num_rects + 1LL;
> +if (new_nb > SIZE_MAX)
>  return AVERROR(ENOMEM);

AVSubtitle.num_rects is unsigned, so this number should always be
bounded by UINT_MAX (and nb_rect_allocated can be a pointer to unsigned,
too).
(Feels like I should resurrect my old av_fast_realloc_array:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200101132758.4452-1-andreas.rheinha...@gmail.com/)

> -sub->rects = rects;
> +
> +if (new_nb) {
> +rects = av_realloc_array(rects, new_nb, sizeof(*sub->rects));
> +if (!rects)
> +return AVERROR(ENOMEM);
> +if (nb_rect_allocated)
> +*nb_rect_allocated = new_nb;
> +sub->rects = rects;
> +}
> +
>  rect   = av_mallocz(sizeof(*rect));
>  if (!rect)
>  return AVERROR(ENOMEM);
> @@ -137,6 +150,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog,
>  return 0;
>  }
>  
> +int ff_ass_add_rect(AVSubtitle *sub, const char *dialog,
> +int readorder, int layer, const char *style,
> +const char *speaker)
> +{
> +return ff_ass_add_rect2(sub, dialog, readorder, layer, style, speaker, 
> NULL);
> +}
> +
>  void ff_ass_decoder_flush(AVCodecContext *avctx)
>  {
>  FFASSDecoderContext *s = avctx->priv_data;
> diff --git a/libavcodec/ass.h b/libavcodec/ass.h
> index 2c260e4e785..784f46c42c5 100644
> --- a/libavcodec/ass.h
> +++ b/libavcodec/ass.h
> @@ -118,6 +118,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog,
>  int readorder, int layer, const char *style,
>  const char *speaker);
>  
> +/**
> + * Add an ASS dialog to a subtitle.
> + */
> +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog,
> + int readorder, int layer, const char *style,
> + const char *speaker, size_t *nb_rect_allocated);
> +
>  /**
>   * Helper to flush a text subtitles decoder making use of the
>   * FFASSDecoderContext.
> 

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

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


Re: [FFmpeg-devel] [PATCH 1/2] tests/dnn: Make DNN tests regular libavfilter tests

2021-12-17 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> They test libavfilter internal API, so they should be libavfilter
> test programs (which implies: linked statically to libavfilter
> to access internal APIs and linked normally (statically or dynamically
> depending upon the build configuration) against all the other libs).
> 
> Right now, they are always linked statically against all libs,
> which is a significant size waste compared to shared libs as all
> of libavcodec has been pulled in despite not being really used.
> This also leads to linking failures on systems for which av_export_avutil
> is intended: libavcodec does not expect to be linked statically
> against the library providing avpriv_(cga|vga16)_font in this case.
> This is fixed by this commit.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavfilter/Makefile  |  3 ++
>  libavfilter/tests/.gitignore  |  8 +
>  .../tests/dnn-layer-avgpool.c |  0
>  .../tests/dnn-layer-conv2d.c  |  0
>  .../tests/dnn-layer-dense.c   |  0
>  .../tests/dnn-layer-depth2space.c |  0
>  .../tests/dnn-layer-mathbinary.c  |  0
>  .../tests/dnn-layer-mathunary.c   |  0
>  .../tests/dnn-layer-maximum.c |  0
>  .../tests/dnn-layer-pad.c |  0
>  tests/Makefile|  5 ++-
>  tests/dnn/.gitignore  |  8 -
>  tests/dnn/Makefile| 18 --
>  tests/fate/dnn.mak| 34 ++-
>  14 files changed, 31 insertions(+), 45 deletions(-)
>  rename tests/dnn/dnn-layer-avgpool-test.c => 
> libavfilter/tests/dnn-layer-avgpool.c (100%)
>  rename tests/dnn/dnn-layer-conv2d-test.c => 
> libavfilter/tests/dnn-layer-conv2d.c (100%)
>  rename tests/dnn/dnn-layer-dense-test.c => 
> libavfilter/tests/dnn-layer-dense.c (100%)
>  rename tests/dnn/dnn-layer-depth2space-test.c => 
> libavfilter/tests/dnn-layer-depth2space.c (100%)
>  rename tests/dnn/dnn-layer-mathbinary-test.c => 
> libavfilter/tests/dnn-layer-mathbinary.c (100%)
>  rename tests/dnn/dnn-layer-mathunary-test.c => 
> libavfilter/tests/dnn-layer-mathunary.c (100%)
>  rename tests/dnn/dnn-layer-maximum-test.c => 
> libavfilter/tests/dnn-layer-maximum.c (100%)
>  rename tests/dnn/dnn-layer-pad-test.c => libavfilter/tests/dnn-layer-pad.c 
> (100%)
>  delete mode 100644 tests/dnn/.gitignore
>  delete mode 100644 tests/dnn/Makefile
> 
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index f4f077af46..d39fab2304 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -587,6 +587,9 @@ SKIPHEADERS-$(CONFIG_VULKAN) += vulkan.h 
> vulkan_filter.h
>  
>  TOOLS = graph2dot
>  TESTPROGS = drawutils filtfmts formats integral
> +TESTPROGS-$(CONFIG_DNN) += dnn-layer-avgpool dnn-layer-conv2d 
> dnn-layer-dense  \
> +   dnn-layer-depth2space dnn-layer-mathbinary
>   \
> +   dnn-layer-mathunary dnn-layer-maximum 
> dnn-layer-pad \
>  
>  TOOLS-$(CONFIG_LIBZMQ) += zmqsend
>  
> diff --git a/libavfilter/tests/.gitignore b/libavfilter/tests/.gitignore
> index 65ef86f2e5..db482cd49b 100644
> --- a/libavfilter/tests/.gitignore
> +++ b/libavfilter/tests/.gitignore
> @@ -1,3 +1,11 @@
> +/dnn-layer-conv2d
> +/dnn-layer-depth2space
> +/dnn-layer-maximum
> +/dnn-layer-pad
> +/dnn-layer-mathbinary
> +/dnn-layer-mathunary
> +/dnn-layer-avgpool
> +/dnn-layer-dense
>  /drawutils
>  /filtfmts
>  /formats
> diff --git a/tests/dnn/dnn-layer-avgpool-test.c 
> b/libavfilter/tests/dnn-layer-avgpool.c
> similarity index 100%
> rename from tests/dnn/dnn-layer-avgpool-test.c
> rename to libavfilter/tests/dnn-layer-avgpool.c
> diff --git a/tests/dnn/dnn-layer-conv2d-test.c 
> b/libavfilter/tests/dnn-layer-conv2d.c
> similarity index 100%
> rename from tests/dnn/dnn-layer-conv2d-test.c
> rename to libavfilter/tests/dnn-layer-conv2d.c
> diff --git a/tests/dnn/dnn-layer-dense-test.c 
> b/libavfilter/tests/dnn-layer-dense.c
> similarity index 100%
> rename from tests/dnn/dnn-layer-dense-test.c
> rename to libavfilter/tests/dnn-layer-dense.c
> diff --git a/tests/dnn/dnn-layer-depth2space-test.c 
> b/libavfilter/tests/dnn-layer-depth2space.c
> similarity index 100%
> rename from tests/dnn/dnn-layer-depth2space-test.c
> rename to libavfilter/tests/dnn-layer-depth2space.c
> diff --git a/tests/dnn/dnn-layer-mathbinary-test.c 
> b/libavfilter/tests/dnn-layer-mathbinary.c
> similarity index 100%
> rename from tests/dnn/dnn-layer-mathbinary-test.c
> rename to libavfilter/tests/dnn-layer-mathbinary.c
> diff --git a/tests/dnn/dnn-layer-mathunary-test.c 
> b/libavfilter/tests/dnn-layer-mathunary.c
> similarity index 100%
> rename from tests/dnn/dnn-layer-mathunary-test.c
> rename to libavfilter/tests/dnn-layer-mathunary.c
> diff --git a/tests/dnn/dnn-layer-maximum-test.c 
> b/libavfilter/tests/dnn-layer-maximum.c
> similarity ind

Re: [FFmpeg-devel] [PATCH 01/11] avcodec/Makefile: Remove superfluous avformat->DNXHD dependencies

2021-12-17 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> There is no mxfenc dependency any more since commit
> b9a26b9d55f77ebbff3596e46be54bb5fed469d3.
> Also remove a dnxhddata.h inclusion in mxfenc that was forgotten
> in the very same commit.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/Makefile  | 2 --
>  libavformat/mxfenc.c | 1 -
>  2 files changed, 3 deletions(-)
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 4122a9b144..fb90ecea84 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -983,14 +983,12 @@ OBJS-$(CONFIG_VP8_QSV_HWACCEL)+= qsvdec.o
>  OBJS-$(CONFIG_ISO_MEDIA)   += mpeg4audio.o mpegaudiodata.o
>  
>  OBJS-$(CONFIG_ADTS_MUXER)  += mpeg4audio.o
> -OBJS-$(CONFIG_DNXHD_DEMUXER)   += dnxhddata.o
>  OBJS-$(CONFIG_FITS_DEMUXER)+= fits.o
>  OBJS-$(CONFIG_LATM_MUXER)  += mpeg4audio.o
>  OBJS-$(CONFIG_MATROSKA_AUDIO_MUXER)+= mpeg4audio.o
>  OBJS-$(CONFIG_MATROSKA_MUXER)  += mpeg4audio.o
>  OBJS-$(CONFIG_MOV_DEMUXER) += ac3tab.o
>  OBJS-$(CONFIG_MATROSKA_DEMUXER)+= mpeg4audio.o
> -OBJS-$(CONFIG_MXF_MUXER)   += dnxhddata.o
>  OBJS-$(CONFIG_NUT_MUXER)   += mpegaudiodata.o
>  OBJS-$(CONFIG_RTP_MUXER)   += mpeg4audio.o
>  OBJS-$(CONFIG_SPDIF_MUXER) += dca.o
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index fcd9afda2a..00bbe58149 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -48,7 +48,6 @@
>  #include "libavutil/pixdesc.h"
>  #include "libavutil/time_internal.h"
>  #include "libavcodec/bytestream.h"
> -#include "libavcodec/dnxhddata.h"
>  #include "libavcodec/dv_profile.h"
>  #include "libavcodec/h264_ps.h"
>  #include "libavcodec/golomb.h"
> 

Ping for this patchset.

- 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] avutil/twofish: Fixed decryption

2021-12-17 Thread Andreas Rheinhardt
Sebastian Kirmayer:
> The previous implementation swapped the two halves of the plaintext. The
> existing tests only decrypted data with a plaintext of all zeroes, which is
> not affected by swapping the halves. Tests which detect the old buggy behavior
> have been added.
> 
> Signed-off-by: Sebastian Kirmayer 
> ---
>  libavutil/tests/twofish.c | 15 ---
>  libavutil/twofish.c   |  8 
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/libavutil/tests/twofish.c b/libavutil/tests/twofish.c
> index 74e0926e..7e8b1292 100644
> --- a/libavutil/tests/twofish.c
> +++ b/libavutil/tests/twofish.c
> @@ -39,7 +39,7 @@ int main(int argc, char *argv[])
>  };
>  uint8_t temp[32], iv[16], rpt[32] = {0};
>  const int kbits[3] = {128, 192, 256};
> -int i, j, err = 0;
> +int i, j, k, err = 0;
>  struct AVTWOFISH *cs;
>  cs = av_twofish_alloc();
>  if (!cs)
> @@ -70,10 +70,19 @@ int main(int argc, char *argv[])
>  memcpy(Key+16,Key,(kbits[j]-128) >> 3);
>  memcpy(Key,rpt,16);
>  memcpy(rpt,temp,16);
> +av_twofish_crypt(cs, temp, temp, 1, NULL, 1);
> +for (k = 0; k < 16; k++) {
> +// Need to compare to Key here, because the plaintext comes
> +// from rpt but was moved over to Key.
> +if (Key[k] != temp[k]) {
> +av_log(NULL, AV_LOG_ERROR, "%d %02x %02x\n", k, Key[k], 
> temp[k]);
> +err = 1;
> +}
> +}
>  }
>  for (i = 0; i < 16; i++) {
> -if (rct[3 + j][i] != temp[i]) {
> -av_log(NULL, AV_LOG_ERROR, "%d %02x %02x\n", i, rct[3 + 
> j][i], temp[i]);
> +if (rct[3 + j][i] != rpt[i]) {
> +av_log(NULL, AV_LOG_ERROR, "%d %02x %02x\n", i, rct[3 + 
> j][i], rpt[i]);
>  err = 1;
>  }
>  }
> diff --git a/libavutil/twofish.c b/libavutil/twofish.c
> index d84fa4f3..649b4bc4 100644
> --- a/libavutil/twofish.c
> +++ b/libavutil/twofish.c
> @@ -260,10 +260,10 @@ static void twofish_decrypt(AVTWOFISH *cs, uint8_t 
> *dst, const uint8_t *src, uin
>  P[3] ^= AV_RL32(iv + 12);
>  memcpy(iv, src, 16);
>  }
> -AV_WL32(dst, P[2]);
> -AV_WL32(dst + 4, P[3]);
> -AV_WL32(dst + 8, P[0]);
> -AV_WL32(dst + 12, P[1]);
> +AV_WL32(dst, P[0]);
> +AV_WL32(dst + 4, P[1]);
> +AV_WL32(dst + 8, P[2]);
> +AV_WL32(dst + 12, P[3]);
>  }
>  
>  av_cold int av_twofish_init(AVTWOFISH *cs, const uint8_t *key, int key_bits)
> 

Confirmed the bug through testing with more interesting data than the
current test does. Will apply this on Sunday unless there are objections.
Thanks.

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