[FFmpeg-devel] [RFC] Specifying KEYINT (-g) has no effect in libx264

Stefano Sabatini stefano.sabatini-lala at poste.it
Thu May 26 17:18:35 CEST 2011


On date Thursday 2011-05-26 01:45:30 +0200, Etienne Buira encoded:
> On Mon, May 23, 2011 at 01:32:15AM -0700, Baptiste Coudurier wrote:
> > On 5/22/11 8:33 PM, Michael Niedermayer wrote:
> > > On Sun, May 22, 2011 at 10:40:07PM +0200, Stefano Sabatini wrote:
> > >> Hi,
> > >>
> > >> see issue #157.
> > >>
> > >> The problem was possibly introduced in:
> > >>
> > >> commit 0140d3f0921e5cbb6ea8706acb0307f7ff57a133
> > >> Author: Baptiste Coudurier <baptiste.coudurier at gmail.com>
> > >> Date:   Sat Apr 16 16:50:50 2011 -0700
> > >>
> > >>     In libx264 wrapper, add -preset and -tune options
> > >>
> > >> and depends on the fact that x264_param_default_preset() which is
> > >> called in x264_init(), calls x264_param_default() which defaults the
> > >> already set options.
> > >>
> > >> Indeed order of options matters, with the current mechanism options
> > >> are set in the relevant contexts, and globally processed in the object
> > >> initialization, so the order specified on the commandline is basically
> > >> ignored.
> > >>
> > >> In the case of libx264 we may process profile/preset *before* the
> > >> other options set in the context, so that specific settings will
> > >> override profile/preset information.
> > 
> > No, order doesn't matter, as long as a preset is specified, it will
> > override other options.
> > 
> > >> This has the problem that most of the times user-set options are
> > >> changed to medium profile or libx264 will complain&exit, see:
> > >>
> > >> thus making fine-tuned user options pointless (since they are changed
> > >> to medium preset and/or will be rejected by libx264).
> > 
> > No, you can still specify fine-tuned options without a preset.
> > 
> > > I see 3 obvious solutions
> > > 
> > > A. is mplayers to just use -x264opts which ive added
> > > 
> > > B. keep track of which options have been set by the user and which are
> > >    default.
> > 
> > 'B' is how it should be done. It's trivial, just remap "g" in the
> > libx264.c the way weightp was mapped.
> 
> Hi all.
> 
> Patch attached.
> 
> But now, I think it would be better to keep track of what options have
> been set at libavutil level. I don't know if ffmpeg would benefit a lot
> from that, but certainly libavutil.

> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index d9bac17..efc8b46 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -20,6 +20,7 @@
>   */
>  
>  #include "libavutil/opt.h"
> +#include "libavutil/avstring.h"
>  #include "avcodec.h"
>  #include <x264.h>
>  #include <math.h>
> @@ -43,6 +44,55 @@ typedef struct X264Context {
>      char *stats;
>      char *weightp;
>      char *x264opts;
> +    char *keyint_max;
> +    char *bframes;
> +    char *cabac;
> +    char *badapt;
> +    char *bbias;
> +    char *keyintmin;
> +    char *scenecut;
> +    char *deblockalpha;
> +    char *deblockbeta;
> +    char *qpmin;
> +    char *qpmax;
> +    char *qpstep;
> +    char *qcomp;
> +    char *qblur;
> +    char *cplxblur;
> +    char *ref;
> +    char *partitions;
> +    char *directpred;
> +    char *me_method;
> +    char *aqmode;
> +    char *aqstrength;
> +    char *rc_lookahead;
> +    char *psy_rd;
> +    char *psy_trellis;
> +    char *me_range;
> +    char *subq;
> +    char *chroma_me;
> +    char *trellis;
> +    char *nr;
> +    char *ip_factor;
> +    char *pb_factor;
> +    char *chromaoffset;
> +    char *pass;
> +    char *slices;
> +    char *flags, *flags2;
> +    char *psnr;
> +    char *b_pyramid;
> +    char *wpred;
> +    char *psy;
> +    char *mixed_refs;
> +    char *dct8x8;
> +    char *fast_pskip;
> +    char *mb_tree;
> +    char *intra_refresh;
> +    char *ssim;
> +    char *aud;
> +    char *interlaced;
> +    char *opengop;
> +    char *repeatheaders;
>  } X264Context;
>  
>  static void X264_log(void *p, int level, const char *fmt, va_list args)
> @@ -170,6 +220,57 @@ static av_cold int X264_close(AVCodecContext *avctx)
>      av_free(x4->level);
>      av_free(x4->stats);
>      av_free(x4->weightp);

