-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Commercial injection clips are too long #72
Comments
ahh, this actually makes sense looking at the code. The problem falls to the following line: 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. |
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. |
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
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. |
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: And a typo in my previous post... of course it checks for a length larger than 10 seconds not 0 |
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:
|
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. |
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: |
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. |
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 |
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. |
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. @justinemter who could judge you for not remembering every little piece of such a script |
@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. |
@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! |
@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 Just for my understanding, you indeed have 5+ minute clips in you commercial db, and they definitly get scheduled? Does
As stated before, editing the PseudoChannelCommercial.py according to @mutto233's advice did the trick. |
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! |
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. |
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?
The text was updated successfully, but these errors were encountered: