Re: [FFmpeg-devel] [PATCH] vf_pseudocolor.c: fix build warning by adding braces

2021-02-08 Thread Paul B Mahol
There is already similar patch by other developer.

On Mon, Feb 8, 2021 at 4:11 AM Guo, Yejun  wrote:

> the warning message is:
> warning: missing braces around initializer [-Wmissing-braces]
>
> Signed-off-by: Guo, Yejun 
> ---
>  libavfilter/vf_pseudocolor.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavfilter/vf_pseudocolor.c b/libavfilter/vf_pseudocolor.c
> index 192839342b..3416ab19a9 100644
> --- a/libavfilter/vf_pseudocolor.c
> +++ b/libavfilter/vf_pseudocolor.c
> @@ -104,8 +104,8 @@ static const Range full_range   = {0, 256};
>  static const Range spec1_range[] = {{0, 16}, {16, 236}, {236, 256}};
>  static const Range spec2_range[] = {{0, 16}, {16, 22}, {22, 226}, {226,
> 236}, {236, 256}};
>
> -static const Fill spec1_fills[] = {{0.5f, 0.f, .5f}, {-1.f, -1.f, -1.f},
> {1.f, 0.f, 0.f}};
> -static const Fill spec2_fills[] = {{0.5f, 0.f, .5f}, {0.f, 1.f, 1.f},
> {-1.f, -1.f, -1.f}, {1.f, 1.f, 0.f}, {1.f, 0.f, 0.f}};
> +static const Fill spec1_fills[] = {{{0.5f, 0.f, .5f}}, {{-1.f, -1.f,
> -1.f}}, {{1.f, 0.f, 0.f}}};
> +static const Fill spec2_fills[] = {{{0.5f, 0.f, .5f}}, {{0.f, 1.f, 1.f}},
> {{-1.f, -1.f, -1.f}}, {{1.f, 1.f, 0.f}}, {{1.f, 0.f, 0.f}}};
>
>  static const Curve curves[] =
>  {
> --
> 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 mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_thumbnail: add support for YUV and GBRP formats

2021-02-08 Thread Paul B Mahol
Will apply soon.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH 1/2] lavc/mpegvideo_enc: mark default_mv_penalty as const

2021-02-08 Thread Anton Khirnov
Nothing is ever written into it.
---
 libavcodec/mpegvideo_enc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 34dcf8c313..a27c80ca37 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -82,7 +82,7 @@ static int sse_mb(MpegEncContext *s);
 static void denoise_dct_c(MpegEncContext *s, int16_t *block);
 static int dct_quantize_trellis_c(MpegEncContext *s, int16_t *block, int n, 
int qscale, int *overflow);
 
-static uint8_t default_mv_penalty[MAX_FCODE + 1][MAX_DMV * 2 + 1];
+static const uint8_t default_mv_penalty[MAX_FCODE + 1][MAX_DMV * 2 + 1];
 static uint8_t default_fcode_tab[MAX_MV * 2 + 1];
 
 const AVOption ff_mpv_generic_options[] = {
-- 
2.28.0

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

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

[FFmpeg-devel] [PATCH 2/2] lavc/ac3enc: rename variable to avoid shadowing

2021-02-08 Thread Anton Khirnov
Harmless, but confusing.
---
 libavcodec/ac3enc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
index bae7405fff..a007723bf3 100644
--- a/libavcodec/ac3enc.c
+++ b/libavcodec/ac3enc.c
@@ -2527,8 +2527,8 @@ av_cold int ff_ac3_encode_init(AVCodecContext *avctx)
 }
 
 if (CONFIG_EAC3_ENCODER && s->eac3) {
-static AVOnce init_static_once = AV_ONCE_INIT;
-ff_thread_once(&init_static_once, ff_eac3_exponent_init);
+static AVOnce init_static_once_eac3 = AV_ONCE_INIT;
+ff_thread_once(&init_static_once_eac3, ff_eac3_exponent_init);
 s->output_frame_header = ff_eac3_output_frame_header;
 } else
 s->output_frame_header = ac3_output_frame_header;
-- 
2.28.0

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

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

Re: [FFmpeg-devel] [PATCH 2/2] lavc/ac3enc: rename variable to avoid shadowing

2021-02-08 Thread Paul B Mahol
LGTM
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/2] lavc/mpegvideo_enc: mark default_mv_penalty as const

2021-02-08 Thread Andreas Rheinhardt
Anton Khirnov:
> Nothing is ever written into it.
> ---
>  libavcodec/mpegvideo_enc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index 34dcf8c313..a27c80ca37 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -82,7 +82,7 @@ static int sse_mb(MpegEncContext *s);
>  static void denoise_dct_c(MpegEncContext *s, int16_t *block);
>  static int dct_quantize_trellis_c(MpegEncContext *s, int16_t *block, int n, 
> int qscale, int *overflow);
>  
> -static uint8_t default_mv_penalty[MAX_FCODE + 1][MAX_DMV * 2 + 1];
> +static const uint8_t default_mv_penalty[MAX_FCODE + 1][MAX_DMV * 2 + 1];
>  static uint8_t default_fcode_tab[MAX_MV * 2 + 1];
>  
>  const AVOption ff_mpv_generic_options[] = {
> 
If you do this, the array will be present in the binary and will
actually occupy memory if it is used; currently, all the pages can be
mapped to the zero page.

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

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

[FFmpeg-devel] [PATCH] avfilter: add monochrome video filter

2021-02-08 Thread Paul B Mahol
Signed-off-by: Paul B Mahol 
---
 doc/filters.texi|  27 
 libavfilter/Makefile|   1 +
 libavfilter/allfilters.c|   1 +
 libavfilter/vf_monochrome.c | 239 
 4 files changed, 268 insertions(+)
 create mode 100644 libavfilter/vf_monochrome.c

diff --git a/doc/filters.texi b/doc/filters.texi
index f50a776897..45afc7efb1 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -14794,6 +14794,33 @@ This filter supports the following commands:
 Syntax is same as option with same name.
 @end table
 
+@section monochrome
+Convert video to gray using custom color filter.
+
+A description of the accepted options follows.
+
+@table @option
+@item cb
+Set the chroma blue spot. Allowed range is from -1 to 1.
+Default value is 0.
+
+@item cr
+Set the chroma red spot. Allowed range is from -1 to 1.
+Default value is 0.
+
+@item size
+Set the color filter size. Allowed range is from .25 to 3.
+Default value is 1.
+
+@item high
+Set the highlights strength. Allowed range is from 0 to 1.
+Default value is 0.
+@end table
+
+@subsection Commands
+
+This filter supports the all above options as @ref{commands}.
+
 @section mpdecimate
 
 Drop frames that do not differ greatly from the previous frame in
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 5a611e7a9e..ad16ef 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -332,6 +332,7 @@ OBJS-$(CONFIG_METADATA_FILTER)   += f_metadata.o
 OBJS-$(CONFIG_MIDEQUALIZER_FILTER)   += vf_midequalizer.o framesync.o
 OBJS-$(CONFIG_MINTERPOLATE_FILTER)   += vf_minterpolate.o 
motion_estimation.o
 OBJS-$(CONFIG_MIX_FILTER)+= vf_mix.o framesync.o
+OBJS-$(CONFIG_MONOCHROME_FILTER) += vf_monochrome.o
 OBJS-$(CONFIG_MPDECIMATE_FILTER) += vf_mpdecimate.o
 OBJS-$(CONFIG_NEGATE_FILTER) += vf_lut.o
 OBJS-$(CONFIG_NLMEANS_FILTER)+= vf_nlmeans.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index 31546c268d..7fa01ff139 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -317,6 +317,7 @@ extern AVFilter ff_vf_metadata;
 extern AVFilter ff_vf_midequalizer;
 extern AVFilter ff_vf_minterpolate;
 extern AVFilter ff_vf_mix;
+extern AVFilter ff_vf_monochrome;
 extern AVFilter ff_vf_mpdecimate;
 extern AVFilter ff_vf_negate;
 extern AVFilter ff_vf_nlmeans;
diff --git a/libavfilter/vf_monochrome.c b/libavfilter/vf_monochrome.c
new file mode 100644
index 00..105b0e98ce
--- /dev/null
+++ b/libavfilter/vf_monochrome.c
@@ -0,0 +1,239 @@
+/*
+ * Copyright (c) 2021 Paul B Mahol
+ *
+ * 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 "libavutil/opt.h"
+#include "libavutil/imgutils.h"
+#include "avfilter.h"
+#include "formats.h"
+#include "internal.h"
+#include "video.h"
+
+typedef struct MonochromeContext {
+const AVClass *class;
+
+float b, r;
+float size;
+float high;
+
+int depth;
+
+int (*do_slice)(AVFilterContext *s, void *arg,
+int jobnr, int nb_jobs);
+} MonochromeContext;
+
+static float envelope(const float x)
+{
+const float beta = 0.6f;
+
+if (x < beta) {
+const float tmp = fabsf(x / beta - 1.f);
+
+return 1.f - tmp * tmp;
+} else {
+const float tmp = (1.f - x) / (1.f - beta);
+
+return tmp * tmp * (3.f - 2.f * tmp);
+}
+}
+
+static float filter(float b, float r, float u, float v, float size)
+{
+return expf(-av_clipf(((b - u) * (b - u) +
+   (r - v) * (r - v)) /
+size, 0.f, 1.f));
+}
+
+#define PROCESS()\
+float y = yptr[x] * imax;\
+float u = uptr[x] * imax - .5f;  \
+float v = vptr[x] * imax - .5f;  \
+float tt, t, ny; \
+ \
+ny = filter(b, r, u, v, size);   \
+tt = envelope(y);\
+t = tt + (1.f - tt) * (1.f - high);  \
+ny = (1.f - t) * y + t * ny * y;
+
+static int monochrome_slice8(AVFilterContext *ctx, void *arg, int jobnr, int 
nb_jobs)
+{
+MonochromeContext *s = ctx->priv;
+AVFram

[FFmpeg-devel] Re: [PATCH 3/4] ffprobe: stop printing deprecated fields

2021-02-08 Thread Anton Khirnov
Quoting Marton Balint (2021-01-28 00:38:22)
> 
> 
> On Wed, 27 Jan 2021, James Almer wrote:
> 
> > On 1/27/2021 7:42 PM, Michael Niedermayer wrote:
> >> On Tue, Jan 26, 2021 at 06:01:01PM +0100, Anton Khirnov wrote:
> >>> Also drop the sections guarded by #if FF_API*
> >>> These macros are private and should not be used by external callers.
> >>> ---
> >>>   fftools/ffprobe.c |  34 --
> >>>   .../ref/fate/concat-demuxer-extended-lavf-mxf |   2 +-
> >>>   .../fate/concat-demuxer-extended-lavf-mxf_d10 |   2 +-
> >>>   .../ref/fate/concat-demuxer-simple1-lavf-mxf  | 248 +-
> >>>   .../fate/concat-demuxer-simple1-lavf-mxf_d10  | 144 +++---
> >>>   tests/ref/fate/concat-demuxer-simple2-lavf-ts | 302 ++--
> >>>   tests/ref/fate/ffprobe_compact|  34 +-
> >>>   tests/ref/fate/ffprobe_csv|  34 +-
> >>>   tests/ref/fate/ffprobe_default|  42 --
> >>>   tests/ref/fate/ffprobe_flat   |  42 --
> >>>   tests/ref/fate/ffprobe_ini|  42 --
> >>>   tests/ref/fate/ffprobe_json   |   9 -
> >>>   tests/ref/fate/ffprobe_xml|   6 +-
> >>>   tests/ref/fate/flcl1905   |  34 +-
> >>>   ...hapqa-extract-nosnappy-to-hapalphaonly-mov |   8 -
> >>>   .../fate/hapqa-extract-nosnappy-to-hapq-mov   |   8 -
> >>>   tests/ref/fate/hls-fmp4_ac3   |   1 -
> >>>   tests/ref/fate/mov-aac-2048-priming   | 434 +-
> >>>   tests/ref/fate/mov-init-nonkeyframe   | 240 +-
> >>>   tests/ref/fate/mov-zombie | 134 +++---
> >>>   tests/ref/fate/mxf-probe-applehdr10   |  10 -
> >>>   tests/ref/fate/mxf-probe-d10  |   8 -
> >>>   tests/ref/fate/mxf-probe-dnxhd|   9 -
> >>>   tests/ref/fate/mxf-probe-dv25 |  10 -
> >>>   24 files changed, 807 insertions(+), 1030 deletions(-)
> >> 
> >> This decreases the amount of fields regression tested.
> >> Iam not sure if thats a good idea. Generally more testing is better
> >
> > These are all deprecated fields, so they will be gone with the bump.
> 
> I thought so too, but on a second look ist->dec_ctx is the codec used at 
> avcodec_open, so that still can be used, and the deprecation #ifs were put 
> there as a mistake or historically. Overall the way I see it these fields 
> can and should still be provided:
> 
>print_int("coded_width",  dec_ctx->coded_width);
>print_int("coded_height", dec_ctx->coded_height);
>print_int("closed_captions", !!(dec_ctx->properties & 
> FF_CODEC_PROPERTY_CLOSED_CAPTIONS));
>print_q("codec_time_base", dec_ctx->time_base, '/');

Use of AVCodecContext.time_base for decoding is deprecated, you are
supposed to read AVCodecContext.time_base instead. But that should be a
new item with a new name.

-- 
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] Re: [PATCH 3/4] ffprobe: stop printing deprecated fields

2021-02-08 Thread Anton Khirnov
Quoting Anton Khirnov (2021-02-08 12:21:57)
> Use of AVCodecContext.time_base for decoding is deprecated, you are
> supposed to read AVCodecContext.time_base instead. But that should be a
  ^
  framerate

Thanks to Andreas for pointing this out.

-- 
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 3/5] ffprobe: drop code accessing deprecated AVStream.codec

2021-02-08 Thread Anton Khirnov
---
 fftools/ffprobe.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index f7d042525e..83c036dd3f 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -3050,11 +3050,6 @@ static int open_input_file(InputFile *ifile, const char 
*filename,
 
 ist->dec_ctx->pkt_timebase = stream->time_base;
 ist->dec_ctx->framerate = stream->avg_frame_rate;
-#if FF_API_LAVF_AVCTX
-ist->dec_ctx->properties = stream->codec->properties;
-ist->dec_ctx->coded_width = stream->codec->coded_width;
-ist->dec_ctx->coded_height = stream->codec->coded_height;
-#endif
 
 if (avcodec_open2(ist->dec_ctx, codec, &opts) < 0) {
 av_log(NULL, AV_LOG_WARNING, "Could not open codec for input 
stream %d\n",
-- 
2.28.0

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

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

[FFmpeg-devel] [PATCH 1/5] ffprobe: remove an unnecessary deprecation guard

2021-02-08 Thread Anton Khirnov
The code it is guarding is not accessing anything deprecated
(disregarding the fact that a library caller must not use FF_API
deprecation guards).
---
 fftools/ffprobe.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 3453aa09ff..4e5beb5710 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2646,13 +2646,11 @@ static int show_stream(WriterContext *w, 
AVFormatContext *fmt_ctx, int stream_id
 case AVMEDIA_TYPE_VIDEO:
 print_int("width",par->width);
 print_int("height",   par->height);
-#if FF_API_LAVF_AVCTX
 if (dec_ctx) {
 print_int("coded_width",  dec_ctx->coded_width);
 print_int("coded_height", dec_ctx->coded_height);
 print_int("closed_captions", !!(dec_ctx->properties & 
FF_CODEC_PROPERTY_CLOSED_CAPTIONS));
 }
-#endif
 print_int("has_b_frames", par->video_delay);
 sar = av_guess_sample_aspect_ratio(fmt_ctx, stream, NULL);
 if (sar.num) {
-- 
2.28.0

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

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

[FFmpeg-devel] [PATCH 2/5] ffprobe: do not use deprecated AVStream.codec for max bitrate

2021-02-08 Thread Anton Khirnov
Use the decoder context instead.
---
 fftools/ffprobe.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 4e5beb5710..f7d042525e 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -2754,10 +2754,10 @@ static int show_stream(WriterContext *w, 
AVFormatContext *fmt_ctx, int stream_id
 print_time("duration",stream->duration, &stream->time_base);
 if (par->bit_rate > 0) print_val("bit_rate", par->bit_rate, 
unit_bit_per_second_str);
 else   print_str_opt("bit_rate", "N/A");
-#if FF_API_LAVF_AVCTX
-if (stream->codec->rc_max_rate > 0) print_val ("max_bit_rate", 
stream->codec->rc_max_rate, unit_bit_per_second_str);
-elseprint_str_opt("max_bit_rate", "N/A");
-#endif
+if (dec_ctx && dec_ctx->rc_max_rate > 0)
+print_val ("max_bit_rate", dec_ctx->rc_max_rate, 
unit_bit_per_second_str);
+else
+print_str_opt("max_bit_rate", "N/A");
 if (dec_ctx && dec_ctx->bits_per_raw_sample > 0) 
print_fmt("bits_per_raw_sample", "%d", dec_ctx->bits_per_raw_sample);
 else 
print_str_opt("bits_per_raw_sample", "N/A");
 if (stream->nb_frames) print_fmt("nb_frames", "%"PRId64, 
stream->nb_frames);
-- 
2.28.0

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

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

[FFmpeg-devel] [PATCH 5/5] ffprobe: stop setting AVCodecContext.framerate

2021-02-08 Thread Anton Khirnov
That field is supposed to be exported by decoders, it makes no sense for
a user to set it.
---
 fftools/ffprobe.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 6a3dd549ec..de70c20eb4 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -3034,7 +3034,6 @@ static int open_input_file(InputFile *ifile, const char 
*filename,
 }
 
 ist->dec_ctx->pkt_timebase = stream->time_base;
-ist->dec_ctx->framerate = stream->avg_frame_rate;
 
 if (avcodec_open2(ist->dec_ctx, codec, &opts) < 0) {
 av_log(NULL, AV_LOG_WARNING, "Could not open codec for input 
stream %d\n",
-- 
2.28.0

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

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

[FFmpeg-devel] [PATCH v3] avformat/concatdec: add support for setting input options

2021-02-08 Thread Jan Ekström
This way protocol or format related options can be set for all
of the files opened during concatenation both globally as well
as per-file.
---

Changes from v2:

1. Added an example, although I had issues figuring out something useful
   that is not a hack for something. Ended up doing a disablement of
   advanced edit list functionality, since that can sometimes lead to
   unwanted behavior.

   * First idea was to override the input format for a file without an
 extension. For that, we have no AVOption it seems.
   * Then came the idea of utilizing the framerate option in the raw
 h264 demuxer. That didn't work because apparently if there is a header
 in there that probed/parsed frame rate field gets utilized.
   * Third idea was either the one I picked, or analyzeduration/probesize
 for MPEG-TS. I opted for the mp4 case.

2. Quoted the : in documentation with @code{:}.
3. Fixed a duplicate space in a log message.
---

 doc/demuxers.texi   | 24 ++
 libavformat/concatdec.c | 69 -
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 3c15ab9eee..20601f9575 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -149,6 +149,14 @@ Metadata of the packets of the file. The specified 
metadata will be set for
 each file packet. You can specify this directive multiple times to add multiple
 metadata entries.
 
+@item @code{input_options @var{key=value:key2=value2}}
+Input options passed on when reading a specific file, using a 
@code{:}-separated
+list of key=value pairs. Requires @code{safe} to be non-positive. Global 
options
+for all files can be set with the @code{input_options} demuxer option. When 
using
+both options on the list of files as well as globally via the demuxer option,
+the global ones get applied first and the file-specific options are then 
applied
+on top of them.
+
 @item @code{stream}
 Introduce a stream in the virtual file.
 All subsequent stream-related directives apply to the last introduced
@@ -204,6 +212,10 @@ expressed in microseconds. The duration metadata is only 
set if it is known
 based on the concat file.
 The default is 0.
 
+@item input_options
+Input options to be passed on for all opened inputs using a :-separated list of
+key=value pairs.
+
 @end table
 
 @subsection Examples
@@ -231,6 +243,18 @@ duration 20.0
 
 file subdir/file-2.wav
 @end example
+
+@item
+Disabling advanced edit list capability for the first input file via
+input_options:
+@example
+ffconcat version 1.0
+
+file file-1.mp4
+input_options advanced_editlist=false
+
+file file-2.mp4
+@end example
 @end itemize
 
 @section dash
diff --git a/libavformat/concatdec.c b/libavformat/concatdec.c
index 6d5b9914f9..89d75cedc6 100644
--- a/libavformat/concatdec.c
+++ b/libavformat/concatdec.c
@@ -52,6 +52,7 @@ typedef struct {
 int64_t outpoint;
 AVDictionary *metadata;
 int nb_streams;
+AVDictionary *input_options;
 } ConcatFile;
 
 typedef struct {
@@ -66,6 +67,7 @@ typedef struct {
 ConcatMatchMode stream_match_mode;
 unsigned auto_convert;
 int segment_time_metadata;
+AVDictionary *input_options;
 } ConcatContext;
 
 static int concat_probe(const AVProbeData *probe)
@@ -329,6 +331,7 @@ static int open_file(AVFormatContext *avf, unsigned fileno)
 {
 ConcatContext *cat = avf->priv_data;
 ConcatFile *file = &cat->files[fileno];
+AVDictionary *options = NULL;
 int ret;
 
 if (cat->avf)
@@ -344,12 +347,37 @@ static int open_file(AVFormatContext *avf, unsigned 
fileno)
 if ((ret = ff_copy_whiteblacklists(cat->avf, avf)) < 0)
 return ret;
 
-if ((ret = avformat_open_input(&cat->avf, file->url, NULL, NULL)) < 0 ||
+// Apply global AVOptions first
+if (cat->input_options &&
+(ret = av_dict_copy(&options, cat->input_options, 0) < 0))
+return ret;
+
+// then apply file-specific AVOptions
+if (file->input_options &&
+(ret = av_dict_copy(&options, file->input_options, 0) < 0))
+return ret;
+
+if ((ret = avformat_open_input(&cat->avf, file->url, NULL, &options)) < 0 
||
 (ret = avformat_find_stream_info(cat->avf, NULL)) < 0) {
 av_log(avf, AV_LOG_ERROR, "Impossible to open '%s'\n", file->url);
 avformat_close_input(&cat->avf);
+av_dict_free(&options);
 return ret;
 }
+
+if (av_dict_count(options)) {
+AVDictionaryEntry *en = NULL;
+
+while ((en = av_dict_get(options, "", en, AV_DICT_IGNORE_SUFFIX))) {
+av_log(avf, AV_LOG_WARNING,
+   "Option '%s' set to '%s' was ignored when opening %s "
+   "with the %s reader!\n",
+   en->key, en->value, file->url, cat->avf->iformat->name);
+}
+}
+
+av_dict_free(&options);
+
 cat->cur_file = file;
 file->start_time = !fileno ? 0 :
cat->files[fileno - 1].start_time +
@@ -386,6 +4

Re: [FFmpeg-devel] [PATCH 1/5] ffprobe: remove an unnecessary deprecation guard

2021-02-08 Thread Paul B Mahol
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 3/5] ffprobe: drop code accessing deprecated AVStream.codec

2021-02-08 Thread Paul B Mahol
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 5/5] ffprobe: stop setting AVCodecContext.framerate

2021-02-08 Thread Paul B Mahol
probably ok
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 2/5] ffprobe: do not use deprecated AVStream.codec for max bitrate

2021-02-08 Thread Paul B Mahol
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Populate field order returned by avs script, Trac Ticket 8757

2021-02-08 Thread emcodem

Am 2021-02-01 09:44, schrieb emco...@ffastrans.com:

Am 2021-01-21 19:08, schrieb emco...@ffastrans.com:

On 2021-01-21 14:10, Stephen Hutchinson wrote:

Yeah, never mind about that.  I didn't notice that those are declared
AVSC_INLINE, not AVSC_API, so they don't get used through the dynamic
API loader.

The comment formatting seems to have been messed up in the second
version, though.

/* The following typically only works when assumetff (-bff) and
 * assumefieldbased is used in-script. Additional
 * logic using GetParity() could deliver more accurate results
 * but also decodes a frame which we want to avoid. */


OK, i have to admit formatting comments is in the top 10 of my
greatest weaknesses :D
Thanks for your patience and also thanks for telling me directly how
the formatting is done correctly.
New patch with formatted comment attached


Is it OK to ping this?


Sorry for Pinging again :-(
I know it's not much that the patch does but the resulting functionality 
from a user perspective can be really useful, especially when using 
programatically calculated avs scripts.From 243bb2cbae9fabc78fe7f48cb90ae1c620c51a51 Mon Sep 17 00:00:00 2001
From: emcodem 
Date: Thu, 21 Jan 2021 18:59:45 +0100
Subject: [PATCH] Populate field order returned by avs script, Trac Ticket 8757

---
 libavformat/avisynth.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 2c08ace8db..22ae8c0dd3 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -241,6 +241,23 @@ static int avisynth_create_stream_video(AVFormatContext *s, AVStream *st)
 st->nb_frames = avs->vi->num_frames;
 avpriv_set_pts_info(st, 32, avs->vi->fps_denominator, avs->vi->fps_numerator);
 
+av_log(s, AV_LOG_TRACE, "avs_is_field_based: %d\n", avs_is_field_based(avs->vi));
+av_log(s, AV_LOG_TRACE, "avs_is_parity_known: %d\n", avs_is_parity_known(avs->vi));
+
+/* The following typically only works when assumetff (-bff) and
+ * assumefieldbased is used in-script. Additional
+ * logic using GetParity() could deliver more accurate results
+ * but also decodes a frame which we want to avoid. */
+st->codecpar->field_order = AV_FIELD_UNKNOWN;
+if (avs_is_field_based(avs->vi)) {
+if (avs_is_tff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_TT;
+}
+else if (avs_is_bff(avs->vi)) {
+st->codecpar->field_order = AV_FIELD_BB;
+}
+}
+
 switch (avs->vi->pixel_type) {
 /* 10~16-bit YUV pix_fmts (AviSynth+) */
 case AVS_CS_YUV444P10:
-- 
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 4/4] avutil/common: Move everything inside inclusion guards

2021-02-08 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> libavutil/common.h is a public header that provides generic math
> functions whereas libavutil/intmath.h is a private header that contains
> plattform-specific optimized versions of said math functions. common.h
> includes intmath.h (when building the FFmpeg libraries) so that the
> optimized versions are used for them.
> 
> This interdependency sometimes causes trouble: intmath.h once contained
> an inlined ff_sqrt function that relied upon av_log2_16bit. In case there
> was no optimized logarithm available on this plattform, intmath.h needed
> to include common.h to get the generic implementation and this has been
> done after the optimized versions (if any) have been provided so that
> common.h used the optimized versions; it also needed to be done before
> ff_sqrt. Yet when intmath.h was included from common.h and if an ordinary
> inclusion guard was used by common.h, the #include "common.h" in intmath.h
> was a no-op and therefore av_log2_16bit was still unknown at the end of
> intmath.h (and also in ff_sqrt) if no optimized version was available.
> 
> Before a955b5965825631986ba854d007d4e934e466c7d this was solved by
> duplicating the #ifndef av_log2_16bit check after the inclusion of
> common.h in intmath.h; said commit instead moved these checks to the
> end of common.h, outside the inclusion guards and made common.h include
> itself to get these unguarded defines. This is still the current
> state of affairs.
> 
> Yet this is unnecessary since 9734b8ba56d05e970c353dfd5baafa43fdb08024
> as said commit removed ff_sqrt as well as the #include "common.h" from
> intmath.h. Therefore this commit moves everything inside the inclusion
> guards and makes common.h not include itself.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavutil/common.h | 140 +
>  1 file changed, 66 insertions(+), 74 deletions(-)
> 
> diff --git a/libavutil/common.h b/libavutil/common.h
> index a60a558b1d..fde90182ee 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -114,8 +114,72 @@
>  #   include "intmath.h"
>  #endif
>  
> -/* Pull in unguarded fallback defines at the end of this file. */
> -#include "common.h"
> +#ifndef av_ceil_log2
> +#   define av_ceil_log2 av_ceil_log2_c
> +#endif
> +#ifndef av_clip
> +#   define av_clip  av_clip_c
> +#endif
> +#ifndef av_clip64
> +#   define av_clip64av_clip64_c
> +#endif
> +#ifndef av_clip_uint8
> +#   define av_clip_uint8av_clip_uint8_c
> +#endif
> +#ifndef av_clip_int8
> +#   define av_clip_int8 av_clip_int8_c
> +#endif
> +#ifndef av_clip_uint16
> +#   define av_clip_uint16   av_clip_uint16_c
> +#endif
> +#ifndef av_clip_int16
> +#   define av_clip_int16av_clip_int16_c
> +#endif
> +#ifndef av_clipl_int32
> +#   define av_clipl_int32   av_clipl_int32_c
> +#endif
> +#ifndef av_clip_intp2
> +#   define av_clip_intp2av_clip_intp2_c
> +#endif
> +#ifndef av_clip_uintp2
> +#   define av_clip_uintp2   av_clip_uintp2_c
> +#endif
> +#ifndef av_mod_uintp2
> +#   define av_mod_uintp2av_mod_uintp2_c
> +#endif
> +#ifndef av_sat_add32
> +#   define av_sat_add32 av_sat_add32_c
> +#endif
> +#ifndef av_sat_dadd32
> +#   define av_sat_dadd32av_sat_dadd32_c
> +#endif
> +#ifndef av_sat_sub32
> +#   define av_sat_sub32 av_sat_sub32_c
> +#endif
> +#ifndef av_sat_dsub32
> +#   define av_sat_dsub32av_sat_dsub32_c
> +#endif
> +#ifndef av_sat_add64
> +#   define av_sat_add64 av_sat_add64_c
> +#endif
> +#ifndef av_sat_sub64
> +#   define av_sat_sub64 av_sat_sub64_c
> +#endif
> +#ifndef av_clipf
> +#   define av_clipf av_clipf_c
> +#endif
> +#ifndef av_clipd
> +#   define av_clipd av_clipd_c
> +#endif
> +#ifndef av_popcount
> +#   define av_popcount  av_popcount_c
> +#endif
> +#ifndef av_popcount64
> +#   define av_popcount64av_popcount64_c
> +#endif
> +#ifndef av_parity
> +#   define av_parityav_parity_c
> +#endif
>  
>  #ifndef av_log2
>  av_const int av_log2(unsigned v);
> @@ -541,75 +605,3 @@ static av_always_inline av_const int 
> av_parity_c(uint32_t v)
>  #endif /* HAVE_AV_CONFIG_H */
>  
>  #endif /* AVUTIL_COMMON_H */
> -
> -/*
> - * The following definitions are outside the multiple inclusion guard
> - * to ensure they are immediately available in intmath.h.
> - */
> -
> -#ifndef av_ceil_log2
> -#   define av_ceil_log2 av_ceil_log2_c
> -#endif
> -#ifndef av_clip
> -#   define av_clip  av_clip_c
> -#endif
> -#ifndef av_clip64
> -#   define av_clip64av_clip64_c
> -#endif
> -#ifndef av_clip_uint8
> -#   define av_clip_uint8av_clip_uint8_c
> -#endif
> -#ifndef av_clip_int8
> -#   define av_clip_int8 av_clip_int8_c
> -#endif
> -#ifndef av_clip_uint16
> -#   define av_clip_uint16   av_clip_uint16_c
> -#endif
> -#ifndef av_clip_int16
> -#   define av_clip_int16av_clip_int16_c
> -#endif
> -#ifndef av_clipl_int32
> -#   define av_clipl_int32   av_clipl_int32_c
> -#endif
> -#ifndef av_clip_intp2
> -#   d

Re: [FFmpeg-devel] [PATCH] avcodec/g722enc: Validate parameters before using them

2021-02-08 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> In case trellis is outside of 0..23, an invalid shift and/or a signed
> integer overflow happens; furthermore, it can lead to the request to
> allocate nonsense amounts of memory. So validate first.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/g722enc.c | 25 -
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/g722enc.c b/libavcodec/g722enc.c
> index 9357f170fe..9e2ebf67c5 100644
> --- a/libavcodec/g722enc.c
> +++ b/libavcodec/g722enc.c
> @@ -64,19 +64,6 @@ static av_cold int g722_encode_init(AVCodecContext * avctx)
>  c->band[1].scale_factor = 2;
>  c->prev_samples_pos = 22;
>  
> -if (avctx->trellis) {
> -int frontier = 1 << avctx->trellis;
> -int max_paths = frontier * FREEZE_INTERVAL;
> -int i;
> -for (i = 0; i < 2; i++) {
> -c->paths[i] = av_mallocz_array(max_paths, sizeof(**c->paths));
> -c->node_buf[i] = av_mallocz_array(frontier, 2 * 
> sizeof(**c->node_buf));
> -c->nodep_buf[i] = av_mallocz_array(frontier, 2 * 
> sizeof(**c->nodep_buf));
> -if (!c->paths[i] || !c->node_buf[i] || !c->nodep_buf[i])
> -return AVERROR(ENOMEM);
> -}
> -}
> -
>  if (avctx->frame_size) {
>  /* validate frame size */
>  if (avctx->frame_size & 1 || avctx->frame_size > MAX_FRAME_SIZE) {
> @@ -110,6 +97,18 @@ static av_cold int g722_encode_init(AVCodecContext * 
> avctx)
> avctx->trellis);
>  avctx->trellis = new_trellis;
>  }
> +if (avctx->trellis) {
> +int frontier = 1 << avctx->trellis;
> +int max_paths = frontier * FREEZE_INTERVAL;
> +
> +for (int i = 0; i < 2; i++) {
> +c->paths[i] = av_calloc(max_paths, sizeof(**c->paths));
> +c->node_buf[i]  = av_calloc(frontier, 2 * 
> sizeof(**c->node_buf));
> +c->nodep_buf[i] = av_calloc(frontier, 2 * 
> sizeof(**c->nodep_buf));
> +if (!c->paths[i] || !c->node_buf[i] || !c->nodep_buf[i])
> +return AVERROR(ENOMEM);
> +}
> +}
>  }
>  
>  ff_g722dsp_init(&c->dsp);
> 
Will apply later today unless there are objections.

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

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

[FFmpeg-devel] [PATCH 1/8] avcodec/frame_thread_encoder: Improve type safety

2021-02-08 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/frame_thread_encoder.c | 15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index 83229f620a..ee289c90e3 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -35,8 +35,8 @@
 #define BUFFER_SIZE (2*MAX_THREADS)
 
 typedef struct{
-void *indata;
-void *outdata;
+AVFrame  *indata;
+AVPacket *outdata;
 int64_t return_code;
 unsigned index;
 } Task;
@@ -255,19 +255,12 @@ void ff_frame_thread_encoder_free(AVCodecContext *avctx){
 
 while (av_fifo_size(c->task_fifo) > 0) {
 Task task;
-AVFrame *frame;
 av_fifo_generic_read(c->task_fifo, &task, sizeof(task), NULL);
-frame = task.indata;
-av_frame_free(&frame);
-task.indata = NULL;
+av_frame_free(&task.indata);
 }
 
 for (i=0; ifinished_tasks[i].outdata != NULL) {
-AVPacket *pkt = c->finished_tasks[i].outdata;
-av_packet_free(&pkt);
-c->finished_tasks[i].outdata = NULL;
-}
+av_packet_free(&c->finished_tasks[i].outdata);
 }
 
 pthread_mutex_destroy(&c->task_fifo_mutex);
-- 
2.27.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 2/8] avcodec/frame_thread_encoder: Fix segfault on allocation error

