Comskip/EDL processing bug in KODI (?)

I’m not certain that this is the best place to post this … if its not, then please point me in the right direction.

I am trying to get Comskip working properly on my Raspberry Pis. I have one RPi running tvheadend and comskip and I have that working pretty well producing EDL files that are as accurate as I could expect given the TV broadcast being processed.

The problem is that when playing the recording on my OSMC Rpi, some of the commercial breaks are sometimes not being skipped.

I created a “fake” EDL file that looked like this …

0009	0100	3
0109	0200	3
0209	0300	3
0309	0400	3

… repeated segments of 9 seconds of “show” followed by 91 seconds of “commercials”.

Most of the time the “commercials” were skipped properly, but sometimes, maybe 1 in 4 times for a HD show and one in 10 times for an SD show, the “commercials” were not skipped.

So I dug into the code, and I think I have found a bug. I actually work in IT, and have programmed for many years, but I have never been a c/c++ programmer, so please forgive me if I’m wrong here, but here is what I think is going on, based on my reading of the code.

In the (main?) loop in VideoPlayer.cpp that starts at line 1357, there are two places that check for EDL skips.

At line 1384, a call to CheckAutoSceneSkip() checks before a chunk of video stream is read, to see if we have entered a commercial and should skip before reading the next chunk of stream.

At line 1625, ProcessPacket() calls ProcessVideoData() which calls CheckSceneSkip() which checks to see if the chunk of video that we are processing is in a commercial an needs to be dropped.

I think that the problem is that both CheckAutoSceneSkip() and CheckSceneSkip() make calls to m_Edl.InCut() which updates the value of m_lastQueryTime within the EDL data structure, but I think that the logic of lastPos in CheckAutoSceneSkip() is based on the last time that m_Edl.InCut was called in that function, but it is also called from CheckSceneSkip() so it can mess up the test at line 2385

For example …

An EDL cut starts at 01:00.00 …

  • Start of main loop at 00:59.99
  • CheckAutoSceneSkip() calls m_Edl.InCut() which sets m_lastQueryTime = 00:59.99
  • Read video stream and time passes to 01:00.01
  • ProcessPacket() calls ProcessVideoData() calls CheckSceneSkip() which sets m_lastQueryTime = 01:00.01
  • (Some tiny imperceptible chunk of video gets dropped?)
  • Loops back top of main loop
  • CheckAutoSceneSkip() calls m_Edl.GetLastQueryTime() which sets lastPos = 01:00.01
  • Test at Line 2385 “if ( lastPos <= cut.start )” is false, so the code to skip the commercial is not executed.

My main problem/question now is, how do I code a fix for this and build an executable to test? I think I can figure out how to hack the code, but building is more of an issue. Is there a guide for coding and building dirty hacks? :

For contributing, bug fixing, building and testing advice for OSMC see here:

https://osmc.tv/wiki/development/overview/

As this would appear to be a bug in Kodi rather than OSMC, you may find similar reports/other fixes on the Kodi forum.

OK, checked out those pages and got as far as

Supported development platforms

You will need a 64-bit computer to work on OSMC. You will need to be running either Ubuntu or Debian.

I was hoping that there was a .configure/make/install method to make “quick” fixes of Kodi directly on the OSMC Pi.

I’ll post in the kodi.tv forums …

You can build Kodi on the device.

git clone https://github.com/osmc/osmc
cd osmc/package/mediacenter-osmc
make (to see list of targets)

Looks like i’m gonna need a bigger card :smiley:

osmc@osmc:~$ df -h
Filesystem      Size  Used Avail Use% Mounted on
...
/dev/mmcblk0p2  3.4G  3.2G     0 100% /

You can build all or any component directly on the device. You may want to enable swap if you’re going to build anything big and also stop any processes you don’t need.

A decent sized good brand SD card (such as the one at https://store.osmc.tv/) will make life easier. Otherwise all you need is plenty of patience and a good supply of caffeine! (It isn’t quick!)

Trust me, if I can do it then anyone can.

OSMC packages that need swap will automatically create an appropriate sized
swap file and swap on and off during build.

 git clone https://github.com/osmc/osmc
 cd osmc/package/mediacenter-osmc
 make rbp2

Nearly 4 hours !

Now what ???

I have some changes I want to test in xbmc\cores\VideoPlayer\VideoPlayer.cpp and Edl.cpp

The only such files I can find are in …

~\osmc\osmc\package\mediacenter-osmc\src\xbmc-147cec4077415c93d3d84fb82cb6b695b5a9094c\xbmc\cores\VideoPlayer

… so I make some changes and then do “make rbp2” again …

And about the first thing it does is wipe out my changes and replaces them with the original files before carrying on … :sob:

I have the feeling that if I knew what I was doing it would be easy, but reading through osmc.tv/wiki I can’t see anywhere that explains to me how to do what I want without signing up a github account.

The simplest process (as I understand it) is as follows:

  1. create account on GitHub
  2. fork OSMC to your own account
  3. git clone to your development device from your own fork
  4. make your changes
  5. run git commit -am "Description of changes" && git push
  6. make rbp2

That should give you a .deb file which you can then install with dpkg.

Test your changes and repeat from step 4 until everything is as you want it.

Once everything is fixed you can create a patch from GitHub and pass that to the Kodi team to be implemented globally or to @sam_nazarko to be implemented for only OSMC users if that is more appropriate.

Make changes as a patch and put it in the patches/ directory.

Alternatively you can chroot in and make changes rapidly to avoid long build times

I really wanted to avoid crating a GitHub account

It took me a bit of digging around in old threads, but I finally figured out how to do this.

(here are the steps, for my own reference and for anyone else foolhardy enough to try)

cd ~
sudo mount --bind $(pwd)/osmc /opt/osmc-tc/armv7-toolchain-osmc/mnt
sudo chroot  /opt/osmc-tc/armv7-toolchain-osmc
cd /mnt/package/mediacenter-osmc/src/xbmc-147cec4077415c93d3d84fb82cb6b695b5a9094c

(edit source code in ./xbmc/cores/VideoPlayer/ )

make

Now I have my own build of kodi.bin in the current directory.

So now I can stop kodi with “sudo systemctl stop mediacenter”, replace /usr/lib/kodi/kodi.bin, and restart kodi and I’m done (? - can’t test this part right now)

Yes, that will work.