add phasher

[aghoulcoder]

A simple phasher program that accepts a video files as a command line arguments and calculates and prints their PHASHes.

$ phasher /tmp/test.mp4
d39239890743d9f6 /tmp/test.mp4

The goal of this separate executable is to have a simple way to calculate phashes that doesn't depend on a full stash instance so that third-party systems and tools can independently generate PHASHes which can be used for interacting with stash and stash-box APIs and data.

Currently phasher is built in the default make target along with stash by simply running make.
Build with make phasher.
Cross-platform targets have not been considered.

Concurrency is intentionally not implemented because it is simpler to use GNU Parallel. For example:

parallel phasher {} ::: *.mp4

P.S. This is the most go I've ever written and my first attempt at reading stash code, so a review and feedback would be appreciated.

[aghoulcoder]

I've been made aware that this tool by @peolic also exists: https://github.com/peolic/videohashes

But I argue that this should be included in stash because stash's PHASH is a bespoke and authoritative implementation of the algorithm for video files. Therefore, anyone who has used stash and is looking for a simple to use implementation of PHASH will be looking here first, as did I.

[kermieisinthehouse]

Can we make it a separate make target?

Maybe make build-phasher

[aghoulcoder]

For sure, it should have it's own make target. I just wanted to push early for review. I'll work on it...

[DingDongSoLong4]

This is definitely very useful, and I agree that having this be part of stash rather than separate is better for discovery and easier to keep up to date.

