search

quietvoid / dovi_tool

dovi_tool is a CLI tool combining multiple utilities for working with Dolby Vision.
MIT License
582 stars 57 forks source link

Converter default output #98

Closed nekno closed 2 years ago

nekno commented 2 years ago

This fixes #97 to dynamically set the filename output for the converter based on the presence/absence of the --discard option.

I'd personally prefer to see "RPU" explicitly included in the filenames to indicate where/when it's present in an HEVC stream, but I understand if your sense is that it should be obvious that it's carried with the EL when present or with the BL when not.

So I went with BL_RPU.hevc when --discard is used and BL_EL_RPU.hevc when not.

I could easily change it if your preference is for BL.hevc with --discard and BL_EL.hevc without.

Testing

quietvoid commented 2 years ago

Can you squash into one commit please? Otherwise it looks good.

nekno commented 2 years ago

Can you squash into one commit please? Otherwise it looks good.

No problem, I'm used to doing a squash commit on merging the PR so didn't think about it.

quietvoid commented 2 years ago

I just don't think squashing on merge keeps the commit/author info.

nekno commented 2 years ago

Squashed.

From prior experience, I believe using the "squash and merge" button rather than the "merge" button on the PR would maintain a contribution for me as "author" and you would gain a contribution as "committer".

When my org has a bot squash and merge PRs automatically, the bot is the committer but the authors of each individual commit are preserved as "authors" on the merge commit.

I believe the same result would happen here, with or without squashing. You're creating the merge commit for your repo, and I would get an author contribution for the one commit rather than multiple commits during the squash.

quietvoid commented 2 years ago

Ah that's good to know. Thanks!