BrettSheleski/comchap

Missing Last Portion of File

Closed this issue · 32 comments

arrmo commented

FYI, the last portion of the file is not included ... from the end of the last line in the EDL file, to the end of the video.

Thanks!

Please confirm that this is the case. I would think for sure that, or others, would have definitely noticed this before.

arrmo commented

Hi,

I'm 95% sure ... :-). Checked the EDL file, looking at commercial times, and the time of the file before and after. Also checking the video, there is definitely a section missing at the end.

And looking at the code logic - I don't see where a segment is generated (and then joined), from the last entry in the EDL file, to the end of the input .ts .. or am I missing it?

Thanks!

To clarify, are we talking about comchap or comcut? I'm assuming we're talking about comcut.

I reviewed comcut, sure enough, it's not handling the last portion of non-commercial content. It is also still using the bc command for math stuff.

I'll have to fix this.

arrmo commented

Yep, comcut. Sorry for the delayed reply, was off at the gym ... ;-). I can dig into this as well if you want, just let me know.

Thanks!

I just pushed a commit (see 6fd39f9). See if that resolves your issue.

I see there was an issue in determining the duration of the final chapter. I fixed that and re-pushed (see e86062b). Try again now.

arrmo commented

Hi,

OK, tried to convert a file - but I don't think it's quite right yet. I could definitely be wrong, but here's why I say this ...

Original File = 3:48:57 (13737 seconds)
EDL cut = 3480 seconds (see EDL below)
Output File (expected) = 10257 seconds
Output File (actual) = 2:46:26 (9986 seconds)
=> Missing ~ 271 seconds, agreed?

Here is the EDL output,
0.00 496.96 0
1008.27 1276.84 0
2160.93 2293.02 0
2740.37 2876.91 0
3044.41 3170.97 0
3699.73 3832.06 0
4069.30 4200.90 0
4487.02 4618.85 0
4783.81 4890.95 0
5147.38 5177.47 0
5207.74 5279.27 0
5509.77 5643.50 0
5825.35 5957.12 0
6230.72 6362.56 0
6688.65 6816.48 0
7232.19 7343.54 0
7785.74 7918.01 0
8828.42 8960.55 0
9410.80 9543.07 0
9953.68 10085.71 0
11006.83 11108.20 0
11612.10 11733.19 0
12025.78 12147.64 0
12800.25 12939.26 0
13403.82 13470.02 0

Thoughts?

Thanks!

arrmo commented

Actually, and I just noticed ... the time from the last commercial (at 13470.02 seconds), to the end of the file (13737 seconds) is 267 seconds => almost exactly what is missing, right?

Trying to check for info, but $metafile doesn't seem to exist (.ffmeta) ... any thoughts?

By default comcut deletes the ffmeta file. Did you specify --keep-meta?

arrmo commented

Doh! That was me, sorry ... ;-).

OK, here is what I'm seeing - I may be misunderstanding, please correct me if I have it wrong. But here are the first few lines from the EDL, then the ffmeta file, with comments inserted (as '=>arrmo:'),

EDL,
0.00 496.96 0
1008.27 1276.84 0
2160.93 2293.02 0

ffmeta,
;FFMETADATA1
[CHAPTER]
TIMEBASE=1/1000
START=496960
END=1008270
=>arrmo: OK, this chapter makes sense, matches EDL
[CHAPTER]
TIMEBASE=1/1000
START=1008270
=>arrmo: This isn't starting the right place, is it? It should start after the commercial, not the same as above .. or is this after cutting? If it is, perhaps log what is used for cutting also?
END=1892360

Thoughts?

Thanks!

The start and end times in the meta file need to subtract out the amount of time that was removed from where the commercials were. This is why the start and end times do not match between the ffmeta and the edl file. However the difference between each start and end times should still match for each entry.

To add to this, the start time of one chapter should be the same as the end time of the previous chapter.

arrmo commented

Gotcha - that makes complete sense, was wondering if that was it. But then, here is the last chapter in the file,

[CHAPTER]
TIMEBASE=1/1000
START=10022430
END=10486990

Hmmm ... this doesn't match my numbers above, either the expected or actual file length. Let me try to log some more output, try to figure it out.

Thanks!

Have you tried actually playing the file in question? It may just be an issue with the ffmeta file, which really only affects if you skip forward/back using chapters during playback.

Try skipping to the last chapter, rewind a few seconds, the play till the end. See if the final commercial break was indeed removed (and at the correct time) and if it plays back till the end.

arrmo commented

Yep, just checked the video files. The output ends right before the last commercial - missing the portion after it. Will try to re-run with some console output to see if the last bit of code is being run.

