Plugin: CommercialDetector

[Fuzzy]

Originally Posted by tmiranda

I setup my dev system and loaded Phoenix 3. I'm no longer convinced the error is due to the SageTV7 dependency in the manifest. I can see that the plugin is loaded and the start() method runs as expected.

Any particular reason you haven't updated the code onto github? (I know you used to have it on google code), and I think you imported 0.7 onto github a while ago. Might help to get some other eyes on it.

[tmiranda]

Originally Posted by Fuzzy

Any particular reason you haven't updated the code onto github? (I know you used to have it on google code), and I think you imported 0.7 onto github a while ago. Might help to get some other eyes on it.

Just lazy. I'll get it on GitHub soon.

[Fuzzy]

Hey, so I found an issue with CD. I noticed that comskip hasn't been running lately, so went to debug mode on it this morning. It is failing out with exit code 2 (this is in the unraid docker). Went to trace mode, and started a recording, and it is passing everything mostly correct, but here's the problem.

I had turned on sage's newer expanded filename option, which now retains spaces in the filename. CD is not wrapping the filename in quotes when passing to comskip, so I get the following comskip command from cd:

/opt/sagetv/comskip/comskip --playnice --ini=/opt/sagetv/comskip/comskip.ini /var/media/tv/High School Basketball - Spalding Hoophall Classic La Lumiere (Ind) vs Sierra Canyon (Calif) - 33104469-0.ts

So it either needs to wrap it in quotes, or escape the spaces (but I think escaping spaces only works in windows, so the quotes is probably the more universal fix).

[tmiranda]

Originally Posted by Fuzzy

Hey, so I found an issue with CD. I noticed that comskip hasn't been running lately, so went to debug mode on it this morning. It is failing out with exit code 2 (this is in the unraid docker). Went to trace mode, and started a recording, and it is passing everything mostly correct, but here's the problem.

I had turned on sage's newer expanded filename option, which now retains spaces in the filename. CD is not wrapping the filename in quotes when passing to comskip, so I get the following comskip command from cd:

/opt/sagetv/comskip/comskip --playnice --ini=/opt/sagetv/comskip/comskip.ini /var/media/tv/High School Basketball - Spalding Hoophall Classic La Lumiere (Ind) vs Sierra Canyon (Calif) - 33104469-0.ts

So it either needs to wrap it in quotes, or escape the spaces (but I think escaping spaces only works in windows, so the quotes is probably the more universal fix).

Looks like I need to get an update out. Thanks for finding this new bug.

[tmiranda]

Originally Posted by Fuzzy

Hey, so I found an issue with CD. I noticed that comskip hasn't been running lately, so went to debug mode on it this morning. It is failing out with exit code 2 (this is in the unraid docker). Went to trace mode, and started a recording, and it is passing everything mostly correct, but here's the problem.

I had turned on sage's newer expanded filename option, which now retains spaces in the filename. CD is not wrapping the filename in quotes when passing to comskip, so I get the following comskip command from cd:

/opt/sagetv/comskip/comskip --playnice --ini=/opt/sagetv/comskip/comskip.ini /var/media/tv/High School Basketball - Spalding Hoophall Classic La Lumiere (Ind) vs Sierra Canyon (Calif) - 33104469-0.ts

So it either needs to wrap it in quotes, or escape the spaces (but I think escaping spaces only works in windows, so the quotes is probably the more universal fix).

comskip doesn't like the name in quotes. I'll have to sort something else out.

[Taddeusz]

Originally Posted by tmiranda

comskip doesn't like the name in quotes. I'll have to sort something else out.

The fix you gave me for this works just fine.

[tmiranda]

Originally Posted by Taddeusz

The fix you gave me for this works just fine.

I sent you the version that is used in the docker image which is what I believe Fuzzy is using. Are you using the expanded files names that contain spaces?

[Taddeusz]

Originally Posted by tmiranda

I sent you the version that is used in the docker image which is what I believe Fuzzy is using. Are you using the expanded files names that contain spaces?

