From 5d58a35f983fa0ce028fad593697903aecae9adf Mon Sep 17 00:00:00 2001 From: Anton Khirnov Date: Thu, 14 Sep 2023 17:07:29 +0200 Subject: [PATCH] fftools/ffmpeg: deprecate the -top option It is badly named (should have been -top_field_first, or at least -tff), underdocumented and underspecified, and (most importantly) entirely redundant with the setfield filter. --- Changelog | 1 + doc/ffmpeg.texi | 2 -- fftools/ffmpeg.h | 9 +++++++++ fftools/ffmpeg_dec.c | 6 +++++- fftools/ffmpeg_demux.c | 2 ++ fftools/ffmpeg_enc.c | 16 +++++++++++++--- fftools/ffmpeg_mux_init.c | 4 ++++ fftools/ffmpeg_opt.c | 6 +++++- tests/fate/lavf-container.mak | 8 ++++---- 9 files changed, 43 insertions(+), 11 deletions(-) diff --git a/Changelog b/Changelog index aaf887b912..3f67a2bba9 100644 --- a/Changelog +++ b/Changelog @@ -33,6 +33,7 @@ version : - OSQ demuxer and decoder - Support HEVC,VP9,AV1 codec fourcclist in enhanced rtmp protocol - CRI USM demuxer +- ffmpeg CLI '-top' option deprecated in favor of the setfield filter version 6.0: diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index 2273c39214..d2864ff37e 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -1052,8 +1052,6 @@ Dump video coding statistics to @var{file}. See the Specify which version of the vstats format to use. Default is @code{2}. See the @ref{vstats_file_format,,vstats file format} section for the format description. -@item -top[:@var{stream_specifier}] @var{n} (@emph{output,per-stream}) -top=1/bottom=0/auto=-1 field first @item -vtag @var{fourcc/tag} (@emph{output}) Force video tag/fourcc. This is an alias for @code{-tag:v}. @item -vbsf @var{bitstream_filter} diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index b146f1f2dc..b059ecbb9f 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -57,6 +57,7 @@ #define FFMPEG_OPT_QPHIST 1 #define FFMPEG_OPT_ADRIFT_THRESHOLD 1 #define FFMPEG_OPT_ENC_TIME_BASE_NUM 1 +#define FFMPEG_OPT_TOP 1 enum VideoSyncMethod { VSYNC_AUTO = -1, @@ -218,8 +219,10 @@ typedef struct OptionsContext { int nb_inter_matrices; SpecifierOpt *chroma_intra_matrices; int nb_chroma_intra_matrices; +#if FFMPEG_OPT_TOP SpecifierOpt *top_field_first; int nb_top_field_first; +#endif SpecifierOpt *metadata_map; int nb_metadata_map; SpecifierOpt *presets; @@ -344,7 +347,9 @@ typedef struct InputStream { AVDictionary *decoder_opts; AVRational framerate; /* framerate forced with -r */ +#if FFMPEG_OPT_TOP int top_field_first; +#endif int autorotate; @@ -538,7 +543,9 @@ typedef struct OutputStream { enum VideoSyncMethod vsync_method; int is_cfr; int force_fps; +#if FFMPEG_OPT_TOP int top_field_first; +#endif #if FFMPEG_ROTATION_METADATA int rotate_overridden; #endif @@ -931,7 +938,9 @@ static inline int err_merge(int err0, int err1) extern const char * const opt_name_codec_names[]; extern const char * const opt_name_codec_tags[]; extern const char * const opt_name_frame_rates[]; +#if FFMPEG_OPT_TOP extern const char * const opt_name_top_field_first[]; +#endif static inline void pkt_move(void *dst, void *src) { diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c index 8a3b52fd7e..1de8234a97 100644 --- a/fftools/ffmpeg_dec.c +++ b/fftools/ffmpeg_dec.c @@ -325,8 +325,12 @@ static int video_frame_process(InputStream *ist, AVFrame *frame) ist->dec_ctx->pix_fmt); } - if(ist->top_field_first>=0) +#if FFMPEG_OPT_TOP + if(ist->top_field_first>=0) { + av_log(ist, AV_LOG_WARNING, "-top is deprecated, use the setfield filter instead\n"); frame->flags |= AV_FRAME_FLAG_TOP_FIELD_FIRST; + } +#endif if (frame->format == d->hwaccel_pix_fmt) { int err = hwaccel_retrieve_data(ist->dec_ctx, frame); diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index 48edbd7f6b..c01852d4cf 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1226,8 +1226,10 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st) } } +#if FFMPEG_OPT_TOP ist->top_field_first = -1; MATCH_PER_STREAM_OPT(top_field_first, i, ist->top_field_first, ic, st); +#endif ist->framerate_guessed = av_guess_frame_rate(ic, st, NULL); diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c index 902ba3c37a..b40a6211a9 100644 --- a/fftools/ffmpeg_enc.c +++ b/fftools/ffmpeg_enc.c @@ -357,9 +357,17 @@ int enc_open(OutputStream *ost, AVFrame *frame) enc_ctx->chroma_sample_location = frame->chroma_location; if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) || - (frame->flags & AV_FRAME_FLAG_INTERLACED) || ost->top_field_first >= 0) { - int top_field_first = ost->top_field_first >= 0 ? - ost->top_field_first : !!(frame->flags & AV_FRAME_FLAG_TOP_FIELD_FIRST); + (frame->flags & AV_FRAME_FLAG_INTERLACED) +#if FFMPEG_OPT_TOP + || ost->top_field_first >= 0 +#endif + ) { + int top_field_first = +#if FFMPEG_OPT_TOP + ost->top_field_first >= 0 ? + ost->top_field_first : +#endif + !!(frame->flags & AV_FRAME_FLAG_TOP_FIELD_FIRST); if (enc->id == AV_CODEC_ID_MJPEG) enc_ctx->field_order = top_field_first ? AV_FIELD_TT : AV_FIELD_BB; @@ -1156,10 +1164,12 @@ static int do_video_out(OutputFile *of, OutputStream *ost, AVFrame *frame) in_picture->quality = enc->global_quality; in_picture->pict_type = forced_kf_apply(ost, &ost->kf, enc->time_base, in_picture, i); +#if FFMPEG_OPT_TOP if (ost->top_field_first >= 0) { in_picture->flags &= ~AV_FRAME_FLAG_TOP_FIELD_FIRST; in_picture->flags |= AV_FRAME_FLAG_TOP_FIELD_FIRST * (!!ost->top_field_first); } +#endif ret = submit_encode_frame(of, ost, in_picture); if (ret == AVERROR_EOF) diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c index cf4cd2d5b7..9d6f442068 100644 --- a/fftools/ffmpeg_mux_init.c +++ b/fftools/ffmpeg_mux_init.c @@ -779,8 +779,12 @@ static int new_stream_video(Muxer *mux, const OptionsContext *o, MATCH_PER_STREAM_OPT(force_fps, i, ost->force_fps, oc, st); +#if FFMPEG_OPT_TOP ost->top_field_first = -1; MATCH_PER_STREAM_OPT(top_field_first, i, ost->top_field_first, oc, st); + if (ost->top_field_first >= 0) + av_log(ost, AV_LOG_WARNING, "-top is deprecated, use the setfield filter instead\n"); +#endif ost->vsync_method = video_sync_method; MATCH_PER_STREAM_OPT(fps_mode, str, fps_mode, oc, st); diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index dc6044120a..304471dd03 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -56,7 +56,9 @@ const char *const opt_name_codec_names[] = {"c", "codec", "acodec", "vcodec", "scodec", "dcodec", NULL}; const char *const opt_name_frame_rates[] = {"r", NULL}; const char *const opt_name_codec_tags[] = {"tag", "atag", "vtag", "stag", NULL}; +#if FFMPEG_OPT_TOP const char *const opt_name_top_field_first[] = {"top", NULL}; +#endif HWDevice *filter_hw_device; @@ -1695,9 +1697,11 @@ const OptionDef options[] = { { "chroma_intra_matrix", OPT_VIDEO | HAS_ARG | OPT_EXPERT | OPT_STRING | OPT_SPEC | OPT_OUTPUT, { .off = OFFSET(chroma_intra_matrices) }, "specify intra matrix coeffs", "matrix" }, +#if FFMPEG_OPT_TOP { "top", OPT_VIDEO | HAS_ARG | OPT_EXPERT | OPT_INT| OPT_SPEC | OPT_INPUT | OPT_OUTPUT, { .off = OFFSET(top_field_first) }, - "top=1/bottom=0/auto=-1 field first", "" }, + "deprecated, use the setfield video filter", "" }, +#endif { "vtag", OPT_VIDEO | HAS_ARG | OPT_EXPERT | OPT_PERFILE | OPT_INPUT | OPT_OUTPUT, { .func_arg = opt_old2new }, "force video tag/fourcc", "fourcc/tag" }, diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak index 0081b45eea..d4a0ac7510 100644 --- a/tests/fate/lavf-container.mak +++ b/tests/fate/lavf-container.mak @@ -54,10 +54,10 @@ fate-lavf-mov_rtphint: CMD = lavf_container "" "-movflags +rtphint -c:a pcm_alaw fate-lavf-mp4: CMD = lavf_container_timecode "-c:v mpeg4 -an -threads 1" fate-lavf-mpg: CMD = lavf_container_timecode "-ar 44100 -threads 1" fate-lavf-mxf: CMD = lavf_container_timecode "-ar 48000 -bf 2 -threads 1" -fate-lavf-mxf_d10: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,pad=720:608:0:32 -c:v mpeg2video -g 0 -flags +ildct+low_delay -dc 10 -non_linear_quant 1 -intra_vlc 1 -qscale 1 -ps 1 -qmin 1 -rc_max_vbv_use 1 -rc_min_vbv_use 1 -pix_fmt yuv422p -minrate 30000k -maxrate 30000k -b 30000k -bufsize 1200000 -top 1 -rc_init_occupancy 1200000 -qmax 12 -f mxf_d10" -fate-lavf-mxf_dv25: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=4/3 -c:v dvvideo -pix_fmt yuv420p -b 25000k -top 0 -f mxf" -fate-lavf-mxf_dvcpro50: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=16/9 -c:v dvvideo -pix_fmt yuv422p -b 50000k -top 0 -f mxf" -fate-lavf-mxf_dvcpro100: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=1440:1080,setdar=16/9 -c:v dvvideo -pix_fmt yuv422p -b 100000k -top 0 -f mxf" +fate-lavf-mxf_d10: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,pad=720:608:0:32,setfield=tff -c:v mpeg2video -g 0 -flags +ildct+low_delay -dc 10 -non_linear_quant 1 -intra_vlc 1 -qscale 1 -ps 1 -qmin 1 -rc_max_vbv_use 1 -rc_min_vbv_use 1 -pix_fmt yuv422p -minrate 30000k -maxrate 30000k -b 30000k -bufsize 1200000 -rc_init_occupancy 1200000 -qmax 12 -f mxf_d10" +fate-lavf-mxf_dv25: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=4/3,setfield=bff -c:v dvvideo -pix_fmt yuv420p -b 25000k -f mxf" +fate-lavf-mxf_dvcpro50: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=720:576,setdar=16/9,setfield=bff -c:v dvvideo -pix_fmt yuv422p -b 50000k -f mxf" +fate-lavf-mxf_dvcpro100: CMD = lavf_container "-ar 48000 -ac 2" "-r 25 -vf scale=1440:1080,setdar=16/9,setfield=bff -c:v dvvideo -pix_fmt yuv422p -b 100000k -f mxf" fate-lavf-mxf_ffv1: CMD = lavf_container "-an" "-r 25 -vf scale=720:576,setdar=4/3 -c:v ffv1 -level 3 -pix_fmt yuv420p -f mxf" fate-lavf-mxf_opatom: CMD = lavf_container "" "-s 1920x1080 -c:v dnxhd -pix_fmt yuv422p -vb 36M -f mxf_opatom -map 0" fate-lavf-mxf_opatom_audio: CMD = lavf_container "-ar 48000 -ac 1" "-f mxf_opatom -mxf_audio_edit_rate 25 -map 1"