Skip to content

tools: keep a drive-letter colon inside the --model path #1638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lusoris wants to merge 1 commit into
base: master
Choose a base branch
from
+175 −1
Open

tools: keep a drive-letter colon inside the --model path #1638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 153 additions & 0 deletions libvmaf/test/test_cli_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/

#include <getopt.h>
#include <stdio.h>
#include <string.h>

#include "test.h"
Expand Down Expand Up @@ -364,6 +365,154 @@ static char *test_default_model_is_sdr_for_bt709_source()
#endif


static void parse_model_option(CLISettings *settings, const char *spec)
{
char *argv[7] = {"vmaf", "-r", "ref.y4m", "-d", "dis.y4m", "--model",
(char *) spec};
optind = 1;
cli_parse(7, argv, settings);
}

static void free_model_option(CLISettings *settings)
{
for (unsigned i = 0; i < settings->model_cnt; i++) {
CLIModelConfig *m = &settings->model_config[i];
for (unsigned j = 0; j < m->overload_cnt; j++)
vmaf_feature_dictionary_free(&m->feature_overload[j].opts_dict);
}
cli_free(settings);
}

static char *test_model_path_drive_letter()
{
const char *const paths[] = {
"C:\\models\\vmaf_v0.6.1.json",
"C:/models/vmaf_v0.6.1.json",
"d:\\vmaf.json",
"Z:/a/b.json",
};
for (unsigned i = 0; i < sizeof(paths) / sizeof(paths[0]); i++) {
char spec[128];
snprintf(spec, sizeof(spec), "path=%s", paths[i]);
CLISettings settings;
parse_model_option(&settings, spec);
mu_assert("cli_parse: --model drive letter path: model count is not 1",
settings.model_cnt == 1);
mu_assert("cli_parse: --model drive letter path was split at the colon",
settings.model_config[0].path &&
!strcmp(settings.model_config[0].path, paths[i]));
mu_assert("cli_parse: --model drive letter path created an overload",
settings.model_config[0].overload_cnt == 0);
free_model_option(&settings);
}
return NULL;
}

static char *test_model_path_drive_letter_with_options()
{
CLISettings settings;
parse_model_option(&settings,
"path=C:\\models\\vmaf.json:name=mine:disable_clip");
mu_assert("cli_parse: --model path then options: model count is not 1",
settings.model_cnt == 1);
CLIModelConfig *m = &settings.model_config[0];
mu_assert("cli_parse: --model path then options: wrong path",
m->path && !strcmp(m->path, "C:\\models\\vmaf.json"));
mu_assert("cli_parse: --model path then options: wrong name",
!strcmp(m->cfg.name, "mine"));
mu_assert("cli_parse: --model path then options: disable_clip not set",
m->cfg.flags & VMAF_MODEL_FLAG_DISABLE_CLIP);
free_model_option(&settings);

parse_model_option(&settings,
"name=mine:path=C:/models/vmaf.json:enable_transform");
m = &settings.model_config[0];
mu_assert("cli_parse: --model options around path: wrong path",
m->path && !strcmp(m->path, "C:/models/vmaf.json"));
mu_assert("cli_parse: --model options around path: wrong name",
!strcmp(m->cfg.name, "mine"));
mu_assert("cli_parse: --model options around path: enable_transform not set",
m->cfg.flags & VMAF_MODEL_FLAG_ENABLE_TRANSFORM);
free_model_option(&settings);

parse_model_option(&settings,
"path=C:\\m.json:vif.vif_enhn_gain_limit=1.5");
m = &settings.model_config[0];
mu_assert("cli_parse: --model path then overload: wrong path",
m->path && !strcmp(m->path, "C:\\m.json"));
mu_assert("cli_parse: --model path then overload: overload count is not 1",
m->overload_cnt == 1);
mu_assert("cli_parse: --model path then overload: wrong feature name",
!strcmp(m->feature_overload[0].name, "vif"));
free_model_option(&settings);

return NULL;
}

