Re: [FFmpeg-devel] [PATCH 2/5] ffmpeg: Add display_matrix option

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Thilo Borgmann:
> From: Jan Ekström <jeebjp@gmail.com>
> 
> This enables overriding the rotation as well as horizontal/vertical
> flip state of a specific video stream on the input side.
> 
> Additionally, switch the singular test that was utilizing the rotation
> metadata to instead override the input display rotation, thus leading
> to the same result.
> ---
>  doc/ffmpeg.texi             |  13 +++++
>  fftools/cmdutils.h          |   1 +
>  fftools/ffmpeg.h            |   2 +
>  fftools/ffmpeg_opt.c        | 107 ++++++++++++++++++++++++++++++++++++
>  tests/fate/filter-video.mak |   2 +-
>  5 files changed, 124 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 42440d93b4..5d3e3b3052 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -912,6 +912,19 @@ If used together with @option{-vcodec copy}, it will affect the aspect ratio
>  stored at container level, but not the aspect ratio stored in encoded
>  frames, if it exists.
>  
> +@item -display_matrix[:@var{stream_specifier}] @var{opt1=val1[,opt2=val2]...} (@emph{input,per-stream})
> +Set the video display matrix according to given options.
> +
> +@table @option
> +@item rotation=@var{number}
> +Set the rotation using a floating point number that describes a pure
> +counter-clockwise rotation in degrees.
> +The @code{-autorotate} logic will be affected.
> +@item hflip=@var{[0,1]}
> +@item vflip=@var{[0,1]}
> +Set a horizontal or vertical flip.

This documentation does not specify the order in which rotation and
flipping will be applied.

> +@end table
> +
>  @item -vn (@emph{input/output})
>  As an input option, blocks all video streams of a file from being filtered or
>  being automatically selected or mapped for any output. See @code{-discard}
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index 6a519c6546..df90cc6958 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -166,6 +166,7 @@ typedef struct OptionDef {
>      } u;
>      const char *help;
>      const char *argname;
> +    const AVClass *args;

Never used(?)