I just would suggest moving phasher.go to cmd/phasher/main.go, to keep in line with the recommended go project layout (https://github.com/golang-standards/project-layout).

If we're going to release phasher binaries then cross-compile targets would be needed, otherwise I don't think they're necessary.

For argument handling, I think spf13/cobra would be overkill. You could use spf13/pflag (which stash already uses) to parse a help flag for a simple usage info printout. Check out the Usage variable in spf13/pflag, which lets you override the usage message so you can display normal arguments in addition to flags.

I think it would also be great to add support for multiple command-line file arguments (concurrency not required), stdin input and JSON output, but that can definitely come in a later PR.

[aghoulcoder]

Thanks for the pointers @DingDongSoLong4

I have:

• Separated phasher into its own make target. But it's disconnected from the reset of the Makefile which is melting my brain a bit.
• Added usage output and a --quiet option that only prints the phash.

Here are some things that have been mentioned, which I recommend not doing:

• Don't handle multiple file inputs. Handling multiple files is inefficient if they are just going to be processed sequentially, compared to using gnu parallel.
• Don't impelment concurrency. If you run phasher as part of a pipeline that includes other tools with concurrency, you will need some way to balance cpu cores between the tools. Instead, you can run the entire pipeline concurrently with gnu parallel.
• JSON output should not be implemented. The current output format is intentional and in-line with gnu coreutils hashing programs (i.e. md5sum, sha1sum, etc..): It prints the hash, followed by a space, and then the filename. With -q it prints the hash only. It does not get any simpler than this.

I'm still trying to untangle this Makefile in my mind to decide how best to restructure it so all the build options and flags are re-used in an efficient and easy to read way for both stash and phasher.

[DingDongSoLong4]

Here are some things that have been mentioned, which I recommend not doing:

* Don't handle multiple file inputs. Handling multiple files is inefficient if they are just going to be processed sequentially, compared to using gnu parallel.

* Don't impelment concurrency. If you run phasher as part of a pipeline that includes other tools with concurrency, you will need some way to balance cpu cores between the tools. Instead, you can run the entire pipeline concurrently with gnu parallel.

* JSON output should not be implemented. The current output format is intentional and in-line with gnu coreutils hashing programs (i.e. `md5sum`, `sha1sum`, etc..): It prints the hash, followed by a space, and then the filename. With `-q` it prints the hash only. It does not get any simpler than this.

Agreed, with the exception of multiple file inputs. I was looking at peolic's implementation and saw it had JSON output so I mentioned it, but that outputs more than just the phash so a JSON output makes sense. This implementation does not need it, the output is simple enough.

Regarding multiple inputs, I do understand your reasoning, but it shouldn't be at all complicated to get working so I don't really see a downside. I'm assuming you use Linux, and I do as well, so I'm totally on board with GNU parallel, but many (most) use Windows so would not have easy access to parallel (plus I myself haven't got around to learning it yet, as I think might be the case for many).

You mention md5sum and sha1sum, which do support hashing multiple file arguments sequentially, so if we're trying to emulate them then phasher should support this as well. Also to be a bit pedantic those utilities actually output two spaces (well most of the time, see the manpage) between the hash and filename, so if we're going for compatibility then that is a minor difference. It doesn't really affect "parseability" though, so your call.

Concurrency on the other hand would definitely add unnecessary complexity, so I agree that it shouldn't be added.

I'm still trying to untangle this Makefile in my mind to decide how best to restructure it so all the build options and flags are re-used in an efficient and easy to read way for both stash and phasher.

This has prompted me to make a few improvements to the Makefile in #3876, in order for the following to work as the phasher target:

.PHONY: build-phasher
build-phasher:
	go build -v $(GO_BUILD_FLAGS) -ldflags "$(LDFLAGS)" ./cmd/phasher

phasher doesn't actually need any of the build flags or variables set by the build-flags and pre-build targets, so it doesn't need them as prerequisites.

For consistency with the rest of the file I've changed the target name to build-phasher rather than phasher. We're basically just using the Makefile as a glorified sort-of-cross-plaform shell script so having the name match the binary isn't really necessary.

You can then just add build-phasher as another prerequisite of build-release and then build-release will build release versions of both phasher and stash itself. The cross-compilation targets should then also work, though I haven't tested that.

[aghoulcoder]

Very quickly put together handling of multiple arguments. Doesn't do anything with stdin.

[aghoulcoder]

@DingDongSoLong4 Thanks for helping out with that Makefile. Hopefully it can be merged soon.

Let me know if there's something more you think could be improved in phasher itself at this point.

Regarding make target names, while I understand that we're quite far from the classic C programs that make was designed for, it still makes sense to name the build target exactly like the resulting executable.

1. The commands make phasher and make stash make sense intuitively and are shorter to type than make build-phasher.

2. With properly named targets you can use the $@ automatic variable to reference the name of the make target, reducing repetition.
   For example, this:

   build-phasher:
       go build -o phasher -trimpath -buildmode=pie -ldflags '-s -w' ./cmd/phasher

   can be written more concisely like this:

   phasher:
       go build -o $@ -trimpath -buildmode=pie -ldflags '-s -w' ./cmd/$@

   And all this:

   .PHONY: cross-compile-linux-arm32v6
   cross-compile-linux-arm32v6: export GOOS := linux
   cross-compile-linux-arm32v6: export GOARCH := arm
   cross-compile-linux-arm32v6: export GOARM := 6
   cross-compile-linux-arm32v6: export CC := arm-linux-gnueabi-gcc
   cross-compile-linux-arm32v6: OUTPUT := -o dist/stash-linux-arm32v6
   cross-compile-linux-arm32v6: build-release-static

   can be written like this:

   .PHONY: linux-arm32v6
   linux-arm32v6: export GOOS := linux
   linux-arm32v6: export GOARCH := arm
   linux-arm32v6: export GOARM := 6
   linux-arm32v6: export CC := arm-linux-gnueabi-gcc
   linux-arm32v6: OUTPUT := -o dist/stash-$@
   linux-arm32v6: build-release-static

[aghoulcoder]

I just realized I did something dumb.
Because I very hastily put together the code for iterating over multiple command line arguments, I accidentally made it so that FFMPEG and FFPROBE are initialized on every loop, which is pointless and inefficient.
Working on it...

[aghoulcoder]

@WithoutPants just fyi, I don't intend on changing anything else on this PR for now. It's ready for review whenever you want.
Original description in the PR is updated and valid.