BrettSheleski / comchap

Commercial detection script to add chapters into video file
MIT License
136 stars 26 forks source link

Update comchap #10

Closed Reed97123 closed 7 years ago

Reed97123 commented 7 years ago

Changes outlined in the Issue #9.

BrettSheleski commented 7 years ago

I've accepted your merge request as-is for now.

I was going to leave it alone even though it now considers a commercial-less input file as error. But after doing some testing I really don't like that.

Again, I'm open to a command line argument to behave this way. Perhaps a --error-on-no-commercials?

Reed97123 commented 7 years ago

Help me understand how you use it. When do you run into cases where you have no commercials?

BrettSheleski commented 7 years ago

My use is mainly as a Plex post-processing script. I do so in another script which also emails me that the recording is done.

In general, it is unknown if a recording has commercials or not. Regardless, it is being ran through comchap or comcut. If there is no commercials, like for the presidential debates for example, it should not fail.

It is unknown what returning a failure status does to Plex (or within some unknown script calling it for that matter), if anything, but should still be avoided as there really was no error.

Also, the message saying comskip failed is misleading. Comskip ran fine and did exactly what it is meant to do. In my testing this confused me as it made me think it is no longer calling comskip. When that was not the case at all.

I liken this to calling make on source that is already built. If there is nothing to do, then there's nothing to do, that's not an error. Same with doing a git pull. If everything is already up to date, that too is not an error. Or starting a systemd service that is already started. Etc.

Reed97123 commented 7 years ago

We can call it an informational message (and still have a return status 0). As long as something gets printed out (even if it's not verbose), then I'm happy. Anything output to stdout/stderr gets e-mailed to me by default.

Does that work with what you are doing?

BrettSheleski commented 7 years ago

Sounds good to me. Probably write it to stderr I'm thinking.