justinemter / pseudo-channel

This is a python based cli-app using the python-plex-api to control a plex-client and act like a real TV channel with show scheduling, commercial breaks, movie nights, etc. "Home-Brewed TV Channel(s) for Hackers"
GNU General Public License v3.0
130 stars 18 forks source link

Commercial injection clips are too long #72

Open bud86 opened 6 years ago

bud86 commented 6 years ago

So using the commercial injection works pretty flawlessly for me, except using clips that are 60+ seconds in duration. During generating the daily schedule the app runs into a endless loop in "def get_random_commercial(self):" as long as it finds no clip between 10-60 seconds. Is there a "MAX_DURATION_FOR_COMMERCIAL" setting to adjust or is it a limitation?

mutto233 commented 6 years ago

ahh, this actually makes sense looking at the code.

The problem falls to the following line: random_commercial_dur_seconds = (int(random_commercial[4])/1000)%60

Notice the modulus operator? This means that any time you surpass 60 seconds it will simply take the value back to zero. Ergo, why you are facing your problem.

As a "first start" fix, I would suggest removing these modulus sections on your code. It may do exactly what you want. If so, please report back and we can edit it on my fork of the code.

bud86 commented 6 years ago

Of course, so simple and obvious! I totally misinterpretet the operator, due to my lack in programming knowlege.

Now it works perfectly for my needs. Appreciate your quick assistance and lesson ;-)

For editing the code, maybe it is enough to give a "No matching clips found for a duration between 0-60 seconds" message for a return value of 0, or something like that. I mean there was probably an idea behind that 60 second "limit" in the beginning. As an average commercial clip is usually no longer than maybe 30-40 seconds (rather shorter) afaik.

mutto233 commented 6 years ago

this function actually isn't looking through "everything" per-say. It is actually simpler than that, and its whole purpose is just to find a commercial that is longer than the shortest time (in this case, 10 seconds).

Here is what the code does 1) Pull random commercial 2) Check length -- Longer than 10 seconds? Return that commercial. -- Shorter than 10 seconds (or longer than 60 according to the bug)? START FROM 1) AGAIN.

The code is not all inclusive, and it isn't the section that searches for a commercial that can fit a given length; I believe that is a different function.

bud86 commented 6 years ago

Thats absoluty how I understood the hole thing. I just assumed that the return value of "random_commercial_dur_seconds" needs to be a minute formated value, to use later on.