There's a simple loop at the end that deletes all temporary files used, one for each chapter. You could try commenting-out the rm command inside. Then take a look at the files and see what's up.

arrmo commented

Actually, added an echo as below,

#dont forget to add the final part from last commercial to end of file
end=$ffmpegPath -hide_banner -loglevel error -nostdin -i "$infile" 2>&1 | grep Duration | awk '{print $2}' | tr -d , | awk -F: '{ printf "%f", ($1*3600)+($2*60)+$3 }'
echo "End: $end"

And I get,
End:

Also added in,
if [ echo "$end" | awk '{printf "%i", $0 * 1000}' -gt echo "$start" | awk '{printf "%i", $0 * 1000}' ]; then
echo "Adding last portion ..."

This echo never gets hit. So it seems the line after "#dont forget to add the final part from last commercial to end of file" is where the issue is, agreed?

arrmo commented

Trying the first portion of that command, I get the response below ...
$ffmpegPath -hide_banner -loglevel error -nostdin -i "$infile" 2>&1

[mpeg2video @ 0x3932960] Invalid frame dimensions 0x0.
Last message repeated 50 times
At least one output file must be specified

So no Duration in the output.

Ah damn. It looks like finding the total duration of the input file is the issue. I copied that from what I did in comchap. Then I went ahead and changed things to deal with the ffmeta file needing entries to be in milliseconds, while what's in the edl file and what needs to get passed to ffmpeg to make the chapter file needs to be done in seconds.

I'm not going to get to this tonight. My ass is firmly on the couch with my feet up. Pull requests are always welcome.

arrmo commented

But, with ffprobe (-i filename),
[mpeg2video @ 0x348c1c0] Invalid frame dimensions 0x0.
Last message repeated 50 times
[mpegts @ 0x3487580] PES packet size mismatch
Last message repeated 1 times
Input #0, mpegts, from 'PGA Tour Golf (2003) - 2017-02-19 00 00 00 - Genesis Open Final Round.ts':
Duration: 03:48:58.41, start: 12699.904589, bitrate: 14807 kb/s
Program 3111
Stream #0:0[0x457]: Video: mpeg2video (Main) ([2][0][0][0] / 0x0002), yuv420p(tv, bt709, top first), 1920x1080 [SAR 1:1 DAR 16:9], Closed Captions, 29.97 fps, 29.97 tbr, 90k tbn, 59.94 tbc
Stream #0:10x458: Audio: ac3 ([129][0][0][0] / 0x0081), 48000 Hz, 5.1(side), fltp, 384 kb/s
Stream #0:20x459: Audio: ac3 ([129][0][0][0] / 0x0081), 48000 Hz, mono, fltp, 192 kb/s

Then, with the rest of the line. ffmpeg likely works also, but ffprobe returns what is needed.

arrmo commented

LOL - NP, turning in now also (early morning tomorrow), but will continue tomorrow night. Have a nice evening!

I think it may be the -log_level error stuff I added to quiet down ffmpeg.i didn't think that would cause an issue. Try removing it and rerunning.

I originally used ffprobe, but opted to go for using just ffmpeg to be more compatible for when specifying ffmpeg-path via command line. I didn't also need to make a ffprobe-path option too. The fewer dependencies the better.

I'm pretty confident removing the -loglevel error part will fix the issue.

arrmo commented

Tried that, no joy => the issue seems to be the noted error,
"At least one output file must be specified"

I can get ffprobe to work (command line below), but I think you want to stick with ffmpeg? If so, we need to find a way around the output file error.

ffprobe -v quiet -show_entries format=duration -of default=noprint_wrappers=1:nokey=1 FILENAME

A link that may help also, https://trac.ffmpeg.org/wiki/FFprobeTips (see Duration)

Removed -loglevel error and commited. In my simple testing it got the duration just fine. Re-pull and try again.

nevermind, still issues. Working them out now.

I think I got it now. I didn't increment the chapter count when writing to the final chapter file. This resulted in the second-to-last chapter file getting overwritten with the final chapter then erroring out later on when cleaning up the chapter files.

arrmo commented

Yep, it seems to work now - thanks! BTW, just a bit odd ... but the first segment is now numbered -2. Does that sound right to you?

Good catch. That can happen if the edl file starts with a zero indicating the first part of the input file is a commercial (and to be cut out). The indexing variable $i gets incremented before it checks for that resulting in $i being 2 during the next iteration of the loop (where there is content to include in the resulting file).

I'll add a fix for that. And also include chapter titles in the metadata in the format of "chapter $i".

arrmo commented

Thanks!!!