Yes

[stuckless]

Originally Posted by tmiranda

comskip doesn't like the name in quotes. I'll have to sort something else out.

If you are starting comskip using the java Runtime.exec() api and you are passing the command and args separately, then spaces should not matter. ie, visually when you log it out it might show the commandline with the spaces not escaped, but as long as the process, and args are all passed separately in the args array, it shouldn't be an issue.

[tmiranda]

Originally Posted by stuckless

If you are starting comskip using the java Runtime.exec() api and you are passing the command and args separately, then spaces should not matter. ie, visually when you log it out it might show the commandline with the spaces not escaped, but as long as the process, and args are all passed separately in the args array, it shouldn't be an issue.

That's what I thought too, and that's what I'm doing. (Using Runtime.exec(String[]))

I don't understand why Fuzzy is having the issue

[Fuzzy]

Originally Posted by stuckless

If you are starting comskip using the java Runtime.exec() api and you are passing the command and args separately, then spaces should not matter. ie, visually when you log it out it might show the commandline with the spaces not escaped, but as long as the process, and args are all passed separately in the args array, it shouldn't be an issue.

well, it definitely is seeing each word as a different arg.

Tue 1/17 12:58:50.559 [Thread-50@b0b6a13] CD: ComskipJob: Starting new ComskipJob
Tue 1/17 12:58:50.567 [Thread-50@b0b6a13] CD: QueuedJob.getComskipIni: Looking for comskip ini.
Tue 1/17 12:58:50.567 [Thread-50@b0b6a13] CD: QueuedJob.getComskipIni: Looking for comskip ini file /opt/sagetv/server/CommercialDetector/comskip/TheBoldandtheBeautiful.ini
Tue 1/17 12:58:50.578 [Thread-50@b0b6a13] CD: QueuedJob.getComskipIni: Looking for comskip ini file /opt/sagetv/server/CommercialDetector/comskip/KCBSDT.ini
Tue 1/17 12:58:50.580 [Thread-50@b0b6a13] CD: QueuedJob.getComskipIni: No ini found, using default.
Tue 1/17 12:58:50.581 [Thread-50@b0b6a13] CD: ComskipJob.executeComskipLinux: WinePath 
Tue 1/17 12:58:50.581 [Thread-50@b0b6a13] CD: ComskipJob.executeComskipLinux: Job parts 1
Tue 1/17 12:58:50.582 [Thread-50@b0b6a13] CD: ComskipJob.executeComskipLinux: Processing /var/media/tv/The Bold and the Beautiful - S30E104 - 33572376-0.ts
Tue 1/17 12:58:50.582 [Thread-50@b0b6a13] CD: ComskipJob.executeComskipLinux: Command sudo -H -u sagetv wine /opt/sagetv/comskip/comskip --playnice --ini=/opt/sagetv/comskip/comskip.ini /var/media/tv/The Bold and the Beautiful - S30E104 - 33572376-0.ts
Tue 1/17 12:58:50.659 [Thread-53@38155dd8] CD: StreamGetter: stdout: The commandline used was:
Tue 1/17 12:58:50.669 [Thread-53@38155dd8] CD: StreamGetter: stdout: /opt/sagetv/comskip/comskip --playnice --ini=/opt/sagetv/comskip/comskip.ini /var/media/tv/The Bold and the Beautiful - S30E104 - 33572376-0.ts
Tue 1/17 12:58:50.670 [Thread-53@38155dd8] CD: StreamGetter: stdout: 
Tue 1/17 12:58:50.670 [Thread-53@38155dd8] CD: StreamGetter: stdout: Usage:
Tue 1/17 12:58:50.670 [Thread-53@38155dd8] CD: StreamGetter: stdout:   comskip  [-h|--help] [-w|--debugwindow] [-n|--playnice] [--zpcut] [--zpchapter] [--videoredo] [--videoredo3] [--csvout] [--quality] [--plist] [-m|--demux] [--hwassist] [--threads=<int>] [-p|--pid=<string>] [-t|--ts] [-d|--detectmethod=<int>] [-v|--verbose=<int>] [-u|--dump=<int>] [-s|--play] [--timing] [-q|--quiet] [--ini=<file>] [--logo=<file>] [--cut=<file>] [--output=<file>] [--selftest=<int>] <file> [<file>]
Tue 1/17 12:58:50.671 [Thread-53@38155dd8] CD: StreamGetter: stdout: 
Tue 1/17 12:58:50.673 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -h, --help                Display syntax
Tue 1/17 12:58:50.674 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -w, --debugwindow         Show debug window
Tue 1/17 12:58:50.675 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -n, --playnice            Slows detection down
Tue 1/17 12:58:50.676 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --zpcut                   Outputs a ZoomPlayer cutlist
Tue 1/17 12:58:50.676 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --zpchapter               Outputs a ZoomPlayer chapter file
Tue 1/17 12:58:50.677 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --videoredo               Outputs a VideoRedo cutlist
Tue 1/17 12:58:50.677 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --videoredo3              Outputs a VideoRedo3 cutlist
Tue 1/17 12:58:50.677 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --csvout                  Outputs a csv of the frame array
Tue 1/17 12:58:50.678 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --quality                 Outputs a csv of false detection segments
Tue 1/17 12:58:50.678 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --plist                   Outputs a mac-style plist for addition to an EyeTV archive as the 'markers' property
Tue 1/17 12:58:50.678 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -m, --demux               Demux the input into elementary streams
Tue 1/17 12:58:50.679 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --hwassist                Activate Hardware Assisted video decoding
Tue 1/17 12:58:50.681 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --threads=<int>           The number of threads to use
Tue 1/17 12:58:50.682 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -p, --pid=<string>        The PID of the video in the TS
Tue 1/17 12:58:50.682 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -t, --ts                  The input file is a Transport Stream
Tue 1/17 12:58:50.683 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -d, --detectmethod=<int>  An integer sum of the detection methods to use
Tue 1/17 12:58:50.683 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -v, --verbose=<int>       Verbose level
Tue 1/17 12:58:50.684 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -u, --dump=<int>          Dump the cutscene at this frame number
Tue 1/17 12:58:50.684 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -s, --play                Play the video
Tue 1/17 12:58:50.685 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --timing                  Dump the timing into a file
Tue 1/17 12:58:50.686 [Thread-53@38155dd8] CD: StreamGetter: stdout:   -q, --quiet               Not output logging to the console window
Tue 1/17 12:58:50.686 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --ini=<file>              Ini file to use
Tue 1/17 12:58:50.687 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --logo=<file>             Logo file to use
Tue 1/17 12:58:50.687 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --cut=<file>              CutScene file to use
Tue 1/17 12:58:50.688 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --output=<file>           Folder to use for all output files
Tue 1/17 12:58:50.688 [Thread-53@38155dd8] CD: StreamGetter: stdout:   --selftest=<int>          Execute a selftest
Tue 1/17 12:58:50.688 [Thread-53@38155dd8] CD: StreamGetter: stdout:   <file>                    Input file
Tue 1/17 12:58:50.689 [Thread-53@38155dd8] CD: StreamGetter: stdout:   <file>                    Output folder for cutlist
Tue 1/17 12:58:50.689 [Thread-53@38155dd8] CD: StreamGetter: stdout: 
Tue 1/17 12:58:50.690 [Thread-53@38155dd8] CD: StreamGetter: stdout: Detection methods available:
Tue 1/17 12:58:50.690 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	  1 - Black Frame
Tue 1/17 12:58:50.691 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	  2 - Logo
Tue 1/17 12:58:50.691 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	  4 - Scene Change
Tue 1/17 12:58:50.691 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	  8 - Resolution Change
Tue 1/17 12:58:50.692 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	 16 - Closed Captions
Tue 1/17 12:58:50.692 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	 32 - Aspect Ratio
Tue 1/17 12:58:50.693 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	 64 - Silence
Tue 1/17 12:58:50.693 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	128 - CutScenes
Tue 1/17 12:58:50.694 [Thread-53@38155dd8] CD: StreamGetter: stdout: 	255 - USE ALL AVAILABLE
Tue 1/17 12:58:50.694 [Thread-53@38155dd8] CD: StreamGetter: stdout: 
Tue 1/17 12:58:50.694 [Thread-53@38155dd8] CD: StreamGetter: stdout: Errors:
Tue 1/17 12:58:50.695 [Thread-53@38155dd8] CD: StreamGetter: stdout: ComSkip: excess option and
Tue 1/17 12:58:50.695 [Thread-53@38155dd8] CD: StreamGetter: stdout: ComSkip: unexpected argument "the"
Tue 1/17 12:58:50.695 [Thread-53@38155dd8] CD: StreamGetter: stdout: ComSkip: unexpected argument "Beautiful"
Tue 1/17 12:58:50.712 [Thread-53@38155dd8] CD: StreamGetter: stdout: ComSkip: unexpected argument "-"
Tue 1/17 12:58:50.712 [Thread-53@38155dd8] CD: StreamGetter: stdout: ComSkip: unexpected argument "S30E104"
Tue 1/17 12:58:50.712 [Thread-53@38155dd8] CD: StreamGetter: stdout: ComSkip: unexpected argument "-"
Tue 1/17 12:58:50.713 [Thread-53@38155dd8] CD: StreamGetter: stdout: ComSkip: unexpected argument "33572376-0.ts"

[Fuzzy]

At a quick clance through ComskipJob.java in the (admittedly old) version of CD on github, I can see that in ExecuteComskipWindows() it is passing the args to Runtime.exe as a String[]. In ExecuteComskipLinux(), however, it is passing the entire line to Runtime.exec as a single String. If this code is still the same, I'm pretty sure this is the issue, and changing the handling in the Linux method would fix it.

[Taddeusz]

I can't find where I brought this problem up before. Both Fuzzy and I are running on unRAID. You sent me an updated jar file that fixed this exact issue for me.

[tmiranda]

Originally Posted by Fuzzy

At a quick clance through ComskipJob.java in the (admittedly old) version of CD on github, I can see that in ExecuteComskipWindows() it is passing the args to Runtime.exe as a String[]. In ExecuteComskipLinux(), however, it is passing the entire line to Runtime.exec as a single String. If this code is still the same, I'm pretty sure this is the issue, and changing the handling in the Linux method would fix it.

That code has been changed.

[Fuzzy]

Originally Posted by tmiranda

That code has been changed.

does the new code still send a single string to the .exec call? Or do they all send a String[], with each full argument it's own element?

[tmiranda]

Originally Posted by Fuzzy

does the new code still send a single string to the .exec call? Or do they all send a String[], with each full argument it's own element?

It uses String[].

I'm super busy at work right now. I'll probably send you a new version to try in a day or two. Sorry I can't get to it sooner

[Fuzzy]

No problem, was easy enough to just turn off extended filenames for now. I only turned it on on a whim anyway, as I don't use the files outside of SageTV (now that the android client is so good).

[sflamm]

now that the android client is so good

What client are you using?

[Fuzzy]

Originally Posted by sflamm

What client are you using?

SageTV miniclient.

[Fuzzy]

So with the new .JAR you sent me, it is now processing files with spaces in it just fine, and it IS now passing a string array instead of a single string.

Old:  Command sudo -H -u sagetv wine /opt/sagetv/comskip/comskip --playnice --ini=/opt/sagetv/comskip/comskip.ini /var/media/tv/Jerry Springer - S25E80 - Betrayed by My Parrot - 33631125-0.ts
New:  Command [/opt/sagetv/comskip/comskip, --playnice, --ini=/opt/sagetv/comskip/comskip.ini, /var/media/tv/Jerry Springer - S25E80 - Betrayed by My Parrot - 33631125-0.ts]