[FFmpeg-devel] [PATCH 01/12] tests/fate/ffmpeg: evaluate thread count in fate-run.sh rather than make

[FFmpeg-devel] [PATCH 01/12] tests/fate/ffmpeg: evaluate thread count in fate-run.sh rather than make

[Anton Khirnov]

Fixes fate-ffmpeg-loopback-decoding with THREADS=random*
---
 tests/fate/ffmpeg.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
index 9cf05ead68..3c549b265e 100644
--- a/tests/fate/ffmpeg.mak
+++ b/tests/fate/ffmpeg.mak
@@ -259,5 +259,5 @@ FATE_FFMPEG-$(call REMUX, RAWVIDEO) += fate-ffmpeg-streamcopy-t
 fate-ffmpeg-loopback-decoding: tests/data/vsynth_lena.yuv
 fate-ffmpeg-loopback-decoding: CMD = transcode \
     "rawvideo -s 352x288 -pix_fmt yuv420p" $(TARGET_PATH)/tests/data/vsynth_lena.yuv nut \
-    "-map 0:v:0 -c:v mpeg2video -f null - -flags +bitexact -idct simple -threads $(THREADS) -dec 0:0 -filter_complex '[0:v][dec:0]hstack[stack]' -map '[stack]' -c:v ffv1" ""
+    "-map 0:v:0 -c:v mpeg2video -f null - -flags +bitexact -idct simple -threads $$threads -dec 0:0 -filter_complex '[0:v][dec:0]hstack[stack]' -map '[stack]' -c:v ffv1" ""
 FATE_FFMPEG-$(call ENCDEC2, MPEG2VIDEO, FFV1, NUT, HSTACK_FILTER PIPE_PROTOCOL FRAMECRC_MUXER) += fate-ffmpeg-loopback-decoding
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 02/12] fftools/ffmpeg_demux: only call filter_codec_opts() when we have a decoder

[Anton Khirnov]

It is pointless otherwise, as decoder options will not be used.
---
 fftools/ffmpeg_demux.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 47312c9fe1..73b0eb0da1 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -1329,10 +1329,12 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     if (ret < 0)
         return ret;
 
-    ret = filter_codec_opts(o->g->codec_opts, ist->st->codecpar->codec_id,
-                            ic, st, ist->dec, &ds->decoder_opts);
-    if (ret < 0)
-        return ret;
+    if (ist->dec) {
+        ret = filter_codec_opts(o->g->codec_opts, ist->st->codecpar->codec_id,
+                                ic, st, ist->dec, &ds->decoder_opts);
+        if (ret < 0)
+            return ret;
+    }
 
     ds->reinit_filters = -1;
     MATCH_PER_STREAM_OPT(reinit_filters, i, ds->reinit_filters, ic, st);
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 03/12] fftools/cmdutils: do not use a random codec's private options

[Anton Khirnov]

