On Wed, 20 Apr 2011 15:53:48 -0700, Alex Converse <alex.conve...@gmail.com> 
wrote:
> diff --git a/cmdutils.c b/cmdutils.c
> index f1cbd55..4aef03d 100644
> --- a/cmdutils.c
> +++ b/cmdutils.c
> @@ -361,8 +361,9 @@ void set_context_opts(void *ctx, void *opts_ctx, int 
> flags, AVCodec *codec)
>          /* We need to use a differnt system to pass options to the private 
> context because
>             it is not known which codec and thus context kind that will be 
> when parsing options
>             we thus use opt_values directly instead of opts_ctx */
> -        if(!str && priv_ctx && av_get_string(priv_ctx, opt_names[i], &opt, 
> buf, sizeof(buf))){
> -            av_set_string3(priv_ctx, opt_names[i], opt_values[i], 1, NULL);
> +        if(!str && priv_ctx) {
> +            if (av_find_opt(priv_ctx, opt_names[i], NULL, flags, flags))
> +                av_set_string3(priv_ctx, opt_names[i], opt_values[i], 0, 
> NULL);
>          }
>      }

This part looks unrelated.

>  }
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 5825945..df4c577 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -19,6 +19,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
>   */
>  
> +#include "libavutil/opt.h"
>  #include "avcodec.h"
>  #include <x264.h>
>  #include <math.h>
> @@ -27,12 +28,17 @@
>  #include <string.h>
>  
>  typedef struct X264Context {
> +    AVClass        *class;
>      x264_param_t    params;
>      x264_t         *enc;
>      x264_picture_t  pic;
>      uint8_t        *sei;
>      int             sei_size;
>      AVFrame         out_pic;
> +    const char *preset;
> +    const char *tune;
> +    const char *profile;
> +    int fastfirstpass;

The strings are leaked.

>  } X264Context;
>  
>  static void X264_log(void *p, int level, const char *fmt, va_list args)
> @@ -163,32 +169,7 @@ static av_cold int X264_init(AVCodecContext *avctx)
>      x4->sei_size = 0;
>      x264_param_default(&x4->params);
>  
> -    x4->params.pf_log               = X264_log;
> -    x4->params.p_log_private        = avctx;
> -
>      x4->params.i_keyint_max         = avctx->gop_size;
> -    x4->params.b_intra_refresh      = avctx->flags2 & 
> CODEC_FLAG2_INTRA_REFRESH;
> -    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_read = 1;
> -    } else {
> -        if (avctx->crf) {
> -            x4->params.rc.i_rc_method   = X264_RC_CRF;
> -            x4->params.rc.f_rf_constant = avctx->crf;
> -            x4->params.rc.f_rf_constant_max = avctx->crf_max;
> -        } else if (avctx->cqp > -1) {
> -            x4->params.rc.i_rc_method   = X264_RC_CQP;
> -            x4->params.rc.i_qp_constant = avctx->cqp;
> -        }
> -    }
> -
> -    // if neither crf nor cqp modes are selected we have to enable the RC
> -    // we do it this way because we cannot check if the bitrate has been set
> -    if (!(avctx->crf || (avctx->cqp > -1)))
> -        x4->params.rc.i_rc_method = X264_RC_ABR;
>  
>      x4->params.i_bframe          = avctx->max_b_frames;
>      x4->params.b_cabac           = avctx->coder_type == FF_CODER_TYPE_AC;
> @@ -217,13 +198,6 @@ static av_cold int X264_init(AVCodecContext *avctx)
>  
>      x4->params.i_frame_reference    = avctx->refs;
>  
> -    x4->params.i_width              = avctx->width;
> -    x4->params.i_height             = avctx->height;
> -    x4->params.vui.i_sar_width      = avctx->sample_aspect_ratio.num;
> -    x4->params.vui.i_sar_height     = avctx->sample_aspect_ratio.den;
> -    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.inter    = 0;
>      if (avctx->partitions) {
>          if (avctx->partitions & X264_PART_I4X4)
> @@ -277,20 +251,65 @@ static av_cold int X264_init(AVCodecContext *avctx)
>      if (avctx->level > 0)
>          x4->params.i_level_idc = avctx->level;
>  
> +    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;
> +

So those parameters can be overwritten by preset/profile? Do we really
want that?

> +    if (x4->preset || x4->tune) {
> +        if (x264_param_default_preset(&x4->params, x4->preset, x4->tune) < 0)
> +            return -1;
> +    }
> +
> +    if (x4->fastfirstpass)
> +        x264_param_apply_fastfirstpass(&x4->params);
> +
> +    if (x4->profile)
> +        if (x264_param_apply_profile(&x4->params, x4->profile) < 0)
> +            return -1;
> +
> +    x4->params.pf_log               = X264_log;
> +    x4->params.p_log_private        = avctx;
> +    x4->params.i_log_level          = X264_LOG_DEBUG;
> +
> +    x4->params.b_intra_refresh      = avctx->flags2 & 
> CODEC_FLAG2_INTRA_REFRESH;
> +    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_read = 1;
> +    } else {
> +        if (avctx->crf) {
> +            x4->params.rc.i_rc_method   = X264_RC_CRF;
> +            x4->params.rc.f_rf_constant = avctx->crf;
> +            x4->params.rc.f_rf_constant_max = avctx->crf_max;
> +        } else if (avctx->cqp > -1) {
> +            x4->params.rc.i_rc_method   = X264_RC_CQP;
> +            x4->params.rc.i_qp_constant = avctx->cqp;
> +        }
> +    }
> +
> +    // if neither crf nor cqp modes are selected we have to enable the RC
> +    // we do it this way because we cannot check if the bitrate has been set
> +    if (!(avctx->crf || (avctx->cqp > -1)))
> +        x4->params.rc.i_rc_method = X264_RC_ABR;
> +
>      if (avctx->rc_buffer_size && avctx->rc_initial_buffer_occupancy &&
>          (avctx->rc_initial_buffer_occupancy <= avctx->rc_buffer_size)) {
>          x4->params.rc.f_vbv_buffer_init =
>              (float)avctx->rc_initial_buffer_occupancy / 
> avctx->rc_buffer_size;
>      }
>  
> -    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;
> +    x4->params.i_width          = avctx->width;
> +    x4->params.i_height         = avctx->height;
> +    x4->params.vui.i_sar_width  = avctx->sample_aspect_ratio.num;
> +    x4->params.vui.i_sar_height = avctx->sample_aspect_ratio.den;
> +    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;
> -    x4->params.i_log_level    = X264_LOG_DEBUG;
>  
>      x4->params.b_aud          = avctx->flags2 & CODEC_FLAG2_AUD;
>  
> @@ -305,6 +324,11 @@ static av_cold int X264_init(AVCodecContext *avctx)
>      if (avctx->flags & CODEC_FLAG_GLOBAL_HEADER)
>          x4->params.b_repeat_headers = 0;
>  
> +    // update AVCodecContext with x264 parameters
> +    avctx->has_b_frames = x4->params.i_bframe_pyramid ? 2 : 
> !!x4->params.i_bframe;
> +    avctx->bit_rate = x4->params.rc.i_bitrate*1000;
> +    avctx->crf = x4->params.rc.f_rf_constant;
> +

I'm wondering what's all this random shuffling for. It also seems to
disagree with recommendation in x264.h:
  In order to replicate x264CLI's option handling, these functions MUST
  be called in the following order:
  1) x264_param_default_preset
  2) Custom user options (via param_parse or directly assigned
     variables)
  3) x264_param_apply_fastfirstpass
  4) x264_param_apply_profile

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

Reply via email to