Re: [FFmpeg-devel] [PATCH] vf_pseudocolor.c: fix build warning by adding braces
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
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
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
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
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
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
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
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
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
--- 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
--- 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()
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()
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()
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()
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
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
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
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
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()
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()
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
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.
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
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
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()
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()
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
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
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.
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.
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.
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
> 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.
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.
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".