| Message ID | 20191210200931.21517-3-martin@martin.st |
|---|---|
| State | Accepted |
| Commit | e10654de2b0db0f3b4828a3cc066d6fa21d02516 |
| Headers | show |
On Tue, 10 Dec 2019, Martin Storsjö wrote: > In these cases, we must pass the full path of the file to ffprobe > (as the current working dir on the remote system, e.g. when invoked > with "ssh remote ffprobe ..." isn't the wanted one). > > The input filename passed to ffprobe is also included in the output, > which is part of the reference test data. Add a new option to > ffprobe to allow overriding what path is printed, to keep the > original relative path in the tests. > > An alternative approach could be an option to allow requesting omitting > the file name from the dumped data, and updating the test references > accordingly. > --- > fftools/ffprobe.c | 22 ++++++++++++++++++---- > tests/fate/ffprobe.mak | 2 +- > 2 files changed, 19 insertions(+), 5 deletions(-) Ping // Martin
On Sun, 15 Dec 2019, Martin Storsjö wrote: > On Tue, 10 Dec 2019, Martin Storsjö wrote: > >> In these cases, we must pass the full path of the file to ffprobe >> (as the current working dir on the remote system, e.g. when invoked >> with "ssh remote ffprobe ..." isn't the wanted one). >> >> The input filename passed to ffprobe is also included in the output, >> which is part of the reference test data. Add a new option to >> ffprobe to allow overriding what path is printed, to keep the >> original relative path in the tests. >> >> An alternative approach could be an option to allow requesting omitting >> the file name from the dumped data, and updating the test references >> accordingly. >> --- >> fftools/ffprobe.c | 22 ++++++++++++++++++---- >> tests/fate/ffprobe.mak | 2 +- >> 2 files changed, 19 insertions(+), 5 deletions(-) > > Ping If there's no opposition to this one, I'll push it tomorrow; it's been out for review for over a week. // Martin
On 12/17/2019 6:27 PM, Martin Storsjö wrote: > On Sun, 15 Dec 2019, Martin Storsjö wrote: > >> On Tue, 10 Dec 2019, Martin Storsjö wrote: >> >>> In these cases, we must pass the full path of the file to ffprobe >>> (as the current working dir on the remote system, e.g. when invoked >>> with "ssh remote ffprobe ..." isn't the wanted one). >>> >>> The input filename passed to ffprobe is also included in the output, >>> which is part of the reference test data. Add a new option to >>> ffprobe to allow overriding what path is printed, to keep the >>> original relative path in the tests. >>> >>> An alternative approach could be an option to allow requesting omitting >>> the file name from the dumped data, and updating the test references >>> accordingly. >>> --- >>> fftools/ffprobe.c | 22 ++++++++++++++++++---- >>> tests/fate/ffprobe.mak | 2 +- >>> 2 files changed, 19 insertions(+), 5 deletions(-) >> >> Ping > > If there's no opposition to this one, I'll push it tomorrow; it's been out > for review for over a week. > > // Martin Not against it, but there have been a couple new tests since you first sent this, so better check they don't break, or require changes.
On Tue, 17 Dec 2019, James Almer wrote: > On 12/17/2019 6:27 PM, Martin Storsjö wrote: >> On Sun, 15 Dec 2019, Martin Storsjö wrote: >> >>> On Tue, 10 Dec 2019, Martin Storsjö wrote: >>> >>>> In these cases, we must pass the full path of the file to ffprobe >>>> (as the current working dir on the remote system, e.g. when invoked >>>> with "ssh remote ffprobe ..." isn't the wanted one). >>>> >>>> The input filename passed to ffprobe is also included in the output, >>>> which is part of the reference test data. Add a new option to >>>> ffprobe to allow overriding what path is printed, to keep the >>>> original relative path in the tests. >>>> >>>> An alternative approach could be an option to allow requesting omitting >>>> the file name from the dumped data, and updating the test references >>>> accordingly. >>>> --- >>>> fftools/ffprobe.c | 22 ++++++++++++++++++---- >>>> tests/fate/ffprobe.mak | 2 +- >>>> 2 files changed, 19 insertions(+), 5 deletions(-) >>> >>> Ping >> >> If there's no opposition to this one, I'll push it tomorrow; it's been out >> for review for over a week. >> >> // Martin > > Not against it, but there have been a couple new tests since you first > sent this, so better check they don't break, or require changes. This change still works fine, and none of the new tests seem to require changes, at least not with respect to the aspects of this (and the other remaining one). Thus pushed this one, and the checkasm one. // Martin
diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c index a95d74346d..b619c1f34e 100644 --- a/fftools/ffprobe.c +++ b/fftools/ffprobe.c @@ -254,6 +254,7 @@ static const OptionDef *options; /* FFprobe context */ static const char *input_filename; +static const char *print_input_filename; static AVInputFormat *iformat = NULL; static struct AVHashContext *hash; @@ -2836,7 +2837,8 @@ static void show_error(WriterContext *w, int err) writer_print_section_footer(w); } -static int open_input_file(InputFile *ifile, const char *filename) +static int open_input_file(InputFile *ifile, const char *filename, + const char *print_filename) { int err, i; AVFormatContext *fmt_ctx = NULL; @@ -2858,6 +2860,10 @@ static int open_input_file(InputFile *ifile, const char *filename) print_error(filename, err); return err; } + if (print_filename) { + av_freep(&fmt_ctx->url); + fmt_ctx->url = av_strdup(print_filename); + } ifile->fmt_ctx = fmt_ctx; if (scan_all_pmts_set) av_dict_set(&format_opts, "scan_all_pmts", NULL, AV_DICT_MATCH_CASE); @@ -2971,7 +2977,8 @@ static void close_input_file(InputFile *ifile) avformat_close_input(&ifile->fmt_ctx); } -static int probe_file(WriterContext *wctx, const char *filename) +static int probe_file(WriterContext *wctx, const char *filename, + const char *print_filename) { InputFile ifile = { 0 }; int ret, i; @@ -2980,7 +2987,7 @@ static int probe_file(WriterContext *wctx, const char *filename) do_read_frames = do_show_frames || do_count_frames; do_read_packets = do_show_packets || do_count_packets; - ret = open_input_file(&ifile, filename); + ret = open_input_file(&ifile, filename, print_filename); if (ret < 0) goto end; @@ -3286,6 +3293,12 @@ static int opt_input_file_i(void *optctx, const char *opt, const char *arg) return 0; } +static int opt_print_filename(void *optctx, const char *opt, const char *arg) +{ + print_input_filename = arg; + return 0; +} + void show_help_default(const char *opt, const char *arg) { av_log_set_callback(log_callback_help); @@ -3544,6 +3557,7 @@ static const OptionDef real_options[] = { { "read_intervals", HAS_ARG, {.func_arg = opt_read_intervals}, "set read intervals", "read_intervals" }, { "default", HAS_ARG | OPT_AUDIO | OPT_VIDEO | OPT_EXPERT, {.func_arg = opt_default}, "generic catch all option", "" }, { "i", HAS_ARG, {.func_arg = opt_input_file_i}, "read specified file", "input_file"}, + { "print_filename", HAS_ARG, {.func_arg = opt_print_filename}, "override the printed input filename", "print_file"}, { "find_stream_info", OPT_BOOL | OPT_INPUT | OPT_EXPERT, { &find_stream_info }, "read and decode the streams to fill missing information with heuristics" }, { NULL, }, @@ -3692,7 +3706,7 @@ int main(int argc, char **argv) av_log(NULL, AV_LOG_ERROR, "Use -h to get full help or, even better, run 'man %s'.\n", program_name); ret = AVERROR(EINVAL); } else if (input_filename) { - ret = probe_file(wctx, input_filename); + ret = probe_file(wctx, input_filename, print_input_filename); if (ret < 0 && do_show_error) show_error(wctx, ret); } diff --git a/tests/fate/ffprobe.mak b/tests/fate/ffprobe.mak index d5fb05cd68..c867bebf41 100644 --- a/tests/fate/ffprobe.mak +++ b/tests/fate/ffprobe.mak @@ -1,5 +1,5 @@ FFPROBE_TEST_FILE=tests/data/ffprobe-test.nut -FFPROBE_COMMAND=ffprobe$(PROGSSUF)$(EXESUF) -show_streams -show_packets -show_format -show_frames -bitexact $(FFPROBE_TEST_FILE) +FFPROBE_COMMAND=ffprobe$(PROGSSUF)$(EXESUF) -show_streams -show_packets -show_format -show_frames -bitexact $(TARGET_PATH)/$(FFPROBE_TEST_FILE) -print_filename $(FFPROBE_TEST_FILE) FATE_FFPROBE-$(CONFIG_AVDEVICE) += fate-ffprobe_compact fate-ffprobe_compact: $(FFPROBE_TEST_FILE)