/* A single letter before a colon is only a drive letter when a path separator
* follows the colon. Option keys never start with one, so a one-letter value
* followed by the next option still splits. */
static char *test_model_one_letter_value_still_splits()
{
CLISettings settings;
parse_model_option(&settings, "name=a:path=/tmp/vmaf.json");
mu_assert("cli_parse: --model one-letter name: model count is not 1",
settings.model_cnt == 1);
CLIModelConfig *m = &settings.model_config[0];
mu_assert("cli_parse: --model one-letter name was merged with the next option",
!strcmp(m->cfg.name, "a"));
mu_assert("cli_parse: --model path after one-letter name is wrong",
m->path && !strcmp(m->path, "/tmp/vmaf.json"));
free_model_option(&settings);

parse_model_option(&settings, "path=m:name=b:disable_clip");
m = &settings.model_config[0];
mu_assert("cli_parse: --model one-letter relative path was merged",
m->path && !strcmp(m->path, "m"));
mu_assert("cli_parse: --model name after one-letter path is wrong",
!strcmp(m->cfg.name, "b"));
mu_assert("cli_parse: --model disable_clip after one-letter path not set",
m->cfg.flags & VMAF_MODEL_FLAG_DISABLE_CLIP);
free_model_option(&settings);

return NULL;
}

static char *test_model_existing_strings_unchanged()
{
CLISettings settings;
parse_model_option(&settings,
"version=vmaf_v0.6.1:name=vmaf_x:disable_clip:enable_transform");
CLIModelConfig *m = &settings.model_config[0];
mu_assert("cli_parse: --model version: wrong version",
m->version && !strcmp(m->version, "vmaf_v0.6.1"));
mu_assert("cli_parse: --model version: wrong name",
!strcmp(m->cfg.name, "vmaf_x"));
mu_assert("cli_parse: --model version: flags not set",
(m->cfg.flags & VMAF_MODEL_FLAG_DISABLE_CLIP) &&
(m->cfg.flags & VMAF_MODEL_FLAG_ENABLE_TRANSFORM));
mu_assert("cli_parse: --model version: path should be unset", !m->path);
free_model_option(&settings);

parse_model_option(&settings,
"path=model/vmaf_v0.6.1.json:name=rel");
m = &settings.model_config[0];
mu_assert("cli_parse: --model relative path: wrong path",
m->path && !strcmp(m->path, "model/vmaf_v0.6.1.json"));
mu_assert("cli_parse: --model relative path: wrong name",
!strcmp(m->cfg.name, "rel"));
free_model_option(&settings);

parse_model_option(&settings, "path=/opt/vmaf/model.json");
m = &settings.model_config[0];
mu_assert("cli_parse: --model absolute path: wrong path",
m->path && !strcmp(m->path, "/opt/vmaf/model.json"));
free_model_option(&settings);

return NULL;
}


char *run_tests()
{
mu_run_test(test_aom_ctc_v1_0);
Expand All @@ -383,5 +532,9 @@ char *run_tests()
mu_run_test(test_default_model_is_sdr_without_color_metadata);
mu_run_test(test_default_model_is_sdr_for_bt709_source);
#endif
mu_run_test(test_model_path_drive_letter);
mu_run_test(test_model_path_drive_letter_with_options);
mu_run_test(test_model_one_letter_value_still_splits);
mu_run_test(test_model_existing_strings_unchanged);
return NULL;
}
23 changes: 22 additions & 1 deletion libvmaf/tools/cli_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <stdarg.h>
#include <stdlib.h>
#include <stdio.h>
#include <ctype.h>
#include <string.h>

#include "cli_parse.h"
Expand Down Expand Up @@ -338,6 +339,26 @@ static char *strsep(char **sp, char *sep)
}
#endif

// Like strsep(s, ":"), but a colon that follows a drive letter ("path=C:\\dir",
// "path=C:/dir") belongs to the value instead of ending the option.
static char *next_option(char **s)
{
char *start = *s;
if (!start) return NULL;
for (char *c = start; *c; c++) {
if (*c != ':') continue;
const int drive = c - start >= 2 && c[-2] == '=' &&
isalpha((unsigned char)c[-1]) &&
(c[1] == '\\' || c[1] == '/');
if (drive) continue;
*c = '\0';
*s = c + 1;
return start;
}
*s = NULL;
return start;
}

static CLIModelConfig parse_model_config(const char *const optarg,
const char *const app)
{
Expand All @@ -359,7 +380,7 @@ static CLIModelConfig parse_model_config(const char *const optarg,
};

char *key_val;
while ((key_val = strsep(&optarg_copy, ":")) != NULL) {
while ((key_val = next_option(&optarg_copy)) != NULL) {
char *key = strsep(&key_val, "=");
char *val = strsep(&key_val, "=");
if (!val) {
Expand Down