2021-02-08 Thread Andreas Rheinhardt
Fixes a segfault from av_fifo_size(NULL) that happens in
ff_frame_thread_encoder_free if the fifo couldn't be allocted;
furthermore the mutexes and conditions that are destroyed in
ff_frame_thread_encoder_free are not even initialized at this point,
so don't call said function.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/frame_thread_encoder.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index ee289c90e3..9ca34e7ffb 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -182,8 +182,10 @@ int ff_frame_thread_encoder_init(AVCodecContext *avctx, 
AVDictionary *options){
 c->parent_avctx = avctx;
 
 c->task_fifo = av_fifo_alloc_array(BUFFER_SIZE, sizeof(Task));
-if(!c->task_fifo)
-goto fail;
+if (!c->task_fifo) {
+av_freep(&avctx->internal->frame_thread_encoder);
+return AVERROR(ENOMEM);
+}
 
 pthread_mutex_init(&c->task_fifo_mutex, NULL);
 pthread_mutex_init(&c->finished_task_mutex, NULL);
-- 
2.27.0

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

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

[FFmpeg-devel] [PATCH 3/8] avcodec/frame_thread_encoder: Avoid allocations of AVPackets, fix deadlock

2021-02-08 Thread Andreas Rheinhardt
Up until now, when doing frame thread encoding, each worker thread
tried to allocate an AVPacket for every AVFrame to be encoded; said
packets would then be handed back to the main thread, where the content
of said packet is copied into the packet actually destined for output;
the temporary AVPacket is then freed.

Besides being wasteful this also has another problem: There is a risk of
deadlock, namely if no AVPacket can be allocated at all. The user
doesn't get an error at all in this case and the worker threads will
simply try to allocate a packet again and again. If the user has
supplied enough frames, the user's thread will block until a task has
been completed, which just doesn't happen if no packet can ever be
allocated.

This patch instead modifies the code to allocate the packets during
init; they are then reused again and again.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/frame_thread_encoder.c | 61 +++
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index 9ca34e7ffb..bcd3c94f8b 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -32,13 +32,18 @@
 #include "thread.h"
 
 #define MAX_THREADS 64
-#define BUFFER_SIZE (2*MAX_THREADS)
+/* There can be as many as MAX_THREADS + 1 outstanding tasks.
+ * An additional + 1 is needed so that one can distinguish
+ * the case of zero and MAX_THREADS + 1 outstanding tasks modulo
+ * the number of buffers. */
+#define BUFFER_SIZE (MAX_THREADS + 2)
 
 typedef struct{
 AVFrame  *indata;
 AVPacket *outdata;
 int64_t return_code;
 unsigned index;
+int   finished;
 } Task;
 
 typedef struct{
@@ -49,8 +54,9 @@ typedef struct{
 pthread_mutex_t task_fifo_mutex;
 pthread_cond_t task_fifo_cond;
 
-Task finished_tasks[BUFFER_SIZE];
-pthread_mutex_t finished_task_mutex;
+unsigned max_tasks;
+Task tasks[BUFFER_SIZE];
+pthread_mutex_t finished_task_mutex; /* Guards tasks[i].finished */
 pthread_cond_t finished_task_cond;
 
 unsigned task_index;
@@ -63,17 +69,13 @@ typedef struct{
 static void * attribute_align_arg worker(void *v){
 AVCodecContext *avctx = v;
 ThreadContext *c = avctx->internal->frame_thread_encoder;
-AVPacket *pkt = NULL;
 
 while (!atomic_load(&c->exit)) {
 int got_packet = 0, ret;
+AVPacket *pkt;
 AVFrame *frame;
 Task task;
 
-if(!pkt) pkt = av_packet_alloc();
-if(!pkt) continue;
-av_init_packet(pkt);
-
 pthread_mutex_lock(&c->task_fifo_mutex);
 while (av_fifo_size(c->task_fifo) <= 0 || atomic_load(&c->exit)) {
 if (atomic_load(&c->exit)) {
@@ -84,7 +86,12 @@ static void * attribute_align_arg worker(void *v){
 }
 av_fifo_generic_read(c->task_fifo, &task, sizeof(task), NULL);
 pthread_mutex_unlock(&c->task_fifo_mutex);
+/* The main thread ensures that any two outstanding tasks have
+ * different indices, ergo each worker thread owns its element
+ * of c->tasks with the exception of finished, which is shared
+ * with the main thread and guarded by finished_task_mutex. */
 frame = task.indata;
+pkt   = c->tasks[task.index].outdata;
 
 ret = avctx->codec->encode2(avctx, pkt, frame, &got_packet);
 if(got_packet) {
@@ -101,13 +108,12 @@ static void * attribute_align_arg worker(void *v){
 pthread_mutex_unlock(&c->buffer_mutex);
 av_frame_free(&frame);
 pthread_mutex_lock(&c->finished_task_mutex);
-c->finished_tasks[task.index].outdata = pkt; pkt = NULL;
-c->finished_tasks[task.index].return_code = ret;
+c->tasks[task.index].return_code = ret;
+c->tasks[task.index].finished= 1;
 pthread_cond_signal(&c->finished_task_cond);
 pthread_mutex_unlock(&c->finished_task_mutex);
 }
 end:
-av_free(pkt);
 pthread_mutex_lock(&c->buffer_mutex);
 avcodec_close(avctx);
 pthread_mutex_unlock(&c->buffer_mutex);
@@ -194,6 +200,12 @@ int ff_frame_thread_encoder_init(AVCodecContext *avctx, 
AVDictionary *options){
 pthread_cond_init(&c->finished_task_cond, NULL);
 atomic_init(&c->exit, 0);
 
+c->max_tasks = avctx->thread_count + 2;
+for (unsigned i = 0; i < c->max_tasks; i++) {
+if (!(c->tasks[i].outdata = av_packet_alloc()))
+goto fail;
+}
+
 for(i=0; ithread_count ; i++){
 AVDictionary *tmp = NULL;
 int ret;
@@ -261,8 +273,8 @@ void ff_frame_thread_encoder_free(AVCodecContext *avctx){
 av_frame_free(&task.indata);
 }
 
-for (i=0; ifinished_tasks[i].outdata);
+for (unsigned i = 0; i < c->max_tasks; i++) {
+av_packet_free(&c->tasks[i].outdata);
 }
 
 pthread_mutex_destroy(&c->task_fifo_mutex);
@@ -276,7 +288,7 @@ void ff_frame_thread_encoder_free(AVCodecContext *avctx){
 
 int ff_thread_video_

[FFmpeg-devel] [PATCH 5/8] avcodec/frame_thread_encoder: Avoid allocations of AVFrames

2021-02-08 Thread Andreas Rheinhardt
Up until now, when using frame threaded encoding, an AVFrame would be
allocated for every frame to be encoded. These AVFrames would reach the
worker threads via a FIFO of tasks, a structure which contained the
AVFrame as well as an index into an array which gives the place where
the worker thread shall put the returned packet; in addition to that,
said structure also contained several unused fields.

This commit changes this: The AVFrames are now allocated during init in
the array that is up until now only used to return the packets. The
contents to be encoded are put into the AVFrame in the same array
element that is also used to return the packets.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/frame_thread_encoder.c | 40 +--
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index 7c2894c933..5fe886aed9 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -42,7 +42,6 @@ typedef struct{
 AVFrame  *indata;
 AVPacket *outdata;
 int64_t return_code;
-unsigned index;
 int   finished;
 } Task;
 
@@ -74,7 +73,8 @@ static void * attribute_align_arg worker(void *v){
 int got_packet = 0, ret;
 AVPacket *pkt;
 AVFrame *frame;
-Task task;
+Task *task;
+unsigned task_index;
 
 pthread_mutex_lock(&c->task_fifo_mutex);
 while (av_fifo_size(c->task_fifo) <= 0 || atomic_load(&c->exit)) {
@@ -84,14 +84,15 @@ static void * attribute_align_arg worker(void *v){
 }
 pthread_cond_wait(&c->task_fifo_cond, &c->task_fifo_mutex);
 }
-av_fifo_generic_read(c->task_fifo, &task, sizeof(task), NULL);
+av_fifo_generic_read(c->task_fifo, &task_index, sizeof(task_index), 
NULL);
 pthread_mutex_unlock(&c->task_fifo_mutex);
 /* The main thread ensures that any two outstanding tasks have
  * different indices, ergo each worker thread owns its element
  * of c->tasks with the exception of finished, which is shared
  * with the main thread and guarded by finished_task_mutex. */
-frame = task.indata;
-pkt   = c->tasks[task.index].outdata;
+task  = &c->tasks[task_index];
+frame = task->indata;
+pkt   = task->outdata;
 
 ret = avctx->codec->encode2(avctx, pkt, frame, &got_packet);
 if(got_packet) {
@@ -106,10 +107,9 @@ static void * attribute_align_arg worker(void *v){
 pthread_mutex_lock(&c->buffer_mutex);
 av_frame_unref(frame);
 pthread_mutex_unlock(&c->buffer_mutex);
-av_frame_free(&frame);
 pthread_mutex_lock(&c->finished_task_mutex);
-c->tasks[task.index].return_code = ret;
-c->tasks[task.index].finished= 1;
+task->return_code = ret;
+task->finished= 1;
 pthread_cond_signal(&c->finished_task_cond);
 pthread_mutex_unlock(&c->finished_task_mutex);
 }
@@ -187,7 +187,7 @@ int ff_frame_thread_encoder_init(AVCodecContext *avctx, 
AVDictionary *options){
 
 c->parent_avctx = avctx;
 
-c->task_fifo = av_fifo_alloc_array(BUFFER_SIZE, sizeof(Task));
+c->task_fifo = av_fifo_alloc_array(BUFFER_SIZE, sizeof(unsigned));
 if (!c->task_fifo) {
 av_freep(&avctx->internal->frame_thread_encoder);
 return AVERROR(ENOMEM);
@@ -202,7 +202,8 @@ int ff_frame_thread_encoder_init(AVCodecContext *avctx, 
AVDictionary *options){
 
 c->max_tasks = avctx->thread_count + 2;
 for (unsigned i = 0; i < c->max_tasks; i++) {
-if (!(c->tasks[i].outdata = av_packet_alloc()))
+if (!(c->tasks[i].indata  = av_frame_alloc()) ||
+!(c->tasks[i].outdata = av_packet_alloc()))
 goto fail;
 }
 
@@ -267,13 +268,8 @@ void ff_frame_thread_encoder_free(AVCodecContext *avctx){
  pthread_join(c->worker[i], NULL);
 }
 
-while (av_fifo_size(c->task_fifo) > 0) {
-Task task;
-av_fifo_generic_read(c->task_fifo, &task, sizeof(task), NULL);
-av_frame_free(&task.indata);
-}
-
 for (unsigned i = 0; i < c->max_tasks; i++) {
+av_frame_free(&c->tasks[i].indata);
 av_packet_free(&c->tasks[i].outdata);
 }
 
@@ -290,20 +286,16 @@ int ff_thread_video_encode_frame(AVCodecContext *avctx, 
AVPacket *pkt,
  AVFrame *frame, int *got_packet_ptr)
 {
 ThreadContext *c = avctx->internal->frame_thread_encoder;
-Task *outtask, task;
+Task *outtask;
 
 av_assert1(!*got_packet_ptr);
 
 if(frame){
-AVFrame *new = av_frame_alloc();
-if(!new)
-return AVERROR(ENOMEM);
-av_frame_move_ref(new, frame);
+av_frame_move_ref(c->tasks[c->task_index].indata, frame);
 
-task.index = c->task_index;
-task.indata = (void*)new;
 pthread_mutex_lock(&c->task_fifo_mutex);
-av_fifo_generic_

[FFmpeg-devel] [PATCH 4/8] avcodec/frame_thread_encoder: Avoid creating reference to frame

2021-02-08 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/encode.c   |  4 
 libavcodec/frame_thread_encoder.c | 11 ---
 libavcodec/frame_thread_encoder.h |  3 ++-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 29f41c3f92..282337e453 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -152,6 +152,10 @@ static int encode_simple_internal(AVCodecContext *avctx, 
AVPacket *avpkt)
 
 if (CONFIG_FRAME_THREAD_ENCODER &&
 avci->frame_thread_encoder && (avctx->active_thread_type & 
FF_THREAD_FRAME))
+/* This might modify frame, but it doesn't matter, because
+ * the frame properties used below are not used for video
+ * (due to the delay inherent in frame threaded encoding, it makes
+ *  no sense to use the properties of the current frame anyway). */
 ret = ff_thread_video_encode_frame(avctx, avpkt, frame, &got_packet);
 else {
 ret = avctx->codec->encode2(avctx, avpkt, frame, &got_packet);
diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index bcd3c94f8b..7c2894c933 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -286,10 +286,11 @@ void ff_frame_thread_encoder_free(AVCodecContext *avctx){
 av_freep(&avctx->internal->frame_thread_encoder);
 }
 
-int ff_thread_video_encode_frame(AVCodecContext *avctx, AVPacket *pkt, const 
AVFrame *frame, int *got_packet_ptr){
+int ff_thread_video_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
+ AVFrame *frame, int *got_packet_ptr)
+{
 ThreadContext *c = avctx->internal->frame_thread_encoder;
 Task *outtask, task;
-int ret;
 
 av_assert1(!*got_packet_ptr);
 
@@ -297,11 +298,7 @@ int ff_thread_video_encode_frame(AVCodecContext *avctx, 
AVPacket *pkt, const AVF
 AVFrame *new = av_frame_alloc();
 if(!new)
 return AVERROR(ENOMEM);
-ret = av_frame_ref(new, frame);
-if(ret < 0) {
-av_frame_free(&new);
-return ret;
-}
+av_frame_move_ref(new, frame);
 
 task.index = c->task_index;
 task.indata = (void*)new;
diff --git a/libavcodec/frame_thread_encoder.h 
b/libavcodec/frame_thread_encoder.h
index 1f79553f20..c400d6b32c 100644
--- a/libavcodec/frame_thread_encoder.h
+++ b/libavcodec/frame_thread_encoder.h
@@ -25,6 +25,7 @@
 
 int ff_frame_thread_encoder_init(AVCodecContext *avctx, AVDictionary *options);
 void ff_frame_thread_encoder_free(AVCodecContext *avctx);
-int ff_thread_video_encode_frame(AVCodecContext *avctx, AVPacket *pkt, const 
AVFrame *frame, int *got_packet_ptr);
+int ff_thread_video_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
+ AVFrame *frame, int *got_packet_ptr);
 
 #endif /* AVCODEC_FRAME_THREAD_ENCODER_H */
-- 
2.27.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 6/8] avcodec/frame_thread_encoder: Avoid FIFO

2021-02-08 Thread Andreas Rheinhardt
It can be replaced by a simple counter.

Signed-off-by: Andreas Rheinhardt 
---
Should I rename task_fifo_mutex/cond?

 libavcodec/frame_thread_encoder.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index 5fe886aed9..c6323b6246 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -22,7 +22,6 @@
 
 #include "frame_thread_encoder.h"
 
-#include "libavutil/fifo.h"
 #include "libavutil/avassert.h"
 #include "libavutil/imgutils.h"
 #include "libavutil/opt.h"
@@ -49,8 +48,7 @@ typedef struct{
 AVCodecContext *parent_avctx;
 pthread_mutex_t buffer_mutex;
 
-AVFifoBuffer *task_fifo;
-pthread_mutex_t task_fifo_mutex;
+pthread_mutex_t task_fifo_mutex; /* Used to guard (next_)task_index */
 pthread_cond_t task_fifo_cond;
 
 unsigned max_tasks;
@@ -58,6 +56,7 @@ typedef struct{
 pthread_mutex_t finished_task_mutex; /* Guards tasks[i].finished */
 pthread_cond_t finished_task_cond;
 
+unsigned next_task_index;
 unsigned task_index;
 unsigned finished_task_index;
 
@@ -77,14 +76,15 @@ static void * attribute_align_arg worker(void *v){
 unsigned task_index;
 
 pthread_mutex_lock(&c->task_fifo_mutex);
-while (av_fifo_size(c->task_fifo) <= 0 || atomic_load(&c->exit)) {
+while (c->next_task_index == c->task_index || atomic_load(&c->exit)) {
 if (atomic_load(&c->exit)) {
 pthread_mutex_unlock(&c->task_fifo_mutex);
 goto end;
 }
 pthread_cond_wait(&c->task_fifo_cond, &c->task_fifo_mutex);
 }
-av_fifo_generic_read(c->task_fifo, &task_index, sizeof(task_index), 
NULL);
+task_index = c->next_task_index;
+c->next_task_index = (c->next_task_index + 1) % c->max_tasks;
 pthread_mutex_unlock(&c->task_fifo_mutex);
 /* The main thread ensures that any two outstanding tasks have
  * different indices, ergo each worker thread owns its element
@@ -187,12 +187,6 @@ int ff_frame_thread_encoder_init(AVCodecContext *avctx, 
AVDictionary *options){
 
 c->parent_avctx = avctx;
 
-c->task_fifo = av_fifo_alloc_array(BUFFER_SIZE, sizeof(unsigned));
-if (!c->task_fifo) {
-av_freep(&avctx->internal->frame_thread_encoder);
-return AVERROR(ENOMEM);
-}
-
 pthread_mutex_init(&c->task_fifo_mutex, NULL);
 pthread_mutex_init(&c->finished_task_mutex, NULL);
 pthread_mutex_init(&c->buffer_mutex, NULL);
@@ -278,7 +272,6 @@ void ff_frame_thread_encoder_free(AVCodecContext *avctx){
 pthread_mutex_destroy(&c->buffer_mutex);
 pthread_cond_destroy(&c->task_fifo_cond);
 pthread_cond_destroy(&c->finished_task_cond);
-av_fifo_freep(&c->task_fifo);
 av_freep(&avctx->internal->frame_thread_encoder);
 }
 
@@ -294,16 +287,15 @@ int ff_thread_video_encode_frame(AVCodecContext *avctx, 
AVPacket *pkt,
 av_frame_move_ref(c->tasks[c->task_index].indata, frame);
 
 pthread_mutex_lock(&c->task_fifo_mutex);
-av_fifo_generic_write(c->task_fifo, &c->task_index,
-  sizeof(c->task_index), NULL);
+c->task_index = (c->task_index + 1) % c->max_tasks;
 pthread_cond_signal(&c->task_fifo_cond);
 pthread_mutex_unlock(&c->task_fifo_mutex);
-
-c->task_index = (c->task_index + 1) % c->max_tasks;
 }
 
 outtask = &c->tasks[c->finished_task_index];
 pthread_mutex_lock(&c->finished_task_mutex);
+/* The access to task_index in the following code is ok,
+ * because it is only ever changed by the main thread. */
 if (c->task_index == c->finished_task_index ||
 (frame && !outtask->finished &&
  (c->task_index - c->finished_task_index + c->max_tasks) % 
c->max_tasks <= avctx->thread_count)) {
-- 
2.27.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 7/8] avcodec/frame_thread_encoder: Reduce amount of code guarded by mutex

2021-02-08 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
This is not done for speed, but for consistency (because the return
statement already accessed outtask after unlocking the mutex).

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

diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index c6323b6246..f1f4fcb490 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -305,6 +305,7 @@ int ff_thread_video_encode_frame(AVCodecContext *avctx, 
AVPacket *pkt,
 while (!outtask->finished) {
 pthread_cond_wait(&c->finished_task_cond, &c->finished_task_mutex);
 }
+pthread_mutex_unlock(&c->finished_task_mutex);
 /* We now own outtask completely: No worker thread touches it any more,
  * because there is no outstanding task with this index. */
 outtask->finished = 0;
@@ -312,7 +313,6 @@ int ff_thread_video_encode_frame(AVCodecContext *avctx, 
AVPacket *pkt,
 if(pkt->data)
 *got_packet_ptr = 1;
 c->finished_task_index = (c->finished_task_index + 1) % c->max_tasks;
-pthread_mutex_unlock(&c->finished_task_mutex);
 
 return outtask->return_code;
 }
-- 
2.27.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 8/8] avcodec/frame_thread_encoder: Use more natural types

2021-02-08 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/frame_thread_encoder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/frame_thread_encoder.c 
b/libavcodec/frame_thread_encoder.c
index f1f4fcb490..778317d60b 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -40,7 +40,7 @@
 typedef struct{
 AVFrame  *indata;
 AVPacket *outdata;
-int64_t return_code;
+int   return_code;
 int   finished;
 } Task;
 
-- 
2.27.0

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

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

Re: [FFmpeg-devel] [PATCH 2/8] avcodec/frame_thread_encoder: Fix segfault on allocation error

2021-02-08 Thread Paul B Mahol
probably ok
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/8] avcodec/frame_thread_encoder: Improve type safety

2021-02-08 Thread Paul B Mahol
probably ok
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 8/8] avcodec/frame_thread_encoder: Use more natural types

2021-02-08 Thread Paul B Mahol
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] GSoC 2021

2021-02-08 Thread Thilo Borgmann
Hi,

>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of
>> Michael Niedermayer
>> Sent: 2021年2月2日 17:48
>> To: FFmpeg development discussions and patches 
>> Subject: [FFmpeg-devel] GSoC 2021
>>
>> Hi all
>>
>> Most people probably already know but just to be sure everyone knows GSoC
>> 2021 is 175h not 350h
>> https://groups.google.com/g/google-summer-of-code-discuss/c/GgvbLrFBcUQ
>> ?pli=1
>>
>> Some project ideas may need to be adjusted accordingly
> 
> hi, for your information.
> 
> just like last year, to get more chances for GSoC project, I proposed my idea 
> at https://01.org/linuxmedia/news/gsoc-2021-ideas, and also copy here for 
> your convenience.
> 
> Async support for TensorFlow backend in FFmpeg
> Description: FFmpeg DNN (deep neural network) module is to enable deep 
> learning based FFmpeg filters with OpenVINO, TensorFlow and Native as its 
> backends. The OpenVINO backend has supported async+batch execution for model 
> inference with commit e67b5d and 64ea15 etc, it shows good performance gain. 
> This project focuses on the async support for TensorFlow backend which 
> invokes TensorFlow C library for model loading and inference. The main 
> difference between the two backends is that OpenVINO API has async execution 
> function while TensorFlow C API does not expose async function, so we need to 
> support async execution by multiple threads within TensorFlow backend with 
> sync function TF_SessionRun. (it is a plus if batch mode is also supported.)
> Difficulty: Medium
> Skill Required: C, multiple threads, DNN knowledge, git

Thanks, see my other mail to have it on the ideas page!

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

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

Re: [FFmpeg-devel] GSoC 2021

2021-02-08 Thread Thilo Borgmann
Hi,


> Most people probably already know but just to be sure everyone knows
> GSoC 2021 is 175h not 350h
> https://groups.google.com/g/google-summer-of-code-discuss/c/GgvbLrFBcUQ?pli=1
> 
> Some project ideas may need to be adjusted accordingly

we've started registering as an Org and I put up the project ideas page for 
everyone to propose a project:

https://trac.ffmpeg.org/wiki/SponsoringPrograms/GSoC/2021

We're still looking for mentors/projects, please feel free to propose a project 
(175 working hours in size this year)

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

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

Re: [FFmpeg-devel] GSoC 2021

2021-02-08 Thread Nicolas George
Steven Liu (12021-02-02):
> And I saw Nicolas make some RFC maybe can split some subproject for mentor 
> project?

As Michael pointed, they are more about design than about coding, which
makes it less suited for this kind of mentor program. They require a
more in depth knowledge of parts of the project as a whole and/or
creative programming, which we cannot expect from an intern.

Another point to consider: we should put the proposed ideas on review,
to avoid letting pass one that is not suitable for FFmpeg for some
reason that the person who proposed missed. It is unfair to the student
if we realize in the middle of the project that it is actually not
wanted.

Regards,

-- 
  Nicolas George


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

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

Re: [FFmpeg-devel] [PATCH 2/8] avformat/mov: Support size = 1 and size = 0 special cases in probing

2021-02-08 Thread Michael Niedermayer
On Sat, Feb 06, 2021 at 11:33:38AM -0800, Chad Fraleigh wrote:
> On 2/6/2021 9:22 AM, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer 
> > ---
> >   libavformat/mov.c | 5 +
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 9406e42f49..70f76caff5 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -7113,6 +7113,11 @@ static int mov_probe(const AVProbeData *p)
> >   if ((offset + 8) > (unsigned int)p->buf_size)
> >   break;
> >   size = AV_RB32(p->buf + offset);
> > +if (size == 1 && offset + 16 > (unsigned int)p->buf_size) {
> > +size = AV_RB64(p->buf+offset + 8);
> 
> Just curious, what happens when size == 1 and the buffer is too small? Is
> leaving it as a size of 1 still valid, or should it be handled as a format
> error (e.g. abort the loop)?

The buffer must have a minimum padding of AVPROBE_PADDING_SIZE
so the buffer cannot be too small. This extra padding requirement is
there for exactly cases like this, otherwise alot more checks would be
needed in many probe functions

thx

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates


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

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

[FFmpeg-devel] [PATCH 2/2] avformat/3dostr: Check sample_rate

2021-02-08 Thread Michael Niedermayer
Fixes: signed integer overflow: -1268324762623155200 * 8 cannot be represented 
in type 'long'
Fixes: 
30123/clusterfuzz-testcase-minimized-ffmpeg_dem_THREEDOSTR_fuzzer-6710765123928064

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

diff --git a/libavformat/3dostr.c b/libavformat/3dostr.c
index 2a35d661c3..534f205fc4 100644
--- a/libavformat/3dostr.c
+++ b/libavformat/3dostr.c
@@ -103,7 +103,7 @@ static int threedostr_read_header(AVFormatContext *s)
 st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
 st->codecpar->sample_rate = avio_rb32(s->pb);
 st->codecpar->channels= avio_rb32(s->pb);
-if (st->codecpar->channels <= 0)
+if (st->codecpar->channels <= 0 || st->codecpar->sample_rate <= 0)
 return AVERROR_INVALIDDATA;
 codec  = avio_rl32(s->pb);
 avio_skip(s->pb, 4);
-- 
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/2] avformat/wtvdec: Check len in parse_chunks() to avoid overflow

2021-02-08 Thread Michael Niedermayer
Fixes: signed integer overflow: 2147483647 + 7 cannot be represented in type 
'int'
Fixes: 
30084/clusterfuzz-testcase-minimized-ffmpeg_dem_WTV_fuzzer-6192261941559296

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

diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
index 6c41e3c1a3..7def9d2348 100644
--- a/libavformat/wtvdec.c
+++ b/libavformat/wtvdec.c
@@ -794,7 +794,7 @@ static int parse_chunks(AVFormatContext *s, int mode, 
int64_t seekts, int *len_p
 
 ff_get_guid(pb, &g);
 len = avio_rl32(pb);
-if (len < 32) {
+if (len < 32 || len > INT_MAX - 7) {
 int ret;
 if (avio_feof(pb))
 return AVERROR_EOF;
-- 
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 2/2] avformat/3dostr: Check sample_rate

2021-02-08 Thread Paul B Mahol
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH] [RFC][v3] Tech Resolution Process

2021-02-08 Thread Jean-Baptiste Kempf
---
 doc/dev_community/resolution_process.md | 91 +
 1 file changed, 91 insertions(+)
 create mode 100644 doc/dev_community/resolution_process.md

diff --git a/doc/dev_community/resolution_process.md 
b/doc/dev_community/resolution_process.md
new file mode 100644
index 00..6da3e8ffb2
--- /dev/null
+++ b/doc/dev_community/resolution_process.md
@@ -0,0 +1,91 @@
+# Technical Committee
+
+_This document only makes sense with the rules from [the community 
document](community)_.
+
+The Technical Committee (**TC**) is here to arbitrate and make decisions when
+technical conflicts occur in the project.
+
+The TC main role is to resolve technical conflicts.
+It is therefore not a technical steering committee, but it is understood that
+some decisions might impact the future of the project.
+
+# Process
+
+## Seizing
+
+The TC can take possession of any technical matter that it sees fit.
+
+To involve the TC in a matter, email tc@ or CC them on an ongoing discussion.
+
+As members of TC are developers, they also can email tc@ to raise an issue.
+
+## Announcement
+
+The TC, once seized, must announce itself on the main mailing list, with a 
_[TC]_ tag.
+
+The TC has 2 modes of operation: a RFC one and an internal one.
+
+If the TC thinks it needs the input from the larger community, the TC can call
+for a RFC. Else, it can decide by itself.
+
+If the disagreement involves a member of the TC, that member should recuse
+themselves from the decision.
+
+The decision to use a RFC process or an internal discussion is a discretionary
+decision of the TC.
+
+The TC can also reject a seizure for a few reasons such as:
+the matter was not discussed enough previously; it lacks expertise to reach a
+beneficial decision on the matter; or the matter is too trivial.
+
+### RFC call
+
+In the RFC mode, one person from the TC posts on the mailing list the
+technical question and will request input from the community.
+
+The mail will have the following specification:
+* a precise title
+* a specific tag [TC RFC]
+* a top-level email
+* contain a precise question that does not exceed 100 words and that is 
answerable by developers
+* may have an extra description, or a link to a previous discussion, if deemed 
necessary,
+* contain a precise end date for the answers.
+
+The answers from the community must be on the main mailing list and must have
+the following specification:
+* keep the tag and the title unchanged
+* limited to 400 words
+* a first-level, answering directly to the main email
+* answering to the question.
+
+Further replies to answers are permitted, as long as they conform to the
+community standards of politeness, they are limited to 100 words, and are not
+nested more than once. (max-depth=2)
+
+After the end-date, mails on the thread will be ignored.
+
+Violations of those rules will be escalated through the Community Committee.
+
+After all the emails are in, the TC has 96 hours to give its final decision.
+Exceptionally, the TC can request an extra delay, that will be notified on the
+mailing list.
+
+### Within TC
+
+In the internal case, the TC has 96 hours to give its final decision.
+Exceptionally, the TC can request an extra delay.
+
+
+## Decisions
+
+The decisions from the TC will be sent on the mailing list, with the _[TC]_ 
tag.
+
+Internally, the TC should take decisions with a majority, or using
+ranked-choice voting.
+
+The decision from the TC should be published with a summary of the reasons that
+lead to this decision.
+
+The decisions from the TC are final, until the matters are reopened after
+no less than one year, by either the GA or the TC auto-seizing.
+
-- 
2.30.0

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

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

Re: [FFmpeg-devel] [PATCH 01/50] avcodec/packet: deprecate av_init_packet()

2021-02-08 Thread James Almer

On 2/4/2021 4:09 PM, James Almer wrote:

Once removed, sizeof(AVPacket) will stop being a part of the public ABI.

Signed-off-by: James Almer 
---
  libavcodec/avpacket.c  | 23 +++
  libavcodec/packet.h| 23 +++
  libavcodec/version.h   |  3 +++
  libavformat/avformat.h |  4 
  4 files changed, 41 insertions(+), 12 deletions(-)


Will add APIChanges entry and version bump, and apply soon.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 01/50] avcodec/packet: deprecate av_init_packet()

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> On 2/4/2021 4:09 PM, James Almer wrote:
>> Once removed, sizeof(AVPacket) will stop being a part of the public ABI.
>>
>> Signed-off-by: James Almer 
>> ---
>>   libavcodec/avpacket.c  | 23 +++
>>   libavcodec/packet.h    | 23 +++
>>   libavcodec/version.h   |  3 +++
>>   libavformat/avformat.h |  4 
>>   4 files changed, 41 insertions(+), 12 deletions(-)
> 
> Will add APIChanges entry and version bump, and apply soon.

Why the hurry? In fact, I have just sent a patchset that makes 3/50
redundant.

- 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 01/50] avcodec/packet: deprecate av_init_packet()

2021-02-08 Thread James Almer

On 2/8/2021 11:37 AM, Andreas Rheinhardt wrote:

James Almer:

On 2/4/2021 4:09 PM, James Almer wrote:

Once removed, sizeof(AVPacket) will stop being a part of the public ABI.

Signed-off-by: James Almer 
---
   libavcodec/avpacket.c  | 23 +++
   libavcodec/packet.h    | 23 +++
   libavcodec/version.h   |  3 +++
   libavformat/avformat.h |  4 
   4 files changed, 41 insertions(+), 12 deletions(-)


Will add APIChanges entry and version bump, and apply soon.


Why the hurry? In fact, I have just sent a patchset that makes 3/50
redundant.


I'm talking about this patch, not the whole set. I'll give the rest some 
more time in case others want to review or test (The vaapi stuff i can't 
test, for example).


And yes, i saw the patches you sent that supersede 3/50, so assuming 
your set is ok i was going to withdraw it.




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

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



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

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

Re: [FFmpeg-devel] [PATCH 01/50] avcodec/packet: deprecate av_init_packet()

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> On 2/8/2021 11:37 AM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 2/4/2021 4:09 PM, James Almer wrote:
 Once removed, sizeof(AVPacket) will stop being a part of the public
 ABI.

 Signed-off-by: James Almer 
 ---
    libavcodec/avpacket.c  | 23 +++
    libavcodec/packet.h    | 23 +++
    libavcodec/version.h   |  3 +++
    libavformat/avformat.h |  4 
    4 files changed, 41 insertions(+), 12 deletions(-)
>>>
>>> Will add APIChanges entry and version bump, and apply soon.
>>
>> Why the hurry? In fact, I have just sent a patchset that makes 3/50
>> redundant.
> 
> I'm talking about this patch, not the whole set. I'll give the rest some
> more time in case others want to review or test (The vaapi stuff i can't
> test, for example).
> 
> And yes, i saw the patches you sent that supersede 3/50, so assuming
> your set is ok i was going to withdraw it.
> 
I still don't see the reason to hurry things; actually I don't even this
direction at all, as it just leads to more allocations and frees. The dv
stuff is a good example of this, the mpegvideo_enc another.

- 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 06/50] avcodec/mpegvideo_enc: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> Signed-off-by: James Almer 
> ---
>  libavcodec/mpegvideo_enc.c | 23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index 34dcf8c313..411cadeae7 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -1366,23 +1366,20 @@ static int skip_check(MpegEncContext *s, Picture *p, 
> Picture *ref)
>  return 0;
>  }
>  
> -static int encode_frame(AVCodecContext *c, AVFrame *frame)
> +static int encode_frame(AVCodecContext *c, AVFrame *frame, AVPacket *pkt)
>  {
> -AVPacket pkt = { 0 };
>  int ret;
>  int size = 0;
>  
> -av_init_packet(&pkt);
> -
>  ret = avcodec_send_frame(c, frame);
>  if (ret < 0)
>  return ret;
>  
>  do {
> -ret = avcodec_receive_packet(c, &pkt);
> +ret = avcodec_receive_packet(c, pkt);
>  if (ret >= 0) {
> -size += pkt.size;
> -av_packet_unref(&pkt);
> +size += pkt->size;
> +av_packet_unref(pkt);
>  } else if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
>  return ret;
>  } while (ret >= 0);
> @@ -1448,6 +1445,7 @@ static int estimate_best_b_count(MpegEncContext *s)
>  
>  for (j = 0; j < s->max_b_frames + 1; j++) {
>  AVCodecContext *c;
> +AVPacket *pkt;
>  int64_t rd = 0;
>  
>  if (!s->input_picture[j])
> @@ -1473,10 +1471,14 @@ static int estimate_best_b_count(MpegEncContext *s)
>  if (ret < 0)
>  goto fail;

The av_packet_free in the fail code uses an uninitialized pointer.

>  
> +pkt = av_packet_alloc();

You are adding s->max_b_frames + 1 allocations + frees per packet to be
encoded (if I am not mistaken). I am speechless.

> +if (!pkt)
> +goto fail;

You forgot to set ret.

> +
>  s->tmp_frames[0]->pict_type = AV_PICTURE_TYPE_I;
>  s->tmp_frames[0]->quality   = 1 * FF_QP2LAMBDA;
>  
> -out_size = encode_frame(c, s->tmp_frames[0]);
> +out_size = encode_frame(c, s->tmp_frames[0], pkt);
>  if (out_size < 0) {
>  ret = out_size;
>  goto fail;
> @@ -1491,7 +1493,7 @@ static int estimate_best_b_count(MpegEncContext *s)
>   AV_PICTURE_TYPE_P : AV_PICTURE_TYPE_B;
>  s->tmp_frames[i + 1]->quality   = is_p ? p_lambda : b_lambda;
>  
> -out_size = encode_frame(c, s->tmp_frames[i + 1]);
> +out_size = encode_frame(c, s->tmp_frames[i + 1], pkt);
>  if (out_size < 0) {
>  ret = out_size;
>  goto fail;
> @@ -1501,7 +1503,7 @@ static int estimate_best_b_count(MpegEncContext *s)
>  }
>  
>  /* get the delayed frames */
> -out_size = encode_frame(c, NULL);
> +out_size = encode_frame(c, NULL, pkt);
>  if (out_size < 0) {
>  ret = out_size;
>  goto fail;
> @@ -1517,6 +1519,7 @@ static int estimate_best_b_count(MpegEncContext *s)
>  
>  fail:
>  avcodec_free_context(&c);
> +av_packet_free(&pkt);
>  if (ret < 0)
>  return ret;
>  }
> 
___
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 06/50] avcodec/mpegvideo_enc: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 11:46 AM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
  libavcodec/mpegvideo_enc.c | 23 +--
  1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 34dcf8c313..411cadeae7 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1366,23 +1366,20 @@ static int skip_check(MpegEncContext *s, Picture *p, 
Picture *ref)
  return 0;
  }
  
-static int encode_frame(AVCodecContext *c, AVFrame *frame)

+static int encode_frame(AVCodecContext *c, AVFrame *frame, AVPacket *pkt)
  {
-AVPacket pkt = { 0 };
  int ret;
  int size = 0;
  
-av_init_packet(&pkt);

-
  ret = avcodec_send_frame(c, frame);
  if (ret < 0)
  return ret;
  
  do {

-ret = avcodec_receive_packet(c, &pkt);
+ret = avcodec_receive_packet(c, pkt);
  if (ret >= 0) {
-size += pkt.size;
-av_packet_unref(&pkt);
+size += pkt->size;
+av_packet_unref(pkt);
  } else if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
  return ret;
  } while (ret >= 0);
@@ -1448,6 +1445,7 @@ static int estimate_best_b_count(MpegEncContext *s)
  
  for (j = 0; j < s->max_b_frames + 1; j++) {

  AVCodecContext *c;
+AVPacket *pkt;
  int64_t rd = 0;
  
  if (!s->input_picture[j])

@@ -1473,10 +1471,14 @@ static int estimate_best_b_count(MpegEncContext *s)
  if (ret < 0)
  goto fail;


The av_packet_free in the fail code uses an uninitialized pointer.

  
+pkt = av_packet_alloc();


You are adding s->max_b_frames + 1 allocations + frees per packet to be
encoded (if I am not mistaken). I am speechless.


I can try to move it outside the loop. But said loop is already 
allocating that many AVCodecContexts, so hardly that much of a difference.





+if (!pkt)
+goto fail;


You forgot to set ret.


+
  s->tmp_frames[0]->pict_type = AV_PICTURE_TYPE_I;
  s->tmp_frames[0]->quality   = 1 * FF_QP2LAMBDA;
  
-out_size = encode_frame(c, s->tmp_frames[0]);

+out_size = encode_frame(c, s->tmp_frames[0], pkt);
  if (out_size < 0) {
  ret = out_size;
  goto fail;
@@ -1491,7 +1493,7 @@ static int estimate_best_b_count(MpegEncContext *s)
   AV_PICTURE_TYPE_P : AV_PICTURE_TYPE_B;
  s->tmp_frames[i + 1]->quality   = is_p ? p_lambda : b_lambda;
  
-out_size = encode_frame(c, s->tmp_frames[i + 1]);

+out_size = encode_frame(c, s->tmp_frames[i + 1], pkt);
  if (out_size < 0) {
  ret = out_size;
  goto fail;
@@ -1501,7 +1503,7 @@ static int estimate_best_b_count(MpegEncContext *s)
  }
  
  /* get the delayed frames */

-out_size = encode_frame(c, NULL);
+out_size = encode_frame(c, NULL, pkt);
  if (out_size < 0) {
  ret = out_size;
  goto fail;
@@ -1517,6 +1519,7 @@ static int estimate_best_b_count(MpegEncContext *s)
  
  fail:

  avcodec_free_context(&c);
+av_packet_free(&pkt);
  if (ret < 0)
  return ret;
  }


___
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 06/50] avcodec/mpegvideo_enc: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> On 2/8/2021 11:46 AM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Signed-off-by: James Almer 
>>> ---
>>>   libavcodec/mpegvideo_enc.c | 23 +--
>>>   1 file changed, 13 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
>>> index 34dcf8c313..411cadeae7 100644
>>> --- a/libavcodec/mpegvideo_enc.c
>>> +++ b/libavcodec/mpegvideo_enc.c
>>> @@ -1366,23 +1366,20 @@ static int skip_check(MpegEncContext *s,
>>> Picture *p, Picture *ref)
>>>   return 0;
>>>   }
>>>   -static int encode_frame(AVCodecContext *c, AVFrame *frame)
>>> +static int encode_frame(AVCodecContext *c, AVFrame *frame, AVPacket
>>> *pkt)
>>>   {
>>> -    AVPacket pkt = { 0 };
>>>   int ret;
>>>   int size = 0;
>>>   -    av_init_packet(&pkt);
>>> -
>>>   ret = avcodec_send_frame(c, frame);
>>>   if (ret < 0)
>>>   return ret;
>>>     do {
>>> -    ret = avcodec_receive_packet(c, &pkt);
>>> +    ret = avcodec_receive_packet(c, pkt);
>>>   if (ret >= 0) {
>>> -    size += pkt.size;
>>> -    av_packet_unref(&pkt);
>>> +    size += pkt->size;
>>> +    av_packet_unref(pkt);
>>>   } else if (ret < 0 && ret != AVERROR(EAGAIN) && ret !=
>>> AVERROR_EOF)
>>>   return ret;
>>>   } while (ret >= 0);
>>> @@ -1448,6 +1445,7 @@ static int estimate_best_b_count(MpegEncContext
>>> *s)
>>>     for (j = 0; j < s->max_b_frames + 1; j++) {
>>>   AVCodecContext *c;
>>> +    AVPacket *pkt;
>>>   int64_t rd = 0;
>>>     if (!s->input_picture[j])
>>> @@ -1473,10 +1471,14 @@ static int
>>> estimate_best_b_count(MpegEncContext *s)
>>>   if (ret < 0)
>>>   goto fail;
>>
>> The av_packet_free in the fail code uses an uninitialized pointer.
>>
>>>   +    pkt = av_packet_alloc();
>>
>> You are adding s->max_b_frames + 1 allocations + frees per packet to be
>> encoded (if I am not mistaken). I am speechless.
> 
> I can try to move it outside the loop. But said loop is already
> allocating that many AVCodecContexts, so hardly that much of a difference.
> 

Still the wrong direction even when the current state is already bad.

>>
>>> +    if (!pkt)
>>> +    goto fail;
>>
>> You forgot to set ret.
>>
>>> +
>>>   s->tmp_frames[0]->pict_type = AV_PICTURE_TYPE_I;
>>>   s->tmp_frames[0]->quality   = 1 * FF_QP2LAMBDA;
>>>   -    out_size = encode_frame(c, s->tmp_frames[0]);
>>> +    out_size = encode_frame(c, s->tmp_frames[0], pkt);
>>>   if (out_size < 0) {
>>>   ret = out_size;
>>>   goto fail;
>>> @@ -1491,7 +1493,7 @@ static int estimate_best_b_count(MpegEncContext
>>> *s)
>>>    AV_PICTURE_TYPE_P :
>>> AV_PICTURE_TYPE_B;
>>>   s->tmp_frames[i + 1]->quality   = is_p ? p_lambda :
>>> b_lambda;
>>>   -    out_size = encode_frame(c, s->tmp_frames[i + 1]);
>>> +    out_size = encode_frame(c, s->tmp_frames[i + 1], pkt);
>>>   if (out_size < 0) {
>>>   ret = out_size;
>>>   goto fail;
>>> @@ -1501,7 +1503,7 @@ static int estimate_best_b_count(MpegEncContext
>>> *s)
>>>   }
>>>     /* get the delayed frames */
>>> -    out_size = encode_frame(c, NULL);
>>> +    out_size = encode_frame(c, NULL, pkt);
>>>   if (out_size < 0) {
>>>   ret = out_size;
>>>   goto fail;
>>> @@ -1517,6 +1519,7 @@ static int estimate_best_b_count(MpegEncContext
>>> *s)
>>>     fail:
>>>   avcodec_free_context(&c);
>>> +    av_packet_free(&pkt);
>>>   if (ret < 0)
>>>   return ret;
>>>   }
>>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>>
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

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

Re: [FFmpeg-devel] [PATCH 06/50] avcodec/mpegvideo_enc: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 11:50 AM, Andreas Rheinhardt wrote:

James Almer:

On 2/8/2021 11:46 AM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
   libavcodec/mpegvideo_enc.c | 23 +--
   1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 34dcf8c313..411cadeae7 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1366,23 +1366,20 @@ static int skip_check(MpegEncContext *s,
Picture *p, Picture *ref)
   return 0;
   }
   -static int encode_frame(AVCodecContext *c, AVFrame *frame)
+static int encode_frame(AVCodecContext *c, AVFrame *frame, AVPacket
*pkt)
   {
-    AVPacket pkt = { 0 };
   int ret;
   int size = 0;
   -    av_init_packet(&pkt);
-
   ret = avcodec_send_frame(c, frame);
   if (ret < 0)
   return ret;
     do {
-    ret = avcodec_receive_packet(c, &pkt);
+    ret = avcodec_receive_packet(c, pkt);
   if (ret >= 0) {
-    size += pkt.size;
-    av_packet_unref(&pkt);
+    size += pkt->size;
+    av_packet_unref(pkt);
   } else if (ret < 0 && ret != AVERROR(EAGAIN) && ret !=
AVERROR_EOF)
   return ret;
   } while (ret >= 0);
@@ -1448,6 +1445,7 @@ static int estimate_best_b_count(MpegEncContext
*s)
     for (j = 0; j < s->max_b_frames + 1; j++) {
   AVCodecContext *c;
+    AVPacket *pkt;
   int64_t rd = 0;
     if (!s->input_picture[j])
@@ -1473,10 +1471,14 @@ static int
estimate_best_b_count(MpegEncContext *s)
   if (ret < 0)
   goto fail;


The av_packet_free in the fail code uses an uninitialized pointer.


   +    pkt = av_packet_alloc();


You are adding s->max_b_frames + 1 allocations + frees per packet to be
encoded (if I am not mistaken). I am speechless.


I can try to move it outside the loop. But said loop is already
allocating that many AVCodecContexts, so hardly that much of a difference.



Still the wrong direction even when the current state is already bad.


I agree. I could also just withdraw this patch altogether (or rather, 
just remove the av_init_packet() call), since it's in lavc, where 
sizeof(AVPacket) can be used just fine.







+    if (!pkt)
+    goto fail;


You forgot to set ret.


+
   s->tmp_frames[0]->pict_type = AV_PICTURE_TYPE_I;
   s->tmp_frames[0]->quality   = 1 * FF_QP2LAMBDA;
   -    out_size = encode_frame(c, s->tmp_frames[0]);
+    out_size = encode_frame(c, s->tmp_frames[0], pkt);
   if (out_size < 0) {
   ret = out_size;
   goto fail;
@@ -1491,7 +1493,7 @@ static int estimate_best_b_count(MpegEncContext
*s)
    AV_PICTURE_TYPE_P :
AV_PICTURE_TYPE_B;
   s->tmp_frames[i + 1]->quality   = is_p ? p_lambda :
b_lambda;
   -    out_size = encode_frame(c, s->tmp_frames[i + 1]);
+    out_size = encode_frame(c, s->tmp_frames[i + 1], pkt);
   if (out_size < 0) {
   ret = out_size;
   goto fail;
@@ -1501,7 +1503,7 @@ static int estimate_best_b_count(MpegEncContext
*s)
   }
     /* get the delayed frames */
-    out_size = encode_frame(c, NULL);
+    out_size = encode_frame(c, NULL, pkt);
   if (out_size < 0) {
   ret = out_size;
   goto fail;
@@ -1517,6 +1519,7 @@ static int estimate_best_b_count(MpegEncContext
*s)
     fail:
   avcodec_free_context(&c);
+    av_packet_free(&pkt);
   if (ret < 0)
   return ret;
   }


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

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



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

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


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

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



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

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

Re: [FFmpeg-devel] [PATCH 01/50] avcodec/packet: deprecate av_init_packet()

2021-02-08 Thread James Almer

On 2/8/2021 11:43 AM, Andreas Rheinhardt wrote:

James Almer:

On 2/8/2021 11:37 AM, Andreas Rheinhardt wrote:

James Almer:

On 2/4/2021 4:09 PM, James Almer wrote:

Once removed, sizeof(AVPacket) will stop being a part of the public
ABI.

Signed-off-by: James Almer 
---
    libavcodec/avpacket.c  | 23 +++
    libavcodec/packet.h    | 23 +++
    libavcodec/version.h   |  3 +++
    libavformat/avformat.h |  4 
    4 files changed, 41 insertions(+), 12 deletions(-)


Will add APIChanges entry and version bump, and apply soon.


Why the hurry? In fact, I have just sent a patchset that makes 3/50
redundant.


I'm talking about this patch, not the whole set. I'll give the rest some
more time in case others want to review or test (The vaapi stuff i can't
test, for example).

And yes, i saw the patches you sent that supersede 3/50, so assuming
your set is ok i was going to withdraw it.


I still don't see the reason to hurry things;


We're overdue for the bump and a new release before it, so it's best to 
move forward at least a bit. And this change, if committed, should go in 
before either of them.



actually I don't even this
direction at all, as it just leads to more allocations and frees. The dv
stuff is a good example of this, the mpegvideo_enc another.


The mpegvideo_enc was an extreme case the way i wrote it at first, yes. 
And the DV stuff is not an issue for needing allocs, but for being 
shared code between lavd and lavf, with one of the modules, the lavd 
device, being undertested (Does any developer here use libiec61883? The 
massive leak i fixed years ago was there for while, i recall).


New AVPacket fields are being discussed to be added, and if 
sizeof(AVPacket) is public then the window to add them is very small 
(About one month every 2 to 4 years). There's also the issue of field 
initialization. Any new fields could end up uninitialized in the 
scenario of users not calling av_init_packet, which is the reason 
av_read_frame, av_packet_copy_props and av_get_packet do it on their own 
(and it can lead to leaks if you pass them a valid, non empty packet, 
for example).


So I suggested putting AVPacket in line with AVFrame, and two other 
developers agreed. This change removes the above corner cases, relaxes 
the requirements to add new fields, and forces the user to do things 
right, and now that the old decode/encode API is being removed, it's the 
ideal time to do it.

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

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

Re: [FFmpeg-devel] [PATCH 01/50] avcodec/packet: deprecate av_init_packet()

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> Once removed, sizeof(AVPacket) will stop being a part of the public ABI.
> 
> Signed-off-by: James Almer 
> ---
>  libavcodec/avpacket.c  | 23 +++
>  libavcodec/packet.h| 23 +++
>  libavcodec/version.h   |  3 +++
>  libavformat/avformat.h |  4 
>  4 files changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index e4ba403cf6..ae0cbfb9f9 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -32,6 +32,7 @@
>  #include "packet.h"
>  #include "packet_internal.h"
>  
> +#if FF_API_INIT_PACKET
>  void av_init_packet(AVPacket *pkt)
>  {
>  pkt->pts  = AV_NOPTS_VALUE;
> @@ -49,6 +50,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  pkt->side_data= NULL;
>  pkt->side_data_elems  = 0;
>  }
> +#endif
> +
> +static void get_packet_defaults(AVPacket *pkt)
> +{
> +memset(pkt, 0, sizeof(*pkt));
> +
> +pkt->pts = AV_NOPTS_VALUE;
> +pkt->dts = AV_NOPTS_VALUE;
> +pkt->pos = -1;
> +}
>  
>  AVPacket *av_packet_alloc(void)
>  {
> @@ -56,7 +67,7 @@ AVPacket *av_packet_alloc(void)
>  if (!pkt)
>  return pkt;
>  
> -av_init_packet(pkt);
> +get_packet_defaults(pkt);
>  
>  return pkt;
>  }
> @@ -92,7 +103,7 @@ int av_new_packet(AVPacket *pkt, int size)
>  if (ret < 0)
>  return ret;
>  
> -av_init_packet(pkt);
> +get_packet_defaults(pkt);
>  pkt->buf  = buf;
>  pkt->data = buf->data;
>  pkt->size = size;
> @@ -607,9 +618,7 @@ void av_packet_unref(AVPacket *pkt)
>  {
>  av_packet_free_side_data(pkt);
>  av_buffer_unref(&pkt->buf);
> -av_init_packet(pkt);
> -pkt->data = NULL;
> -pkt->size = 0;
> +get_packet_defaults(pkt);
>  }
>  
>  int av_packet_ref(AVPacket *dst, const AVPacket *src)
> @@ -664,9 +673,7 @@ AVPacket *av_packet_clone(const AVPacket *src)
>  void av_packet_move_ref(AVPacket *dst, AVPacket *src)
>  {
>  *dst = *src;
> -av_init_packet(src);
> -src->data = NULL;
> -src->size = 0;
> +get_packet_defaults(src);
>  }
>  
>  int av_packet_make_refcounted(AVPacket *pkt)
> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> index b9d4c9c2c8..c442b6a6eb 100644
> --- a/libavcodec/packet.h
> +++ b/libavcodec/packet.h
> @@ -319,10 +319,6 @@ typedef struct AVPacketSideData {
>   * packets, with no compressed data, containing only side data
>   * (e.g. to update some stream parameters at the end of encoding).
>   *
> - * AVPacket is one of the few structs in FFmpeg, whose size is a part of 
> public
> - * ABI. Thus it may be allocated on stack and no new fields can be added to 
> it
> - * without libavcodec and libavformat major bump.
> - *
>   * The semantics of data ownership depends on the buf field.
>   * If it is set, the packet data is dynamically allocated and is
>   * valid indefinitely until a call to av_packet_unref() reduces the
> @@ -334,6 +330,12 @@ typedef struct AVPacketSideData {
>   * The side data is always allocated with av_malloc(), copied by
>   * av_packet_ref() and freed by av_packet_unref().
>   *
> + * sizeof(AVPacket) being a part of the public ABI is deprecated. once
> + * av_init_packet() is removed, new packets will only be able to be allocated
> + * with av_packet_alloc(), and new fields may be added to the end of the 
> struct
> + * with a minor bump.
> + *
> + * @see av_packet_alloc
>   * @see av_packet_ref
>   * @see av_packet_unref
>   */
> @@ -394,7 +396,11 @@ typedef struct AVPacket {
>  } AVPacket;
>  
>  typedef struct AVPacketList {
> +#if FF_API_INIT_PACKET
>  AVPacket pkt;
> +#else
> +AVPacket *pkt;
> +#endif
>  struct AVPacketList *next;
>  } AVPacketList;

As long as the packet-list functions use this structure, there will be
an unnecessary allocation for every packet list entry. This is
unnecessary even if one wants to make sizeof(AVPacket) not part of the
ABI any more: Just move the next pointer to the front.
Of course this will make this structure useless to someone who doesn't
have functions that deal with AVPacketList entries. But given that there
are no public functions dedicated to this task at all and given that
anyone can write a packet list like the above without problems, this
structure should be removed from the public API altogether.
(If the packet list helper functions were to be made public, the above
would need to be revised. But I don't think that adding a packet list
API that forces us to use a linked list should be made public.)

>  
> @@ -460,6 +466,7 @@ AVPacket *av_packet_clone(const AVPacket *src);
>   */
>  void av_packet_free(AVPacket **pkt);
>  
> +#if FF_API_INIT_PACKET
>  /**
>   * Initialize optional fields of a packet with default values.
>   *
> @@ -467,8 +474,16 @@ void av_packet_free(AVPacket **pkt);
>   * initialized separately.
>   *
>   * @param pkt packet
> + *
> + * @see av_packet_alloc
> + * @see av_packet_

Re: [FFmpeg-devel] [PATCH 1/2] avformat/wtvdec: Check len in parse_chunks() to avoid overflow

2021-02-08 Thread Peter Ross
On Mon, Feb 08, 2021 at 02:29:01PM +0100, Michael Niedermayer wrote:
> Fixes: signed integer overflow: 2147483647 + 7 cannot be represented in type 
> 'int'
> Fixes: 
> 30084/clusterfuzz-testcase-minimized-ffmpeg_dem_WTV_fuzzer-6192261941559296
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/wtvdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
> index 6c41e3c1a3..7def9d2348 100644
> --- a/libavformat/wtvdec.c
> +++ b/libavformat/wtvdec.c
> @@ -794,7 +794,7 @@ static int parse_chunks(AVFormatContext *s, int mode, 
> int64_t seekts, int *len_p
>  
>  ff_get_guid(pb, &g);
>  len = avio_rl32(pb);
> -if (len < 32) {
> +if (len < 32 || len > INT_MAX - 7) {
>  int ret;
>  if (avio_feof(pb))
>  return AVERROR_EOF;

the + 7 comes from WTV_PAD calculation
looks good

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
___
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/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> Signed-off-by: James Almer 
> ---
>  libavformat/matroskadec.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 374831baa3..9fad78c78b 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
>  /* byte position of the segment inside the stream */
>  int64_t segment_start;
>  
> +AVPacket *pkt;
> +
>  /* the packet queue */
>  AVPacketList *queue;
>  AVPacketList *queue_end;
> @@ -2885,6 +2887,10 @@ static int matroska_read_header(AVFormatContext *s)
>  }
>  ebml_free(ebml_syntax, &ebml);
>  
> +matroska->pkt = av_packet_alloc();
> +if (!matroska->pkt)

Missing AVERROR(ENOMEM).
(I actually have an idea to completely remove the packet list from
matroskadec.)

> +goto fail;
> +
>  /* The next thing is a segment. */
>  pos = avio_tell(matroska->ctx->pb);
>  res = ebml_parse(matroska, matroska_segments, matroska);
> @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext *s)
>  st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
>  st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>  
> -av_init_packet(pkt);
> +av_packet_unref(pkt);
>  pkt->buf  = attachments[j].bin.buf;
>  attachments[j].bin.buf = NULL;
>  pkt->data = attachments[j].bin.data;
> @@ -3180,7 +3186,7 @@ static int matroska_parse_rm_audio(MatroskaDemuxContext 
> *matroska,
>  
>  while (track->audio.pkt_cnt) {
>  int ret;
> -AVPacket pktl, *pkt = &pktl;
> +AVPacket *pkt = matroska->pkt;
>  
>  ret = av_new_packet(pkt, a);
>  if (ret < 0) {
> @@ -3317,7 +3323,7 @@ static int matroska_parse_webvtt(MatroskaDemuxContext 
> *matroska,
>   uint64_t duration,
>   int64_t pos)
>  {
> -AVPacket pktl, *pkt = &pktl;
> +AVPacket *pkt = matroska->pkt;
>  uint8_t *id, *settings, *text, *buf;
>  int id_len, settings_len, text_len;
>  uint8_t *p, *q;
> @@ -3434,7 +3440,7 @@ static int matroska_parse_frame(MatroskaDemuxContext 
> *matroska,
>  {
>  uint8_t *pkt_data = data;
>  int res = 0;
> -AVPacket pktl, *pkt = &pktl;
> +AVPacket *pkt = matroska->pkt;
>  
>  if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) {
>  res = matroska_parse_wavpack(track, &pkt_data, &pkt_size);
> @@ -3464,7 +3470,7 @@ static int matroska_parse_frame(MatroskaDemuxContext 
> *matroska,
>  if (!pkt_size && !additional_size)
>  goto no_output;
>  
> -av_init_packet(pkt);
> +av_packet_unref(pkt);
>  if (!buf)
>  pkt->buf = av_buffer_create(pkt_data, pkt_size + 
> AV_INPUT_BUFFER_PADDING_SIZE,
>  NULL, NULL, 0);
> @@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s)
>  int n;
>  
>  matroska_clear_queue(matroska);
> +av_packet_free(&matroska->pkt);
>  
>  for (n = 0; n < matroska->tracks.nb_elem; n++)
>  if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)
> 

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

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

Re: [FFmpeg-devel] [PATCH 01/50] avcodec/packet: deprecate av_init_packet()

2021-02-08 Thread James Almer

On 2/8/2021 12:16 PM, Andreas Rheinhardt wrote:

  typedef struct AVPacketList {
+#if FF_API_INIT_PACKET
  AVPacket pkt;
+#else
+AVPacket *pkt;
+#endif
  struct AVPacketList *next;
  } AVPacketList;

As long as the packet-list functions use this structure, there will be
an unnecessary allocation for every packet list entry. This is
unnecessary even if one wants to make sizeof(AVPacket) not part of the
ABI any more: Just move the next pointer to the front.
Of course this will make this structure useless to someone who doesn't
have functions that deal with AVPacketList entries. But given that there
are no public functions dedicated to this task at all and given that
anyone can write a packet list like the above without problems, this
structure should be removed from the public API altogether.


I agree. I was considering to either make this an opaque struct, so new 
public API that makes use of it can be added down the line without 
worrying about what fields are defined in it, or just remove it.
FFplay was defining its own struct based on it because it needed one 
extra field per element (Which will become unnecessary once an opaque 
user field is added to AVPacket).



(If the packet list helper functions were to be made public, the above
would need to be revised. But I don't think that adding a packet list
API that forces us to use a linked list should be made public.)


A public packet list API could have its internals hidden, and then use a 
FIFO (sizeof(AVPacket) will be allowed within lavc, so no allocs). As 
far as the user can see, the API would simply consume are return packet 
references. But that's for later, if ever.

___
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/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
  libavformat/matroskadec.c | 17 -
  1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 374831baa3..9fad78c78b 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
  /* byte position of the segment inside the stream */
  int64_t segment_start;
  
+AVPacket *pkt;

+
  /* the packet queue */
  AVPacketList *queue;
  AVPacketList *queue_end;
@@ -2885,6 +2887,10 @@ static int matroska_read_header(AVFormatContext *s)
  }
  ebml_free(ebml_syntax, &ebml);
  
+matroska->pkt = av_packet_alloc();

+if (!matroska->pkt)


Missing AVERROR(ENOMEM).


This seems to be a common mistake in this set. Sorry.


(I actually have an idea to completely remove the packet list from
matroskadec.)


Should i commit this while you work on that, or just withdraw this 
patch? It's not urgent at all, since any deprecation period is 2+ years. 
Either option is fine by me.





+goto fail;
+
  /* The next thing is a segment. */
  pos = avio_tell(matroska->ctx->pb);
  res = ebml_parse(matroska, matroska_segments, matroska);
@@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext *s)
  st->disposition |= AV_DISPOSITION_ATTACHED_PIC;
  st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
  
-av_init_packet(pkt);

+av_packet_unref(pkt);
  pkt->buf  = attachments[j].bin.buf;
  attachments[j].bin.buf = NULL;
  pkt->data = attachments[j].bin.data;
@@ -3180,7 +3186,7 @@ static int matroska_parse_rm_audio(MatroskaDemuxContext 
*matroska,
  
  while (track->audio.pkt_cnt) {

  int ret;
-AVPacket pktl, *pkt = &pktl;
+AVPacket *pkt = matroska->pkt;
  
  ret = av_new_packet(pkt, a);

  if (ret < 0) {
@@ -3317,7 +3323,7 @@ static int matroska_parse_webvtt(MatroskaDemuxContext 
*matroska,
   uint64_t duration,
   int64_t pos)
  {
-AVPacket pktl, *pkt = &pktl;
+AVPacket *pkt = matroska->pkt;
  uint8_t *id, *settings, *text, *buf;
  int id_len, settings_len, text_len;
  uint8_t *p, *q;
@@ -3434,7 +3440,7 @@ static int matroska_parse_frame(MatroskaDemuxContext 
*matroska,
  {
  uint8_t *pkt_data = data;
  int res = 0;
-AVPacket pktl, *pkt = &pktl;
+AVPacket *pkt = matroska->pkt;
  
  if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) {

  res = matroska_parse_wavpack(track, &pkt_data, &pkt_size);
@@ -3464,7 +3470,7 @@ static int matroska_parse_frame(MatroskaDemuxContext 
*matroska,
  if (!pkt_size && !additional_size)
  goto no_output;
  
-av_init_packet(pkt);

+av_packet_unref(pkt);
  if (!buf)
  pkt->buf = av_buffer_create(pkt_data, pkt_size + 
AV_INPUT_BUFFER_PADDING_SIZE,
  NULL, NULL, 0);
@@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s)
  int n;
  
  matroska_clear_queue(matroska);

+av_packet_free(&matroska->pkt);
  
  for (n = 0; n < matroska->tracks.nb_elem; n++)

  if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)



___
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 21/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Signed-off-by: James Almer 
>>> ---
>>>   libavformat/matroskadec.c | 17 -
>>>   1 file changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index 374831baa3..9fad78c78b 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
>>>   /* byte position of the segment inside the stream */
>>>   int64_t segment_start;
>>>   +    AVPacket *pkt;
>>> +
>>>   /* the packet queue */
>>>   AVPacketList *queue;
>>>   AVPacketList *queue_end;
>>> @@ -2885,6 +2887,10 @@ static int
>>> matroska_read_header(AVFormatContext *s)
>>>   }
>>>   ebml_free(ebml_syntax, &ebml);
>>>   +    matroska->pkt = av_packet_alloc();
>>> +    if (!matroska->pkt)
>>
>> Missing AVERROR(ENOMEM).
> 
> This seems to be a common mistake in this set. Sorry.
> 

Yeah, the failure paths seem untested.

>> (I actually have an idea to completely remove the packet list from
>> matroskadec.)
> 
> Should i commit this while you work on that, or just withdraw this
> patch? It's not urgent at all, since any deprecation period is 2+ years.
> Either option is fine by me.
> 
I don't want to have deprecation warnings.

>>
>>> +    goto fail;
>>> +
>>>   /* The next thing is a segment. */
>>>   pos = avio_tell(matroska->ctx->pb);
>>>   res = ebml_parse(matroska, matroska_segments, matroska);
>>> @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext
>>> *s)
>>>   st->disposition |=
>>> AV_DISPOSITION_ATTACHED_PIC;
>>>   st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>>>   -    av_init_packet(pkt);
>>> +    av_packet_unref(pkt);
>>>   pkt->buf  = attachments[j].bin.buf;
>>>   attachments[j].bin.buf = NULL;
>>>   pkt->data = attachments[j].bin.data;
>>> @@ -3180,7 +3186,7 @@ static int
>>> matroska_parse_rm_audio(MatroskaDemuxContext *matroska,
>>>     while (track->audio.pkt_cnt) {
>>>   int ret;
>>> -    AVPacket pktl, *pkt = &pktl;
>>> +    AVPacket *pkt = matroska->pkt;
>>>     ret = av_new_packet(pkt, a);
>>>   if (ret < 0) {
>>> @@ -3317,7 +3323,7 @@ static int
>>> matroska_parse_webvtt(MatroskaDemuxContext *matroska,
>>>    uint64_t duration,
>>>    int64_t pos)
>>>   {
>>> -    AVPacket pktl, *pkt = &pktl;
>>> +    AVPacket *pkt = matroska->pkt;
>>>   uint8_t *id, *settings, *text, *buf;
>>>   int id_len, settings_len, text_len;
>>>   uint8_t *p, *q;
>>> @@ -3434,7 +3440,7 @@ static int
>>> matroska_parse_frame(MatroskaDemuxContext *matroska,
>>>   {
>>>   uint8_t *pkt_data = data;
>>>   int res = 0;
>>> -    AVPacket pktl, *pkt = &pktl;
>>> +    AVPacket *pkt = matroska->pkt;
>>>     if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) {
>>>   res = matroska_parse_wavpack(track, &pkt_data, &pkt_size);
>>> @@ -3464,7 +3470,7 @@ static int
>>> matroska_parse_frame(MatroskaDemuxContext *matroska,
>>>   if (!pkt_size && !additional_size)
>>>   goto no_output;
>>>   -    av_init_packet(pkt);
>>> +    av_packet_unref(pkt);
>>>   if (!buf)
>>>   pkt->buf = av_buffer_create(pkt_data, pkt_size +
>>> AV_INPUT_BUFFER_PADDING_SIZE,
>>>   NULL, NULL, 0);
>>> @@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s)
>>>   int n;
>>>     matroska_clear_queue(matroska);
>>> +    av_packet_free(&matroska->pkt);
>>>     for (n = 0; n < matroska->tracks.nb_elem; n++)
>>>   if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)
>>>
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>>
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

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

Re: [FFmpeg-devel] [PATCH 30/50] avformat/subtitles: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> Signed-off-by: James Almer 
> ---
>  libavformat/jacosubdec.c |  2 +-
>  libavformat/mpeg.c   |  4 +--
>  libavformat/mpsubdec.c   |  4 +--
>  libavformat/subtitles.c  | 64 +++-
>  libavformat/subtitles.h  |  2 +-
>  libavformat/tedcaptionsdec.c |  4 +--
>  6 files changed, 42 insertions(+), 38 deletions(-)
> 
> diff --git a/libavformat/jacosubdec.c b/libavformat/jacosubdec.c
> index 14221b166c..b44e3b7783 100644
> --- a/libavformat/jacosubdec.c
> +++ b/libavformat/jacosubdec.c
> @@ -250,7 +250,7 @@ static int jacosub_read_header(AVFormatContext *s)
>  /* SHIFT and TIMERES affect the whole script so packet timing can only be
>   * done in a second pass */
>  for (i = 0; i < jacosub->q.nb_subs; i++) {
> -AVPacket *sub = &jacosub->q.subs[i];
> +AVPacket *sub = jacosub->q.subs[i];
>  read_ts(jacosub, sub->data, &sub->pts, &sub->duration);
>  }
>  ff_subtitles_queue_finalize(s, &jacosub->q);
> diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
> index 20d1e10168..79610ec600 100644
> --- a/libavformat/mpeg.c
> +++ b/libavformat/mpeg.c
> @@ -934,7 +934,7 @@ static int vobsub_read_packet(AVFormatContext *s, 
> AVPacket *pkt)
>  if (tmpq->current_sub_idx >= tmpq->nb_subs)
>  continue;
>  
> -ts = tmpq->subs[tmpq->current_sub_idx].pts;
> +ts = tmpq->subs[tmpq->current_sub_idx]->pts;
>  if (ts < min_ts) {
>  min_ts = ts;
>  sid = i;
> @@ -950,7 +950,7 @@ static int vobsub_read_packet(AVFormatContext *s, 
> AVPacket *pkt)
>  /* compute maximum packet size using the next packet position. This is
>   * useful when the len in the header is non-sense */
>  if (q->current_sub_idx < q->nb_subs) {
> -psize = q->subs[q->current_sub_idx].pos - pkt->pos;
> +psize = q->subs[q->current_sub_idx]->pos - pkt->pos;
>  } else {
>  int64_t fsize = avio_size(pb);
>  psize = fsize < 0 ? 0x : fsize - pkt->pos;
> diff --git a/libavformat/mpsubdec.c b/libavformat/mpsubdec.c
> index 2e6dc883eb..c113be5eba 100644
> --- a/libavformat/mpsubdec.c
> +++ b/libavformat/mpsubdec.c
> @@ -147,8 +147,8 @@ static int mpsub_read_header(AVFormatContext *s)
>  if (common_factor > 1) {
>  common_factor = av_gcd(pts_info.num, common_factor);
>  for (i = 0; i < mpsub->q.nb_subs; i++) {
> -mpsub->q.subs[i].pts  /= common_factor;
> -mpsub->q.subs[i].duration /= common_factor;
> +mpsub->q.subs[i]->pts  /= common_factor;
> +mpsub->q.subs[i]->duration /= common_factor;
>  }
>  pts_info.num /= common_factor;
>  }
> diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
> index ad7f68938e..ec10b99822 100644
> --- a/libavformat/subtitles.c
> +++ b/libavformat/subtitles.c
> @@ -111,13 +111,13 @@ int ff_text_peek_r8(FFTextReader *r)
>  AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue *q,
>  const uint8_t *event, size_t len, int 
> merge)
>  {
> -AVPacket *subs, *sub;
> +AVPacket **subs, *sub;
>  
>  if (merge && q->nb_subs > 0) {
>  /* merge with previous event */
>  
>  int old_len;
> -sub = &q->subs[q->nb_subs - 1];
> +sub = q->subs[q->nb_subs - 1];
>  old_len = sub->size;
>  if (av_grow_packet(sub, len) < 0)
>  return NULL;
> @@ -132,7 +132,10 @@ AVPacket 
> *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue *q,
>  if (!subs)
>  return NULL;
>  q->subs = subs;
> -sub = &subs[q->nb_subs];
> +subs[q->nb_subs] = av_packet_alloc();
> +if (!subs[q->nb_subs])
> +return NULL;
> +sub = subs[q->nb_subs];

q->nb_subs++. And remove the q->nb_subs++ below. Otherwise the packet
leaks if av_new_packet fails below.

>  if (av_new_packet(sub, len) < 0)
>  return NULL;
>  q->nb_subs++;
> @@ -145,8 +148,8 @@ AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue 
> *q,
>  
>  static int cmp_pkt_sub_ts_pos(const void *a, const void *b)
>  {
> -const AVPacket *s1 = a;
> -const AVPacket *s2 = b;
> +const AVPacket *s1 = *(const AVPacket **)a;
> +const AVPacket *s2 = *(const AVPacket **)b;
>  if (s1->pts == s2->pts)
>  return FFDIFFSIGN(s1->pos, s2->pos);
>  return FFDIFFSIGN(s1->pts , s2->pts);
> @@ -154,8 +157,8 @@ static int cmp_pkt_sub_ts_pos(const void *a, const void 
> *b)
>  
>  static int cmp_pkt_sub_pos_ts(const void *a, const void *b)
>  {
> -const AVPacket *s1 = a;
> -const AVPacket *s2 = b;
> +const AVPacket *s1 = *(const AVPacket **)a;
> +const AVPacket *s2 = *(const AVPacket **)b;
>  if (s1->pos == s2->pos) {
>  if (s1->pts == s2->pts)
>  return 0;
> @@ -170,18 +173,18 @@ static void drop_dups(void *log_ctx, 
> FFDemuxSubtitlesQueue *q)
>  
>  for (i 

Re: [FFmpeg-devel] [PATCH 21/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote:

James Almer:

On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
   libavformat/matroskadec.c | 17 -
   1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 374831baa3..9fad78c78b 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
   /* byte position of the segment inside the stream */
   int64_t segment_start;
   +    AVPacket *pkt;
+
   /* the packet queue */
   AVPacketList *queue;
   AVPacketList *queue_end;
@@ -2885,6 +2887,10 @@ static int
matroska_read_header(AVFormatContext *s)
   }
   ebml_free(ebml_syntax, &ebml);
   +    matroska->pkt = av_packet_alloc();
+    if (!matroska->pkt)


Missing AVERROR(ENOMEM).


This seems to be a common mistake in this set. Sorry.



Yeah, the failure paths seem untested.


make fate doesn't test failure paths. I recall someone did some manual 
runs limiting available memory long ago, which detected and removed a 
lot of unchecked av_mallocs, but nothing automated.

Fuzzing has handled failure path issue detection since then.




(I actually have an idea to completely remove the packet list from
matroskadec.)


Should i commit this while you work on that, or just withdraw this
patch? It's not urgent at all, since any deprecation period is 2+ years.
Either option is fine by me.


I don't want to have deprecation warnings.


Ok.






+    goto fail;
+
   /* The next thing is a segment. */
   pos = avio_tell(matroska->ctx->pb);
   res = ebml_parse(matroska, matroska_segments, matroska);
@@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext
*s)
   st->disposition |=
AV_DISPOSITION_ATTACHED_PIC;
   st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
   -    av_init_packet(pkt);
+    av_packet_unref(pkt);
   pkt->buf  = attachments[j].bin.buf;
   attachments[j].bin.buf = NULL;
   pkt->data = attachments[j].bin.data;
@@ -3180,7 +3186,7 @@ static int
matroska_parse_rm_audio(MatroskaDemuxContext *matroska,
     while (track->audio.pkt_cnt) {
   int ret;
-    AVPacket pktl, *pkt = &pktl;
+    AVPacket *pkt = matroska->pkt;
     ret = av_new_packet(pkt, a);
   if (ret < 0) {
@@ -3317,7 +3323,7 @@ static int
matroska_parse_webvtt(MatroskaDemuxContext *matroska,
    uint64_t duration,
    int64_t pos)
   {
-    AVPacket pktl, *pkt = &pktl;
+    AVPacket *pkt = matroska->pkt;
   uint8_t *id, *settings, *text, *buf;
   int id_len, settings_len, text_len;
   uint8_t *p, *q;
@@ -3434,7 +3440,7 @@ static int
matroska_parse_frame(MatroskaDemuxContext *matroska,
   {
   uint8_t *pkt_data = data;
   int res = 0;
-    AVPacket pktl, *pkt = &pktl;
+    AVPacket *pkt = matroska->pkt;
     if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) {
   res = matroska_parse_wavpack(track, &pkt_data, &pkt_size);
@@ -3464,7 +3470,7 @@ static int
matroska_parse_frame(MatroskaDemuxContext *matroska,
   if (!pkt_size && !additional_size)
   goto no_output;
   -    av_init_packet(pkt);
+    av_packet_unref(pkt);
   if (!buf)
   pkt->buf = av_buffer_create(pkt_data, pkt_size +
AV_INPUT_BUFFER_PADDING_SIZE,
   NULL, NULL, 0);
@@ -3838,6 +3844,7 @@ static int matroska_read_close(AVFormatContext *s)
   int n;
     matroska_clear_queue(matroska);
+    av_packet_free(&matroska->pkt);
     for (n = 0; n < matroska->tracks.nb_elem; n++)
   if (tracks[n].type == MATROSKA_TRACK_TYPE_AUDIO)



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

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



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

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


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

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



___
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/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:
 James Almer:
> Signed-off-by: James Almer 
> ---
>    libavformat/matroskadec.c | 17 -
>    1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 374831baa3..9fad78c78b 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
>    /* byte position of the segment inside the stream */
>    int64_t segment_start;
>    +    AVPacket *pkt;
> +
>    /* the packet queue */
>    AVPacketList *queue;
>    AVPacketList *queue_end;
> @@ -2885,6 +2887,10 @@ static int
> matroska_read_header(AVFormatContext *s)
>    }
>    ebml_free(ebml_syntax, &ebml);
>    +    matroska->pkt = av_packet_alloc();
> +    if (!matroska->pkt)

 Missing AVERROR(ENOMEM).
>>>
>>> This seems to be a common mistake in this set. Sorry.
>>>
>>
>> Yeah, the failure paths seem untested.
> 
> make fate doesn't test failure paths. I recall someone did some manual
> runs limiting available memory long ago, which detected and removed a
> lot of unchecked av_mallocs, but nothing automated.
> Fuzzing has handled failure path issue detection since then.
> 

No, fuzzing does not simulate low-memory environments (at least ossfuzz
as-is does not). I think it errors out on OOM when one tries to allocate
a lot, but small stuff like AVPackets don't trigger this. I have a long
list of patches in libavcodec where an earlier allocation in an init
function would leak if a subsequent allocation fails; or even something
like 96061c5a4f690c3ab49e4458701bb013fd3dd57f where the close function
assumed that several allocations were successful. These bugs existed for
years and the fuzzer never found them.
This means that all these errors paths need to be manually tested, so
that something that adds error paths (like requiring things to be
allocated) adds a burden on devs (in addition to the runtime overhead).

>>
 (I actually have an idea to completely remove the packet list from
 matroskadec.)
>>>
>>> Should i commit this while you work on that, or just withdraw this
>>> patch? It's not urgent at all, since any deprecation period is 2+ years.
>>> Either option is fine by me.
>>>
>> I don't want to have deprecation warnings.
> 
> Ok.
> 
>>

> +    goto fail;
> +
>    /* The next thing is a segment. */
>    pos = avio_tell(matroska->ctx->pb);
>    res = ebml_parse(matroska, matroska_segments, matroska);
> @@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext
> *s)
>    st->disposition |=
> AV_DISPOSITION_ATTACHED_PIC;
>    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>    -    av_init_packet(pkt);
> +    av_packet_unref(pkt);
>    pkt->buf  = attachments[j].bin.buf;
>    attachments[j].bin.buf = NULL;
>    pkt->data = attachments[j].bin.data;
> @@ -3180,7 +3186,7 @@ static int
> matroska_parse_rm_audio(MatroskaDemuxContext *matroska,
>      while (track->audio.pkt_cnt) {
>    int ret;
> -    AVPacket pktl, *pkt = &pktl;
> +    AVPacket *pkt = matroska->pkt;
>      ret = av_new_packet(pkt, a);
>    if (ret < 0) {
> @@ -3317,7 +3323,7 @@ static int
> matroska_parse_webvtt(MatroskaDemuxContext *matroska,
>     uint64_t duration,
>     int64_t pos)
>    {
> -    AVPacket pktl, *pkt = &pktl;
> +    AVPacket *pkt = matroska->pkt;
>    uint8_t *id, *settings, *text, *buf;
>    int id_len, settings_len, text_len;
>    uint8_t *p, *q;
> @@ -3434,7 +3440,7 @@ static int
> matroska_parse_frame(MatroskaDemuxContext *matroska,
>    {
>    uint8_t *pkt_data = data;
>    int res = 0;
> -    AVPacket pktl, *pkt = &pktl;
> +    AVPacket *pkt = matroska->pkt;
>      if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) {
>    res = matroska_parse_wavpack(track, &pkt_data, &pkt_size);
> @@ -3464,7 +3470,7 @@ static int
> matroska_parse_frame(MatroskaDemuxContext *matroska,
>    if (!pkt_size && !additional_size)
>    goto no_output;
>    -    av_init_packet(pkt);
> +    av_packet_unref(pkt);
>    if (!buf)
>    pkt->buf = av_buffer_create(pkt_data, pkt_size +
> AV_INPUT_BUFFER_PADDING_SIZE,
>    NULL, NULL, 0);
> @@ -3838,6 +3844,7 @@ static int
> matroska_read_close(AVFormatContext *s)
>   

Re: [FFmpeg-devel] [PATCH 21/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 12:57 PM, Andreas Rheinhardt wrote:

James Almer:

On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote:

James Almer:

On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
    libavformat/matroskadec.c | 17 -
    1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 374831baa3..9fad78c78b 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
    /* byte position of the segment inside the stream */
    int64_t segment_start;
    +    AVPacket *pkt;
+
    /* the packet queue */
    AVPacketList *queue;
    AVPacketList *queue_end;
@@ -2885,6 +2887,10 @@ static int
matroska_read_header(AVFormatContext *s)
    }
    ebml_free(ebml_syntax, &ebml);
    +    matroska->pkt = av_packet_alloc();
+    if (!matroska->pkt)


Missing AVERROR(ENOMEM).


This seems to be a common mistake in this set. Sorry.



Yeah, the failure paths seem untested.


make fate doesn't test failure paths. I recall someone did some manual
runs limiting available memory long ago, which detected and removed a
lot of unchecked av_mallocs, but nothing automated.
Fuzzing has handled failure path issue detection since then.



No, fuzzing does not simulate low-memory environments (at least ossfuzz
as-is does not).


I know, which is why i said failure path issues, not OOM specifically.
Most failure paths it triggers are due to invalid data. But outside of 
fuzzing, nothing tests any kind of failure path (Maybe static 
analyzers?). All FATE tests do is ensure working scenarios keep working.



I think it errors out on OOM when one tries to allocate
a lot, but small stuff like AVPackets don't trigger this. I have a long
list of patches in libavcodec where an earlier allocation in an init
function would leak if a subsequent allocation fails; or even something
like 96061c5a4f690c3ab49e4458701bb013fd3dd57f where the close function
assumed that several allocations were successful. These bugs existed for
years and the fuzzer never found them.
This means that all these errors paths need to be manually tested, so
that something that adds error paths (like requiring things to be
allocated) adds a burden on devs (in addition to the runtime overhead).




(I actually have an idea to completely remove the packet list from
matroskadec.)


Should i commit this while you work on that, or just withdraw this
patch? It's not urgent at all, since any deprecation period is 2+ years.
Either option is fine by me.


I don't want to have deprecation warnings.


Ok.






+    goto fail;
+
    /* The next thing is a segment. */
    pos = avio_tell(matroska->ctx->pb);
    res = ebml_parse(matroska, matroska_segments, matroska);
@@ -2947,7 +2953,7 @@ static int matroska_read_header(AVFormatContext
*s)
    st->disposition |=
AV_DISPOSITION_ATTACHED_PIC;
    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
    -    av_init_packet(pkt);
+    av_packet_unref(pkt);
    pkt->buf  = attachments[j].bin.buf;
    attachments[j].bin.buf = NULL;
    pkt->data = attachments[j].bin.data;
@@ -3180,7 +3186,7 @@ static int
matroska_parse_rm_audio(MatroskaDemuxContext *matroska,
      while (track->audio.pkt_cnt) {
    int ret;
-    AVPacket pktl, *pkt = &pktl;
+    AVPacket *pkt = matroska->pkt;
      ret = av_new_packet(pkt, a);
    if (ret < 0) {
@@ -3317,7 +3323,7 @@ static int
matroska_parse_webvtt(MatroskaDemuxContext *matroska,
     uint64_t duration,
     int64_t pos)
    {
-    AVPacket pktl, *pkt = &pktl;
+    AVPacket *pkt = matroska->pkt;
    uint8_t *id, *settings, *text, *buf;
    int id_len, settings_len, text_len;
    uint8_t *p, *q;
@@ -3434,7 +3440,7 @@ static int
matroska_parse_frame(MatroskaDemuxContext *matroska,
    {
    uint8_t *pkt_data = data;
    int res = 0;
-    AVPacket pktl, *pkt = &pktl;
+    AVPacket *pkt = matroska->pkt;
      if (st->codecpar->codec_id == AV_CODEC_ID_WAVPACK) {
    res = matroska_parse_wavpack(track, &pkt_data, &pkt_size);
@@ -3464,7 +3470,7 @@ static int
matroska_parse_frame(MatroskaDemuxContext *matroska,
    if (!pkt_size && !additional_size)
    goto no_output;
    -    av_init_packet(pkt);
+    av_packet_unref(pkt);
    if (!buf)
    pkt->buf = av_buffer_create(pkt_data, pkt_size +
AV_INPUT_BUFFER_PADDING_SIZE,
    NULL, NULL, 0);
@@ -3838,6 +3844,7 @@ static int
matroska_read_close(AVFormatContext *s)
    int n;
      matroska_clear_queue(matroska);
+    av_packet_free(&matroska->pkt);
      for (n = 0; n < matroska->tracks.nb_elem; n++)
    if (tracks[n

Re: [FFmpeg-devel] [PATCH 21/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
  libavformat/matroskadec.c | 17 -
  1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 374831baa3..9fad78c78b 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
  /* byte position of the segment inside the stream */
  int64_t segment_start;
  
+AVPacket *pkt;

+
  /* the packet queue */
  AVPacketList *queue;
  AVPacketList *queue_end;
@@ -2885,6 +2887,10 @@ static int matroska_read_header(AVFormatContext *s)
  }
  ebml_free(ebml_syntax, &ebml);
  
+matroska->pkt = av_packet_alloc();

+if (!matroska->pkt)


Missing AVERROR(ENOMEM).


I think at this point i can just return ENOMEM without jumping to fail.
___
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/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> On 2/8/2021 12:57 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote:
 James Almer:
> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Signed-off-by: James Almer 
>>> ---
>>>     libavformat/matroskadec.c | 17 -
>>>     1 file changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index 374831baa3..9fad78c78b 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
>>>     /* byte position of the segment inside the stream */
>>>     int64_t segment_start;
>>>     +    AVPacket *pkt;
>>> +
>>>     /* the packet queue */
>>>     AVPacketList *queue;
>>>     AVPacketList *queue_end;
>>> @@ -2885,6 +2887,10 @@ static int
>>> matroska_read_header(AVFormatContext *s)
>>>     }
>>>     ebml_free(ebml_syntax, &ebml);
>>>     +    matroska->pkt = av_packet_alloc();
>>> +    if (!matroska->pkt)
>>
>> Missing AVERROR(ENOMEM).
>
> This seems to be a common mistake in this set. Sorry.
>

 Yeah, the failure paths seem untested.
>>>
>>> make fate doesn't test failure paths. I recall someone did some manual
>>> runs limiting available memory long ago, which detected and removed a
>>> lot of unchecked av_mallocs, but nothing automated.
>>> Fuzzing has handled failure path issue detection since then.
>>>
>>
>> No, fuzzing does not simulate low-memory environments (at least ossfuzz
>> as-is does not).
> 
> I know, which is why i said failure path issues, not OOM specifically.

Then I don't get the point of you mentioning fuzzing as handling failure
path issues in your reply to my comment about untested allocation error
paths.

> Most failure paths it triggers are due to invalid data. But outside of
> fuzzing, nothing tests any kind of failure path (Maybe static
> analyzers?). All FATE tests do is ensure working scenarios keep working.
> 

Static analyzers do, but they typically only find the easy ones. E.g.
Coverity would probably have found the memleak in avpriv_dv_init_demux
that your patch introduced, but not the memleaks in the dv demuxer
(because in the latter case, the packets are still accessible via the
still existing DVDemuxContext and it doesn't know that they will not be
freed).
(And of course it should be mentioned that not all parts of FFmpeg are
systematically fuzzed: Encoders, muxers and filters are not.)

>> I think it errors out on OOM when one tries to allocate
>> a lot, but small stuff like AVPackets don't trigger this. I have a long
>> list of patches in libavcodec where an earlier allocation in an init
>> function would leak if a subsequent allocation fails; or even something
>> like 96061c5a4f690c3ab49e4458701bb013fd3dd57f where the close function
>> assumed that several allocations were successful. These bugs existed for
>> years and the fuzzer never found them.
>> This means that all these errors paths need to be manually tested, so
>> that something that adds error paths (like requiring things to be
>> allocated) adds a burden on devs (in addition to the runtime overhead).
>>

>> (I actually have an idea to completely remove the packet list from
>> matroskadec.)
>
> Should i commit this while you work on that, or just withdraw this
> patch? It's not urgent at all, since any deprecation period is 2+
> years.
> Either option is fine by me.
>
 I don't want to have deprecation warnings.
>>>
>>> Ok.
>>>

>>
>>> +    goto fail;
>>> +
>>>     /* The next thing is a segment. */
>>>     pos = avio_tell(matroska->ctx->pb);
>>>     res = ebml_parse(matroska, matroska_segments, matroska);
>>> @@ -2947,7 +2953,7 @@ static int
>>> matroska_read_header(AVFormatContext
>>> *s)
>>>     st->disposition |=
>>> AV_DISPOSITION_ATTACHED_PIC;
>>>     st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>>>     -    av_init_packet(pkt);
>>> +    av_packet_unref(pkt);
>>>     pkt->buf  = attachments[j].bin.buf;
>>>     attachments[j].bin.buf = NULL;
>>>     pkt->data = attachments[j].bin.data;
>>> @@ -3180,7 +3186,7 @@ static int
>>> matroska_parse_rm_audio(MatroskaDemuxContext *matroska,
>>>       while (track->audio.pkt_cnt) {
>>>     int ret;
>>> -    AVPacket pktl, *pkt = &pktl;
>>> +    AVPacket *pkt = matroska->pkt;
>>>       ret = av_new_packet(pkt, a);
>>>     if (ret < 0) {
>>> @@ -3317,7 +3323,7 @@ static int
>>> matroska_parse_webvtt(MatroskaDemuxContext *matroska,
>>>    

Re: [FFmpeg-devel] [PATCH 21/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Signed-off-by: James Almer 
>>> ---
>>>   libavformat/matroskadec.c | 17 -
>>>   1 file changed, 12 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>>> index 374831baa3..9fad78c78b 100644
>>> --- a/libavformat/matroskadec.c
>>> +++ b/libavformat/matroskadec.c
>>> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
>>>   /* byte position of the segment inside the stream */
>>>   int64_t segment_start;
>>>   +    AVPacket *pkt;
>>> +
>>>   /* the packet queue */
>>>   AVPacketList *queue;
>>>   AVPacketList *queue_end;
>>> @@ -2885,6 +2887,10 @@ static int
>>> matroska_read_header(AVFormatContext *s)
>>>   }
>>>   ebml_free(ebml_syntax, &ebml);
>>>   +    matroska->pkt = av_packet_alloc();
>>> +    if (!matroska->pkt)
>>
>> Missing AVERROR(ENOMEM).
> 
> I think at this point i can just return ENOMEM without jumping to fail.

Yes.

- 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 21/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 1:44 PM, Andreas Rheinhardt wrote:

James Almer:

On 2/8/2021 12:57 PM, Andreas Rheinhardt wrote:

James Almer:

On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote:

James Almer:

On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
     libavformat/matroskadec.c | 17 -
     1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 374831baa3..9fad78c78b 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
     /* byte position of the segment inside the stream */
     int64_t segment_start;
     +    AVPacket *pkt;
+
     /* the packet queue */
     AVPacketList *queue;
     AVPacketList *queue_end;
@@ -2885,6 +2887,10 @@ static int
matroska_read_header(AVFormatContext *s)
     }
     ebml_free(ebml_syntax, &ebml);
     +    matroska->pkt = av_packet_alloc();
+    if (!matroska->pkt)


Missing AVERROR(ENOMEM).


This seems to be a common mistake in this set. Sorry.



Yeah, the failure paths seem untested.


make fate doesn't test failure paths. I recall someone did some manual
runs limiting available memory long ago, which detected and removed a
lot of unchecked av_mallocs, but nothing automated.
Fuzzing has handled failure path issue detection since then.



No, fuzzing does not simulate low-memory environments (at least ossfuzz
as-is does not).


I know, which is why i said failure path issues, not OOM specifically.


Then I don't get the point of you mentioning fuzzing as handling failure
path issues in your reply to my comment about untested allocation error
paths.


If you look at the above exchange, you started with the general 
statement "failure paths seem untested", and i continued from there.

Apologies if it was confusing.
___
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/50] avformat/matroskadec: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> On 2/8/2021 1:44 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 2/8/2021 12:57 PM, Andreas Rheinhardt wrote:
 James Almer:
> On 2/8/2021 12:32 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 2/8/2021 12:22 PM, Andreas Rheinhardt wrote:
 James Almer:
> Signed-off-by: James Almer 
> ---
>  libavformat/matroskadec.c | 17 -
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 374831baa3..9fad78c78b 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -365,6 +365,8 @@ typedef struct MatroskaDemuxContext {
>  /* byte position of the segment inside the stream */
>  int64_t segment_start;
>  +    AVPacket *pkt;
> +
>  /* the packet queue */
>  AVPacketList *queue;
>  AVPacketList *queue_end;
> @@ -2885,6 +2887,10 @@ static int
> matroska_read_header(AVFormatContext *s)
>  }
>  ebml_free(ebml_syntax, &ebml);
>  +    matroska->pkt = av_packet_alloc();
> +    if (!matroska->pkt)

 Missing AVERROR(ENOMEM).
>>>
>>> This seems to be a common mistake in this set. Sorry.
>>>
>>
>> Yeah, the failure paths seem untested.
>
> make fate doesn't test failure paths. I recall someone did some manual
> runs limiting available memory long ago, which detected and removed a
> lot of unchecked av_mallocs, but nothing automated.
> Fuzzing has handled failure path issue detection since then.
>

 No, fuzzing does not simulate low-memory environments (at least ossfuzz
 as-is does not).
>>>
>>> I know, which is why i said failure path issues, not OOM specifically.
>>
>> Then I don't get the point of you mentioning fuzzing as handling failure
>> path issues in your reply to my comment about untested allocation error
>> paths.
> 
> If you look at the above exchange, you started with the general
> statement "failure paths seem untested", and i continued from there.
> Apologies if it was confusing.

I was actually thinking about the failure paths added by your set (after
all, you started the above exchange with a comment about your set); but
yes, the issue is more widespread, unfortunately.
Apologies for being so confusing.

- 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 1/2] avformat/wtvdec: Check len in parse_chunks() to avoid overflow

2021-02-08 Thread Michael Niedermayer
On Tue, Feb 09, 2021 at 02:20:41AM +1100, Peter Ross wrote:
> On Mon, Feb 08, 2021 at 02:29:01PM +0100, Michael Niedermayer wrote:
> > Fixes: signed integer overflow: 2147483647 + 7 cannot be represented in 
> > type 'int'
> > Fixes: 
> > 30084/clusterfuzz-testcase-minimized-ffmpeg_dem_WTV_fuzzer-6192261941559296
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/wtvdec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/wtvdec.c b/libavformat/wtvdec.c
> > index 6c41e3c1a3..7def9d2348 100644
> > --- a/libavformat/wtvdec.c
> > +++ b/libavformat/wtvdec.c
> > @@ -794,7 +794,7 @@ static int parse_chunks(AVFormatContext *s, int mode, 
> > int64_t seekts, int *len_p
> >  
> >  ff_get_guid(pb, &g);
> >  len = avio_rl32(pb);
> > -if (len < 32) {
> > +if (len < 32 || len > INT_MAX - 7) {
> >  int ret;
> >  if (avio_feof(pb))
> >  return AVERROR_EOF;
> 
> the + 7 comes from WTV_PAD calculation

yes


> looks good

will apply

thx

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

Those who are best at talking, realize last or never when they are wrong.


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

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

Re: [FFmpeg-devel] [PATCH 1/8] avformat/mov: factor size out of probe code

2021-02-08 Thread Michael Niedermayer
On Sat, Feb 06, 2021 at 11:05:35PM +0100, Paul B Mahol wrote:
> LGTM

will apply

thx

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

Observe your enemies, for they first find out your faults. -- Antisthenes


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

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

[FFmpeg-devel] [PATCH 1/2] avcodec/fitsdec: properly initialize header->data_max

2021-02-08 Thread Paul B Mahol
Signed-off-by: Paul B Mahol 
---
 libavcodec/fitsdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/fitsdec.c b/libavcodec/fitsdec.c
index 32a79cdd0d..802aa5b509 100644
--- a/libavcodec/fitsdec.c
+++ b/libavcodec/fitsdec.c
@@ -63,7 +63,7 @@ static int fill_data_min_max(const uint8_t *ptr8, FITSHeader 
*header, const uint
 int i, j;
 
 header->data_min = DBL_MAX;
-header->data_max = DBL_MIN;
+header->data_max = -DBL_MAX;
 switch (header->bitpix) {
 #define CASE_N(a, t, rd) \
 case a: \
-- 
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/2] avformat/fitsenc: write DATAMIN/DATAMAX to encoded output

2021-02-08 Thread Paul B Mahol
There is no point in doing normalization when such files are decoded.

Update fate test with new results.

Signed-off-by: Paul B Mahol 
---
 libavformat/fitsenc.c | 39 ++-
 tests/ref/fate/fits-demux | 10 -
 tests/ref/fate/fitsdec-gray   |  2 +-
 tests/ref/lavf/gbrap.fits |  2 +-
 tests/ref/lavf/gbrap16be.fits |  2 +-
 tests/ref/lavf/gbrp.fits  |  2 +-
 tests/ref/lavf/gbrp16be.fits  |  2 +-
 tests/ref/lavf/gray.fits  |  2 +-
 tests/ref/lavf/gray16be.fits  |  4 ++--
 9 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/libavformat/fitsenc.c b/libavformat/fitsenc.c
index cc3999aa8a..212c769df1 100644
--- a/libavformat/fitsenc.c
+++ b/libavformat/fitsenc.c
@@ -45,7 +45,8 @@ static int fits_write_header(AVFormatContext *s)
  * @param lines_written to keep track of lines written so far
  * @return 0
  */
-static int write_keyword_value(AVFormatContext *s, const char *keyword, int 
value, int *lines_written)
+static int write_keyword_value(AVFormatContext *s, const char *fmt,
+   const char *keyword, void *value, int 
*lines_written)
 {
 int len, ret;
 uint8_t header[80];
@@ -57,7 +58,12 @@ static int write_keyword_value(AVFormatContext *s, const 
char *keyword, int valu
 header[8] = '=';
 header[9] = ' ';
 
-ret = snprintf(header + 10, 70, "%d", value);
+if (!strcmp(fmt, "%d")) {
+ret = snprintf(header + 10, 70, fmt, *(int *)value);
+} else {
+ret = snprintf(header + 10, 70, fmt, *(float *)value);
+}
+
 memset(&header[ret + 10], ' ', sizeof(header) - (ret + 10));
 
 avio_write(s->pb, header, sizeof(header));
@@ -72,16 +78,22 @@ static int write_image_header(AVFormatContext *s)
 FITSContext *fitsctx = s->priv_data;
 uint8_t buffer[80];
 int bitpix, naxis, naxis3 = 1, bzero = 0, rgb = 0, lines_written = 0, 
lines_left;
+int pcount = 0, gcount = 1;
+float datamax, datamin;
 
 switch (encctx->format) {
 case AV_PIX_FMT_GRAY8:
 bitpix = 8;
 naxis = 2;
+datamin = 0;
+datamax = 255;
 break;
 case AV_PIX_FMT_GRAY16BE:
 bitpix = 16;
 naxis = 2;
 bzero = 32768;
+datamin = 0;
+datamax = 65535;
 break;
 case AV_PIX_FMT_GBRP:
 case AV_PIX_FMT_GBRAP:
@@ -93,6 +105,8 @@ static int write_image_header(AVFormatContext *s)
 } else {
 naxis3 = 4;
 }
+datamin = 0;
+datamax = 255;
 break;
 case AV_PIX_FMT_GBRP16BE:
 case AV_PIX_FMT_GBRAP16BE:
@@ -105,6 +119,8 @@ static int write_image_header(AVFormatContext *s)
 naxis3 = 4;
 }
 bzero = 32768;
+datamin = 0;
+datamax = 65535;
 break;
 default:
 return AVERROR(EINVAL);
@@ -122,28 +138,31 @@ static int write_image_header(AVFormatContext *s)
 }
 lines_written++;
 
-write_keyword_value(s, "BITPIX", bitpix, &lines_written); // no of 
bits per pixel
-write_keyword_value(s, "NAXIS", naxis, &lines_written);   // no of 
dimensions of image
-write_keyword_value(s, "NAXIS1", encctx->width, &lines_written);   // 
first dimension i.e. width
-write_keyword_value(s, "NAXIS2", encctx->height, &lines_written);  // 
second dimension i.e. height
+write_keyword_value(s, "%d", "BITPIX", &bitpix, &lines_written); 
// no of bits per pixel
+write_keyword_value(s, "%d", "NAXIS", &naxis, &lines_written);   
// no of dimensions of image
+write_keyword_value(s, "%d", "NAXIS1", &encctx->width, &lines_written);   
// first dimension i.e. width
+write_keyword_value(s, "%d", "NAXIS2", &encctx->height, &lines_written);  
// second dimension i.e. height
 
 if (rgb)
-write_keyword_value(s, "NAXIS3", naxis3, &lines_written); // third 
dimension to store RGBA planes
+write_keyword_value(s, "%d", "NAXIS3", &naxis3, &lines_written); 
// third dimension to store RGBA planes
 
 if (!fitsctx->first_image) {
-write_keyword_value(s, "PCOUNT", 0, &lines_written);
-write_keyword_value(s, "GCOUNT", 1, &lines_written);
+write_keyword_value(s, "%d", "PCOUNT", &pcount, &lines_written);
+write_keyword_value(s, "%d", "GCOUNT", &gcount, &lines_written);
 } else {
 fitsctx->first_image = 0;
 }
 
+write_keyword_value(s, "%g", "DATAMIN", &datamin, &lines_written);
+write_keyword_value(s, "%g", "DATAMAX", &datamax, &lines_written);
+
 /*
  * Since FITS does not support unsigned 16 bit integers,
  * BZERO = 32768 is used to store unsigned 16 bit integers as
  * signed integers so that it can be read properly.
  */
 if (bitpix == 16)
-write_keyword_value(s, "BZERO", bzero, &lines_written);
+write_keyword_value(s, "%d", "B

Re: [FFmpeg-devel] [PATCH 15/50] avformat/avienc: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> Signed-off-by: James Almer 
> ---
>  libavformat/avienc.c | 18 --
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/avienc.c b/libavformat/avienc.c
> index 1b2cb529b9..58bd081fcb 100644
> --- a/libavformat/avienc.c
> +++ b/libavformat/avienc.c
> @@ -85,6 +85,8 @@ typedef struct AVIStream {
>  
>  int64_t last_dts;
>  
> +AVPacket *empty_packet;
> +
>  AVIIndex indexes;
>  
>  int64_t strh_flags_offset;
> @@ -275,9 +277,17 @@ static int avi_write_header(AVFormatContext *s)
>  }
>  
>  for (n = 0; n < s->nb_streams; n++) {
> +AVIStream *avist;
> +
>  s->streams[n]->priv_data = av_mallocz(sizeof(AVIStream));
>  if (!s->streams[n]->priv_data)
>  return AVERROR(ENOMEM);
> +
> +avist = s->streams[n]->priv_data;
> +avist->empty_packet = av_packet_alloc();
> +if (!avist->empty_packet)
> +return AVERROR(ENOMEM);
> +avist->empty_packet->stream_index = n;

There is no need to allocate a packet per AVIStream.

>  }
>  
>  /* header list */
> @@ -745,18 +755,13 @@ static int write_skip_frames(AVFormatContext *s, int 
> stream_index, int64_t dts)
>  ff_dlog(s, "dts:%s packet_count:%d stream_index:%d\n", av_ts2str(dts), 
> avist->packet_count, stream_index);
>  while (par->block_align == 0 && dts != AV_NOPTS_VALUE &&
> dts > avist->packet_count && par->codec_id != AV_CODEC_ID_XSUB && 
> avist->packet_count) {
> -AVPacket empty_packet;
>  
>  if (dts - avist->packet_count > 6) {
>  av_log(s, AV_LOG_ERROR, "Too large number of skipped frames 
> %"PRId64" > 6\n", dts - avist->packet_count);
>  return AVERROR(EINVAL);
>  }
>  
> -av_init_packet(&empty_packet);
> -empty_packet.size = 0;
> -empty_packet.data = NULL;
> -empty_packet.stream_index = stream_index;
> -avi_write_packet_internal(s, &empty_packet);
> +avi_write_packet_internal(s, avist->empty_packet);
>  ff_dlog(s, "dup dts:%s packet_count:%d\n", av_ts2str(dts), 
> avist->packet_count);
>  }
>  
> @@ -985,6 +990,7 @@ static void avi_deinit(AVFormatContext *s)
>  for (int j = 0; j < avist->indexes.ents_allocated / 
> AVI_INDEX_CLUSTER_SIZE; j++)
>  av_freep(&avist->indexes.cluster[j]);
>  av_freep(&avist->indexes.cluster);
> +av_packet_free(&avist->empty_packet);
>  avist->indexes.ents_allocated = avist->indexes.entry = 0;
>  }
>  }
> 

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

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

Re: [FFmpeg-devel] [PATCH 23/50] avformat/mux: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> Signed-off-by: James Almer 
> ---
>  libavformat/internal.h |  5 +
>  libavformat/mux.c  | 44 ++
>  libavformat/options.c  |  6 ++
>  libavformat/utils.c|  1 +
>  4 files changed, 35 insertions(+), 21 deletions(-)
> 
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index d0db331b96..69a7caff93 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -92,6 +92,11 @@ struct AVFormatInternal {
>   */
>  struct AVPacketList *parse_queue;
>  struct AVPacketList *parse_queue_end;
> +
> +/**
> + * Used to hold temporary packets.
> + */
> +AVPacket *pkt;
>  /**
>   * Remaining size available for raw_packet_buffer, in bytes.
>   */
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 84c56ac6ba..3600e74a50 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -1052,7 +1052,9 @@ int ff_interleaved_peek(AVFormatContext *s, int stream,
>  AVPacketList *pktl = s->internal->packet_buffer;
>  while (pktl) {
>  if (pktl->pkt.stream_index == stream) {
> -*pkt = pktl->pkt;
> +int ret = av_packet_ref(pkt, &pktl->pkt);
> +if (ret < 0)
> +return ret;

This will lead to memleaks, because up until now the callers just
received a non-ownership packets, ergo they did not unref the packet.
(The fate-movenc test fails with this when run under valgrind/asan.)

Returning a pointer to a const AVPacket and signaling the offsetted
timestamps via other pointer arguments would avoid this problem.

>  if (add_offset) {
>  AVStream *st = s->streams[pkt->stream_index];
>  int64_t offset = st->internal->mux_ts_offset;
> @@ -1208,7 +1210,7 @@ static int write_packets_common(AVFormatContext *s, 
> AVPacket *pkt, int interleav
>  
>  int av_write_frame(AVFormatContext *s, AVPacket *in)
>  {
> -AVPacket local_pkt, *pkt = &local_pkt;
> +AVPacket *pkt = s->internal->pkt;
>  int ret;
>  
>  if (!in) {
> @@ -1229,6 +1231,7 @@ int av_write_frame(AVFormatContext *s, AVPacket *in)
>   * The following avoids copying in's data unnecessarily.
>   * Copying side data is unavoidable as a bitstream filter
>   * may change it, e.g. free it on errors. */
> +av_packet_unref(pkt);
>  pkt->buf  = NULL;
>  pkt->data = in->data;
>  pkt->size = in->size;
> @@ -1270,14 +1273,14 @@ int av_interleaved_write_frame(AVFormatContext *s, 
> AVPacket *pkt)
>  int av_write_trailer(AVFormatContext *s)
>  {
>  int i, ret1, ret = 0;
> -AVPacket pkt = {0};
> -av_init_packet(&pkt);
> +AVPacket *pkt = s->internal->pkt;
>  
> +av_packet_unref(pkt);
>  for (i = 0; i < s->nb_streams; i++) {
>  if (s->streams[i]->internal->bsfc) {
> -ret1 = write_packets_from_bsfs(s, s->streams[i], &pkt, 
> 1/*interleaved*/);
> +ret1 = write_packets_from_bsfs(s, s->streams[i], pkt, 
> 1/*interleaved*/);
>  if (ret1 < 0)
> -av_packet_unref(&pkt);
> +av_packet_unref(pkt);
>  if (ret >= 0)
>  ret = ret1;
>  }
> @@ -1351,7 +1354,7 @@ static void uncoded_frame_free(void *unused, uint8_t 
> *data)
>  static int write_uncoded_frame_internal(AVFormatContext *s, int stream_index,
>  AVFrame *frame, int interleaved)
>  {
> -AVPacket pkt, *pktp;
> +AVPacket *pkt = s->internal->pkt;
>  
>  av_assert0(s->oformat);
>  if (!s->oformat->write_uncoded_frame) {
> @@ -1360,18 +1363,17 @@ static int 
> write_uncoded_frame_internal(AVFormatContext *s, int stream_index,
>  }
>  
>  if (!frame) {
> -pktp = NULL;
> +pkt = NULL;
>  } else {
>  size_t   bufsize = sizeof(frame) + AV_INPUT_BUFFER_PADDING_SIZE;
>  AVFrame **framep = av_mallocz(bufsize);
>  
>  if (!framep)
>  goto fail;
> -pktp = &pkt;
> -av_init_packet(&pkt);
> -pkt.buf = av_buffer_create((void *)framep, bufsize,
> +av_packet_unref(pkt);
> +pkt->buf = av_buffer_create((void *)framep, bufsize,
> uncoded_frame_free, NULL, 0);
> -if (!pkt.buf) {
> +if (!pkt->buf) {
>  av_free(framep);
>  fail:
>  av_frame_free(&frame);
> @@ -1379,17 +1381,17 @@ static int 
> write_uncoded_frame_internal(AVFormatContext *s, int stream_index,
>  }
>  *framep = frame;
>  
> -pkt.data = (void *)framep;
> -pkt.size = sizeof(frame);
> -pkt.pts  =
> -pkt.dts  = frame->pts;
> -pkt.duration = frame->pkt_duration;
> -pkt.stream_index = stream_index;
> -pkt.flags |= AV_PKT_FLAG_UNCODED_FRAME;
> +pkt->data = (void *)framep;
> +pkt->size = sizeof(frame

Re: [FFmpeg-devel] [PATCH 19/50] avformat/flacdec: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> Signed-off-by: James Almer 
> ---
>  libavformat/flacdec.c | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
> index 6aca4755a1..7852a79d39 100644
> --- a/libavformat/flacdec.c
> +++ b/libavformat/flacdec.c
> @@ -259,7 +259,7 @@ static int flac_probe(const AVProbeData *p)
>  static av_unused int64_t flac_read_timestamp(AVFormatContext *s, int 
> stream_index,
>   int64_t *ppos, int64_t 
> pos_limit)
>  {
> -AVPacket pkt;
> +AVPacket *pkt;
>  AVStream *st = s->streams[stream_index];
>  AVCodecParserContext *parser;
>  int ret;
> @@ -268,9 +268,12 @@ static av_unused int64_t 
> flac_read_timestamp(AVFormatContext *s, int stream_inde
>  if (avio_seek(s->pb, *ppos, SEEK_SET) < 0)
>  return AV_NOPTS_VALUE;
>  
> -av_init_packet(&pkt);
> +pkt = av_packet_alloc();
> +if (!pkt)
> +return AV_NOPTS_VALUE;
>  parser = av_parser_init(st->codecpar->codec_id);
>  if (!parser){
> +av_packet_free(&pkt);
>  return AV_NOPTS_VALUE;
>  }
>  parser->flags |= PARSER_FLAG_USE_CODEC_TS;
> @@ -279,20 +282,20 @@ static av_unused int64_t 
> flac_read_timestamp(AVFormatContext *s, int stream_inde
>  uint8_t *data;
>  int size;
>  
> -ret = ff_raw_read_partial_packet(s, &pkt);
> +ret = ff_raw_read_partial_packet(s, pkt);
>  if (ret < 0){
>  if (ret == AVERROR(EAGAIN))
>  continue;
>  else {
> -av_packet_unref(&pkt);
> -av_assert1(!pkt.size);
> +av_packet_unref(pkt);
> +av_assert1(!pkt->size);
>  }
>  }
>  av_parser_parse2(parser, st->internal->avctx,
> - &data, &size, pkt.data, pkt.size,
> - pkt.pts, pkt.dts, *ppos);
> + &data, &size, pkt->data, pkt->size,
> + pkt->pts, pkt->dts, *ppos);
>  
> -av_packet_unref(&pkt);
> +av_packet_unref(pkt);
>  if (size) {
>  if (parser->pts != AV_NOPTS_VALUE){
>  // seeking may not have started from beginning of a frame
> @@ -304,6 +307,7 @@ static av_unused int64_t 
> flac_read_timestamp(AVFormatContext *s, int stream_inde
>  } else if (ret < 0)
>  break;
>  }
> +av_packet_free(&pkt);
>  av_parser_close(parser);
>  return pts;
>  }
> 
The parse_packet is unused during this function, so it can be reused.

- 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 23/50] avformat/mux: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 3:22 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer
---
  libavformat/internal.h |  5 +
  libavformat/mux.c  | 44 ++
  libavformat/options.c  |  6 ++
  libavformat/utils.c|  1 +
  4 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/libavformat/internal.h b/libavformat/internal.h
index d0db331b96..69a7caff93 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -92,6 +92,11 @@ struct AVFormatInternal {
   */
  struct AVPacketList *parse_queue;
  struct AVPacketList *parse_queue_end;
+
+/**
+ * Used to hold temporary packets.
+ */
+AVPacket *pkt;
  /**
   * Remaining size available for raw_packet_buffer, in bytes.
   */
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 84c56ac6ba..3600e74a50 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -1052,7 +1052,9 @@ int ff_interleaved_peek(AVFormatContext *s, int stream,
  AVPacketList *pktl = s->internal->packet_buffer;
  while (pktl) {
  if (pktl->pkt.stream_index == stream) {
-*pkt = pktl->pkt;
+int ret = av_packet_ref(pkt, &pktl->pkt);
+if (ret < 0)
+return ret;

This will lead to memleaks, because up until now the callers just
received a non-ownership packets, ergo they did not unref the packet.
(The fate-movenc test fails with this when run under valgrind/asan.)

Returning a pointer to a const AVPacket and signaling the offsetted
timestamps via other pointer arguments would avoid this problem.


There's a single user of ff_interleaved_peek(), which is the movenc one, 
and i made it unref the packet right after using it. But that's done in 
the following patch, so i guess i should change this function and its 
one caller at the same time.


Also, i could just return the offsetted pts and dts values and not 
bother at all with packets. No allocations that way.

___
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 19/50] avformat/flacdec: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 3:43 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer 
---
  libavformat/flacdec.c | 20 
  1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
index 6aca4755a1..7852a79d39 100644
--- a/libavformat/flacdec.c
+++ b/libavformat/flacdec.c
@@ -259,7 +259,7 @@ static int flac_probe(const AVProbeData *p)
  static av_unused int64_t flac_read_timestamp(AVFormatContext *s, int 
stream_index,
   int64_t *ppos, int64_t pos_limit)
  {
-AVPacket pkt;
+AVPacket *pkt;
  AVStream *st = s->streams[stream_index];
  AVCodecParserContext *parser;
  int ret;
@@ -268,9 +268,12 @@ static av_unused int64_t 
flac_read_timestamp(AVFormatContext *s, int stream_inde
  if (avio_seek(s->pb, *ppos, SEEK_SET) < 0)
  return AV_NOPTS_VALUE;
  
-av_init_packet(&pkt);

+pkt = av_packet_alloc();
+if (!pkt)
+return AV_NOPTS_VALUE;
  parser = av_parser_init(st->codecpar->codec_id);
  if (!parser){
+av_packet_free(&pkt);
  return AV_NOPTS_VALUE;
  }
  parser->flags |= PARSER_FLAG_USE_CODEC_TS;
@@ -279,20 +282,20 @@ static av_unused int64_t 
flac_read_timestamp(AVFormatContext *s, int stream_inde
  uint8_t *data;
  int size;
  
-ret = ff_raw_read_partial_packet(s, &pkt);

+ret = ff_raw_read_partial_packet(s, pkt);
  if (ret < 0){
  if (ret == AVERROR(EAGAIN))
  continue;
  else {
-av_packet_unref(&pkt);
-av_assert1(!pkt.size);
+av_packet_unref(pkt);
+av_assert1(!pkt->size);
  }
  }
  av_parser_parse2(parser, st->internal->avctx,
- &data, &size, pkt.data, pkt.size,
- pkt.pts, pkt.dts, *ppos);
+ &data, &size, pkt->data, pkt->size,
+ pkt->pts, pkt->dts, *ppos);
  
-av_packet_unref(&pkt);

+av_packet_unref(pkt);
  if (size) {
  if (parser->pts != AV_NOPTS_VALUE){
  // seeking may not have started from beginning of a frame
@@ -304,6 +307,7 @@ static av_unused int64_t 
flac_read_timestamp(AVFormatContext *s, int stream_inde
  } else if (ret < 0)
  break;
  }
+av_packet_free(&pkt);
  av_parser_close(parser);
  return pts;
  }


The parse_packet is unused during this function, so it can be reused.


The AVFormatInternal one? Good idea.
___
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] [RFC][v3] Tech Resolution Process

2021-02-08 Thread Lynne
Feb 8, 2021, 15:23 by j...@videolan.org:

> ---
>  doc/dev_community/resolution_process.md | 91 +
>  1 file changed, 91 insertions(+)
>  create mode 100644 doc/dev_community/resolution_process.md
>
> diff --git a/doc/dev_community/resolution_process.md 
> b/doc/dev_community/resolution_process.md
> new file mode 100644
> index 00..6da3e8ffb2
> --- /dev/null
> +++ b/doc/dev_community/resolution_process.md
> @@ -0,0 +1,91 @@
> +# Technical Committee
> +
> +_This document only makes sense with the rules from [the community 
> document](community)_.
> +
> +The Technical Committee (**TC**) is here to arbitrate and make decisions when
> +technical conflicts occur in the project.
> +
> +The TC main role is to resolve technical conflicts.
> +It is therefore not a technical steering committee, but it is understood that
> +some decisions might impact the future of the project.
> +
> +# Process
> +
> +## Seizing
> +
> +The TC can take possession of any technical matter that it sees fit.
> +
> +To involve the TC in a matter, email tc@ or CC them on an ongoing discussion.
> +
> +As members of TC are developers, they also can email tc@ to raise an issue.
> +
> +## Announcement
> +
> +The TC, once seized, must announce itself on the main mailing list, with a 
> _[TC]_ tag.
> +
> +The TC has 2 modes of operation: a RFC one and an internal one.
> +
> +If the TC thinks it needs the input from the larger community, the TC can 
> call
> +for a RFC. Else, it can decide by itself.
> +
> +If the disagreement involves a member of the TC, that member should recuse
> +themselves from the decision.
> +
> +The decision to use a RFC process or an internal discussion is a 
> discretionary
> +decision of the TC.
> +
> +The TC can also reject a seizure for a few reasons such as:
> +the matter was not discussed enough previously; it lacks expertise to reach a
> +beneficial decision on the matter; or the matter is too trivial.
> +
> +### RFC call
> +
> +In the RFC mode, one person from the TC posts on the mailing list the
> +technical question and will request input from the community.
> +
> +The mail will have the following specification:
> +* a precise title
> +* a specific tag [TC RFC]
> +* a top-level email
> +* contain a precise question that does not exceed 100 words and that is 
> answerable by developers
> +* may have an extra description, or a link to a previous discussion, if 
> deemed necessary,
> +* contain a precise end date for the answers.
> +
> +The answers from the community must be on the main mailing list and must have
> +the following specification:
> +* keep the tag and the title unchanged
> +* limited to 400 words
> +* a first-level, answering directly to the main email
> +* answering to the question.
> +
> +Further replies to answers are permitted, as long as they conform to the
> +community standards of politeness, they are limited to 100 words, and are not
> +nested more than once. (max-depth=2)
> +
> +After the end-date, mails on the thread will be ignored.
> +
> +Violations of those rules will be escalated through the Community Committee.
> +
> +After all the emails are in, the TC has 96 hours to give its final decision.
> +Exceptionally, the TC can request an extra delay, that will be notified on 
> the
> +mailing list.
> +
> +### Within TC
> +
> +In the internal case, the TC has 96 hours to give its final decision.
> +Exceptionally, the TC can request an extra delay.
> +
> +
> +## Decisions
> +
> +The decisions from the TC will be sent on the mailing list, with the _[TC]_ 
> tag.
> +
> +Internally, the TC should take decisions with a majority, or using
> +ranked-choice voting.
> +
> +The decision from the TC should be published with a summary of the reasons 
> that
> +lead to this decision.
> +
> +The decisions from the TC are final, until the matters are reopened after
> +no less than one year, by either the GA or the TC auto-seizing.
>

Is there an OR there? Can the question be raised again after more than
a year by anyone, OR at any time by either the GA or the TC?
___
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 36/50] avfilter/vf_mcdeint: use av_packet_alloc() to allocate packets

2021-02-08 Thread Andreas Rheinhardt
James Almer:
> Signed-off-by: James Almer 
> ---
>  libavfilter/vf_mcdeint.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vf_mcdeint.c b/libavfilter/vf_mcdeint.c
> index bc7b3230d3..26baf94adb 100644
> --- a/libavfilter/vf_mcdeint.c
> +++ b/libavfilter/vf_mcdeint.c
> @@ -74,6 +74,7 @@ typedef struct MCDeintContext {
>  int mode;   ///< MCDeintMode
>  int parity; ///< MCDeintParity
>  int qp;
> +AVPacket *pkt;
>  AVCodecContext *enc_ctx;
>  } MCDeintContext;
>  
> @@ -112,6 +113,9 @@ static int config_props(AVFilterLink *inlink)
>  return AVERROR(EINVAL);
>  }
>  
> +mcdeint->pkt = av_packet_alloc();
> +if (!mcdeint->pkt)
> +return AVERROR(ENOMEM);
>  mcdeint->enc_ctx = avcodec_alloc_context3(enc);
>  if (!mcdeint->enc_ctx)
>  return AVERROR(ENOMEM);
> @@ -154,6 +158,7 @@ static av_cold void uninit(AVFilterContext *ctx)
>  {
>  MCDeintContext *mcdeint = ctx->priv;
>  
> +av_packet_free(&mcdeint->pkt);
>  avcodec_free_context(&mcdeint->enc_ctx);
>  }
>  
> @@ -173,7 +178,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *inpic)
>  MCDeintContext *mcdeint = inlink->dst->priv;
>  AVFilterLink *outlink = inlink->dst->outputs[0];
>  AVFrame *outpic, *frame_dec;
> -AVPacket pkt = {0};
> +AVPacket *pkt = mcdeint->pkt;
>  int x, y, i, ret, got_frame = 0;
>  
>  outpic = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> @@ -184,9 +189,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *inpic)
>  av_frame_copy_props(outpic, inpic);
>  inpic->quality = mcdeint->qp * FF_QP2LAMBDA;
>  
> -av_init_packet(&pkt);
> +av_packet_unref(pkt);

Unnecessary: The packet is clean at the beginning, because it will
always be uninitialized at the end.

>  
> -ret = avcodec_encode_video2(mcdeint->enc_ctx, &pkt, inpic, &got_frame);
> +ret = avcodec_encode_video2(mcdeint->enc_ctx, pkt, inpic, &got_frame);
>  if (ret < 0)
>  goto end;
>  
> @@ -274,7 +279,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *inpic)
>  mcdeint->parity ^= 1;
>  
>  end:
> -av_packet_unref(&pkt);
> +av_packet_unref(pkt);
>  av_frame_free(&inpic);
>  if (ret < 0) {
>  av_frame_free(&outpic);
> 

___
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 support for the new key & value API in libaom.

2021-02-08 Thread Bohan Li
Gentle ping :)

On Thu, Feb 4, 2021 at 1:45 PM Bohan Li  wrote:

> Thanks for the review! Indeed the buf string does not play any role here.
> I have removed it in the new patch.
>
> Bohan
>
> On Thu, Feb 4, 2021 at 1:02 PM Bohan Li  wrote:
>
>> This key & value API can greatly help with users who wants to try
>> libaom-av1 specific options that are not supported by ffmpeg options.
>>
>> As was previously discussed in this thread:
>> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658.
>>
>> The commit that added the API to libaom:
>> https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257
>>
>> The libaom issue tracker:
>> https://bugs.chromium.org/p/aomedia/issues/detail?id=2875
>>
>> Signed-off-by: Bohan Li 
>> ---
>>  doc/encoders.texi  | 11 +++
>>  libavcodec/libaomenc.c | 30 ++
>>  2 files changed, 41 insertions(+)
>>
>> diff --git a/doc/encoders.texi b/doc/encoders.texi
>> index c2ba7d3e6f..9fab512892 100644
>> --- a/doc/encoders.texi
>> +++ b/doc/encoders.texi
>> @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true.
>>  @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >=
>> v2.0.0)
>>  Enable smooth interintra mode. Default is true.
>>
>> +@item libaom-params
>> +Set libaom options using a list of @var{key}=@var{value} pairs separated
>> +by ":". For a list of supported options, see @command{aomenc --help}
>> under the
>> +section "AV1 Specific Options".
>> +
>> +For example to specify libaom encoding options with
>> @option{-libaom-params}:
>> +
>> +@example
>> +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params
>> tune=psnr:enable-tpl-model=1 output.mp4
>> +@end example
>> +
>>  @end table
>>
>>  @section libsvtav1
>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
>> index 342d0883e4..c7a87e01cd 100644
>> --- a/libavcodec/libaomenc.c
>> +++ b/libavcodec/libaomenc.c
>> @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
>>  int enable_diff_wtd_comp;
>>  int enable_dist_wtd_comp;
>>  int enable_dual_filter;
>> +AVDictionary *extra_params;
>>  } AOMContext;
>>
>>  static const char *const ctlidstr[] = {
>> @@ -318,6 +319,25 @@ static av_cold int codecctl_int(AVCodecContext
>> *avctx,
>>  return 0;
>>  }
>>
>> +static av_cold int codec_set_option(AVCodecContext *avctx,
>> +const char* key,
>> +const char* value)
>> +{
>> +AOMContext *ctx = avctx->priv_data;
>> +int width = -30;
>> +int res;
>> +
>> +av_log(avctx, AV_LOG_DEBUG, "  %*s: %s\n", width, key, value);
>> +
>> +res = aom_codec_set_option(&ctx->encoder, key, value);
>> +if (res != AOM_CODEC_OK) {
>> +log_encoder_error(avctx, key);
>> +return AVERROR(EINVAL);
>> +}
>> +
>> +return 0;
>> +}
>> +
>>  static av_cold int aom_free(AVCodecContext *avctx)
>>  {
>>  AOMContext *ctx = avctx->priv_data;
>> @@ -874,6 +894,15 @@ static av_cold int aom_init(AVCodecContext *avctx,
>>  codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC,
>> ctx->enable_intrabc);
>>  #endif
>>
>> +#if AOM_ENCODER_ABI_VERSION >= 23
>> +{
>> +  AVDictionaryEntry *en = NULL;
>> +  while ((en = av_dict_get(ctx->extra_params, "", en,
>> AV_DICT_IGNORE_SUFFIX))) {
>> +codec_set_option(avctx, en->key, en->value);
>> +  }
>> +}
>> +#endif
>> +
>>  // provide dummy value to initialize wrapper, values will be updated
>> each _encode()
>>  aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1,
>>   (unsigned char*)1);
>> @@ -1299,6 +1328,7 @@ static const AVOption options[] = {
>>  { "enable-masked-comp",   "Enable masked compound",
>>   OFFSET(enable_masked_comp),   AV_OPT_TYPE_BOOL,
>> {.i64 = -1}, -1, 1, VE},
>>  { "enable-interintra-comp",   "Enable interintra compound",
>>   OFFSET(enable_interintra_comp),   AV_OPT_TYPE_BOOL,
>> {.i64 = -1}, -1, 1, VE},
>>  { "enable-smooth-interintra", "Enable smooth interintra mode",
>>OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL,
>> {.i64 = -1}, -1, 1, VE},
>> +{ "libaom-params","Extra parameters for libaom",
>>OFFSET(extra_params), AV_OPT_TYPE_DICT,
>> { 0 },0, 0, VE },
>>  { NULL },
>>  };
>>
>> --
>> 2.30.0.365.g02bc693789-goog
>>
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Add support for the new key & value API in libaom.

2021-02-08 Thread Christopher Degawa
Quick question as a user that is not entirely related to this patch, would
it be possible to add something like a `ffmpeg ... -libaom-params help -f
null -` or someother way that would print out the options available in the
compiled libaom? (In the case where we do not have aomenc compiled or it's
compiled using a different libaom version)
___
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 support for the new key & value API in libaom.

2021-02-08 Thread Jan Ekström
On Mon, Feb 8, 2021 at 10:40 PM Christopher Degawa  wrote:
>
> Quick question as a user that is not entirely related to this patch, would
> it be possible to add something like a `ffmpeg ... -libaom-params help -f
> null -` or someother way that would print out the options available in the
> compiled libaom? (In the case where we do not have aomenc compiled or it's
> compiled using a different libaom version)

I don't think this is currently possible, but I did hack up a quick
PoC for dynamic help entries once after someone #ffmpeg raised that
it'd be nice to have a listing of things.

https://github.com/jeeb/ffmpeg/commits/avoption_dynamic_help

This PoC just added the presets from libx265 into a dynamic help text
for the preset option, visible with `ffmpeg -h encoder=libx265`. Of
course this just goes into the explanation field, so not like the
alternatives in some other fields where it iterates over things.

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

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

Re: [FFmpeg-devel] [PATCH 2/2] lavf/libsrt: deduplicate libsrt_network_wait_fd_timeout

2021-02-08 Thread Marton Balint



On Mon, 8 Feb 2021, "zhilizhao(赵志立)" wrote:





On Feb 8, 2021, at 2:36 AM, Marton Balint  wrote:



On Mon, 8 Feb 2021, Zhao Zhili wrote:


---
libavformat/libsrt.c | 45 +---
1 file changed, 17 insertions(+), 28 deletions(-)


Hmm, it seems my latest srt patches conflics with this a bit, and you will have to 
somewhat rebase this, sorry. On the bright side, now only eid is passed to 
libsrt_network_wait_fd, so maybe you can pass eid instead of fd, especially since 
it is not always the context->eid that is used for wait.


Thanks for review. Rebase is done:

https://patchwork.ffmpeg.org/project/ffmpeg/patch/tencent_ff259aef9f5aab614e6c70ef2d7a9a684...@qq.com/

libsrt_network_wait_fd_timeout is implemented on top of 
ff_network_wait_fd_timeout
to keep minimum code changes. libsrt_network_wait_fd_timeout can be removed if 
add a NetworkWaitFdCB field to SRTContext but more code need to be changed.

What do you think?


I think it is fine as is.

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 1/2] avformat/network: fix timeout inaccurate in wait_fd_timeout

2021-02-08 Thread Marton Balint



The wait_start was about POLLING_TIME larger which leads to timeout
100ms late than the option setting.
---
libavformat/network.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/libavformat/network.c b/libavformat/network.c
index 0f5a575f77..7a9a4be5bb 100644
--- a/libavformat/network.c
+++ b/libavformat/network.c
@@ -78,7 +78,10 @@ int ff_network_wait_fd(int fd, int write)
int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, 
AVIOInterruptCB *int_cb)
{
int ret;
-int64_t wait_start = 0;
+int64_t wait_start;
+
+if (timeout > 0)
+wait_start = av_gettime_relative();


I think we intentionally wanted to avoid calling gettime on the fast path.



while (1) {
if (ff_check_interrupt(int_cb))
@@ -86,12 +89,8 @@ int ff_network_wait_fd_timeout(int fd, int write, int64_t 
timeout, AVIOInterrupt
ret = ff_network_wait_fd(fd, write);
if (ret != AVERROR(EAGAIN))
return ret;
-if (timeout > 0) {
-if (!wait_start)
-wait_start = av_gettime_relative();


Why not simply wait_start = av_gettime_relative() - POLLING_TIME? It seems 
to achieve the same result.


Thanks,
Marton


-else if (av_gettime_relative() - wait_start > timeout)
-return AVERROR(ETIMEDOUT);
-}
+if (timeout > 0 && (av_gettime_relative() - wait_start > timeout))
+return AVERROR(ETIMEDOUT);
}
}

--
2.28.0

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

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

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

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

[FFmpeg-devel] [PATCH 1/2] avformat/mux: add ff_get_muxer_ts_offset()

2021-02-08 Thread James Almer
Will be useful in the next patch

Signed-off-by: James Almer 
---
 libavformat/internal.h |  1 +
 libavformat/mux.c  | 16 
 2 files changed, 17 insertions(+)

diff --git a/libavformat/internal.h b/libavformat/internal.h
index d0db331b96..33ece6b172 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -874,6 +874,7 @@ int ff_bprint_to_codecpar_extradata(AVCodecParameters *par, 
struct AVBPrint *buf
 int ff_interleaved_peek(AVFormatContext *s, int stream,
 AVPacket *pkt, int add_offset);
 
+int ff_get_muxer_ts_offset(AVFormatContext *s, int stream_index, int64_t 
*offset);
 
 int ff_lock_avformat(void);
 int ff_unlock_avformat(void);
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 84c56ac6ba..ae46844c66 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -1046,6 +1046,22 @@ int ff_interleave_packet_per_dts(AVFormatContext *s, 
AVPacket *out,
 }
 }
 
+int ff_get_muxer_ts_offset(AVFormatContext *s, int stream_index, int64_t 
*offset)
+{
+AVStream *st;
+
+if (stream_index < 0 || stream_index >= s->nb_streams)
+return AVERROR(EINVAL);
+
+st = s->streams[stream_index];
+*offset = st->internal->mux_ts_offset;
+
+if (s->output_ts_offset)
+*offset += av_rescale_q(s->output_ts_offset, AV_TIME_BASE_Q, 
st->time_base);
+
+return 0;
+}
+
 int ff_interleaved_peek(AVFormatContext *s, int stream,
 AVPacket *pkt, int add_offset)
 {
-- 
2.30.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 2/2] avformat/mux: return a pointer to the packet in ff_interleaved_peek()

2021-02-08 Thread James Almer
And make it const, so the caller doesn't attempt to change it.
ff_get_muxer_ts_offset() should be used to get the muxer timestamp offset.

Signed-off-by: James Almer 
---
Went with Andreas' idea, but splitting off the muxer ts offset part of the
process, so ff_interleaved_peek() doesn't need to use an extra output
argument for it.

 libavformat/internal.h |  8 ++--
 libavformat/movenc.c   | 17 ++---
 libavformat/mux.c  | 20 +++-
 3 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/libavformat/internal.h b/libavformat/internal.h
index 33ece6b172..0ffdc87b6a 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -865,14 +865,10 @@ int ff_bprint_to_codecpar_extradata(AVCodecParameters 
*par, struct AVBPrint *buf
 
 /**
  * Find the next packet in the interleaving queue for the given stream.
- * The pkt parameter is filled in with the queued packet, including
- * references to the data (which the caller is not allowed to keep or
- * modify).
  *
- * @return 0 if a packet was found, a negative value if no packet was found
+ * @return a pointer to a packet if one was found, NULL otherwise.
  */
-int ff_interleaved_peek(AVFormatContext *s, int stream,
-AVPacket *pkt, int add_offset);
+const AVPacket *ff_interleaved_peek(AVFormatContext *s, int stream);
 
 int ff_get_muxer_ts_offset(AVFormatContext *s, int stream_index, int64_t 
*offset);
 
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 372c04295d..db9b6122e6 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5303,15 +5303,18 @@ static int mov_flush_fragment(AVFormatContext *s, int 
force)
 for (i = 0; i < s->nb_streams; i++) {
 MOVTrack *track = &mov->tracks[i];
 if (!track->end_reliable) {
-AVPacket pkt;
-if (!ff_interleaved_peek(s, i, &pkt, 1)) {
+const AVPacket *pkt = ff_interleaved_peek(s, i);
+if (pkt) {
+int64_t offset, dts = pkt->dts, pts = pkt->pts;
+ff_get_muxer_ts_offset(s, i, &offset);
+dts += offset;
 if (track->dts_shift != AV_NOPTS_VALUE)
-pkt.dts += track->dts_shift;
-track->track_duration = pkt.dts - track->start_dts;
-if (pkt.pts != AV_NOPTS_VALUE)
-track->end_pts = pkt.pts;
+dts += track->dts_shift;
+track->track_duration = dts - track->start_dts;
+if (pts != AV_NOPTS_VALUE)
+track->end_pts = pts + offset;
 else
-track->end_pts = pkt.dts;
+track->end_pts = dts;
 }
 }
 }
diff --git a/libavformat/mux.c b/libavformat/mux.c
index ae46844c66..062ba8d789 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -1062,30 +1062,16 @@ int ff_get_muxer_ts_offset(AVFormatContext *s, int 
stream_index, int64_t *offset
 return 0;
 }
 
-int ff_interleaved_peek(AVFormatContext *s, int stream,
-AVPacket *pkt, int add_offset)
+const AVPacket *ff_interleaved_peek(AVFormatContext *s, int stream)
 {
 AVPacketList *pktl = s->internal->packet_buffer;
 while (pktl) {
 if (pktl->pkt.stream_index == stream) {
-*pkt = pktl->pkt;
-if (add_offset) {
-AVStream *st = s->streams[pkt->stream_index];
-int64_t offset = st->internal->mux_ts_offset;
-
-if (s->output_ts_offset)
-offset += av_rescale_q(s->output_ts_offset, 
AV_TIME_BASE_Q, st->time_base);
-
-if (pkt->dts != AV_NOPTS_VALUE)
-pkt->dts += offset;
-if (pkt->pts != AV_NOPTS_VALUE)
-pkt->pts += offset;
-}
-return 0;
+return &pktl->pkt;
 }
 pktl = pktl->next;
 }
-return AVERROR(ENOENT);
+return NULL;
 }
 
 /**
-- 
2.30.0

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

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

Re: [FFmpeg-devel] [PATCH 23/50] avformat/mux: use av_packet_alloc() to allocate packets

2021-02-08 Thread James Almer

On 2/8/2021 3:50 PM, James Almer wrote:

On 2/8/2021 3:22 PM, Andreas Rheinhardt wrote:

James Almer:

Signed-off-by: James Almer
---
  libavformat/internal.h |  5 +
  libavformat/mux.c  | 44 ++
  libavformat/options.c  |  6 ++
  libavformat/utils.c    |  1 +
  4 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/libavformat/internal.h b/libavformat/internal.h
index d0db331b96..69a7caff93 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -92,6 +92,11 @@ struct AVFormatInternal {
   */
  struct AVPacketList *parse_queue;
  struct AVPacketList *parse_queue_end;
+
+    /**
+ * Used to hold temporary packets.
+ */
+    AVPacket *pkt;
  /**
   * Remaining size available for raw_packet_buffer, in bytes.
   */
diff --git a/libavformat/mux.c b/libavformat/mux.c
index 84c56ac6ba..3600e74a50 100644
--- a/libavformat/mux.c
+++ b/libavformat/mux.c
@@ -1052,7 +1052,9 @@ int ff_interleaved_peek(AVFormatContext *s, int 
stream,

  AVPacketList *pktl = s->internal->packet_buffer;
  while (pktl) {
  if (pktl->pkt.stream_index == stream) {
-    *pkt = pktl->pkt;
+    int ret = av_packet_ref(pkt, &pktl->pkt);
+    if (ret < 0)
+    return ret;

This will lead to memleaks, because up until now the callers just
received a non-ownership packets, ergo they did not unref the packet.
(The fate-movenc test fails with this when run under valgrind/asan.)

Returning a pointer to a const AVPacket and signaling the offsetted
timestamps via other pointer arguments would avoid this problem.


There's a single user of ff_interleaved_peek(), which is the movenc one, 
and i made it unref the packet right after using it. But that's done in 
the following patch, so i guess i should change this function and its 
one caller at the same time.


Also, i could just return the offsetted pts and dts values and not 
bother at all with packets. No allocations that way.


Decided to implement your idea with a few changes in a separate patchset.
___
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] lavc/mpegvideo_enc: mark default_mv_penalty as const

2021-02-08 Thread Michael Niedermayer
On Mon, Feb 08, 2021 at 10:53:20AM +0100, Anton Khirnov wrote:
> Nothing is ever written into it.
> ---
>  libavcodec/mpegvideo_enc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index 34dcf8c313..a27c80ca37 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -82,7 +82,7 @@ static int sse_mb(MpegEncContext *s);
>  static void denoise_dct_c(MpegEncContext *s, int16_t *block);
>  static int dct_quantize_trellis_c(MpegEncContext *s, int16_t *block, int n, 
> int qscale, int *overflow);
>  
> -static uint8_t default_mv_penalty[MAX_FCODE + 1][MAX_DMV * 2 + 1];
> +static const uint8_t default_mv_penalty[MAX_FCODE + 1][MAX_DMV * 2 + 1];
>  static uint8_t default_fcode_tab[MAX_MV * 2 + 1];

if its optimal to have this the zero table instead of some non zero content
then i suggest to rename it to zero_mv_penalty

thx

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


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] Add support for the new key & value API in libaom.

2021-02-08 Thread Jan Ekström
On Thu, Feb 4, 2021 at 11:02 PM Bohan Li
 wrote:
>
> This key & value API can greatly help with users who wants to try
> libaom-av1 specific options that are not supported by ffmpeg options.
>

Excellent! Thank you for moving this forward :) .

I noticed various things which I will also notice here, but made a possible
fixup commit on a branch:
https://github.com/jeeb/ffmpeg/commits/libaom_key_value_api

If you think that is OK (in case I didn't mess anything up), I can
trim the commit message a bit and we should be done with this :) .

> As was previously discussed in this thread:
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658.
>
> The commit that added the API to libaom:
> https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257
>
> The libaom issue tracker:
> https://bugs.chromium.org/p/aomedia/issues/detail?id=2875
>
> Signed-off-by: Bohan Li 
> ---
>  doc/encoders.texi  | 11 +++
>  libavcodec/libaomenc.c | 30 ++
>  2 files changed, 41 insertions(+)
>
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index c2ba7d3e6f..9fab512892 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true.
>  @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0)
>  Enable smooth interintra mode. Default is true.
>
> +@item libaom-params
> +Set libaom options using a list of @var{key}=@var{value} pairs separated
> +by ":". For a list of supported options, see @command{aomenc --help} under 
> the
> +section "AV1 Specific Options".
> +
> +For example to specify libaom encoding options with @option{-libaom-params}:
> +
> +@example
> +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params 
> tune=psnr:enable-tpl-model=1 output.mp4
> +@end example
> +
>  @end table
>
>  @section libsvtav1
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index 342d0883e4..c7a87e01cd 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
>  int enable_diff_wtd_comp;
>  int enable_dist_wtd_comp;
>  int enable_dual_filter;
> +AVDictionary *extra_params;
>  } AOMContext;
>
>  static const char *const ctlidstr[] = {
> @@ -318,6 +319,25 @@ static av_cold int codecctl_int(AVCodecContext *avctx,
>  return 0;
>  }
>
> +static av_cold int codec_set_option(AVCodecContext *avctx,
> +const char* key,
> +const char* value)
> +{
> +AOMContext *ctx = avctx->priv_data;
> +int width = -30;
> +int res;
> +
> +av_log(avctx, AV_LOG_DEBUG, "  %*s: %s\n", width, key, value);
> +

My guess this was a left-over from some debugging. I think the width
and debug log line can be removed if log_encoder_error gives one a
useful error in the log?

> +res = aom_codec_set_option(&ctx->encoder, key, value);
> +if (res != AOM_CODEC_OK) {
> +log_encoder_error(avctx, key);
> +return AVERROR(EINVAL);

AVERROR_EXTERNAL seems to be utilized when something fails inside an
external library. To be fair, in this case maybe if we had separate
paths for AOM_CODEC_INVALID_PARAM and AOM_CODEC_ERROR we could utilize
EINVAL for one of them? But if we just handle both of them then
AVERROR_EXTERNAL might match both?

> +}
> +
> +return 0;
> +}
> +
>  static av_cold int aom_free(AVCodecContext *avctx)
>  {
>  AOMContext *ctx = avctx->priv_data;
> @@ -874,6 +894,15 @@ static av_cold int aom_init(AVCodecContext *avctx,
>  codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc);
>  #endif
>
> +#if AOM_ENCODER_ABI_VERSION >= 23
> +{

Indentation 2 VS 4 in this block here :) Probably just left-over
options from somewhere.

> +  AVDictionaryEntry *en = NULL;
> +  while ((en = av_dict_get(ctx->extra_params, "", en, 
> AV_DICT_IGNORE_SUFFIX))) {
> +codec_set_option(avctx, en->key, en->value);

This function does return an error, yet it is not handled here and
passed on. I will guess this is just a forgotten case, and not that
you want to specifically ignore those errors.

Thus in the review notes commit I just moved the little code that was
in codec_set_option to within this loop.

> +  }
> +}
> +#endif
> +
>  // provide dummy value to initialize wrapper, values will be updated 
> each _encode()
>  aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1,
>   (unsigned char*)1);
> @@ -1299,6 +1328,7 @@ static const AVOption options[] = {
>  { "enable-masked-comp",   "Enable masked compound",  
>   OFFSET(enable_masked_comp),   AV_OPT_TYPE_BOOL, {.i64 = 
> -1}, -1, 1, VE},
>  { "enable-interintra-comp",   "Enable interintra compound",  
>   OFFSET(enable_interintra_comp),   AV_OPT_TYPE_BOOL, {.i64 = 
> -1}, -1, 1, VE},
>  { "enable-smooth-interintra", "Ena

Re: [FFmpeg-devel] [PATCH] Add support for the new key & value API in libaom.

2021-02-08 Thread Bohan Li
Thank you very much for the reviews and the fixes, Jan!
I went over the fixup commit and I do agree with the patches you proposed.
It looks good to me.

Just to make sure I don't misunderstand, should I submit a new patch with
the modifications you mentioned, or leave the patch as is and you would
modify and apply it?

Thanks a lot!
Bohan

On Mon, Feb 8, 2021 at 3:47 PM Jan Ekström  wrote:

> On Thu, Feb 4, 2021 at 11:02 PM Bohan Li
>  wrote:
> >
> > This key & value API can greatly help with users who wants to try
> > libaom-av1 specific options that are not supported by ffmpeg options.
> >
>
> Excellent! Thank you for moving this forward :) .
>
> I noticed various things which I will also notice here, but made a possible
> fixup commit on a branch:
> https://github.com/jeeb/ffmpeg/commits/libaom_key_value_api
>
> If you think that is OK (in case I didn't mess anything up), I can
> trim the commit message a bit and we should be done with this :) .
>
> > As was previously discussed in this thread:
> > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658.
> >
> > The commit that added the API to libaom:
> > https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257
> >
> > The libaom issue tracker:
> > https://bugs.chromium.org/p/aomedia/issues/detail?id=2875
> >
> > Signed-off-by: Bohan Li 
> > ---
> >  doc/encoders.texi  | 11 +++
> >  libavcodec/libaomenc.c | 30 ++
> >  2 files changed, 41 insertions(+)
> >
> > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > index c2ba7d3e6f..9fab512892 100644
> > --- a/doc/encoders.texi
> > +++ b/doc/encoders.texi
> > @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true.
> >  @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >=
> v2.0.0)
> >  Enable smooth interintra mode. Default is true.
> >
> > +@item libaom-params
> > +Set libaom options using a list of @var{key}=@var{value} pairs separated
> > +by ":". For a list of supported options, see @command{aomenc --help}
> under the
> > +section "AV1 Specific Options".
> > +
> > +For example to specify libaom encoding options with
> @option{-libaom-params}:
> > +
> > +@example
> > +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params
> tune=psnr:enable-tpl-model=1 output.mp4
> > +@end example
> > +
> >  @end table
> >
> >  @section libsvtav1
> > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> > index 342d0883e4..c7a87e01cd 100644
> > --- a/libavcodec/libaomenc.c
> > +++ b/libavcodec/libaomenc.c
> > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
> >  int enable_diff_wtd_comp;
> >  int enable_dist_wtd_comp;
> >  int enable_dual_filter;
> > +AVDictionary *extra_params;
> >  } AOMContext;
> >
> >  static const char *const ctlidstr[] = {
> > @@ -318,6 +319,25 @@ static av_cold int codecctl_int(AVCodecContext
> *avctx,
> >  return 0;
> >  }
> >
> > +static av_cold int codec_set_option(AVCodecContext *avctx,
> > +const char* key,
> > +const char* value)
> > +{
> > +AOMContext *ctx = avctx->priv_data;
> > +int width = -30;
> > +int res;
> > +
> > +av_log(avctx, AV_LOG_DEBUG, "  %*s: %s\n", width, key, value);
> > +
>
> My guess this was a left-over from some debugging. I think the width
> and debug log line can be removed if log_encoder_error gives one a
> useful error in the log?
>
> > +res = aom_codec_set_option(&ctx->encoder, key, value);
> > +if (res != AOM_CODEC_OK) {
> > +log_encoder_error(avctx, key);
> > +return AVERROR(EINVAL);
>
> AVERROR_EXTERNAL seems to be utilized when something fails inside an
> external library. To be fair, in this case maybe if we had separate
> paths for AOM_CODEC_INVALID_PARAM and AOM_CODEC_ERROR we could utilize
> EINVAL for one of them? But if we just handle both of them then
> AVERROR_EXTERNAL might match both?
>
> > +}
> > +
> > +return 0;
> > +}
> > +
> >  static av_cold int aom_free(AVCodecContext *avctx)
> >  {
> >  AOMContext *ctx = avctx->priv_data;
> > @@ -874,6 +894,15 @@ static av_cold int aom_init(AVCodecContext *avctx,
> >  codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC,
> ctx->enable_intrabc);
> >  #endif
> >
> > +#if AOM_ENCODER_ABI_VERSION >= 23
> > +{
>
> Indentation 2 VS 4 in this block here :) Probably just left-over
> options from somewhere.
>
> > +  AVDictionaryEntry *en = NULL;
> > +  while ((en = av_dict_get(ctx->extra_params, "", en,
> AV_DICT_IGNORE_SUFFIX))) {
> > +codec_set_option(avctx, en->key, en->value);
>
> This function does return an error, yet it is not handled here and
> passed on. I will guess this is just a forgotten case, and not that
> you want to specifically ignore those errors.
>
> Thus in the review notes commit I just moved the little code that was
> in codec_set_option to within this loop.
>
> > +  }
> > +}
> > +#endif
> > +
> >  // provide dummy value to

Re: [FFmpeg-devel] [PATCH] Add support for the new key & value API in libaom.

2021-02-08 Thread Steven Liu
Bohan Li  于2021年2月9日周二 上午8:24写道:
>
> Thank you very much for the reviews and the fixes, Jan!
> I went over the fixup commit and I do agree with the patches you proposed.
> It looks good to me.
>
> Just to make sure I don't misunderstand, should I submit a new patch with
> the modifications you mentioned, or leave the patch as is and you would
> modify and apply it?

You should remake a new version patch use git format-patch -v 2 -s -1
and use git send-email --to with --in-reply-to jeeb's comments mail
message-id.

>
> Thanks a lot!
> Bohan
>
> On Mon, Feb 8, 2021 at 3:47 PM Jan Ekström  wrote:
>
> > On Thu, Feb 4, 2021 at 11:02 PM Bohan Li
> >  wrote:
> > >
> > > This key & value API can greatly help with users who wants to try
> > > libaom-av1 specific options that are not supported by ffmpeg options.
> > >
> >
> > Excellent! Thank you for moving this forward :) .
> >
> > I noticed various things which I will also notice here, but made a possible
> > fixup commit on a branch:
> > https://github.com/jeeb/ffmpeg/commits/libaom_key_value_api
> >
> > If you think that is OK (in case I didn't mess anything up), I can
> > trim the commit message a bit and we should be done with this :) .
> >
> > > As was previously discussed in this thread:
> > > https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658.
> > >
> > > The commit that added the API to libaom:
> > > https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257
> > >
> > > The libaom issue tracker:
> > > https://bugs.chromium.org/p/aomedia/issues/detail?id=2875
> > >
> > > Signed-off-by: Bohan Li 
> > > ---
> > >  doc/encoders.texi  | 11 +++
> > >  libavcodec/libaomenc.c | 30 ++
> > >  2 files changed, 41 insertions(+)
> > >
> > > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > > index c2ba7d3e6f..9fab512892 100644
> > > --- a/doc/encoders.texi
> > > +++ b/doc/encoders.texi
> > > @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true.
> > >  @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >=
> > v2.0.0)
> > >  Enable smooth interintra mode. Default is true.
> > >
> > > +@item libaom-params
> > > +Set libaom options using a list of @var{key}=@var{value} pairs separated
> > > +by ":". For a list of supported options, see @command{aomenc --help}
> > under the
> > > +section "AV1 Specific Options".
> > > +
> > > +For example to specify libaom encoding options with
> > @option{-libaom-params}:
> > > +
> > > +@example
> > > +ffmpeg -i input -c:v libaom-av1 -b:v 500K -libaom-params
> > tune=psnr:enable-tpl-model=1 output.mp4
> > > +@end example
> > > +
> > >  @end table
> > >
> > >  @section libsvtav1
> > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> > > index 342d0883e4..c7a87e01cd 100644
> > > --- a/libavcodec/libaomenc.c
> > > +++ b/libavcodec/libaomenc.c
> > > @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
> > >  int enable_diff_wtd_comp;
> > >  int enable_dist_wtd_comp;
> > >  int enable_dual_filter;
> > > +AVDictionary *extra_params;
> > >  } AOMContext;
> > >
> > >  static const char *const ctlidstr[] = {
> > > @@ -318,6 +319,25 @@ static av_cold int codecctl_int(AVCodecContext
> > *avctx,
> > >  return 0;
> > >  }
> > >
> > > +static av_cold int codec_set_option(AVCodecContext *avctx,
> > > +const char* key,
> > > +const char* value)
> > > +{
> > > +AOMContext *ctx = avctx->priv_data;
> > > +int width = -30;
> > > +int res;
> > > +
> > > +av_log(avctx, AV_LOG_DEBUG, "  %*s: %s\n", width, key, value);
> > > +
> >
> > My guess this was a left-over from some debugging. I think the width
> > and debug log line can be removed if log_encoder_error gives one a
> > useful error in the log?
> >
> > > +res = aom_codec_set_option(&ctx->encoder, key, value);
> > > +if (res != AOM_CODEC_OK) {
> > > +log_encoder_error(avctx, key);
> > > +return AVERROR(EINVAL);
> >
> > AVERROR_EXTERNAL seems to be utilized when something fails inside an
> > external library. To be fair, in this case maybe if we had separate
> > paths for AOM_CODEC_INVALID_PARAM and AOM_CODEC_ERROR we could utilize
> > EINVAL for one of them? But if we just handle both of them then
> > AVERROR_EXTERNAL might match both?
> >
> > > +}
> > > +
> > > +return 0;
> > > +}
> > > +
> > >  static av_cold int aom_free(AVCodecContext *avctx)
> > >  {
> > >  AOMContext *ctx = avctx->priv_data;
> > > @@ -874,6 +894,15 @@ static av_cold int aom_init(AVCodecContext *avctx,
> > >  codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC,
> > ctx->enable_intrabc);
> > >  #endif
> > >
> > > +#if AOM_ENCODER_ABI_VERSION >= 23
> > > +{
> >
> > Indentation 2 VS 4 in this block here :) Probably just left-over
> > options from somewhere.
> >
> > > +  AVDictionaryEntry *en = NULL;
> > > +  while ((en = av_dict_get(ctx->extra_params, "", en,
> > AV_DICT_IGNORE_S

Re: [FFmpeg-devel] [PATCH 1/2] avformat/network: fix timeout inaccurate in wait_fd_timeout

2021-02-08 Thread zhilizhao(赵志立)


> On Feb 9, 2021, at 6:03 AM, Marton Balint  wrote:
> 
>> 
>> The wait_start was about POLLING_TIME larger which leads to timeout
>> 100ms late than the option setting.
>> ---
>> libavformat/network.c | 13 ++---
>> 1 file changed, 6 insertions(+), 7 deletions(-)
>> 
>> diff --git a/libavformat/network.c b/libavformat/network.c
>> index 0f5a575f77..7a9a4be5bb 100644
>> --- a/libavformat/network.c
>> +++ b/libavformat/network.c
>> @@ -78,7 +78,10 @@ int ff_network_wait_fd(int fd, int write)
>> int ff_network_wait_fd_timeout(int fd, int write, int64_t timeout, 
>> AVIOInterruptCB *int_cb)
>> {
>>int ret;
>> -int64_t wait_start = 0;
>> +int64_t wait_start;
>> +
>> +if (timeout > 0)
>> +wait_start = av_gettime_relative();
> 
> I think we intentionally wanted to avoid calling gettime on the fast path.
> 
>> 
>>while (1) {
>>if (ff_check_interrupt(int_cb))
>> @@ -86,12 +89,8 @@ int ff_network_wait_fd_timeout(int fd, int write, int64_t 
>> timeout, AVIOInterrupt
>>ret = ff_network_wait_fd(fd, write);
>>if (ret != AVERROR(EAGAIN))
>>return ret;
>> -if (timeout > 0) {
>> -if (!wait_start)
>> -wait_start = av_gettime_relative();
> 
> Why not simply wait_start = av_gettime_relative() - POLLING_TIME? It seems to 
> achieve the same result.

Then it depends on the implementation of ff_network_wait_fd. After adding 
wait_fd as a callback,
there is no guarantee that it takes POLLING_TIME. It can be solved by adding a 
polling_time field
to NetworkWaitFdCB:

typedef struct NetworkWaitFdCB {
int (*wait_fd)(int /*fd*/, int /*write*/, void* /*opaque*/);
void *opaque;
int polling_time;
} NetworkWaitFdCB;

This is trying to get rid of calling gettime once. We can go further and 
sacrifice the accuracy by:

int64_t loop_count = (timeout > 0) ? (timeout / polling_time) : timeout;
for (int64_t i = 0; loop_count <= 0 || i < loop_count; i++) {
...
}

In my opinion, either depends on gettime or loop_count, but not both.

> 
> Thanks,
> Marton
> 
>> -else if (av_gettime_relative() - wait_start > timeout)
>> -return AVERROR(ETIMEDOUT);
>> -}
>> +if (timeout > 0 && (av_gettime_relative() - wait_start > timeout))
>> +return AVERROR(ETIMEDOUT);
>>}
>> }
>> -- 
>> 2.28.0
>> 
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> ___
> ffmpeg-devel 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 v2] Add support for the new key & value API in libaom.

2021-02-08 Thread Bohan Li
This key & value API can greatly help with users who wants to try
libaom-av1 specific options that are not supported by ffmpeg options.

As was previously discussed in this thread:
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658.

The commit that added the API to libaom:
https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257

The libaom issue tracker:
https://bugs.chromium.org/p/aomedia/issues/detail?id=2875

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

diff --git a/doc/encoders.texi b/doc/encoders.texi
index c2ba7d3e6f..8fb573c416 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true.
 @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0)
 Enable smooth interintra mode. Default is true.
 
+@item aom-params
+Set libaom options using a list of @var{key}=@var{value} pairs separated
+by ":". For a list of supported options, see @command{aomenc --help} under the
+section "AV1 Specific Options".
+
+For example to specify libaom encoding options with @option{-aom-params}:
+
+@example
+ffmpeg -i input -c:v libaom-av1 -b:v 500K -aom-params 
tune=psnr:enable-tpl-model=1 output.mp4
+@end example
+
 @end table
 
 @section libsvtav1
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 342d0883e4..9a26b5f9ef 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
 int enable_diff_wtd_comp;
 int enable_dist_wtd_comp;
 int enable_dual_filter;
+AVDictionary *aom_params;
 } AOMContext;
 
 static const char *const ctlidstr[] = {
@@ -874,6 +875,20 @@ static av_cold int aom_init(AVCodecContext *avctx,
 codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc);
 #endif
 
+#if AOM_ENCODER_ABI_VERSION >= 23
+{
+AVDictionaryEntry *en = NULL;
+
+while ((en = av_dict_get(ctx->aom_params, "", en, 
AV_DICT_IGNORE_SUFFIX))) {
+int ret = aom_codec_set_option(&ctx->encoder, en->key, en->value);
+if (ret != AOM_CODEC_OK) {
+log_encoder_error(avctx, en->key);
+return AVERROR_EXTERNAL;
+}
+}
+}
+#endif
+
 // provide dummy value to initialize wrapper, values will be updated each 
_encode()
 aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1,
  (unsigned char*)1);
@@ -1299,6 +1314,9 @@ static const AVOption options[] = {
 { "enable-masked-comp",   "Enable masked compound",
OFFSET(enable_masked_comp),   AV_OPT_TYPE_BOOL, {.i64 = 
-1}, -1, 1, VE},
 { "enable-interintra-comp",   "Enable interintra compound",
OFFSET(enable_interintra_comp),   AV_OPT_TYPE_BOOL, {.i64 = 
-1}, -1, 1, VE},
 { "enable-smooth-interintra", "Enable smooth interintra mode", 
OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL, {.i64 = 
-1}, -1, 1, VE},
+#if AOM_ENCODER_ABI_VERSION >= 23
+{ "aom-params",   "Set libaom options using a :-separated 
list of key=value pairs", OFFSET(aom_params), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE 
},
+#endif
 { NULL },
 };
 
-- 
2.30.0.478.g8a0d178c01-goog

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

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

Re: [FFmpeg-devel] [PATCH v2] Add support for the new key & value API in libaom.

2021-02-08 Thread Bohan Li
Thanks for the info! Just uploaded the patch v2 as suggested.

Best,
Bohan

On Mon, Feb 8, 2021 at 8:04 PM Bohan Li  wrote:

> This key & value API can greatly help with users who wants to try
> libaom-av1 specific options that are not supported by ffmpeg options.
>
> As was previously discussed in this thread:
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2020-October/271658.
>
> The commit that added the API to libaom:
> https://aomedia.googlesource.com/aom/+/c1d42fe6615c96fc929257
>
> The libaom issue tracker:
> https://bugs.chromium.org/p/aomedia/issues/detail?id=2875
>
> Signed-off-by: Bohan Li 
> ---
>  doc/encoders.texi  | 11 +++
>  libavcodec/libaomenc.c | 18 ++
>  2 files changed, 29 insertions(+)
>
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index c2ba7d3e6f..8fb573c416 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -1684,6 +1684,17 @@ Enable interintra compound. Default is true.
>  @item enable-smooth-interintra (@emph{boolean}) (Requires libaom >=
> v2.0.0)
>  Enable smooth interintra mode. Default is true.
>
> +@item aom-params
> +Set libaom options using a list of @var{key}=@var{value} pairs separated
> +by ":". For a list of supported options, see @command{aomenc --help}
> under the
> +section "AV1 Specific Options".
> +
> +For example to specify libaom encoding options with @option{-aom-params}:
> +
> +@example
> +ffmpeg -i input -c:v libaom-av1 -b:v 500K -aom-params
> tune=psnr:enable-tpl-model=1 output.mp4
> +@end example
> +
>  @end table
>
>  @section libsvtav1
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index 342d0883e4..9a26b5f9ef 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -124,6 +124,7 @@ typedef struct AOMEncoderContext {
>  int enable_diff_wtd_comp;
>  int enable_dist_wtd_comp;
>  int enable_dual_filter;
> +AVDictionary *aom_params;
>  } AOMContext;
>
>  static const char *const ctlidstr[] = {
> @@ -874,6 +875,20 @@ static av_cold int aom_init(AVCodecContext *avctx,
>  codecctl_int(avctx, AV1E_SET_ENABLE_INTRABC, ctx->enable_intrabc);
>  #endif
>
> +#if AOM_ENCODER_ABI_VERSION >= 23
> +{
> +AVDictionaryEntry *en = NULL;
> +
> +while ((en = av_dict_get(ctx->aom_params, "", en,
> AV_DICT_IGNORE_SUFFIX))) {
> +int ret = aom_codec_set_option(&ctx->encoder, en->key,
> en->value);
> +if (ret != AOM_CODEC_OK) {
> +log_encoder_error(avctx, en->key);
> +return AVERROR_EXTERNAL;
> +}
> +}
> +}
> +#endif
> +
>  // provide dummy value to initialize wrapper, values will be updated
> each _encode()
>  aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1,
>   (unsigned char*)1);
> @@ -1299,6 +1314,9 @@ static const AVOption options[] = {
>  { "enable-masked-comp",   "Enable masked compound",
>   OFFSET(enable_masked_comp),   AV_OPT_TYPE_BOOL,
> {.i64 = -1}, -1, 1, VE},
>  { "enable-interintra-comp",   "Enable interintra compound",
>   OFFSET(enable_interintra_comp),   AV_OPT_TYPE_BOOL,
> {.i64 = -1}, -1, 1, VE},
>  { "enable-smooth-interintra", "Enable smooth interintra mode",
>  OFFSET(enable_smooth_interintra), AV_OPT_TYPE_BOOL,
> {.i64 = -1}, -1, 1, VE},
> +#if AOM_ENCODER_ABI_VERSION >= 23
> +{ "aom-params",   "Set libaom options using a
> :-separated list of key=value pairs", OFFSET(aom_params), AV_OPT_TYPE_DICT,
> { 0 }, 0, 0, VE },
> +#endif
>  { NULL },
>  };
>
> --
> 2.30.0.478.g8a0d178c01-goog
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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