> +    av_free(x4->x264opts);
> +    av_free(x4->keyint_max);
> +    av_free(x4->bframes);
> +    av_free(x4->cabac);
> +    av_free(x4->badapt);
> +    av_free(x4->bbias);
> +    av_free(x4->keyintmin);
> +    av_free(x4->scenecut);
> +    av_free(x4->deblockalpha);
> +    av_free(x4->deblockbeta);
> +    av_free(x4->qpmin);
> +    av_free(x4->qpmax);
> +    av_free(x4->qpstep);
> +    av_free(x4->qcomp);
> +    av_free(x4->qblur);
> +    av_free(x4->cplxblur);
> +    av_free(x4->ref);
> +    av_free(x4->partitions);
> +    av_free(x4->directpred);
> +    av_free(x4->me_method);
> +    av_free(x4->aqmode);
> +    av_free(x4->aqstrength);
> +    av_free(x4->rc_lookahead);
> +    av_free(x4->psy_rd);
> +    av_free(x4->psy_trellis);
> +    av_free(x4->me_range);
> +    av_free(x4->subq);
> +    av_free(x4->chroma_me);
> +    av_free(x4->trellis);
> +    av_free(x4->nr);
> +    av_free(x4->ip_factor);
> +    av_free(x4->pb_factor);
> +    av_free(x4->chromaoffset);
> +    av_free(x4->pass);
> +    av_free(x4->slices);
> +    av_free(x4->flags);
> +    av_free(x4->flags2);
> +    av_free(x4->psnr);
> +    av_free(x4->b_pyramid);
> +    av_free(x4->wpred);
> +    av_free(x4->psy);
> +    av_free(x4->mixed_refs);
> +    av_free(x4->dct8x8);
> +    av_free(x4->fast_pskip);
> +    av_free(x4->mb_tree);
> +    av_free(x4->intra_refresh);
> +    av_free(x4->ssim);
> +    av_free(x4->aud);
> +    av_free(x4->interlaced);
> +    av_free(x4->opengop);
> +    av_free(x4->repeatheaders);

Note: we need a function for freeing the private context strings
  