There is only a single caller of filter_codec_opts() that passes
a NULL codec to it, which is streamcopy in ffmpeg CLI. In that case we
only want generic AVCodecContext options, not private options of any
specific encoder.
---
 fftools/cmdutils.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index f3c258bb99..1bb26f44f4 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -998,10 +998,6 @@ int filter_codec_opts(const AVDictionary *opts, enum AVCodecID codec_id,
     char          prefix = 0;
     const AVClass    *cc = avcodec_get_class();
 
-    if (!codec)
-        codec            = s->oformat ? avcodec_find_encoder(codec_id)
-                                      : avcodec_find_decoder(codec_id);
-
     switch (st->codecpar->codec_type) {
     case AVMEDIA_TYPE_VIDEO:
         prefix  = 'v';
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 04/12] fftools/ffmpeg_dec: apply decoder options manually

[Anton Khirnov]

Do not pass an options dictionary to avcodec_open2().

This should be equivalent to current behaviour, but will allow
overriding caller-supplied options in a cleaner and more robust manner.

We can now set the COPY_OPAQUE flag directly rather going through
dec_opts.
---
 fftools/ffmpeg_dec.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index ed411b6bf8..ad02a64b60 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -21,6 +21,7 @@
 #include "libavutil/dict.h"
 #include "libavutil/error.h"
 #include "libavutil/log.h"
+#include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
 #include "libavutil/pixfmt.h"
 #include "libavutil/time.h"
@@ -1191,8 +1192,6 @@ static int dec_open(DecoderPriv *dp, AVDictionary **dec_opts,
     if (!av_dict_get(*dec_opts, "threads", NULL, 0))
         av_dict_set(dec_opts, "threads", "auto", 0);
 
-    av_dict_set(dec_opts, "flags", "+copy_opaque", AV_DICT_MULTIKEY);
-
     ret = hw_device_setup_for_decode(dp, codec, o->hwaccel_device);
     if (ret < 0) {
         av_log(dp, AV_LOG_ERROR,
@@ -1201,7 +1200,19 @@ static int dec_open(DecoderPriv *dp, AVDictionary **dec_opts,
         return ret;
     }
 
-    if ((ret = avcodec_open2(dp->dec_ctx, codec, dec_opts)) < 0) {
+    ret = av_opt_set_dict2(dp->dec_ctx, dec_opts, AV_OPT_SEARCH_CHILDREN);
+    if (ret < 0) {
+        av_log(dp, AV_LOG_ERROR, "Error applying decoder options: %s\n",
+               av_err2str(ret));
+        return ret;
+    }
+    ret = check_avoptions(*dec_opts);
+    if (ret < 0)
+        return ret;
+
+    dp->dec_ctx->flags |= AV_CODEC_FLAG_COPY_OPAQUE;
+
+    if ((ret = avcodec_open2(dp->dec_ctx, codec, NULL)) < 0) {
         av_log(dp, AV_LOG_ERROR, "Error while opening decoder: %s\n",
                av_err2str(ret));
         return ret;
@@ -1220,10 +1231,6 @@ static int dec_open(DecoderPriv *dp, AVDictionary **dec_opts,
             dp->dec_ctx->extra_hw_frames = extra_frames;
     }
 
-    ret = check_avoptions(*dec_opts);
-    if (ret < 0)
-        return ret;
-
     dp->dec.subtitle_header      = dp->dec_ctx->subtitle_header;
     dp->dec.subtitle_header_size = dp->dec_ctx->subtitle_header_size;
 
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 05/12] fftools/ffmpeg_{demux, dec}: pass -bitexact through DecoderFlags

[Anton Khirnov]

Avoids abusing AV_DICT_MULTIKEY and relying on undocumented AVDictionary
ordering behaviour.
---
 fftools/ffmpeg.h       |  2 ++
 fftools/ffmpeg_dec.c   |  2 ++
 fftools/ffmpeg_demux.c | 11 +++++------
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 7454089c2d..1437b36b0d 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -304,6 +304,8 @@ enum DecoderFlags {
     DECODER_FLAG_TOP_FIELD_FIRST  = (1 << 3),
 #endif
     DECODER_FLAG_SEND_END_TS      = (1 << 4),
+    // force bitexact decoding
+    DECODER_FLAG_BITEXACT         = (1 << 5),
 };
 
 typedef struct DecoderOpts {
diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index ad02a64b60..b8bfae469f 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -1211,6 +1211,8 @@ static int dec_open(DecoderPriv *dp, AVDictionary **dec_opts,
         return ret;
 
     dp->dec_ctx->flags |= AV_CODEC_FLAG_COPY_OPAQUE;
+    if (o->flags & DECODER_FLAG_BITEXACT)
+        dp->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
 
     if ((ret = avcodec_open2(dp->dec_ctx, codec, NULL)) < 0) {
         av_log(dp, AV_LOG_ERROR, "Error while opening decoder: %s\n",
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 73b0eb0da1..af4b4cfd1e 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -908,11 +908,11 @@ static int ist_use(InputStream *ist, int decoding_needed)
     if (decoding_needed && ds->sch_idx_dec < 0) {
         int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
 
-        ds->dec_opts.flags = (!!ist->fix_sub_duration * DECODER_FLAG_FIX_SUB_DURATION) |
-                             (!!(d->f.ctx->iformat->flags & AVFMT_NOTIMESTAMPS) * DECODER_FLAG_TS_UNRELIABLE) |
-                             (!!(d->loop && is_audio) * DECODER_FLAG_SEND_END_TS)
+        ds->dec_opts.flags |= (!!ist->fix_sub_duration * DECODER_FLAG_FIX_SUB_DURATION) |
+                              (!!(d->f.ctx->iformat->flags & AVFMT_NOTIMESTAMPS) * DECODER_FLAG_TS_UNRELIABLE) |
+                              (!!(d->loop && is_audio) * DECODER_FLAG_SEND_END_TS)
 #if FFMPEG_OPT_TOP
-                             | ((ist->top_field_first >= 0) * DECODER_FLAG_TOP_FIELD_FIRST)
+                              | ((ist->top_field_first >= 0) * DECODER_FLAG_TOP_FIELD_FIRST)
 #endif
                              ;
 
@@ -1357,8 +1357,7 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
         ist->user_set_discard = ist->st->discard;
     }
 
-    if (o->bitexact)
-        av_dict_set(&ds->decoder_opts, "flags", "+bitexact", AV_DICT_MULTIKEY);
+    ds->dec_opts.flags |= DECODER_FLAG_BITEXACT * !!o->bitexact;
 
     /* Attached pics are sparse, therefore we would not want to delay their decoding
      * till EOF. */
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 06/12] fftools/ffmpeg_dec: apply cropping manually

[Anton Khirnov]

lavfi does not require aligned buffers, so we can safely apply top/left
cropping by any amount, without passing any special flags to lavc.
Longer term, an even better solution would probably be auto-inserting
the crop filter (or its hwaccel versions) as needed.

Multiple FATE tests no longer need -flags unaligned.
---
 fftools/ffmpeg_dec.c | 14 ++++++++++++++
 tests/fate/h264.mak  |  2 +-
 tests/fate/hevc.mak  |  8 ++++----
 tests/fate/vvc.mak   |  2 +-
 4 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index b8bfae469f..e3ed0b3978 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -51,6 +51,7 @@ typedef struct DecoderPriv {
 
     // a combination of DECODER_FLAG_*, provided to dec_open()
     int                 flags;
+    int                 apply_cropping;
 
     enum AVPixelFormat  hwaccel_pix_fmt;
     enum HWAccelID      hwaccel_id;
@@ -404,6 +405,15 @@ static int video_frame_process(DecoderPriv *dp, AVFrame *frame)
     if (dp->sar_override.num)
         frame->sample_aspect_ratio = dp->sar_override;
 
+    if (dp->apply_cropping) {
+        // lavfi does not require aligned frame data
+        int ret = av_frame_apply_cropping(frame, AV_FRAME_CROP_UNALIGNED);
+        if (ret < 0) {
+            av_log(dp, AV_LOG_ERROR, "Error applying decoder cropping\n");
+            return ret;
+        }
+    }
+
     return 0;
 }
 
@@ -1214,6 +1224,10 @@ static int dec_open(DecoderPriv *dp, AVDictionary **dec_opts,
     if (o->flags & DECODER_FLAG_BITEXACT)
         dp->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
 
+    // we apply cropping outselves
+    dp->apply_cropping          = dp->dec_ctx->apply_cropping;
+    dp->dec_ctx->apply_cropping = 0;
+
     if ((ret = avcodec_open2(dp->dec_ctx, codec, NULL)) < 0) {
         av_log(dp, AV_LOG_ERROR, "Error while opening decoder: %s\n",
                av_err2str(ret));
diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
index 674054560b..88f06d7b15 100644
--- a/tests/fate/h264.mak
+++ b/tests/fate/h264.mak
@@ -312,7 +312,7 @@ fate-h264-conformance-ci1_ft_b:                   CMD = framecrc -i $(TARGET_SAM
 fate-h264-conformance-ci_mw_d:                    CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CI_MW_D.264
 fate-h264-conformance-cvbs3_sony_c:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVBS3_Sony_C.jsv
 fate-h264-conformance-cvcanlma2_sony_c:           CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVCANLMA2_Sony_C.jsv
-fate-h264-conformance-cvfc1_sony_c:               CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/h264-conformance/CVFC1_Sony_C.jsv
+fate-h264-conformance-cvfc1_sony_c:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFC1_Sony_C.jsv
 fate-h264-conformance-cvfi1_sony_d:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI1_Sony_D.jsv
 fate-h264-conformance-cvfi1_sva_c:                CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI1_SVA_C.264
 fate-h264-conformance-cvfi2_sony_h:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI2_Sony_H.jsv
diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
index 4889ee8237..720603c112 100644
--- a/tests/fate/hevc.mak
+++ b/tests/fate/hevc.mak
@@ -200,8 +200,8 @@ $(HEVC_TESTS_444_8BIT): SCALE_OPTS := -pix_fmt yuv444p
 $(HEVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
 $(HEVC_TESTS_422_10BIT) $(HEVC_TESTS_422_10BIN): SCALE_OPTS := -pix_fmt yuv422p10le -vf scale
 $(HEVC_TESTS_444_12BIT): SCALE_OPTS := -pix_fmt yuv444p12le -vf scale
-fate-hevc-conformance-%: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
-$(HEVC_TESTS_422_10BIN): CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
+fate-hevc-conformance-%: CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
+$(HEVC_TESTS_422_10BIN): CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
 
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER) += $(HEVC_TESTS_8BIT) $(HEVC_TESTS_444_8BIT)
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) +=         \
@@ -252,10 +252,10 @@ FATE_HEVC_FFPROBE-$(call DEMDEC, MOV, HEVC) += fate-hevc-dv-rpu
 fate-hevc-two-first-slice: CMD = threads=2 framemd5 -i $(TARGET_SAMPLES)/hevc/two_first_slice.mp4 -sws_flags bitexact -t 00:02.00 -an
 FATE_HEVC-$(call FRAMEMD5, MOV, HEVC) += fate-hevc-two-first-slice
 
-fate-hevc-cabac-tudepth: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
+fate-hevc-cabac-tudepth: CMD = framecrc -i $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC) += fate-hevc-cabac-tudepth
 
-fate-hevc-small422chroma: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc/food.hevc -pix_fmt yuv422p10le -vf scale
+fate-hevc-small422chroma: CMD = framecrc -i $(TARGET_SAMPLES)/hevc/food.hevc -pix_fmt yuv422p10le -vf scale
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) += fate-hevc-small422chroma
 
 FATE_SAMPLES_AVCONV += $(FATE_HEVC-yes)
diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak
index f5a45cc4ca..6cb3471b55 100644
--- a/tests/fate/vvc.mak
+++ b/tests/fate/vvc.mak
@@ -38,7 +38,7 @@ $(foreach VAR,$(FATE_VVC_VARS), $(eval VVC_TESTS_$(VAR) := $(addprefix fate-vvc-
 $(VVC_TESTS_8BIT): SCALE_OPTS := -pix_fmt yuv420p
 $(VVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
 $(VVC_TESTS_444_10BIT): SCALE_OPTS := -pix_fmt yuv444p10le -vf scale
-fate-vvc-conformance-%: CMD = framecrc -flags unaligned -c:v vvc -strict experimental -i $(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit $(SCALE_OPTS)
+fate-vvc-conformance-%: CMD = framecrc -c:v vvc -strict experimental -i $(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit $(SCALE_OPTS)
 
 FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER) += $(VVC_TESTS_8BIT)
 FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER SCALE_FILTER) +=            \
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 07/12] fftools/ffmpeg_filter: remove display matrix if we have applied it

[Anton Khirnov]

---
 fftools/ffmpeg_filter.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 3d88482d07..9aa499a89e 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -143,6 +143,7 @@ typedef struct InputFilterPriv {
     AVBufferRef        *hw_frames_ctx;
 
     int                 displaymatrix_present;
+    int                 displaymatrix_applied;
     int32_t             displaymatrix[9];
 
     // fallback parameters to use when no input is ever sent
@@ -1568,6 +1569,7 @@ static int configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
     av_assert0(desc);
 
     // TODO: insert hwaccel enabled filters like transpose_vaapi into the graph
+    ifp->displaymatrix_applied = 0;
     if ((ifp->opts.flags & IFILTER_FLAG_AUTOROTATE) &&
         !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
         int32_t *displaymatrix = ifp->displaymatrix;
@@ -1601,6 +1603,8 @@ static int configure_input_video_filter(FilterGraph *fg, AVFilterGraph *graph,
         }
         if (ret < 0)
             return ret;
+
+        ifp->displaymatrix_applied = 1;
     }
 
     snprintf(name, sizeof(name), "trim_in_%s", ifp->opts.name);
@@ -2697,6 +2701,9 @@ static int send_frame(FilterGraph *fg, FilterGraphThread *fgt,
     frame->duration  = av_rescale_q(frame->duration, frame->time_base, ifp->time_base);
     frame->time_base = ifp->time_base;
 
+    if (ifp->displaymatrix_applied)
+        av_frame_remove_side_data(frame, AV_FRAME_DATA_DISPLAYMATRIX);
+
     fd = frame_data(frame);
     if (!fd)
         return AVERROR(ENOMEM);
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Anton Khirnov]

It is highly unsafe, as AVCodecContext contains many allocated fields.
Copying information via AVCodecParameters and with av_opt_copy() should
handle everything needed by thread workers.
---
 libavcodec/frame_thread_encoder.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
index cda5158117..744062b776 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -28,6 +28,7 @@
 #include "libavutil/thread.h"
 #include "avcodec.h"
 #include "avcodec_internal.h"
+#include "codec_par.h"
 #include "encode.h"
 #include "internal.h"
 #include "pthread_internal.h"
@@ -121,6 +122,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
     int i=0;
     ThreadContext *c;
     AVCodecContext *thread_avctx = NULL;
+    AVCodecParameters *par = NULL;
     int ret;
 
     if(   !(avctx->thread_type & FF_THREAD_FRAME)
@@ -194,18 +196,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         }
     }
 
+    par = avcodec_parameters_alloc();
+    if (!par) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
+
+    ret = avcodec_parameters_from_context(par, avctx);
+    if (ret < 0)
+        goto fail;
+
     for(i=0; i<avctx->thread_count ; i++){
-        void *tmpv;
         thread_avctx = avcodec_alloc_context3(avctx->codec);
         if (!thread_avctx) {
             ret = AVERROR(ENOMEM);
             goto fail;
         }
-        tmpv = thread_avctx->priv_data;
-        *thread_avctx = *avctx;
-        thread_avctx->priv_data = tmpv;
-        thread_avctx->internal = NULL;
-        thread_avctx->hw_frames_ctx = NULL;
+
+        ret = avcodec_parameters_to_context(thread_avctx, par);
+        if (ret < 0)
+            goto fail;
+
         ret = av_opt_copy(thread_avctx, avctx);
         if (ret < 0)
             goto fail;
@@ -227,10 +238,13 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         }
     }
 
+    avcodec_parameters_free(&par);
+
     avctx->active_thread_type = FF_THREAD_FRAME;
 
     return 0;
 fail:
+    avcodec_parameters_free(&par);
     ff_codec_close(thread_avctx);
     av_freep(&thread_avctx);
     avctx->thread_count = i;
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 09/12] lavc/decode: move sd_global_map to avcodec

[Anton Khirnov]

It will be shared with encoding code.
---
 libavcodec/avcodec.c          | 14 ++++++++++++++
 libavcodec/avcodec_internal.h | 16 +++++++++++++++-
 libavcodec/decode.c           | 33 +++++++++------------------------
 3 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index a9a87bb58c..525fe516bd 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -54,6 +54,20 @@
  */
 #define FF_MAX_EXTRADATA_SIZE ((1 << 28) - AV_INPUT_BUFFER_PADDING_SIZE)
 
+const SideDataMap ff_sd_global_map[] = {
+    { AV_PKT_DATA_REPLAYGAIN ,                AV_FRAME_DATA_REPLAYGAIN },
+    { AV_PKT_DATA_DISPLAYMATRIX,              AV_FRAME_DATA_DISPLAYMATRIX },
+    { AV_PKT_DATA_SPHERICAL,                  AV_FRAME_DATA_SPHERICAL },
+    { AV_PKT_DATA_STEREO3D,                   AV_FRAME_DATA_STEREO3D },
+    { AV_PKT_DATA_AUDIO_SERVICE_TYPE,         AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
+    { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
+    { AV_PKT_DATA_CONTENT_LIGHT_LEVEL,        AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
+    { AV_PKT_DATA_ICC_PROFILE,                AV_FRAME_DATA_ICC_PROFILE },
+    { AV_PKT_DATA_AMBIENT_VIEWING_ENVIRONMENT,AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT },
+    { AV_PKT_DATA_NB },
+};
+
+
 int avcodec_default_execute(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2), void *arg, int *ret, int count, int size)
 {
     size_t i;
diff --git a/libavcodec/avcodec_internal.h b/libavcodec/avcodec_internal.h
index 4d1cb3a314..0a024378ae 100644
--- a/libavcodec/avcodec_internal.h
+++ b/libavcodec/avcodec_internal.h
@@ -25,8 +25,22 @@
 #ifndef AVCODEC_AVCODEC_INTERNAL_H
 #define AVCODEC_AVCODEC_INTERNAL_H
 
+#include "libavutil/frame.h"
+
+#include "packet.h"
+
 struct AVCodecContext;
-struct AVFrame;
+
+typedef struct SideDataMap {
+    enum AVPacketSideDataType packet;
+    enum AVFrameSideDataType frame;
+} SideDataMap;
+
+/**
+ * A map between packet and frame side data types.
+ * Terminated with an entry where packet=AV_PKT_DATA_NB.
+ */
+extern const SideDataMap ff_sd_global_map[];
 
 /**
  * avcodec_receive_frame() implementation for decoders.
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 34bcb7cc64..ddb73b6934 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1376,21 +1376,6 @@ static int add_metadata_from_side_data(const AVPacket *avpkt, AVFrame *frame)
     return av_packet_unpack_dictionary(side_metadata, size, frame_md);
 }
 
-static const struct {
-    enum AVPacketSideDataType packet;
-    enum AVFrameSideDataType frame;
-} sd_global_map[] = {
-    { AV_PKT_DATA_REPLAYGAIN ,                AV_FRAME_DATA_REPLAYGAIN },
-    { AV_PKT_DATA_DISPLAYMATRIX,              AV_FRAME_DATA_DISPLAYMATRIX },
-    { AV_PKT_DATA_SPHERICAL,                  AV_FRAME_DATA_SPHERICAL },
-    { AV_PKT_DATA_STEREO3D,                   AV_FRAME_DATA_STEREO3D },
-    { AV_PKT_DATA_AUDIO_SERVICE_TYPE,         AV_FRAME_DATA_AUDIO_SERVICE_TYPE },
-    { AV_PKT_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA },
-    { AV_PKT_DATA_CONTENT_LIGHT_LEVEL,        AV_FRAME_DATA_CONTENT_LIGHT_LEVEL },
-    { AV_PKT_DATA_ICC_PROFILE,                AV_FRAME_DATA_ICC_PROFILE },
-    { AV_PKT_DATA_AMBIENT_VIEWING_ENVIRONMENT,AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT },
-};
-
 int ff_decode_frame_props_from_pkt(const AVCodecContext *avctx,
                                    AVFrame *frame, const AVPacket *pkt)
 {
@@ -1414,13 +1399,13 @@ FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
-    for (int i = 0; i < FF_ARRAY_ELEMS(sd_global_map); i++) {
+    for (int i = 0; ff_sd_global_map[i].packet < AV_PKT_DATA_NB; i++) {
         size_t size;
-        const uint8_t *packet_sd = av_packet_get_side_data(pkt, sd_global_map[i].packet, &size);
+        const uint8_t *packet_sd = av_packet_get_side_data(pkt, ff_sd_global_map[i].packet, &size);
         if (packet_sd) {
             AVFrameSideData *frame_sd;
 
-            frame_sd = av_frame_new_side_data(frame, sd_global_map[i].frame, size);
+            frame_sd = av_frame_new_side_data(frame, ff_sd_global_map[i].frame, size);
             if (!frame_sd)
                 return AVERROR(ENOMEM);
             memcpy(frame_sd->data, packet_sd, size);
@@ -1461,12 +1446,12 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
 {
     int ret;
 
-    for (int i = 0; i < FF_ARRAY_ELEMS(sd_global_map); i++) {
+    for (int i = 0; ff_sd_global_map[i].packet < AV_PKT_DATA_NB; i++) {
         const AVPacketSideData *packet_sd = ff_get_coded_side_data(avctx,
-                                                                   sd_global_map[i].packet);
+                                                                   ff_sd_global_map[i].packet);
         if (packet_sd) {
             AVFrameSideData *frame_sd = av_frame_new_side_data(frame,
-                                                               sd_global_map[i].frame,
+                                                               ff_sd_global_map[i].frame,
                                                                packet_sd->size);
             if (!frame_sd)
                 return AVERROR(ENOMEM);
@@ -1758,9 +1743,9 @@ int ff_decode_preinit(AVCodecContext *avctx)
                 return AVERROR(EINVAL);
             }
 
-            for (unsigned j = 0; j < FF_ARRAY_ELEMS(sd_global_map); j++) {
-                if (sd_global_map[j].packet == val) {
-                    val = sd_global_map[j].frame;
+            for (unsigned j = 0; ff_sd_global_map[j].packet < AV_PKT_DATA_NB; j++) {
+                if (ff_sd_global_map[j].packet == val) {
+                    val = ff_sd_global_map[j].frame;
 
                     // this code will need to be changed when we have more than
                     // 64 frame side data types
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 10/12] lavc/encode: map AVCodecContext.decoded_side_data to coded_side_data

[Anton Khirnov]

This way it can be automagically propagated through the encoder to
muxing.
---
 libavcodec/encode.c          | 23 +++++++++++++++++++++++
 tests/ref/fate/libx265-hdr10 | 24 ++++++++++++------------
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 7fc9737e93..46e46a055e 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -782,6 +782,29 @@ int ff_encode_preinit(AVCodecContext *avctx)
             return AVERROR(ENOMEM);
     }
 
+    for (int i = 0; ff_sd_global_map[i].packet < AV_PKT_DATA_NB; i++) {
+        const enum AVPacketSideDataType type_packet = ff_sd_global_map[i].packet;
+        const enum AVFrameSideDataType  type_frame  = ff_sd_global_map[i].frame;
+        const AVFrameSideData *sd_frame;
+        AVPacketSideData      *sd_packet;
+
+        sd_frame = av_frame_side_data_get(avctx->decoded_side_data,
+                                          avctx->nb_decoded_side_data,
+                                          type_frame);
+        if (!sd_frame ||
+            av_packet_side_data_get(avctx->coded_side_data, avctx->nb_coded_side_data,
+                                    type_packet))
+
+            continue;
+
+        sd_packet = av_packet_side_data_new(&avctx->coded_side_data, &avctx->nb_coded_side_data,
+                                            type_packet, sd_frame->size, 0);
+        if (!sd_packet)
+            return AVERROR(ENOMEM);
+
+        memcpy(sd_packet->data, sd_frame->data, sd_frame->size);
+    }
+
     if (CONFIG_FRAME_THREAD_ENCODER) {
         ret = ff_frame_thread_encoder_init(avctx);
         if (ret < 0)
diff --git a/tests/ref/fate/libx265-hdr10 b/tests/ref/fate/libx265-hdr10
index 571c837cac..68511202a5 100644
--- a/tests/ref/fate/libx265-hdr10
+++ b/tests/ref/fate/libx265-hdr10
@@ -1,16 +1,16 @@
 frames.frame.0.side_data_list.side_data.0.side_data_type="H.26[45] User Data Unregistered SEI message"
-frames.frame.0.side_data_list.side_data.1.side_data_type="H.26[45] User Data Unregistered SEI message"
-frames.frame.0.side_data_list.side_data.2.side_data_type="Mastering display metadata"
-frames.frame.0.side_data_list.side_data.2.red_x="13250/50000"
-frames.frame.0.side_data_list.side_data.2.red_y="34500/50000"
-frames.frame.0.side_data_list.side_data.2.green_x="7500/50000"
-frames.frame.0.side_data_list.side_data.2.green_y="3000/50000"
-frames.frame.0.side_data_list.side_data.2.blue_x="34000/50000"
-frames.frame.0.side_data_list.side_data.2.blue_y="16000/50000"
-frames.frame.0.side_data_list.side_data.2.white_point_x="15635/50000"
-frames.frame.0.side_data_list.side_data.2.white_point_y="16450/50000"
-frames.frame.0.side_data_list.side_data.2.min_luminance="50/10000"
-frames.frame.0.side_data_list.side_data.2.max_luminance="10000000/10000"
+frames.frame.0.side_data_list.side_data.1.side_data_type="Mastering display metadata"
+frames.frame.0.side_data_list.side_data.1.red_x="13250/50000"
+frames.frame.0.side_data_list.side_data.1.red_y="34500/50000"
+frames.frame.0.side_data_list.side_data.1.green_x="7500/50000"
+frames.frame.0.side_data_list.side_data.1.green_y="3000/50000"
+frames.frame.0.side_data_list.side_data.1.blue_x="34000/50000"
+frames.frame.0.side_data_list.side_data.1.blue_y="16000/50000"
+frames.frame.0.side_data_list.side_data.1.white_point_x="15635/50000"
+frames.frame.0.side_data_list.side_data.1.white_point_y="16450/50000"
+frames.frame.0.side_data_list.side_data.1.min_luminance="50/10000"
+frames.frame.0.side_data_list.side_data.1.max_luminance="10000000/10000"
+frames.frame.0.side_data_list.side_data.2.side_data_type="H.26[45] User Data Unregistered SEI message"
 frames.frame.0.side_data_list.side_data.3.side_data_type="Content light level metadata"
 frames.frame.0.side_data_list.side_data.3.max_content=1000
 frames.frame.0.side_data_list.side_data.3.max_average=200
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 11/12] fftools/ffmpeg_enc: stop copying demuxer side data to the muxer

[Anton Khirnov]

All side data should be propagated through the trancoding pipeline.
---
 fftools/ffmpeg_enc.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index f01be1c22f..138044da24 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -360,29 +360,6 @@ int enc_open(void *opaque, const AVFrame *frame)
         return ret;
     }
 
-    /*
-     * Add global input side data. For now this is naive, and copies it
-     * from the input stream's global side data. All side data should
-     * really be funneled over AVFrame and libavfilter, then added back to
-     * packet side data, and then potentially using the first packet for
-     * global side data.
-     */
-    if (ist) {
-        for (int i = 0; i < ist->st->codecpar->nb_coded_side_data; i++) {
-            AVPacketSideData *sd_src = &ist->st->codecpar->coded_side_data[i];
-            if (sd_src->type != AV_PKT_DATA_CPB_PROPERTIES) {
-                AVPacketSideData *sd_dst = av_packet_side_data_new(&ost->par_in->coded_side_data,
-                                                                   &ost->par_in->nb_coded_side_data,
-                                                                   sd_src->type, sd_src->size, 0);
-                if (!sd_dst)
-                    return AVERROR(ENOMEM);
-                memcpy(sd_dst->data, sd_src->data, sd_src->size);
-                if (ist->autorotate && sd_src->type == AV_PKT_DATA_DISPLAYMATRIX)
-                    av_display_rotation_set((int32_t *)sd_dst->data, 0);
-            }
-        }
-    }
-
     // copy timebase while removing common factors
     if (ost->st->time_base.num <= 0 || ost->st->time_base.den <= 0)
         ost->st->time_base = av_add_q(ost->enc_ctx->time_base, (AVRational){0, 1});
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 12/12] fftools/ffmpeg_demux: make InputStream.autorotate private

[Anton Khirnov]

It is no longer accessed outside of ffmpeg_demux.
---
 fftools/ffmpeg.h       | 2 --
 fftools/ffmpeg_demux.c | 8 +++++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 1437b36b0d..84475434f3 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -370,8 +370,6 @@ typedef struct InputStream {
     int                   top_field_first;
 #endif
 
-    int                   autorotate;
-
     int                   fix_sub_duration;
 
     /* decoded data from this stream goes into all those filters
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index af4b4cfd1e..d815dd3696 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -64,6 +64,8 @@ typedef struct DemuxStream {
     int                      streamcopy_needed;
     int                      have_sub2video;
     int                      reinit_filters;
+    int                      autorotate;
+
 
     int                      wrap_correction_done;
     int                      saw_first_ts;
@@ -1055,7 +1057,7 @@ int ist_filter_add(InputStream *ist, InputFilter *ifilter, int is_simple,
     if (!opts->name)
         return AVERROR(ENOMEM);
 
-    opts->flags |= IFILTER_FLAG_AUTOROTATE * !!(ist->autorotate) |
+    opts->flags |= IFILTER_FLAG_AUTOROTATE * !!(ds->autorotate) |
                    IFILTER_FLAG_REINIT     * !!(ds->reinit_filters);
 
     return ds->sch_idx_dec;
@@ -1235,8 +1237,8 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     ds->ts_scale = 1.0;
     MATCH_PER_STREAM_OPT(ts_scale, dbl, ds->ts_scale, ic, st);
 
-    ist->autorotate = 1;
-    MATCH_PER_STREAM_OPT(autorotate, i, ist->autorotate, ic, st);
+    ds->autorotate = 1;
+    MATCH_PER_STREAM_OPT(autorotate, i, ds->autorotate, ic, st);
 
     MATCH_PER_STREAM_OPT(codec_tags, str, codec_tag, ic, st);
     if (codec_tag) {
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Andreas Rheinhardt]

Anton Khirnov:
> It is highly unsafe, as AVCodecContext contains many allocated fields.
> Copying information via AVCodecParameters and with av_opt_copy() should
> handle everything needed by thread workers.
> ---
>  libavcodec/frame_thread_encoder.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
> index cda5158117..744062b776 100644
> --- a/libavcodec/frame_thread_encoder.c
> +++ b/libavcodec/frame_thread_encoder.c
> @@ -28,6 +28,7 @@
>  #include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "avcodec_internal.h"
> +#include "codec_par.h"
>  #include "encode.h"
>  #include "internal.h"
>  #include "pthread_internal.h"
> @@ -121,6 +122,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>      int i=0;
>      ThreadContext *c;
>      AVCodecContext *thread_avctx = NULL;
> +    AVCodecParameters *par = NULL;
>      int ret;
>  
>      if(   !(avctx->thread_type & FF_THREAD_FRAME)
> @@ -194,18 +196,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>          }
>      }
>  
> +    par = avcodec_parameters_alloc();
> +    if (!par) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
> +
> +    ret = avcodec_parameters_from_context(par, avctx);
> +    if (ret < 0)
> +        goto fail;
> +
>      for(i=0; i<avctx->thread_count ; i++){
> -        void *tmpv;
>          thread_avctx = avcodec_alloc_context3(avctx->codec);
>          if (!thread_avctx) {
>              ret = AVERROR(ENOMEM);
>              goto fail;
>          }
> -        tmpv = thread_avctx->priv_data;
> -        *thread_avctx = *avctx;
> -        thread_avctx->priv_data = tmpv;
> -        thread_avctx->internal = NULL;
> -        thread_avctx->hw_frames_ctx = NULL;
> +
> +        ret = avcodec_parameters_to_context(thread_avctx, par);
> +        if (ret < 0)
> +            goto fail;
> +
>          ret = av_opt_copy(thread_avctx, avctx);
>          if (ret < 0)
>              goto fail;
> @@ -227,10 +238,13 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>          }
>      }
>  
> +    avcodec_parameters_free(&par);
> +
>      avctx->active_thread_type = FF_THREAD_FRAME;
>  
>      return 0;
>  fail:
> +    avcodec_parameters_free(&par);
>      ff_codec_close(thread_avctx);
>      av_freep(&thread_avctx);
>      avctx->thread_count = i;

IIRC the mjpeg encoders use intra_matrix and this will now no longer be
copied to the worker threads.

- 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 10/12] lavc/encode: map AVCodecContext.decoded_side_data to coded_side_data

[James Almer]

On 3/22/2024 5:28 PM, Anton Khirnov wrote:
> This way it can be automagically propagated through the encoder to
> muxing.
> ---
>   libavcodec/encode.c          | 23 +++++++++++++++++++++++
>   tests/ref/fate/libx265-hdr10 | 24 ++++++++++++------------
>   2 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> index 7fc9737e93..46e46a055e 100644
> --- a/libavcodec/encode.c
> +++ b/libavcodec/encode.c
> @@ -782,6 +782,29 @@ int ff_encode_preinit(AVCodecContext *avctx)
>               return AVERROR(ENOMEM);
>       }
>   
> +    for (int i = 0; ff_sd_global_map[i].packet < AV_PKT_DATA_NB; i++) {
> +        const enum AVPacketSideDataType type_packet = ff_sd_global_map[i].packet;
> +        const enum AVFrameSideDataType  type_frame  = ff_sd_global_map[i].frame;
> +        const AVFrameSideData *sd_frame;
> +        AVPacketSideData      *sd_packet;
> +
> +        sd_frame = av_frame_side_data_get(avctx->decoded_side_data,
> +                                          avctx->nb_decoded_side_data,
> +                                          type_frame);
> +        if (!sd_frame ||
> +            av_packet_side_data_get(avctx->coded_side_data, avctx->nb_coded_side_data,
> +                                    type_packet))
> +
> +            continue;
> +
> +        sd_packet = av_packet_side_data_new(&avctx->coded_side_data, &avctx->nb_coded_side_data,
> +                                            type_packet, sd_frame->size, 0);
> +        if (!sd_packet)
> +            return AVERROR(ENOMEM);
> +
> +        memcpy(sd_packet->data, sd_frame->data, sd_frame->size);
> +    }
> +
>       if (CONFIG_FRAME_THREAD_ENCODER) {
>           ret = ff_frame_thread_encoder_init(avctx);
>           if (ret < 0)
> diff --git a/tests/ref/fate/libx265-hdr10 b/tests/ref/fate/libx265-hdr10
> index 571c837cac..68511202a5 100644
> --- a/tests/ref/fate/libx265-hdr10
> +++ b/tests/ref/fate/libx265-hdr10
> @@ -1,16 +1,16 @@
>   frames.frame.0.side_data_list.side_data.0.side_data_type="H.26[45] User Data Unregistered SEI message"
> -frames.frame.0.side_data_list.side_data.1.side_data_type="H.26[45] User Data Unregistered SEI message"
> -frames.frame.0.side_data_list.side_data.2.side_data_type="Mastering display metadata"
> -frames.frame.0.side_data_list.side_data.2.red_x="13250/50000"
> -frames.frame.0.side_data_list.side_data.2.red_y="34500/50000"
> -frames.frame.0.side_data_list.side_data.2.green_x="7500/50000"
> -frames.frame.0.side_data_list.side_data.2.green_y="3000/50000"
> -frames.frame.0.side_data_list.side_data.2.blue_x="34000/50000"
> -frames.frame.0.side_data_list.side_data.2.blue_y="16000/50000"
> -frames.frame.0.side_data_list.side_data.2.white_point_x="15635/50000"
> -frames.frame.0.side_data_list.side_data.2.white_point_y="16450/50000"
> -frames.frame.0.side_data_list.side_data.2.min_luminance="50/10000"
> -frames.frame.0.side_data_list.side_data.2.max_luminance="10000000/10000"
> +frames.frame.0.side_data_list.side_data.1.side_data_type="Mastering display metadata"
> +frames.frame.0.side_data_list.side_data.1.red_x="13250/50000"
> +frames.frame.0.side_data_list.side_data.1.red_y="34500/50000"
> +frames.frame.0.side_data_list.side_data.1.green_x="7500/50000"
> +frames.frame.0.side_data_list.side_data.1.green_y="3000/50000"
> +frames.frame.0.side_data_list.side_data.1.blue_x="34000/50000"
> +frames.frame.0.side_data_list.side_data.1.blue_y="16000/50000"
> +frames.frame.0.side_data_list.side_data.1.white_point_x="15635/50000"
> +frames.frame.0.side_data_list.side_data.1.white_point_y="16450/50000"
> +frames.frame.0.side_data_list.side_data.1.min_luminance="50/10000"
> +frames.frame.0.side_data_list.side_data.1.max_luminance="10000000/10000"
> +frames.frame.0.side_data_list.side_data.2.side_data_type="H.26[45] User Data Unregistered SEI message"
>   frames.frame.0.side_data_list.side_data.3.side_data_type="Content light level metadata"
>   frames.frame.0.side_data_list.side_data.3.max_content=1000
>   frames.frame.0.side_data_list.side_data.3.max_average=200

Why does this test change? And is it just the order or side data in the 
output frame?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 06/12] fftools/ffmpeg_dec: apply cropping manually

[James Almer]

On 3/22/2024 5:28 PM, Anton Khirnov wrote:
> lavfi does not require aligned buffers, so we can safely apply top/left
> cropping by any amount, without passing any special flags to lavc.
> Longer term, an even better solution would probably be auto-inserting
> the crop filter (or its hwaccel versions) as needed.

It's what i did in my container cropping set. One problem with it is 
that the cropping filter doesn't look at cropping values in input frames 
but at filter options instead, and will in fact *add* cropping values to 
the output frames if you pass it something it can't handle, like hwaccel 
formats.

And for that matter, cropping fields in frames should be handled by 
encoders, or by the generic encoding code in some form. Right now they 
are outright ignored.

> 
> Multiple FATE tests no longer need -flags unaligned.
> ---
>   fftools/ffmpeg_dec.c | 14 ++++++++++++++
>   tests/fate/h264.mak  |  2 +-
>   tests/fate/hevc.mak  |  8 ++++----
>   tests/fate/vvc.mak   |  2 +-
>   4 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
> index b8bfae469f..e3ed0b3978 100644
> --- a/fftools/ffmpeg_dec.c
> +++ b/fftools/ffmpeg_dec.c
> @@ -51,6 +51,7 @@ typedef struct DecoderPriv {
>   
>       // a combination of DECODER_FLAG_*, provided to dec_open()
>       int                 flags;
> +    int                 apply_cropping;
>   
>       enum AVPixelFormat  hwaccel_pix_fmt;
>       enum HWAccelID      hwaccel_id;
> @@ -404,6 +405,15 @@ static int video_frame_process(DecoderPriv *dp, AVFrame *frame)
>       if (dp->sar_override.num)
>           frame->sample_aspect_ratio = dp->sar_override;
>   
> +    if (dp->apply_cropping) {
> +        // lavfi does not require aligned frame data
> +        int ret = av_frame_apply_cropping(frame, AV_FRAME_CROP_UNALIGNED);
> +        if (ret < 0) {
> +            av_log(dp, AV_LOG_ERROR, "Error applying decoder cropping\n");
> +            return ret;
> +        }
> +    }
> +
>       return 0;
>   }
>   
> @@ -1214,6 +1224,10 @@ static int dec_open(DecoderPriv *dp, AVDictionary **dec_opts,
>       if (o->flags & DECODER_FLAG_BITEXACT)
>           dp->dec_ctx->flags |= AV_CODEC_FLAG_BITEXACT;
>   
> +    // we apply cropping outselves
> +    dp->apply_cropping          = dp->dec_ctx->apply_cropping;
> +    dp->dec_ctx->apply_cropping = 0;
> +
>       if ((ret = avcodec_open2(dp->dec_ctx, codec, NULL)) < 0) {
>           av_log(dp, AV_LOG_ERROR, "Error while opening decoder: %s\n",
>                  av_err2str(ret));
> diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
> index 674054560b..88f06d7b15 100644
> --- a/tests/fate/h264.mak
> +++ b/tests/fate/h264.mak
> @@ -312,7 +312,7 @@ fate-h264-conformance-ci1_ft_b:                   CMD = framecrc -i $(TARGET_SAM
>   fate-h264-conformance-ci_mw_d:                    CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CI_MW_D.264
>   fate-h264-conformance-cvbs3_sony_c:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVBS3_Sony_C.jsv
>   fate-h264-conformance-cvcanlma2_sony_c:           CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVCANLMA2_Sony_C.jsv
> -fate-h264-conformance-cvfc1_sony_c:               CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/h264-conformance/CVFC1_Sony_C.jsv
> +fate-h264-conformance-cvfc1_sony_c:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFC1_Sony_C.jsv
>   fate-h264-conformance-cvfi1_sony_d:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI1_Sony_D.jsv
>   fate-h264-conformance-cvfi1_sva_c:                CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI1_SVA_C.264
>   fate-h264-conformance-cvfi2_sony_h:               CMD = framecrc -i $(TARGET_SAMPLES)/h264-conformance/CVFI2_Sony_H.jsv
> diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> index 4889ee8237..720603c112 100644
> --- a/tests/fate/hevc.mak
> +++ b/tests/fate/hevc.mak
> @@ -200,8 +200,8 @@ $(HEVC_TESTS_444_8BIT): SCALE_OPTS := -pix_fmt yuv444p
>   $(HEVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
>   $(HEVC_TESTS_422_10BIT) $(HEVC_TESTS_422_10BIN): SCALE_OPTS := -pix_fmt yuv422p10le -vf scale
>   $(HEVC_TESTS_444_12BIT): SCALE_OPTS := -pix_fmt yuv444p12le -vf scale
> -fate-hevc-conformance-%: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
> -$(HEVC_TESTS_422_10BIN): CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
> +fate-hevc-conformance-%: CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
> +$(HEVC_TESTS_422_10BIN): CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
>   
>   FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER) += $(HEVC_TESTS_8BIT) $(HEVC_TESTS_444_8BIT)
>   FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) +=         \
> @@ -252,10 +252,10 @@ FATE_HEVC_FFPROBE-$(call DEMDEC, MOV, HEVC) += fate-hevc-dv-rpu
>   fate-hevc-two-first-slice: CMD = threads=2 framemd5 -i $(TARGET_SAMPLES)/hevc/two_first_slice.mp4 -sws_flags bitexact -t 00:02.00 -an
>   FATE_HEVC-$(call FRAMEMD5, MOV, HEVC) += fate-hevc-two-first-slice
>   
> -fate-hevc-cabac-tudepth: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
> +fate-hevc-cabac-tudepth: CMD = framecrc -i $(TARGET_SAMPLES)/hevc/cbf_cr_cb_TUDepth_4_circle.h265 -pix_fmt yuv444p
>   FATE_HEVC-$(call FRAMECRC, HEVC, HEVC) += fate-hevc-cabac-tudepth
>   
> -fate-hevc-small422chroma: CMD = framecrc -flags unaligned -i $(TARGET_SAMPLES)/hevc/food.hevc -pix_fmt yuv422p10le -vf scale
> +fate-hevc-small422chroma: CMD = framecrc -i $(TARGET_SAMPLES)/hevc/food.hevc -pix_fmt yuv422p10le -vf scale
>   FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) += fate-hevc-small422chroma
>   
>   FATE_SAMPLES_AVCONV += $(FATE_HEVC-yes)
> diff --git a/tests/fate/vvc.mak b/tests/fate/vvc.mak
> index f5a45cc4ca..6cb3471b55 100644
> --- a/tests/fate/vvc.mak
> +++ b/tests/fate/vvc.mak
> @@ -38,7 +38,7 @@ $(foreach VAR,$(FATE_VVC_VARS), $(eval VVC_TESTS_$(VAR) := $(addprefix fate-vvc-
>   $(VVC_TESTS_8BIT): SCALE_OPTS := -pix_fmt yuv420p
>   $(VVC_TESTS_10BIT): SCALE_OPTS := -pix_fmt yuv420p10le -vf scale
>   $(VVC_TESTS_444_10BIT): SCALE_OPTS := -pix_fmt yuv444p10le -vf scale
> -fate-vvc-conformance-%: CMD = framecrc -flags unaligned -c:v vvc -strict experimental -i $(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit $(SCALE_OPTS)
> +fate-vvc-conformance-%: CMD = framecrc -c:v vvc -strict experimental -i $(TARGET_SAMPLES)/vvc-conformance/$(subst fate-vvc-conformance-,,$(@)).bit $(SCALE_OPTS)
>   
>   FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER) += $(VVC_TESTS_8BIT)
>   FATE_VVC-$(call FRAMECRC, VVC, VVC, VVC_PARSER SCALE_FILTER) +=            \
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 06/12] fftools/ffmpeg_dec: apply cropping manually

[Anton Khirnov]

Quoting James Almer (2024-03-23 03:53:26)
> On 3/22/2024 5:28 PM, Anton Khirnov wrote:
> > lavfi does not require aligned buffers, so we can safely apply top/left
> > cropping by any amount, without passing any special flags to lavc.
> > Longer term, an even better solution would probably be auto-inserting
> > the crop filter (or its hwaccel versions) as needed.
> 
> It's what i did in my container cropping set. One problem with it is 
> that the cropping filter doesn't look at cropping values in input frames 
> but at filter options instead, and will in fact *add* cropping values to 
> the output frames if you pass it something it can't handle, like hwaccel 
> formats.

The idea is that ffmpeg_filter would set up the crop filter, then remove
the values from the frame. The crop filter then either actually performs
cropping or adds them back.

> And for that matter, cropping fields in frames should be handled by 
> encoders, or by the generic encoding code in some form. Right now they 
> are outright ignored.

Sure I guess. Patches welcome?

-- 
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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 10/12] lavc/encode: map AVCodecContext.decoded_side_data to coded_side_data

[Anton Khirnov]

Quoting James Almer (2024-03-23 03:33:13)
> Why does this test change? And is it just the order or side data in the 
> output frame?

Yes, just the order. And the test is being removed, so it doesn't matter
either way.

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

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

[FFmpeg-devel] [PATCH v2 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Anton Khirnov]

It is highly unsafe, as AVCodecContext contains many allocated fields.
Everything needed by worked threads should be covered by
* routing through AVCodecParameters
* av_opt_copy()
* copying quantisation matrices manually

avcodec_free_context() can now be used for per-thread contexts.
---
 libavcodec/frame_thread_encoder.c | 44 ++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
index cda5158117..5ea6386dfb 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -28,6 +28,7 @@
 #include "libavutil/thread.h"
 #include "avcodec.h"
 #include "avcodec_internal.h"
+#include "codec_par.h"
 #include "encode.h"
 #include "internal.h"
 #include "pthread_internal.h"
@@ -111,8 +112,7 @@ static void * attribute_align_arg worker(void *v){
         pthread_mutex_unlock(&c->finished_task_mutex);
     }
 end:
-    ff_codec_close(avctx);
-    av_freep(&avctx);
+    avcodec_free_context(&avctx);
     return NULL;
 }
 
@@ -121,6 +121,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
     int i=0;
     ThreadContext *c;
     AVCodecContext *thread_avctx = NULL;
+    AVCodecParameters *par = NULL;
     int ret;
 
     if(   !(avctx->thread_type & FF_THREAD_FRAME)
@@ -194,18 +195,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         }
     }
 
+    par = avcodec_parameters_alloc();
+    if (!par) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
+
+    ret = avcodec_parameters_from_context(par, avctx);
+    if (ret < 0)
+        goto fail;
+
     for(i=0; i<avctx->thread_count ; i++){
-        void *tmpv;
         thread_avctx = avcodec_alloc_context3(avctx->codec);
         if (!thread_avctx) {
             ret = AVERROR(ENOMEM);
             goto fail;
         }
-        tmpv = thread_avctx->priv_data;
-        *thread_avctx = *avctx;
-        thread_avctx->priv_data = tmpv;
-        thread_avctx->internal = NULL;
-        thread_avctx->hw_frames_ctx = NULL;
+
+        ret = avcodec_parameters_to_context(thread_avctx, par);
+        if (ret < 0)
+            goto fail;
+
         ret = av_opt_copy(thread_avctx, avctx);
         if (ret < 0)
             goto fail;
@@ -217,6 +227,18 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         thread_avctx->thread_count = 1;
         thread_avctx->active_thread_type &= ~FF_THREAD_FRAME;
 
+#define DUP_MATRIX(m)                                                       \
+        if (avctx->m) {                                                     \
+            thread_avctx->m = av_memdup(avctx->m, 64 * sizeof(avctx->m));   \
+            if (!thread_avctx->m) {                                         \
+                ret = AVERROR(ENOMEM);                                      \
+                goto fail;                                                  \
+            }                                                               \
+        }
+        DUP_MATRIX(intra_matrix);
+        DUP_MATRIX(chroma_intra_matrix);
+        DUP_MATRIX(inter_matrix);
+
         if ((ret = avcodec_open2(thread_avctx, avctx->codec, NULL)) < 0)
             goto fail;
         av_assert0(!thread_avctx->internal->frame_thread_encoder);
@@ -227,12 +249,14 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         }
     }
 
+    avcodec_parameters_free(&par);
+
     avctx->active_thread_type = FF_THREAD_FRAME;
 
     return 0;
 fail:
-    ff_codec_close(thread_avctx);
-    av_freep(&thread_avctx);
+    avcodec_parameters_free(&par);
+    avcodec_free_context(&thread_avctx);
     avctx->thread_count = i;
     av_log(avctx, AV_LOG_ERROR, "ff_frame_thread_encoder_init failed\n");
     ff_frame_thread_encoder_free(avctx);
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Andreas Rheinhardt]

Anton Khirnov:
> It is highly unsafe, as AVCodecContext contains many allocated fields.
> Everything needed by worked threads should be covered by
> * routing through AVCodecParameters
> * av_opt_copy()
> * copying quantisation matrices manually
> 
> avcodec_free_context() can now be used for per-thread contexts.
> ---
>  libavcodec/frame_thread_encoder.c | 44 ++++++++++++++++++++++++-------
>  1 file changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
> index cda5158117..5ea6386dfb 100644
> --- a/libavcodec/frame_thread_encoder.c
> +++ b/libavcodec/frame_thread_encoder.c
> @@ -28,6 +28,7 @@
>  #include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "avcodec_internal.h"
> +#include "codec_par.h"
>  #include "encode.h"
>  #include "internal.h"
>  #include "pthread_internal.h"
> @@ -111,8 +112,7 @@ static void * attribute_align_arg worker(void *v){
>          pthread_mutex_unlock(&c->finished_task_mutex);
>      }
>  end:
> -    ff_codec_close(avctx);
> -    av_freep(&avctx);
> +    avcodec_free_context(&avctx);
>      return NULL;
>  }
>  
> @@ -121,6 +121,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>      int i=0;
>      ThreadContext *c;
>      AVCodecContext *thread_avctx = NULL;
> +    AVCodecParameters *par = NULL;
>      int ret;
>  
>      if(   !(avctx->thread_type & FF_THREAD_FRAME)
> @@ -194,18 +195,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>          }
>      }
>  
> +    par = avcodec_parameters_alloc();
> +    if (!par) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
> +
> +    ret = avcodec_parameters_from_context(par, avctx);
> +    if (ret < 0)
> +        goto fail;
> +
>      for(i=0; i<avctx->thread_count ; i++){
> -        void *tmpv;
>          thread_avctx = avcodec_alloc_context3(avctx->codec);
>          if (!thread_avctx) {
>              ret = AVERROR(ENOMEM);
>              goto fail;
>          }
> -        tmpv = thread_avctx->priv_data;
> -        *thread_avctx = *avctx;
> -        thread_avctx->priv_data = tmpv;
> -        thread_avctx->internal = NULL;
> -        thread_avctx->hw_frames_ctx = NULL;
> +
> +        ret = avcodec_parameters_to_context(thread_avctx, par);
> +        if (ret < 0)
> +            goto fail;
> +
>          ret = av_opt_copy(thread_avctx, avctx);
>          if (ret < 0)
>              goto fail;
> @@ -217,6 +227,18 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>          thread_avctx->thread_count = 1;
>          thread_avctx->active_thread_type &= ~FF_THREAD_FRAME;
>  
> +#define DUP_MATRIX(m)                                                       \
> +        if (avctx->m) {                                                     \
> +            thread_avctx->m = av_memdup(avctx->m, 64 * sizeof(avctx->m));   \
> +            if (!thread_avctx->m) {                                         \
> +                ret = AVERROR(ENOMEM);                                      \
> +                goto fail;                                                  \
> +            }                                                               \
> +        }
> +        DUP_MATRIX(intra_matrix);
> +        DUP_MATRIX(chroma_intra_matrix);
> +        DUP_MATRIX(inter_matrix);
> +
>          if ((ret = avcodec_open2(thread_avctx, avctx->codec, NULL)) < 0)
>              goto fail;
>          av_assert0(!thread_avctx->internal->frame_thread_encoder);
> @@ -227,12 +249,14 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>          }
>      }
>  
> +    avcodec_parameters_free(&par);
> +
>      avctx->active_thread_type = FF_THREAD_FRAME;
>  
>      return 0;
>  fail:
> -    ff_codec_close(thread_avctx);
> -    av_freep(&thread_avctx);
> +    avcodec_parameters_free(&par);
> +    avcodec_free_context(&thread_avctx);
>      avctx->thread_count = i;
>      av_log(avctx, AV_LOG_ERROR, "ff_frame_thread_encoder_init failed\n");
>      ff_frame_thread_encoder_free(avctx);

1. The earlier code would just work in case the user used a smaller
number of elements for the matrices if these matrices were not used at
all (which happens for the majority of encoders). This is no longer true
with this patch.
2. Did you test this? Did it work? "av_memdup(avctx->m, 64 *
sizeof(avctx->m))" uses sizeof a pointer and therefore leads to a buffer
overflow.

- 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 608 bytes --]

On Fri, Mar 22, 2024 at 09:28:37PM +0100, Anton Khirnov wrote:
> It is highly unsafe, as AVCodecContext contains many allocated fields.
> Copying information via AVCodecParameters and with av_opt_copy() should
> handle everything needed by thread workers.
> ---
>  libavcodec/frame_thread_encoder.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)

this breaks huffyuv with 2 pass

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

Re: [FFmpeg-devel] [PATCH v2 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Anton Khirnov]

Quoting Andreas Rheinhardt (2024-03-23 15:11:59)
> 1. The earlier code would just work in case the user used a smaller
> number of elements for the matrices if these matrices were not used at
> all (which happens for the majority of encoders). This is no longer true
> with this patch.

So?

-- 
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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Anton Khirnov]

It is highly unsafe, as AVCodecContext contains many allocated fields.
Everything needed by worked threads should be covered by
* routing through AVCodecParameters
* av_opt_copy()
* copying quantisation matrices manually

avcodec_free_context() can now be used for per-thread contexts.
---
 libavcodec/frame_thread_encoder.c | 48 ++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
index cda5158117..d34fe022a5 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -28,6 +28,7 @@
 #include "libavutil/thread.h"
 #include "avcodec.h"
 #include "avcodec_internal.h"
+#include "codec_par.h"
 #include "encode.h"
 #include "internal.h"
 #include "pthread_internal.h"
@@ -111,8 +112,7 @@ static void * attribute_align_arg worker(void *v){
         pthread_mutex_unlock(&c->finished_task_mutex);
     }
 end:
-    ff_codec_close(avctx);
-    av_freep(&avctx);
+    avcodec_free_context(&avctx);
     return NULL;
 }
 
@@ -121,6 +121,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
     int i=0;
     ThreadContext *c;
     AVCodecContext *thread_avctx = NULL;
+    AVCodecParameters *par = NULL;
     int ret;
 
     if(   !(avctx->thread_type & FF_THREAD_FRAME)
@@ -194,18 +195,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         }
     }
 
+    par = avcodec_parameters_alloc();
+    if (!par) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
+
+    ret = avcodec_parameters_from_context(par, avctx);
+    if (ret < 0)
+        goto fail;
+
     for(i=0; i<avctx->thread_count ; i++){
-        void *tmpv;
         thread_avctx = avcodec_alloc_context3(avctx->codec);
         if (!thread_avctx) {
             ret = AVERROR(ENOMEM);
             goto fail;
         }
-        tmpv = thread_avctx->priv_data;
-        *thread_avctx = *avctx;
-        thread_avctx->priv_data = tmpv;
-        thread_avctx->internal = NULL;
-        thread_avctx->hw_frames_ctx = NULL;
+
+        ret = avcodec_parameters_to_context(thread_avctx, par);
+        if (ret < 0)
+            goto fail;
+
         ret = av_opt_copy(thread_avctx, avctx);
         if (ret < 0)
             goto fail;
@@ -217,6 +227,22 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         thread_avctx->thread_count = 1;
         thread_avctx->active_thread_type &= ~FF_THREAD_FRAME;
 
+#define DUP_MATRIX(m)                                                       \
+        if (avctx->m) {                                                     \
+            thread_avctx->m = av_memdup(avctx->m, 64 * sizeof(*avctx->m));  \
+            if (!thread_avctx->m) {                                         \
+                ret = AVERROR(ENOMEM);                                      \
+                goto fail;                                                  \
+            }                                                               \
+        }
+        DUP_MATRIX(intra_matrix);
+        DUP_MATRIX(chroma_intra_matrix);
+        DUP_MATRIX(inter_matrix);
+
+#undef DUP_MATRIX
+
+        thread_avctx->stats_in = avctx->stats_in;
+
         if ((ret = avcodec_open2(thread_avctx, avctx->codec, NULL)) < 0)
             goto fail;
         av_assert0(!thread_avctx->internal->frame_thread_encoder);
@@ -227,12 +253,14 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         }
     }
 
+    avcodec_parameters_free(&par);
+
     avctx->active_thread_type = FF_THREAD_FRAME;
 
     return 0;
 fail:
-    ff_codec_close(thread_avctx);
-    av_freep(&thread_avctx);
+    avcodec_parameters_free(&par);
+    avcodec_free_context(&thread_avctx);
     avctx->thread_count = i;
     av_log(avctx, AV_LOG_ERROR, "ff_frame_thread_encoder_init failed\n");
     ff_frame_thread_encoder_free(avctx);
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-03-23 15:11:59)
>> 1. The earlier code would just work in case the user used a smaller
>> number of elements for the matrices if these matrices were not used at
>> all (which happens for the majority of encoders). This is no longer true
>> with this patch.
> 
> So?
> 

It means there is a scenario where you break something.

- 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Anton Khirnov]

Quoting Andreas Rheinhardt (2024-03-24 11:19:19)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2024-03-23 15:11:59)
> >> 1. The earlier code would just work in case the user used a smaller
> >> number of elements for the matrices if these matrices were not used at
> >> all (which happens for the majority of encoders). This is no longer true
> >> with this patch.
> > 
> > So?
> > 
> 
> It means there is a scenario where you break something.

There is no way for the caller to know whether, and how much, will lavc
read from those tables, so it's invalid API use.

-- 
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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-03-24 11:19:19)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2024-03-23 15:11:59)
>>>> 1. The earlier code would just work in case the user used a smaller
>>>> number of elements for the matrices if these matrices were not used at
>>>> all (which happens for the majority of encoders). This is no longer true
>>>> with this patch.
>>>
>>> So?
>>>
>>
>> It means there is a scenario where you break something.
> 
> There is no way for the caller to know whether, and how much, will lavc
> read from those tables, so it's invalid API use.
> 

Incorrect: Given that these fields do not a length field, it is legal to
put pointers to matrices of arbitrary length in the relevant
AVCodecContext field. Just because the current code presumes this to
have at least 64 elements for certain mpegvideo encoders and just
because there are no other encoders using different values does not make
it illegal.
The reason we are in this predicament is of course that the API itself
is broken.

- 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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Anton Khirnov]

Quoting Andreas Rheinhardt (2024-03-24 12:10:24)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2024-03-24 11:19:19)
> >> Anton Khirnov:
> >>> Quoting Andreas Rheinhardt (2024-03-23 15:11:59)
> >>>> 1. The earlier code would just work in case the user used a smaller
> >>>> number of elements for the matrices if these matrices were not used at
> >>>> all (which happens for the majority of encoders). This is no longer true
> >>>> with this patch.
> >>>
> >>> So?
> >>>
> >>
> >> It means there is a scenario where you break something.
> > 
> > There is no way for the caller to know whether, and how much, will lavc
> > read from those tables, so it's invalid API use.
> > 
> 
> Incorrect: Given that these fields do not a length field, it is legal to
> put pointers to matrices of arbitrary length in the relevant
> AVCodecContext field.

No. Users are not allowed to write random values to random places.
The only meaningful way to use these arrays is with 64 elements,
therefore any caller using a different number is broken.

-- 
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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Andreas Rheinhardt]

Anton Khirnov:
> It is highly unsafe, as AVCodecContext contains many allocated fields.
> Everything needed by worked threads should be covered by
> * routing through AVCodecParameters
> * av_opt_copy()
> * copying quantisation matrices manually
> 
> avcodec_free_context() can now be used for per-thread contexts.

So now we have to maintain a list of stuff to be copied just to avoid
this "unsafe" copying. Sounds worse to me.
There is another usecase you are breaking: The execute-callbacks. The
user may have used custom ones that use a thread pool which would allow
using both slice- and frame-threading together.
And you also forgot to copy get_encode_buffer.

> ---
>  libavcodec/frame_thread_encoder.c | 48 ++++++++++++++++++++++++-------
>  1 file changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
> index cda5158117..d34fe022a5 100644
> --- a/libavcodec/frame_thread_encoder.c
> +++ b/libavcodec/frame_thread_encoder.c
> @@ -28,6 +28,7 @@
>  #include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "avcodec_internal.h"
> +#include "codec_par.h"
>  #include "encode.h"
>  #include "internal.h"
>  #include "pthread_internal.h"
> @@ -111,8 +112,7 @@ static void * attribute_align_arg worker(void *v){
>          pthread_mutex_unlock(&c->finished_task_mutex);
>      }
>  end:
> -    ff_codec_close(avctx);
> -    av_freep(&avctx);
> +    avcodec_free_context(&avctx);
>      return NULL;
>  }
>  
> @@ -121,6 +121,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>      int i=0;
>      ThreadContext *c;
>      AVCodecContext *thread_avctx = NULL;
> +    AVCodecParameters *par = NULL;
>      int ret;
>  
>      if(   !(avctx->thread_type & FF_THREAD_FRAME)
> @@ -194,18 +195,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>          }
>      }
>  
> +    par = avcodec_parameters_alloc();
> +    if (!par) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
> +
> +    ret = avcodec_parameters_from_context(par, avctx);
> +    if (ret < 0)
> +        goto fail;
> +
>      for(i=0; i<avctx->thread_count ; i++){
> -        void *tmpv;
>          thread_avctx = avcodec_alloc_context3(avctx->codec);
>          if (!thread_avctx) {
>              ret = AVERROR(ENOMEM);
>              goto fail;
>          }
> -        tmpv = thread_avctx->priv_data;
> -        *thread_avctx = *avctx;
> -        thread_avctx->priv_data = tmpv;
> -        thread_avctx->internal = NULL;
> -        thread_avctx->hw_frames_ctx = NULL;
> +
> +        ret = avcodec_parameters_to_context(thread_avctx, par);
> +        if (ret < 0)
> +            goto fail;
> +
>          ret = av_opt_copy(thread_avctx, avctx);
>          if (ret < 0)
>              goto fail;
> @@ -217,6 +227,22 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>          thread_avctx->thread_count = 1;
>          thread_avctx->active_thread_type &= ~FF_THREAD_FRAME;
>  
> +#define DUP_MATRIX(m)                                                       \
> +        if (avctx->m) {                                                     \
> +            thread_avctx->m = av_memdup(avctx->m, 64 * sizeof(*avctx->m));  \
> +            if (!thread_avctx->m) {                                         \
> +                ret = AVERROR(ENOMEM);                                      \
> +                goto fail;                                                  \
> +            }                                                               \
> +        }
> +        DUP_MATRIX(intra_matrix);
> +        DUP_MATRIX(chroma_intra_matrix);
> +        DUP_MATRIX(inter_matrix);
> +
> +#undef DUP_MATRIX
> +
> +        thread_avctx->stats_in = avctx->stats_in;
> +
>          if ((ret = avcodec_open2(thread_avctx, avctx->codec, NULL)) < 0)
>              goto fail;
>          av_assert0(!thread_avctx->internal->frame_thread_encoder);
> @@ -227,12 +253,14 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
>          }
>      }
>  
> +    avcodec_parameters_free(&par);
> +
>      avctx->active_thread_type = FF_THREAD_FRAME;
>  
>      return 0;
>  fail:
> -    ff_codec_close(thread_avctx);
> -    av_freep(&thread_avctx);
> +    avcodec_parameters_free(&par);
> +    avcodec_free_context(&thread_avctx);
>      avctx->thread_count = i;
>      av_log(avctx, AV_LOG_ERROR, "ff_frame_thread_encoder_init failed\n");
>      ff_frame_thread_encoder_free(avctx);

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

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

Re: [FFmpeg-devel] [PATCH v3 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Anton Khirnov]

Quoting Andreas Rheinhardt (2024-03-24 12:25:53)
> Anton Khirnov:
> > It is highly unsafe, as AVCodecContext contains many allocated fields.
> > Everything needed by worked threads should be covered by
> > * routing through AVCodecParameters
> > * av_opt_copy()
> > * copying quantisation matrices manually
> > 
> > avcodec_free_context() can now be used for per-thread contexts.
> 
> So now we have to maintain a list of stuff to be copied just to avoid
> this "unsafe" copying. Sounds worse to me.

We have to maintain a list of stuff either way, the difference is that
with my patch forgetting an item may result in some features not
working, while with current code forgetting an item will typically cause
a double free.

-- 
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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v4 08/12] lavc/frame_thread_encoder: avoid assigning a whole AVCodecContext

[Anton Khirnov]

It is highly unsafe, as AVCodecContext contains many allocated fields.
Almost everything needed by worker threads should be covered by
routing through AVCodecParameters and av_opt_copy(), except for a few
fields that are copied manually.

avcodec_free_context() can now be used for per-thread contexts.
---
 libavcodec/frame_thread_encoder.c | 52 +++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
index cda5158117..17712e68ee 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -28,6 +28,7 @@
 #include "libavutil/thread.h"
 #include "avcodec.h"
 #include "avcodec_internal.h"
+#include "codec_par.h"
 #include "encode.h"
 #include "internal.h"
 #include "pthread_internal.h"
@@ -111,8 +112,7 @@ static void * attribute_align_arg worker(void *v){
         pthread_mutex_unlock(&c->finished_task_mutex);
     }
 end:
-    ff_codec_close(avctx);
-    av_freep(&avctx);
+    avcodec_free_context(&avctx);
     return NULL;
 }
 
@@ -121,6 +121,7 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
     int i=0;
     ThreadContext *c;
     AVCodecContext *thread_avctx = NULL;
+    AVCodecParameters *par = NULL;
     int ret;
 
     if(   !(avctx->thread_type & FF_THREAD_FRAME)
@@ -194,18 +195,27 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         }
     }
 
+    par = avcodec_parameters_alloc();
+    if (!par) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
+
+    ret = avcodec_parameters_from_context(par, avctx);
+    if (ret < 0)
+        goto fail;
+
     for(i=0; i<avctx->thread_count ; i++){
-        void *tmpv;
         thread_avctx = avcodec_alloc_context3(avctx->codec);
         if (!thread_avctx) {
             ret = AVERROR(ENOMEM);
             goto fail;
         }
-        tmpv = thread_avctx->priv_data;
-        *thread_avctx = *avctx;
-        thread_avctx->priv_data = tmpv;
-        thread_avctx->internal = NULL;
-        thread_avctx->hw_frames_ctx = NULL;
+
+        ret = avcodec_parameters_to_context(thread_avctx, par);
+        if (ret < 0)
+            goto fail;
+
         ret = av_opt_copy(thread_avctx, avctx);
         if (ret < 0)
             goto fail;
@@ -217,6 +227,26 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         thread_avctx->thread_count = 1;
         thread_avctx->active_thread_type &= ~FF_THREAD_FRAME;
 
+#define DUP_MATRIX(m)                                                       \
+        if (avctx->m) {                                                     \
+            thread_avctx->m = av_memdup(avctx->m, 64 * sizeof(*avctx->m));  \
+            if (!thread_avctx->m) {                                         \
+                ret = AVERROR(ENOMEM);                                      \
+                goto fail;                                                  \
+            }                                                               \
+        }
+        DUP_MATRIX(intra_matrix);
+        DUP_MATRIX(chroma_intra_matrix);
+        DUP_MATRIX(inter_matrix);
+
+#undef DUP_MATRIX
+
+        thread_avctx->opaque            = avctx->opaque;
+        thread_avctx->get_encode_buffer = avctx->get_encode_buffer;
+        thread_avctx->execute           = avctx->execute;
+        thread_avctx->execute2          = avctx->execute2;
+        thread_avctx->stats_in          = avctx->stats_in;
+
         if ((ret = avcodec_open2(thread_avctx, avctx->codec, NULL)) < 0)
             goto fail;
         av_assert0(!thread_avctx->internal->frame_thread_encoder);
@@ -227,12 +257,14 @@ av_cold int ff_frame_thread_encoder_init(AVCodecContext *avctx)
         }
     }
 
+    avcodec_parameters_free(&par);
+
     avctx->active_thread_type = FF_THREAD_FRAME;
 
     return 0;
 fail:
-    ff_codec_close(thread_avctx);
-    av_freep(&thread_avctx);
+    avcodec_parameters_free(&par);
+    avcodec_free_context(&thread_avctx);
     avctx->thread_count = i;
     av_log(avctx, AV_LOG_ERROR, "ff_frame_thread_encoder_init failed\n");
     ff_frame_thread_encoder_free(avctx);
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".