Skip to content

Conversation

jsyrjala
Copy link

By ignoring warning messages about unhandled key out_time_us
are not displayed.

Fixes #191

By ignoring warning messages about unhandled key `out_time_us`
are not displayed.

Fixes bramp#191
@coveralls
Copy link

Coverage Status

Coverage increased (+2.8%) to 84.515% when pulling 3310441 on jsyrjala:progress-out_time_us into 3819b15 on bramp:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+2.8%) to 84.515% when pulling 3310441 on jsyrjala:progress-out_time_us into 3819b15 on bramp:master.

return false;

case "out_time_us":
return false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll be great if we could parse this time, and store it in out_time_ns.

It'll also be great if you could fix the out_time_ms as well :)

Copy link
Author

@jsyrjala jsyrjala Dec 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Progress output looks like this:

frame=31677 fps=371 q=29.0 size=    8192kB time=00:01:45.81 bitrate= 634.2kbits/s speed=12.4x    
fps=370.91
stream_0_0_q=29.0
bitrate= 634.2kbits/s
total_size=8388656
out_time_us=105813333
out_time_ms=105813333
out_time=00:01:45.813333
dup_frames=0
drop_frames=0
speed=12.4x
progress=continue

out_time_us has integer value in microseconds, out_time_ms has the same value which is not in milliseconds like the name would hint due a bug in ffmpeg https://trac.ffmpeg.org/ticket/7345.

If I parse out_time_us and out_time_ms and add them to this.out_time_ns, the value gets overwritten by value parsed from out_time.

I could do following, but I think that it would be better to parse this.out_time_ns just from one source like this PR currently does.

      case "out_time_ms":
        this.out_time_ns = Long.parseLong(value) * 1000;
        return false;

      case "out_time_us":
        this.out_time_ns = Long.parseLong(value) * 1000;
        return false;

      case "out_time":
        this.out_time_ns = fromTimecode(value);
        return false;

For reference: ffmpeg patch where key out_time_us was added: https://patchwork.ffmpeg.org/patch/9894/ . They are also discussing about removing out_time_ms key at some point in the future.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another options would be to add a ignore/skip list for specific key and ignore key out_time_ms and out_time_us there.

  protected boolean parseLine(String line) {
    line = checkNotNull(line).trim();
    if (line.isEmpty()) {
      return false; // Skip empty lines
    }

    final String[] args = line.split("=", 2);
    if (args.length != 2) {
      // invalid argument, so skip
      return false;
    }

    final String key = checkNotNull(args[0]);
    final String value = checkNotNull(args[1]);

    List<String> ignoreList = Arrays.asList(
          "out_time_ms", // duplicate with out_time
          "out_time_us" // duplicate with out_time
     );

    if (ignoreList.contains(key)) {
        return false;
    }

    switch (key) {
      case "frame":
        frame = Long.parseLong(value);
        return false;

      case "fps":
        fps = Fraction.getFraction(value);
        return false;
...

Yet another option would be to remove the logging line.
LOG.warn("skipping unhandled key: {} = {}", key, value);

@bramp
Copy link
Owner

bramp commented Oct 26, 2023

OMG Sorry it's taken me 5 years to reply! The mix of out_time_ms, out_time_us and out_time seems messy, especially when they are mis labelled.

Someone else has fixed this in #289, and we've decided to just ignore all but out_time.

Thanks!

@bramp bramp closed this Oct 26, 2023
@jsyrjala jsyrjala deleted the progress-out_time_us branch November 12, 2023 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Progress doesn't support out_time_us
3 participants