I read it like: random_commercial_dur_seconds = (int(random_commercial[4])/1000)**/**60

And a typo in my previous post... of course it checks for a length larger than 10 seconds not 0 Anyway, thanks again

justinemter commented 6 years ago

To be honest I cannot remember my reasoning for adding the modulusoperator to this function. However, this hasn't caused an issue like @bud86 mentioned for me. In fact, I inject my channels with long movie trailers and old 90's smashing pumpkins music videos just fine (5+ minute durations). In regard to your endless loop issue, make sure you have a lot of commercials in your library. I used a tool called "youtube-dl" and downloaded hundreds of commercials from youtube playlists. This, plus the movie trailers/music videos gave me plenty of content that varied widely in duration. The commercial logic is pretty solid, here's how it works:

mutto233 commented 6 years ago

it makes me wonder @justinemter if that specific function (get_commercial) is a "legacy" function that isn't really being used anymore. I bet if I go back and check all the logic it may never be called if done correctly.

Maybe we have just debugged an old function.

justinemter commented 6 years ago

It looks like it is being used. Also, I realized that I do have a variable up top in that class called "MIN_DURATION_FOR_COMMERCIAL" and it is being set to "10" (seconds). So it looks like that logic is in there to simply check to make sure that it meets that single condition: random_commercial > MIN_DURATION_FOR_COMMERCIAL. It really shouldn't be the cause of @bud86's issue with it endlessly looping when encountering a commercial over 60 seconds. This is a strange one. Also, thanks for all your help on this project! I got really into it about a year ago, then got busy and have to remember everything again.

mutto233 commented 6 years ago

Not a problem; this project has helped me cut the chord and will be saving me a lot of money over the course of just the next few months!

So the thing is that modulus operator threw a wrench into the logic of get_commercial. Anything over 60 seconds will therefore effectively "wrap" back to 0 seconds, so it will think it is too short.

justinemter commented 6 years ago

Man, I wish I remember what I was doing there. I suppose one fix is to just completely remove that condition entirely and have that function return a random commercial (disregarding the MIN_DURATION_FOR_COMMERCIAL condition and the modulus thing). But keep in mind I've been running my channels with that code for a while now, injecting gaps with commercials well over 60 seconds. Either way I don't think it would be a problem to just remove that excess logic and have that function return a random commercial: return random.choice(self.commercials).

mutto233 commented 6 years ago

gotcha. Well if thats the case, I will keep that change in my fork. I have removed the modulus, so hopefully this won't be an issue for the near future.

bud86 commented 6 years ago

Just to make it clear, I started to test everything with around 8 yt clips all about 15-30 secons long. After this worked very well I switched to my custom made 5 minute clips. And run into the "bug"

I only end up in a loop as long as there is no clip in my "db" that is below 60 seconds. Honestly I never tried a clip below ten seconds, I just assumed it would not work due to the "MIN_DURATION_FOR_COMMERCIAL" value of 10.

As soon as I place at least one clip shorter than 60 seconds, the gaps get filled up with only this clip.

I can absolutly reproduce this "bug". If you like I could provide you with my script and if your 60+ clips work, we know there is an issue on my side. Anyway, I will keep the change as well, as it works perfect now.

@justinemter who could judge you for not remembering every little piece of such a script

mutto233 commented 6 years ago

@bud86 I suggest checking out my fork to see some more changes that have been made. If not interested in that, simply comment out the modulus part of the script on your end, and try again. If it works, bug found! If not, there may be some deeper reason for its existence. But I doubt it, since that length is never actually returned outside of that script if I remember correctly.

justinemter commented 6 years ago

@bud86 I think it is due to the script anticipating hundreds of commercial videos of varying length. I can't quite wrap my head around how it isn't working with your current situation but it seems that this fix we've been discussing is fixing that. Either way, I'd go ahead and stock up on commercial content. I have over 500 (maybe even 1000) commercials and haven't experienced any issues. Cheers!

bud86 commented 6 years ago

@justinemter I see no logic in stocking up my commercials. Why should it work with hundreds of clips but not with 5. It doesn't matter how much clips are listed in my db or how long a gap is, as long as there is at least one clip random_commercial_dur_seconds does not return 0

Just for my understanding, you indeed have 5+ minute clips in you commercial db, and they definitly get scheduled? Does def get_random_commercial(self): in your script looks according to following:

def get_random_commercial(self):

        random_commercial = random.choice(self.commercials)
        random_commercial_dur_seconds = (int(random_commercial[4])/1000)%60
        while random_commercial_dur_seconds < self.MIN_DURATION_FOR_COMMERCIAL:
             random_commercial = random.choice(self.commercials)
             random_commercial_dur_seconds = (int(random_commercial[4])/1000)%60

As stated before, editing the PseudoChannelCommercial.py according to @mutto233's advice did the trick. If you guys like I can test @mutto233's fork by time and give some feedback if the bug still appears for me, otherwise if its just me being able to reproduce this bug, I'll just stay with the work around and be happy with the app.

mutto233 commented 6 years ago

Here is my guess. @justinemter has commercials that are long but their modulus just HAPPENS to fall in that range. So this bug was never noticed. We have noticed it now, so it seems like we should be good to go.

@justinemter is merely suggesting that having more commercials will add to your variety, and allow a larger amount of "Gaps" to be filled. But, as we talked about, this script in particular does not actually look at filling a gap, it just finds a random commercial that can be tested to fill a gap.

I would love more people to test out my fork. I have been trying to make improvements beyond just this to a great program, so let me know what you think. Either leave an issue on the GitHub or come chat in the discord!

justinemter commented 6 years ago

I'm not sure why it works for me (and others) but is breaking for you. I guess I was suggesting that for some reason having more commercials maybe the reason it's not breaking for us. Hopefully this discussed fix works for you. Def checkout mutto's branch as he's been doing a lot.