>      return 0;
>  }
> @@ -208,95 +309,137 @@ static void check_default_settings(AVCodecContext *avctx)
>          }                                                               \
>      } while (0);                                                        \
>  
> -static av_cold int X264_init(AVCodecContext *avctx)
> -{
> -    X264Context *x4 = avctx->priv_data;
> -
> -    x4->sei_size = 0;
> -    x264_param_default(&x4->params);
> -
> -    x4->params.i_keyint_max         = avctx->gop_size;
> -
> -    x4->params.i_bframe          = avctx->max_b_frames;
> -    x4->params.b_cabac           = avctx->coder_type == FF_CODER_TYPE_AC;
> -    x4->params.i_bframe_adaptive = avctx->b_frame_strategy;
> -    x4->params.i_bframe_bias     = avctx->bframebias;
> -    x4->params.i_bframe_pyramid  = avctx->flags2 & CODEC_FLAG2_BPYRAMID ? X264_B_PYRAMID_NORMAL : X264_B_PYRAMID_NONE;
> -    avctx->has_b_frames          = avctx->flags2 & CODEC_FLAG2_BPYRAMID ? 2 : !!avctx->max_b_frames;

> +#define OPT_DBLSTR(opt, param1, param2, err)                                \
> +    do {                                                                    \

> +        if (param1) {                                                       \
> +            char buf[255];                                                  \
> +            if (snprintf(buf, 254, "%s%s%s", param1,                        \

sizeof(buf) => safer

> +                         param2 ? ":" : "", param2 ? param2 : "") > 254) {  \

254 => safer

> +                av_log(avctx, AV_LOG_ERROR,                                 \
> +                       "String too long when dealing with %s param\n", opt);\

> +                return -1;                                                  \

please meaningful error codes

> +            }                                                               \


> +            OPT_STR(opt, buf);                                              \
> +        } else if (param2) {                                                \
> +            av_log(avctx, AV_LOG_ERROR, "%s\n", err);                       \
> +            return -1;                                                      \
> +        }                                                                   \
> +    } while(0);                                                             \

making a function of these macros may reduce object code size

> +
> +#define OPT_FLAG(opt, override, flarr, flnm, flagstr)                       \

flnm? Please use more descriptive names.

> +    do {                                                                    \
> +        char *optdstr; int ret;                                             \
> +        if (override) {                                                     \
> +            OPT_STR(opt, override);                                         \
> +        } else if ((ret=flagset(flnm, flagstr, flarr, &optdstr)) > 0) {     \
> +            OPT_STR(opt, optdstr);                                          \
> +        } else if (ret < 0) {                                               \
> +            av_log(avctx, AV_LOG_ERROR, "Error when parsing flags\n");      \
> +            return -1;                                                      \
> +        }                                                                   \
> +    } while (0);                                                            \

Same notes as before.

> +
> +static struct {
> +    const char *name;
> +    unsigned int part;
> +} x264parts[] = {
> +    {"i4x4", X264_ANALYSE_I4x4},
> +    {"i8x8", X264_ANALYSE_I8x8},
> +    {"p8x8", X264_ANALYSE_PSUB16x16},
> +    {"p4x4", X264_ANALYSE_PSUB8x8},
> +    {"b8x8", X264_ANALYSE_BSUB16x16},
> +    {NULL},
> +};

Ditto.

> -    x4->params.i_keyint_min = avctx->keyint_min;
> -    if (x4->params.i_keyint_min > x4->params.i_keyint_max)
> -        x4->params.i_keyint_min = x4->params.i_keyint_max;
> -
> -    x4->params.i_scenecut_threshold        = avctx->scenechange_threshold;
> -
> -    x4->params.b_deblocking_filter         = avctx->flags & CODEC_FLAG_LOOP_FILTER;
> -    x4->params.i_deblocking_filter_alphac0 = avctx->deblockalpha;
> -    x4->params.i_deblocking_filter_beta    = avctx->deblockbeta;
> -
> -    x4->params.rc.i_qp_min                 = avctx->qmin;
> -    x4->params.rc.i_qp_max                 = avctx->qmax;
> -    x4->params.rc.i_qp_step                = avctx->max_qdiff;
> -
> -    x4->params.rc.f_qcompress       = avctx->qcompress; /* 0.0 => cbr, 1.0 => constant qp */
> -    x4->params.rc.f_qblur           = avctx->qblur;     /* temporally blur quants */
> -    x4->params.rc.f_complexity_blur = avctx->complexityblur;
> -
> -    x4->params.i_frame_reference    = avctx->refs;
> -
> -    x4->params.analyse.inter    = 0;
> -    if (avctx->partitions) {
> -        if (avctx->partitions & X264_PART_I4X4)
> -            x4->params.analyse.inter |= X264_ANALYSE_I4x4;
> -        if (avctx->partitions & X264_PART_I8X8)
> -            x4->params.analyse.inter |= X264_ANALYSE_I8x8;
> -        if (avctx->partitions & X264_PART_P8X8)
> -            x4->params.analyse.inter |= X264_ANALYSE_PSUB16x16;
> -        if (avctx->partitions & X264_PART_P4X4)
> -            x4->params.analyse.inter |= X264_ANALYSE_PSUB8x8;
> -        if (avctx->partitions & X264_PART_B8X8)
> -            x4->params.analyse.inter |= X264_ANALYSE_BSUB16x16;
> -    }
> +static struct {
> +    const char *ffname;
> +    const char *x264name;
> +} x264mes[] = {
> +    {"epzs", "dia"},
> +    {"hex", "hex"},
> +    {"umh", "umh"},
> +    {"full", "esa"},
> +    {"tesa", "tesa"},
> +    {NULL},
> +};
>  
> -    x4->params.analyse.i_direct_mv_pred  = avctx->directpred;
> +struct avflags2x264 {
> +    const char *avname;
> +    const char *ifset;
> +    const char *ifnotset;
> +};
>  
> -    x4->params.analyse.b_weighted_bipred = avctx->flags2 & CODEC_FLAG2_WPRED;

> +static struct avflags2x264 flags1[] = {
> +    {"loop", "0", "1"},
> +    {"pass1", "1", "0"},
> +    {"pass2", "1", "0"},
> +    {"psnr", "1", "0"},
> +    {"ildct", "1", "0"},
> +    {"cgop", "0", "1"},
> +    {"global_header", "0", "1"},
> +    {NULL},
> +};

can't options be used for these constants?

>  
> -    if (avctx->me_method == ME_EPZS)
> -        x4->params.analyse.i_me_method = X264_ME_DIA;
> -    else if (avctx->me_method == ME_HEX)
> -        x4->params.analyse.i_me_method = X264_ME_HEX;
> -    else if (avctx->me_method == ME_UMH)
> -        x4->params.analyse.i_me_method = X264_ME_UMH;
> -    else if (avctx->me_method == ME_FULL)
> -        x4->params.analyse.i_me_method = X264_ME_ESA;
> -    else if (avctx->me_method == ME_TESA)
> -        x4->params.analyse.i_me_method = X264_ME_TESA;
> -    else x4->params.analyse.i_me_method = X264_ME_HEX;

> +static struct avflags2x264 flags2[] = {
> +    {"bpyramid", "2", "0"},
> +    {"wpred", "1", "0"},
> +    {"psy", "1", "0"},
> +    {"mixed_refs", "1", "0"},
> +    {"dct8x8", "1", "0"},
> +    {"fastpskip", "1", "0"},
> +    {"mbtree", "1", "0"},
> +    {"intra_refresh", "1", "0"},
> +    {"ssim", "1", "0"},
> +    {"aud", "1", "0"},
> +    {NULL},
> +};

same note

> -    x4->params.rc.i_aq_mode               = avctx->aq_mode;
> -    x4->params.rc.f_aq_strength           = avctx->aq_strength;
> -    x4->params.rc.i_lookahead             = avctx->rc_lookahead;

> +static int flagset(const char *flagname, char *flagstr, struct avflags2x264 flarr[], char **opt)
> +/* Returns:
> + * -1 => Error
> + *  0 => Not set
> + *  1 => Explicitely unset, *opt set
> + *  2 => Explicitely set, *opt set
> + */
> +{
> +    char sign, *p;
> +    int i;
>  
> -    x4->params.analyse.b_psy              = avctx->flags2 & CODEC_FLAG2_PSY;
> -    x4->params.analyse.f_psy_rd           = avctx->psy_rd;
> -    x4->params.analyse.f_psy_trellis      = avctx->psy_trellis;
> +    if (!flagstr)
> +        return 0;
> +    p = strstr(flagstr, flagname);
> +    if (!p)
> +        return 0;
> +    if (p>flagstr)
> +        sign = *(--p);
> +    else
> +        return -1;
> +    for (i=0; flarr[i].avname && strcmp(flagname, flarr[i].avname); i++);
> +    if (flarr[i].avname) {
> +        switch(sign) {
> +        case '+':   if (opt) *opt = flarr[i].ifset;
> +                    return 2;
> +        case '-':   if (opt) *opt = flarr[i].ifnotset;
> +                    return 1;
> +        default:    return -1;
> +        }
> +    } else
> +        return -1;
> +}
>  
> -    x4->params.analyse.i_me_range         = avctx->me_range;
> -    x4->params.analyse.i_subpel_refine    = avctx->me_subpel_quality;
> +static av_cold int X264_init(AVCodecContext *avctx)
> +{
> +    X264Context *x4 = avctx->priv_data;
> +    int ret;
>  
> -    x4->params.analyse.b_mixed_references = avctx->flags2 & CODEC_FLAG2_MIXED_REFS;
> -    x4->params.analyse.b_chroma_me        = avctx->me_cmp & FF_CMP_CHROMA;
> -    x4->params.analyse.b_transform_8x8    = avctx->flags2 & CODEC_FLAG2_8X8DCT;
> -    x4->params.analyse.b_fast_pskip       = avctx->flags2 & CODEC_FLAG2_FASTPSKIP;

> +    x4->sei_size = 0;

Is this required?

> +    x264_param_default(&x4->params);  
> -    x4->params.analyse.i_trellis          = avctx->trellis;
> -    x4->params.analyse.i_noise_reduction  = avctx->noise_reduction;
> +    /* Defaults from ffmpeg before OPT_x move */
> +    x4->params.b_open_gop = 1;
>  
> -    x4->params.rc.b_mb_tree               = !!(avctx->flags2 & CODEC_FLAG2_MBTREE);
> -    x4->params.rc.f_ip_factor             = 1 / fabs(avctx->i_quant_factor);
> -    x4->params.rc.f_pb_factor             = avctx->b_quant_factor;
> -    x4->params.analyse.i_chroma_qp_offset = avctx->chromaoffset;

> +    /* Should be cleaned out */
> +    avctx->has_b_frames          = avctx->flags2 & CODEC_FLAG2_BPYRAMID ? 2 : !!avctx->max_b_frames;

Please can you explain the meaning of the comment? Also shouldn't this
set in the framework (that is: what's libx264 specific here?).

>      if (!x4->preset)
>          check_default_settings(avctx);
> @@ -306,18 +449,115 @@ static av_cold int X264_init(AVCodecContext *avctx)
>              return -1;
>      }
>  
> +    if ((ret = flagset("pass1", x4->flags, flags1, NULL)) < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Error while parsing flags/pass1\n");
> +        return -1;
> +    }
> +    if ((ret == 2 || avctx->flags & CODEC_FLAG_PASS1) && !x4->pass)
> +        x4->pass = av_strdup("1");
> +    if ((ret = flagset("pass2", x4->flags, flags2, NULL)) < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Error while parsing flags/pass2\n");
> +        return -1;
> +    }
> +    if ((ret == 2 || avctx->flags & CODEC_FLAG_PASS2) && !x4->pass)
> +        x4->pass = av_strdup("2");
> +
>      x4->params.pf_log               = X264_log;
>      x4->params.p_log_private        = avctx;
>      x4->params.i_log_level          = X264_LOG_DEBUG;
>  
>      OPT_STR("weightp", x4->weightp);
> -
> -    x4->params.b_intra_refresh      = avctx->flags2 & CODEC_FLAG2_INTRA_REFRESH;
> +    OPT_STR("keyint", x4->keyint_max);
> +    OPT_STR("bframes", x4->bframes);
> +    OPT_STR("cabac", x4->cabac);
> +    OPT_STR("b-adapt", x4->badapt);
> +    OPT_STR("b-bias", x4->bbias);
> +    OPT_FLAG("b-pyramid", x4->b_pyramid, flags2, "bpyramid", x4->flags2);
> +    OPT_STR("keyint-min", x4->keyintmin);
> +    OPT_STR("scenecut", x4->scenecut);
> +    OPT_FLAG("nf", 0, flags1, "loop", x4->flags);
> +    OPT_DBLSTR("deblock", x4->deblockalpha, x4->deblockbeta, "You must set deblockalpha to set deblockbeta");
> +    OPT_STR("qpmin", x4->qpmin);
> +    OPT_STR("qpmax", x4->qpmax);
> +    OPT_STR("qpstep", x4->qpstep);
> +    OPT_STR("qcomp", x4->qcomp);    /* 0.0 => cbr, 1.0 => constant qp */
> +    OPT_STR("qblur", x4->qblur);    /* temporally blur quants */
> +    OPT_STR("cplxblur", x4->cplxblur);
> +    OPT_STR("ref", x4->ref);

> +    if (x4->partitions) {
> +        const char *p = x4->partitions;
> +        while (*p) {
> +            int i;
> +            char sign = *(p++);
> +            for (i=0; x264parts[i].name && !av_strstart(p, x264parts[i].name, &p); i++);
> +            if (!x264parts[i].name) {
> +                av_log(avctx, AV_LOG_ERROR, "Unable to parse partitions (unknown partition name)\n");
> +                return -1;
> +            }
> +            switch(sign) {
> +            case '+': x4->params.analyse.inter |= x264parts[i].part; break;
> +            case '-': x4->params.analyse.inter &= ~x264parts[i].part; break;
> +            default:  av_log(avctx, AV_LOG_ERROR, "Unable to parse partitions (sign not found)\n");
> +                      return -1;

You may print the string which causes the parsing failure.

> +            }
> +            if (*p == ',') p++;
> +        }
> +    }

But I'm not sure I understand this code, why avoptions flags setting
can't be used? 

> +    if (x4->directpred) {
> +        if (isdigit(x4->directpred[0])) {
> +            int i, v = atoi(x4->directpred);
> +            for (i=0 ; x264_direct_pred_names[i] && i<v ; i++);
> +            if (i==v) {
> +                OPT_STR("direct-pred", x264_direct_pred_names[i]);
> +            } else {
> +                av_log(avctx, AV_LOG_ERROR, "Wrong value for directpred\n");
> +                return -1;
> +            }
> +        } else
> +            OPT_STR("direct-pred", x4->directpred);
> +    }
> +    if (x4->me_method) {
> +        int i;
> +        for (i=0; x264mes[i].ffname && (strcmp(x4->me_method, x264mes[i].ffname) && strcmp(x4->me_method, x264mes[i].x264name)); i++);
> +        if (!x264mes[i].ffname) {
> +            av_log(avctx, AV_LOG_ERROR, "Unable to parse motion estimation method\n");
> +            return -1;
> +        }
> +        OPT_STR("me", x264mes[i].x264name);
> +    }
> +    OPT_FLAG("weightb", x4->wpred, flags2, "wpred", x4->flags2);
> +    OPT_STR("aq-mode", x4->aqmode);
> +    OPT_STR("aq-strength", x4->aqstrength);
> +    OPT_STR("rc-lookahead", x4->rc_lookahead);
> +    OPT_FLAG("psy", x4->psy, flags2, "psy", x4->flags2);
> +    OPT_DBLSTR("psy-rd", x4->psy_rd, x4->psy_trellis, "You must set psy_rd to set psy_trellis");
> +    OPT_STR("me-range", x4->me_range);
> +    OPT_STR("subq", x4->subq);
> +    OPT_FLAG("mixed-refs", x4->mixed_refs, flags2, "mixed_refs", x4->flags2);
> +    OPT_STR("chroma-me", x4->chroma_me);
> +    OPT_FLAG("8x8dct", x4->dct8x8, flags2, "dct8x8", x4->flags2);
> +    OPT_FLAG("fast-pskip", x4->fast_pskip, flags2, "fastpskip", x4->flags2);
> +    OPT_STR("trellis", x4->trellis);
> +    OPT_STR("nr", x4->nr);
> +    OPT_FLAG("mbtree", x4->mb_tree, flags2, "mbtree", x4->flags2);
> +    if (x4->ip_factor) {
> +        float f;
> +        char param[255];
> +        if (sscanf(x4->ip_factor, "%f", &f) != 1) {
> +            av_log(avctx, AV_LOG_ERROR, "Invalid i_qfactor\n");
> +            return -1;
> +        }
> +        snprintf(param, 254, "%f", 1/fabs(f));
> +        OPT_STR("ip-factor", param);
> +    }
> +    OPT_STR("pb-factor", x4->pb_factor);
> +    OPT_STR("chroma-qp-offset", x4->chromaoffset);
> +    OPT_FLAG("intra-refresh", x4->intra_refresh, flags2, "intra_refresh", x4->flags2);
>      x4->params.rc.i_bitrate         = avctx->bit_rate       / 1000;
>      x4->params.rc.i_vbv_buffer_size = avctx->rc_buffer_size / 1000;
>      x4->params.rc.i_vbv_max_bitrate = avctx->rc_max_rate    / 1000;
> -    x4->params.rc.b_stat_write      = avctx->flags & CODEC_FLAG_PASS1;
> -    if (avctx->flags & CODEC_FLAG_PASS2) {
> +    x4->params.rc.b_stat_write      = x4->pass && x4->pass[0]=='1';
> +    if (x4->pass && x4->pass[0]=='2') {
>          x4->params.rc.b_stat_read = 1;
>      } else {
>          if (avctx->crf) {
> @@ -370,23 +610,22 @@ static av_cold int X264_init(AVCodecContext *avctx)
>      x4->params.i_fps_num = x4->params.i_timebase_den = avctx->time_base.den;
>      x4->params.i_fps_den = x4->params.i_timebase_num = avctx->time_base.num;
>  
> -    x4->params.analyse.b_psnr = avctx->flags & CODEC_FLAG_PSNR;
> -    x4->params.analyse.b_ssim = avctx->flags2 & CODEC_FLAG2_SSIM;
> +    OPT_FLAG("psnr", x4->psnr, flags1, "psnr", x4->flags);
> +    OPT_FLAG("ssim", x4->ssim, flags2, "ssim", x4->flags2);
>  
> -    x4->params.b_aud          = avctx->flags2 & CODEC_FLAG2_AUD;
> +    OPT_FLAG("aud", x4->aud, flags2, "aud", x4->flags2);
>  
>      x4->params.i_threads      = avctx->thread_count;
>  
> -    x4->params.b_interlaced   = avctx->flags & CODEC_FLAG_INTERLACED_DCT;
> +    OPT_FLAG("interlaced", x4->interlaced, flags1, "ildct", x4->flags);
>  
> -    x4->params.b_open_gop     = !(avctx->flags & CODEC_FLAG_CLOSED_GOP);
> +    OPT_FLAG("open-gop", x4->opengop, flags1, "cgop", x4->flags);
>  
> -    x4->params.i_slice_count  = avctx->slices;
> +    OPT_STR("slices", x4->slices);
>  
>      x4->params.vui.b_fullrange = avctx->pix_fmt == PIX_FMT_YUVJ420P;
>  
> -    if (avctx->flags & CODEC_FLAG_GLOBAL_HEADER)
> -        x4->params.b_repeat_headers = 0;
> +    OPT_FLAG("repeat-headers", x4->repeatheaders, flags1, "global_header", x4->flags);
>  
>      // update AVCodecContext with x264 parameters
>      avctx->has_b_frames = x4->params.i_bframe ?
> @@ -400,7 +639,7 @@ static av_cold int X264_init(AVCodecContext *avctx)
>  
>      avctx->coded_frame = &x4->out_pic;
>  
> -    if (avctx->flags & CODEC_FLAG_GLOBAL_HEADER) {
> +    if (x4->params.b_repeat_headers) {
>          x264_nal_t *nal;
>          int nnal, s, i;
>  
> @@ -429,6 +668,56 @@ static const AVOption options[] = {
>      {"passlogfile", "Filename for 2 pass stats", OFFSET(stats), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
>      {"wpredp", "Weighted prediction for P-frames", OFFSET(weightp), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
>      {"x264opts", "x264 options", OFFSET(x264opts), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},

> +    {"g", "Set the group of picture (GOP) size (int)", OFFSET(keyint_max), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"bf", "use 'frames' B frame (int)", OFFSET(bframes), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"coder", "Use cabac (boolean)", OFFSET(cabac), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"b_strategy", "strategy to choose between I/P/B-frames (see x264 --fullhelp) (int 0..2)", OFFSET(badapt), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"bframebias", "influences how often B-frames are used (int)", OFFSET(bbias), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"keyint_min", "minimum interval between IDR-frames (int)", OFFSET(keyintmin), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"sc_threshold", "scene change threshold (int)", OFFSET(scenecut), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"deblockalpha", "in-loop deblocking filter alphac0 parameter (int)", OFFSET(deblockalpha), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"deblockbeta", "in-loop deblocking filter beta parameter (int)", OFFSET(deblockbeta), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"qmin", "min video quantizer scale (VBR) (int)", OFFSET(qpmin), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"qmax", "max video quantizer scale (VBR) (int)", OFFSET(qpmax), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"qdiff", "max difference between the quantizer scale (VBR) (int)", OFFSET(qpstep), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"qcomp", "video quantizer scale compression (VBR) (float)", OFFSET(qcomp), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"qblur", "video quantizer scale blur (VBR) (float)", OFFSET(qblur), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"complexityblur", "reduce fluctuations in qp (before curve compression) (float)", OFFSET(cplxblur), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"refs", "reference frames to consider for motion compensation (int)", OFFSET(ref), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"partitions", "macroblock subpartition sizes to consider, list of (+|-)(i4x4|i8x8|p8x8|p4x4|b8x8)", OFFSET(partitions), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"directpred", "direct mv prediction mode - (0|none)|(1|spatial)|(2|temporal)|(3|auto)", OFFSET(directpred), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"me_method", "set motion estimation method (epzs|dia)|hex|umh|(full|esa)|tesa", OFFSET(me_method), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"aq_mode", "specify aq method (int)", OFFSET(aqmode), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"aq_strength", "specify aq strength (float)", OFFSET(aqstrength), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"rc_lookahead", "specify number of frames to look ahead for frametype (int)", OFFSET(rc_lookahead), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"psy_rd", "specify psycho visual strength (float)", OFFSET(psy_rd), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"psy_trellis", "specify psycho visual trellis (float)", OFFSET(psy_trellis), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"me_range", "limit motion vectors range (int)", OFFSET(me_range), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"subq", "sub pel motion estimation quality (int)", OFFSET(subq), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"cmp", "full pel me compare function (bool)", OFFSET(chroma_me), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"trellis", "rate-distortion optimal quantization (int)", OFFSET(trellis), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"nr", "noise reduction (int)", OFFSET(nr), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"i_qfactor", "qp factor between P and I frames (float)", OFFSET(ip_factor), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"b_qfactor", "qp factor between p and b frames (float)", OFFSET(pb_factor), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"chromaoffset", "chroma qp offset from luma (int)", OFFSET(chromaoffset), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"pass", "use multipass mode (1..3)", OFFSET(pass), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"slices", "number of slices, used in parallelized decoding (int)", OFFSET(slices), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"flags", "", OFFSET(flags), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"flags2", "", OFFSET(flags2), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"psnr", "enable psnr computation (overrides -flags psnr) (bool)", OFFSET(psnr), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"b-pyramid", "allows B-frames to be used as references for predicting (0|none)|(1|strict)|(2|normal) (overrides -flags2 bpyramid)", OFFSET(b_pyramid), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"wpred", "weighted biprediction for b-frames (overrides -flags2 wpred) (bool)", OFFSET(wpred), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"psy", "use psycho visual optimization (overrides -flags2 psy) (bool)", OFFSET(psy), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"mixed_refs", "one reference per partition, as opposed to one reference per macroblock (overrides -flags2 mixed_refs) (bool)", OFFSET(mixed_refs), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"dct8x8", "high profile 8x8 transform (overrides -flags2 dct8x8) (bool)", OFFSET(dct8x8), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"fastpskip", "fast pskip (overrides -flags2 fastpskip) (bool)", OFFSET(fast_pskip), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"mbtree", "use macroblock tree ratecontrol (overrides -flags2 mbtree) (bool)", OFFSET(mb_tree), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"intra_refresh", "use periodic insertion of intra blocks instead of keyframes (overrides -flags2 intra_refresh) (bool)", OFFSET(intra_refresh), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"ssim", "ssim will be calculated during encoding (overrides -flags2 ssim) (bool)", OFFSET(ssim), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"aud", "access unit delimiters (overrides -flags2 aud) (bool)", OFFSET(aud), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"interlaced", "use interlaced dct (overrides -flags ildct) (bool)", OFFSET(interlaced), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"opengop", "open gop (overrides -flags cgop) (bool)", OFFSET(opengop), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
> +    {"repeatheaders", "place global headers in every keyframe (overrides -flags global_header) (bool)", OFFSET(repeatheaders), FF_OPT_TYPE_STRING, {.str=NULL}, 0, 0, VE},
>      { NULL },
>  };

Uhm so the idea is that options are set in the private context.

> diff --git a/libavcodec/options.c b/libavcodec/options.c
> index ccf1b87..e4f2cf5 100644
> --- a/libavcodec/options.c
> +++ b/libavcodec/options.c
> @@ -120,7 +120,6 @@ static const AVOption options[]={
>  {"b_qfactor", "qp factor between p and b frames", OFFSET(b_quant_factor), FF_OPT_TYPE_FLOAT, {.dbl = 1.25 }, -FLT_MAX, FLT_MAX, V|E},
>  {"rc_strategy", "ratecontrol method", OFFSET(rc_strategy), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, INT_MIN, INT_MAX, V|E},
>  {"b_strategy", "strategy to choose between I/P/B-frames", OFFSET(b_frame_strategy), FF_OPT_TYPE_INT, {.dbl = 0 }, INT_MIN, INT_MAX, V|E},
> -{"wpredp", "weighted prediction analysis method", OFFSET(weighted_p_pred), FF_OPT_TYPE_INT, {.dbl = 0 }, INT_MIN, INT_MAX, V|E},
>  {"ps", "rtp payload size in bytes", OFFSET(rtp_payload_size), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, INT_MIN, INT_MAX, V|E},
>  {"mv_bits", NULL, OFFSET(mv_bits), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, INT_MIN, INT_MAX},
>  {"header_bits", NULL, OFFSET(header_bits), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, INT_MIN, INT_MAX},
> @@ -353,22 +352,18 @@ static const AVOption options[]={
>  {"brd_scale", "downscales frames for dynamic B-frame decision", OFFSET(brd_scale), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, 0, 10, V|E},
>  {"crf", "enables constant quality mode, and selects the quality (x264/VP8)", OFFSET(crf), FF_OPT_TYPE_FLOAT, {.dbl = DEFAULT }, 0, 63, V|E},
>  {"cqp", "constant quantization parameter rate control method", OFFSET(cqp), FF_OPT_TYPE_INT, {.dbl = -1 }, INT_MIN, INT_MAX, V|E},
> -{"keyint_min", "minimum interval between IDR-frames (x264)", OFFSET(keyint_min), FF_OPT_TYPE_INT, {.dbl = 25 }, INT_MIN, INT_MAX, V|E},

> -{"refs", "reference frames to consider for motion compensation (Snow)", OFFSET(refs), FF_OPT_TYPE_INT, {.dbl = 1 }, INT_MIN, INT_MAX, V|E},
                                                                 ^^^^^^^
> +{"keyint_min", "minimum interval between IDR-frames", OFFSET(keyint_min), FF_OPT_TYPE_INT, {.dbl = 25 }, INT_MIN, INT_MAX, V|E},

> +{"refs", "reference frames to consider for motion compensation", OFFSET(refs), FF_OPT_TYPE_INT, {.dbl = 1 }, INT_MIN, INT_MAX, V|E},

Unrelated?

>  {"chromaoffset", "chroma qp offset from luma", OFFSET(chromaoffset), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, INT_MIN, INT_MAX, V|E},
>  {"bframebias", "influences how often B-frames are used", OFFSET(bframebias), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, INT_MIN, INT_MAX, V|E},
>  {"trellis", "rate-distortion optimal quantization", OFFSET(trellis), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, INT_MIN, INT_MAX, V|A|E},
> -{"directpred", "direct mv prediction mode - 0 (none), 1 (spatial), 2 (temporal), 3 (auto)", OFFSET(directpred), FF_OPT_TYPE_INT, {.dbl = 2 }, INT_MIN, INT_MAX, V|E},
>  {"bpyramid", "allows B-frames to be used as references for predicting", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_BPYRAMID }, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"wpred", "weighted biprediction for b-frames (H.264)", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_WPRED }, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"mixed_refs", "one reference per partition, as opposed to one reference per macroblock", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_MIXED_REFS }, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"dct8x8", "high profile 8x8 transform (H.264)", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_8X8DCT }, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"fastpskip", "fast pskip (H.264)", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_FASTPSKIP }, INT_MIN, INT_MAX, V|E, "flags2"},
> -{"aud", "access unit delimiters (H.264)", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_AUD }, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"skiprd", "RD optimal MB level residual skipping", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_SKIP_RD }, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"complexityblur", "reduce fluctuations in qp (before curve compression)", OFFSET(complexityblur), FF_OPT_TYPE_FLOAT, {.dbl = 20.0 }, FLT_MIN, FLT_MAX, V|E},
> -{"deblockalpha", "in-loop deblocking filter alphac0 parameter", OFFSET(deblockalpha), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, -6, 6, V|E},
> -{"deblockbeta", "in-loop deblocking filter beta parameter", OFFSET(deblockbeta), FF_OPT_TYPE_INT, {.dbl = DEFAULT }, -6, 6, V|E},
>  {"partitions", "macroblock subpartition sizes to consider", OFFSET(partitions), FF_OPT_TYPE_FLAGS, {.dbl = DEFAULT }, INT_MIN, INT_MAX, V|E, "partitions"},
>  {"parti4x4", NULL, 0, FF_OPT_TYPE_CONST, {.dbl = X264_PART_I4X4 }, INT_MIN, INT_MAX, V|E, "partitions"},
>  {"parti8x8", NULL, 0, FF_OPT_TYPE_CONST, {.dbl = X264_PART_I8X8 }, INT_MIN, INT_MAX, V|E, "partitions"},
> @@ -408,14 +403,7 @@ static const AVOption options[]={
>  {"colorspace", NULL, OFFSET(colorspace), FF_OPT_TYPE_INT, {.dbl = AVCOL_SPC_UNSPECIFIED }, 1, AVCOL_SPC_NB-1, V|E|D},
>  {"color_range", NULL, OFFSET(color_range), FF_OPT_TYPE_INT, {.dbl = AVCOL_RANGE_UNSPECIFIED }, 0, AVCOL_RANGE_NB-1, V|E|D},
>  {"chroma_sample_location", NULL, OFFSET(chroma_sample_location), FF_OPT_TYPE_INT, {.dbl = AVCHROMA_LOC_UNSPECIFIED }, 0, AVCHROMA_LOC_NB-1, V|E|D},
> -{"psy", "use psycho visual optimization", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_PSY }, INT_MIN, INT_MAX, V|E, "flags2"},
> -{"psy_rd", "specify psycho visual strength", OFFSET(psy_rd), FF_OPT_TYPE_FLOAT, {.dbl = 1.0 }, 0, FLT_MAX, V|E},
> -{"psy_trellis", "specify psycho visual trellis", OFFSET(psy_trellis), FF_OPT_TYPE_FLOAT, {.dbl = 0 }, 0, FLT_MAX, V|E},
> -{"aq_mode", "specify aq method", OFFSET(aq_mode), FF_OPT_TYPE_INT, {.dbl = 1 }, 0, INT_MAX, V|E},
> -{"aq_strength", "specify aq strength", OFFSET(aq_strength), FF_OPT_TYPE_FLOAT, {.dbl = 1.0 }, 0, FLT_MAX, V|E},
> -{"rc_lookahead", "specify number of frames to look ahead for frametype", OFFSET(rc_lookahead), FF_OPT_TYPE_INT, {.dbl = 40 }, 0, INT_MAX, V|E},
>  {"ssim", "ssim will be calculated during encoding", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_SSIM }, INT_MIN, INT_MAX, V|E, "flags2"},
> -{"intra_refresh", "use periodic insertion of intra blocks instead of keyframes", 0, FF_OPT_TYPE_CONST, {.dbl = CODEC_FLAG2_INTRA_REFRESH }, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"crf_max", "in crf mode, prevents vbv from lowering quality beyond this point", OFFSET(crf_max), FF_OPT_TYPE_FLOAT, {.dbl = DEFAULT }, 0, 51, V|E},
>  {"log_level_offset", "set the log level offset", OFFSET(log_level_offset), FF_OPT_TYPE_INT, {.dbl = 0 }, INT_MIN, INT_MAX },
>  #if FF_API_FLAC_GLOBAL_OPTS

The idea of removing the libx264 options from AVCodecContext is *good*
(TM), theoretically this involves an API break but then practically
should not be a problem.

So basically the idea is this:
* set libx264 defaults in x4->params
* if an option has been set in the private context (that is if the
  user specified them) set it in the x4->params context. This is
  needed if there are global FFmpeg options which are mapped to
  specific libx264 options.

For other libx264 options which don't have a corresponding in the
global context there is the possibility to either use x264params
(preferred?) or specify the option in the private context.

This way libx264 defaults override the FFmpeg defaults.

Is my understanding correct?
-- 
FFmpeg = Funny and Fancy Murdering Purposeless Evangelical Genius


More information about the ffmpeg-devel mailing list