Kyle Swanson:
> Hi,
>
> On Sun, Jan 2, 2022 at 9:21 PM Andreas Rheinhardt
> <andreas.rheinha...@outlook.com> wrote:
>>>
>>> static const AVOption libvmaf_options[] = {
>>> - {"model_path", "Set the model to be used for computing vmaf.",
>>> OFFSET(model_path), AV_OPT_TYPE_STRING,
>>> {.str="/usr/local/share/model/vmaf_v0.6.1.pkl"}, 0, 1, FLAGS},
>>> - {"log_path", "Set the file path to be used to store logs.",
>>> OFFSET(log_path), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1,
>>> FLAGS},
>>> - {"log_fmt", "Set the format of the log (csv, json or xml).",
>>> OFFSET(log_fmt), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1, FLAGS},
>>> - {"enable_transform", "Enables transform for computing vmaf.",
>>> OFFSET(enable_transform), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1,
>>> FLAGS},
>>> - {"phone_model", "Invokes the phone model that will generate higher
>>> VMAF scores.", OFFSET(phone_model), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1,
>>> FLAGS},
>>> - {"psnr", "Enables computing psnr along with vmaf.",
>>> OFFSET(psnr), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
>>> - {"ssim", "Enables computing ssim along with vmaf.",
>>> OFFSET(ssim), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
>>> - {"ms_ssim", "Enables computing ms-ssim along with vmaf.",
>>> OFFSET(ms_ssim), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS},
>>> - {"pool", "Set the pool method to be used for computing vmaf.",
>>> OFFSET(pool), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1, FLAGS},
>>> + {"model", "Set the model to be used for computing vmaf.",
>>> OFFSET(model_cfg), AV_OPT_TYPE_STRING,
>>> {.str="version=vmaf_v0.6.1"}, 0, 1, FLAGS},
>>> + {"feature", "Set the feature to be used for computing vmaf.",
>>> OFFSET(feature_cfg), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1,
>>> FLAGS},
>>> + {"log_path", "Set the file path to be used to write log.",
>>> OFFSET(log_path), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 1,
>>> FLAGS},
>>> + {"log_fmt", "Set the format of the log (csv, json, xml, or sub).",
>>> OFFSET(log_fmt), AV_OPT_TYPE_STRING, {.str="xml"}, 0, 1,
>>> FLAGS},
>>> {"n_threads", "Set number of threads to be used when computing vmaf.",
>>> OFFSET(n_threads), AV_OPT_TYPE_INT, {.i64=0}, 0, UINT_MAX,
>>> FLAGS},
>>> {"n_subsample", "Set interval for frame subsampling used when
>>> computing vmaf.", OFFSET(n_subsample), AV_OPT_TYPE_INT, {.i64=1}, 1,
>>> UINT_MAX, FLAGS},
>>> - {"enable_conf_interval", "Enables confidence interval.",
>>> OFFSET(enable_conf_interval), AV_OPT_TYPE_BOOL, {.i64=0}, 0,
>>> 1, FLAGS},
>>> { NULL }
>>> };
>>
>> You are removing lots of options; removing options is only permissible
>> during a major break and even then the options need to have been
>> deprecated before that.
>
> Good point, thanks. New patch attached. No more missing options,
> everything that should be deprecated has been marked as deprecated and
> appropriate fallback behavior implemented.
>
> +static AVDictionary **delimited_dict_parse(char *str, unsigned *cnt)
> {
> - LIBVMAFContext *s = (LIBVMAFContext *) ctx;
> - compute_vmaf_score(s);
> - if (!s->error) {
> - av_log(ctx, AV_LOG_INFO, "VMAF score: %f\n",s->vmaf_score);
> - } else {
> - pthread_mutex_lock(&s->lock);
> - pthread_cond_signal(&s->cond);
> - pthread_mutex_unlock(&s->lock);
> + int err = 0;
> + if (!str)
> + return NULL;
> +
> + *cnt = 1;
> + for (char *p = str; *p; p++) {
> + if (*p == '|')
> + (*cnt)++;
> }
> - pthread_exit(NULL);
> +
> + AVDictionary **dict = av_calloc(*cnt, sizeof(*dict));
> + if (!dict)
> + goto fail;
> +
> + char *str_copy = av_strdup(str);
> + if (!str_copy)
> + goto fail;
> +
> + char *saveptr = NULL;
> + for (unsigned i = 0; i < *cnt; i++) {
> + char *s = av_strtok(i == 0 ? str_copy : NULL, "|", &saveptr);
> + err = av_dict_parse_string(&dict[i], s, "=", ":", 0);
> + if (err)
> + goto fail;
> + }
> +
> + av_free(str_copy);
> + return dict;
> +
> +fail:
> + if (dict) {
> + for (unsigned i = 0; i < *cnt; i++) {
> + if (dict[i])
> + av_dict_free(&dict[i]);
> + }
> + av_free(dict);
> + }
> +
> + av_free(str_copy);
> + *cnt = 0;
> return NULL;
> }
1. FFmpeg uses the ancient C90 rule that only allows variable
declarations at the beginning of a block (before all statements). You
should actually have received a compiler warning because of the above
code (unless you use Clang, which claims to support this type of
warning, yet doesn't).
2. You jump over the initializion of str_copy if the dict allocation
fails and consequently you free an uninitialized string.
- 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-requ...@ffmpeg.org with subject "unsubscribe".