>  } OptionDef;
>  
>  /**
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 6991ba7632..0ea730fd42 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -193,6 +193,8 @@ typedef struct OptionsContext {
>      int        nb_force_fps;
>      SpecifierOpt *frame_aspect_ratios;
>      int        nb_frame_aspect_ratios;
> +    SpecifierOpt *display_matrixes;
> +    int        nb_display_matrixes;
>      SpecifierOpt *rc_overrides;
>      int        nb_rc_overrides;
>      SpecifierOpt *intra_matrices;
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index cc038aae6b..e184b4239c 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -20,6 +20,7 @@
>  
>  #include "config.h"
>  
> +#include <float.h>
>  #include <stdint.h>
>  
>  #if HAVE_SYS_RESOURCE_H
> @@ -45,6 +46,7 @@
>  #include "libavutil/avutil.h"
>  #include "libavutil/bprint.h"
>  #include "libavutil/channel_layout.h"
> +#include "libavutil/display.h"
>  #include "libavutil/getenv_utf8.h"
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/fifo.h"
> @@ -87,6 +89,7 @@ static const char *const opt_name_forced_key_frames[]         = {"forced_key_fra
>  static const char *const opt_name_fps_mode[]                  = {"fps_mode", NULL};
>  static const char *const opt_name_force_fps[]                 = {"force_fps", NULL};
>  static const char *const opt_name_frame_aspect_ratios[]       = {"aspect", NULL};
> +static const char *const opt_name_display_matrixes[]          = {"display_matrix", NULL};
>  static const char *const opt_name_rc_overrides[]              = {"rc_override", NULL};
>  static const char *const opt_name_intra_matrices[]            = {"intra_matrix", NULL};
>  static const char *const opt_name_inter_matrices[]            = {"inter_matrix", NULL};
> @@ -112,6 +115,32 @@ static const char *const opt_name_time_bases[]                = {"time_base", NU
>  static const char *const opt_name_enc_time_bases[]            = {"enc_time_base", NULL};
>  static const char *const opt_name_bits_per_raw_sample[]       = {"bits_per_raw_sample", NULL};
>  
> +// XXX this should probably go into a seperate file <name>_args.c and #included here
> +    struct display_matrix_s {

We actually use CamelCase for struct tags and typedefs.

> +        const AVClass *class;
> +        double  rotation;
> +        int     hflip;
> +        int     vflip;
> +    };
> +#define OFFSET(x) offsetof(struct display_matrix_s, x)
> +    static const AVOption display_matrix_args[] = {
> +        { "rotation", "set rotation", OFFSET(rotation), AV_OPT_TYPE_DOUBLE,
> +            { .dbl = DBL_MAX }, -(DBL_MAX), DBL_MAX - 1.0f, AV_OPT_FLAG_ARGUMENT},
> +        { "hflip",    "set hflip", OFFSET(hflip),    AV_OPT_TYPE_BOOL,
> +            { .i64 = -1 }, 0, 1, AV_OPT_FLAG_ARGUMENT},
> +        { "vflip",    "set vflip", OFFSET(vflip),    AV_OPT_TYPE_BOOL,
> +            { .i64 = -1 }, 0, 1, AV_OPT_FLAG_ARGUMENT},
> +        { NULL },
> +    };
> +    static const AVClass class_display_matrix_args = {
> +        .class_name = "display_matrix_args",
> +        .item_name  = av_default_item_name,
> +        .option     = display_matrix_args,
> +        .version    = LIBAVUTIL_VERSION_INT,
> +    };
> +#undef OFFSET
> +// XXX
> +
>  #define WARN_MULTIPLE_OPT_USAGE(name, type, so, st)\
>  {\
>      char namestr[128] = "";\
> @@ -808,6 +837,75 @@ static int opt_recording_timestamp(void *optctx, const char *opt, const char *ar
>      return 0;
>  }
>  
> +static void add_display_matrix_to_stream(OptionsContext *o,
> +                                         AVFormatContext *ctx, AVStream *st)
> +{
> +    int hflip_set = 0, vflip_set = 0, display_rotation_set = 0;
> +    uint8_t *buf = NULL;
> +
> +    static struct display_matrix_s test_args = {

Why static?

> +        .class    = &class_display_matrix_args,
> +        .rotation = DBL_MAX,
> +        .hflip    = -1,
> +        .vflip    = -1,
> +    };
> +
> +    AVDictionary *global_args = NULL;
> +    AVDictionary *local_args  = NULL;
> +    AVDictionaryEntry *en = NULL;
> +
> +    MATCH_PER_STREAM_OPT(display_matrixes, dict, global_args, ctx, st);
> +
> +    if (!global_args)
> +        return;
> +
> +    // make a copy of the dict so it doesn't get freed from underneath us
> +    if (av_dict_copy(&local_args, global_args, 0) < 0) {
> +        av_log(NULL, AV_LOG_FATAL,
> +               "Failed to copy argument dict for display matrix!\n");

No exit_program here?

> +    }
> +
> +    if (av_opt_set_dict2(&test_args, &local_args, 0) < 0) {
> +        av_log(NULL, AV_LOG_FATAL,
> +               "Failed to set options for a display matrix!\n");
> +        exit_program(1);
> +    }
> +
> +    while ((en = av_dict_get(local_args, "", en, AV_DICT_IGNORE_SUFFIX))) {
> +        av_log(NULL, AV_LOG_FATAL,
> +               "Unknown option=value pair for display matrix: "
> +               "key: '%s', value: '%s'!\n",
> +               en->key, en->value);
> +    }
> +
> +    if (av_dict_count(local_args)) {
> +        exit_program(1);
> +    }
> +
> +    av_dict_free(&local_args);
> +
> +    display_rotation_set = test_args.rotation != DBL_MAX;
> +    hflip_set            = test_args.hflip != -1;
> +    vflip_set            = test_args.vflip != -1;
> +
> +    if (!display_rotation_set && !hflip_set && !vflip_set)
> +        return;
> +
> +    if (!(buf = av_stream_new_side_data(st, AV_PKT_DATA_DISPLAYMATRIX,
> +                                        sizeof(int32_t) * 9))) {
> +        av_log(NULL, AV_LOG_FATAL, "Failed to generate a display matrix!\n");
> +        exit_program(1);
> +    }
> +
> +    av_display_rotation_set((int32_t *)buf,
> +                            display_rotation_set ? -(test_args.rotation) :
> +                                                   -0.0f);
> +    av_display_matrix_flip((int32_t *)buf,
> +                           hflip_set ? test_args.hflip : 0,
> +                           vflip_set ? test_args.vflip : 0);
> +}
> +
> +
>  static const AVCodec *find_codec_or_die(const char *name, enum AVMediaType type, int encoder)
>  {
>      const AVCodecDescriptor *desc;
> @@ -942,6 +1040,8 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
>          }
>  
>          if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
> +            add_display_matrix_to_stream(o, ic, st);
> +
>              MATCH_PER_STREAM_OPT(hwaccels, str, hwaccel, ic, st);
>              MATCH_PER_STREAM_OPT(hwaccel_output_formats, str,
>                                   hwaccel_output_format, ic, st);
> @@ -1865,6 +1965,8 @@ static OutputStream *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
>          ost->frame_aspect_ratio = q;
>      }
>  
> +    add_display_matrix_to_stream(o, oc, st);
> +
>      MATCH_PER_STREAM_OPT(filter_scripts, str, ost->filters_script, oc, st);
>      MATCH_PER_STREAM_OPT(filters,        str, ost->filters,        oc, st);
>  
> @@ -3992,6 +4094,11 @@ const OptionDef options[] = {
>      { "aspect",       OPT_VIDEO | HAS_ARG  | OPT_STRING | OPT_SPEC |
>                        OPT_OUTPUT,                                                { .off = OFFSET(frame_aspect_ratios) },
>          "set aspect ratio (4:3, 16:9 or 1.3333, 1.7777)", "aspect" },
> +    { "display_matrix", OPT_VIDEO | HAS_ARG | OPT_DICT | OPT_SPEC |
> +                        OPT_INPUT,                                              { .off = OFFSET(display_matrixes) },
> +        "define a display matrix with rotation and/or horizontal/vertical "
> +        "flip for stream(s)",
> +        "arguments", &class_display_matrix_args },
>      { "pix_fmt",      OPT_VIDEO | HAS_ARG | OPT_EXPERT  | OPT_STRING | OPT_SPEC |
>                        OPT_INPUT | OPT_OUTPUT,                                    { .off = OFFSET(frame_pix_fmts) },
>          "set pixel format", "format" },
> diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
> index 372c70bba7..763390ea51 100644
> --- a/tests/fate/filter-video.mak
> +++ b/tests/fate/filter-video.mak
> @@ -691,7 +691,7 @@ fate-filter-metadata-avf-aphase-meter-out-of-phase: SRC = $(TARGET_SAMPLES)/filt
>  fate-filter-metadata-avf-aphase-meter-out-of-phase: CMD = run $(FILTER_METADATA_COMMAND) "amovie='$(SRC)',aphasemeter=video=0"
>  
>  FATE_FILTER_SAMPLES-$(call TRANSCODE, RAWVIDEO H264, MOV, ARESAMPLE_FILTER  AAC_FIXED_DECODER) += fate-filter-meta-4560-rotate0
> -fate-filter-meta-4560-rotate0: CMD = transcode mov $(TARGET_SAMPLES)/filter/sample-in-issue-505.mov mov "-c copy -metadata:s:v:0 rotate=0" "-af aresample" "" "" "-flags +bitexact -c:a aac_fixed"
> +fate-filter-meta-4560-rotate0: CMD = transcode "mov -display_matrix:v:0 rotation=0" $(TARGET_SAMPLES)/filter/sample-in-issue-505.mov mov "-c copy" "-af aresample" "" "" "-flags +bitexact -c:a aac_fixed"
>  
>  FATE_FILTER_CMP_METADATA-$(CONFIG_BLOCKDETECT_FILTER) += fate-filter-refcmp-blockdetect-yuv
>  fate-filter-refcmp-blockdetect-yuv: CMD = cmp_metadata blockdetect yuv420p 0.015

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