Commercial skipping notifications and toggle action #9399

b-pass · 2016-03-19T21:27:25Z

An implementation to address this feature request from the forums: http://forum.kodi.tv/showthread.php?tid=208206

This adds a new action, TOGGLE_COMMSKIP which turns Commercial Skipping on/off for the current recording only (next record starts back at the default setting from advancedsettings.xml).

Also adds an option to display notifications of commercials and their lengths.

This is a resubmit of #7330 and #7520.

xbmc/video/windows/GUIWindowFullScreen.cpp

@@ -177,6 +177,11 @@ bool CGUIWindowFullScreen::OnAction(const CAction &action)
    }
    break;

+  case ACTION_TOGGLE_COMMSKIP:


b-pass · 2016-03-19T22:28:06Z

@FernetMenta this was requested to be in advanced settings by @ksooo, @MartijnKaijser, and you in #7330 because everyone was worried it would be a support problem if it showed up in a more prominent place.

FernetMenta · 2016-03-19T22:47:12Z

@b-pass I can't find any reference that I requested this to be in advancedsettings. I only found a single post of myself in this thread:

support for comskip is no requirement for a pvr addon. hence adding those options to general settings is not correct. this needs to be enabled by something private to an addon and conveyed through the pvr api.

what sources do require this feature?

b-pass · 2016-03-19T22:52:29Z

From @ksooo

@MartijnKaijser yes, I'm actually afraid of this "support nightmare", too. We have to be really very clear about what this power user feature is/is not if we expose it through any UI.

Sounds to me like the point was to keep it out of the GUI entirely. It's an EDL feature, which is just sprinkled throughout VideoPlayer and already itself has a few settings in Advanced settings. In #7330 there's some discussion of a "support nightmare" if the setting is left in the GUI anywhere.

FernetMenta · 2016-03-20T08:09:32Z

Why don't you disable the function by default and have users enable it with the toggle on demand? THat would eliminate the need for a setting.

b-pass · 2016-03-20T10:38:10Z

@FernetMenta That's fair. The setting was to allow people to turn it off. But since it can't be turned off today either, there's not really a loss in functionality. I've made the changes you requested.

FernetMenta · 2016-03-20T11:47:59Z

you still have ToggleCommSkip in the interface. This is not required because VideoPlayer listens to action event.

xbmc/cores/VideoPlayer/VideoPlayer.cpp

@@ -4391,6 +4418,9 @@ bool CVideoPlayer::OnAction(const CAction &action)
      }
      else
        break;
+    case ACTION_TOGGLE_COMMSKIP:
+      ToggleCommSkip();


b-pass · 2016-03-20T13:41:38Z

@BrettEBowman I've done as you said, and EDL will continue to be enabled by default, users can now use the action to toggle it off. The setting which I've now removed was to allow it to be disabled by default.

@FernetMenta I've removed the unnecessary 2 line function now as well.

FernetMenta · 2016-03-20T14:12:11Z

jenkins build this please

Commercial skipping notifications and toggle action

FernetMenta · 2016-03-20T19:16:11Z

thanks!

jpribyl · 2022-11-26T20:39:52Z

To anyone who wound up here from this thread and is looking to disable the "Commercial" popup- it's possible to do in your skin files without needing to build kodi from source. This notification uses the DialogNotification template which will be somewhere roughly like this:

~.kodi/addons/skin.embuary-matrix/xml/DialogNotification.xml

In that file, you can include a visible tag like this:

<?xml version="1.0" encoding="UTF-8"?>
<window>
    ...
    <controls>
    <visible>!String.Contains(Control.GetLabel(400), "Commercial") + !String.Contains(Control.GetLabel(401), "Commercial") + !String.Contains(Control.GetLabel(402), "Commercial")</visible>
    ...
    </controls>
</window>

Restart kodi and you should find that the pop up is gone. YMMV depending upon your skin

enen92 · 2022-11-26T21:55:22Z

@jpribyl see #20711 (Kodi 20 and up)

jpribyl · 2022-11-27T16:57:23Z

Amazing! Nice work 😄 - @enen92 just out of curiosity, does that setting in the new build also suppress the progress bar from becoming visible?

enen92 · 2022-11-27T17:11:28Z

No, unfortunately not. That's sort of on my todo list but we still lack any way of checking from the skin engine if we "come" from an edl mark after a seek.
At the moment you'll have to patch estuary to avoid showing the seek bar. That can be done removing Player.DisplayAfterSeek calls for < 20 or removing Player.HasPerformedSeek(x) calls for v20 and up.
I'll likely come back to this at some point after v20 is out.

b-pass · 2022-11-27T17:42:17Z

If you don't want to see the progress bar or the notification then you should be setting your EDL lists to "Cut" type instead of "CommBreak" type. That's the whole purpose of "Cut" type -- to make it seem like the skipped section was not there at all (skip as smoothly as possible, etc).

How exactly you do that depends on the source and/or file format of the EDL.

enen92 · 2022-11-27T17:54:07Z

Cut effectively removes the block from the timeline, you're not allowed to jump back into the removed section if for some reason the software marks the advertisement on an incorrect position (quite common). Also, internally the player does a seek anyway so the seekbar will also display..

jpribyl · 2022-11-27T21:53:30Z

Oh snap, that's a totally reasonable workaround 💪 thank you! For anyone else using embuary on v19 the necessary line to change is here:

https://github.com/sualfred/skin.embuary/blob/master/xml/Includes.xml#L90

calania · 2023-09-13T07:02:32Z

Am really interested in the TOGGLE_COMMSKIP function but can't I can't find how to use it. Could anybody here please explain how it works?

b-pass · 2023-09-13T10:29:57Z

Am really interested in the TOGGLE_COMMSKIP function but can't I can't find how to use it. Could anybody here please explain how it works?

Yes, you can reference it in a keymap xml file. For example, to have the button named "green" do this action while watching video:

<keymap><fullscreenvideo><remote><green>ToggleCommSkip</green></remote></fullscreenvideo>

calania · 2023-09-13T15:24:54Z

Didn't expect the original creator of this pull request to answer after so many years :) . Thanks for the answer but it's not exactly what I want. I would want it to by default not skip the commercials and having to always press a remote on to toggle auto skip off is quite annoying. In an ideal scenario I would want a popup along the lines of "Commercial detected! Do you want to skip?" but simply disabling auto skip would work as well. Looking through your pull requests it seems like there once was a way to disable auto skip in the advancedsettings.xml so it didn't skip by default (you even mentioned it in the first post of this pr). But I cant find any mention of it now. Did you or someone else remove the ability to set the default behavior?

b-pass · 2023-09-15T00:36:42Z

Looking through your pull requests it seems like there once was a way to disable auto skip in the advancedsettings.xml so it didn't skip by default (you even mentioned it in the first post of this pr). But I cant find any mention of it now. Did you or someone else remove the ability to set the default behavior?

At the time, they did not want to add things to advancedsettings.xml, but they also did not want to add options to the main Kodi settings pages which implied commercial skipping could be done by kodi alone. So there isn't a way to configure it, because there didn't seem to be something that the PR approvers would agree on.

calania · 2023-09-16T20:18:53Z

Well that's a shame. Seems like quite a basic setting and can't really understand why at least not add it to advancedsettings.xml

FernetMenta reviewed Mar 19, 2016
View reviewed changes

xbmc/video/windows/GUIWindowFullScreen.cpp

@@ -177,6 +177,11 @@ bool CGUIWindowFullScreen::OnAction(const CAction &action)

}

break;

case ACTION_TOGGLE_COMMSKIP:

This comment was marked as spam.

Sign in to view

b-pass force-pushed the comm-skip-toggle branch from aa6b7ab to f52cb05 Compare March 19, 2016 23:11

b-pass force-pushed the comm-skip-toggle branch 2 times, most recently from bd4e551 to 2055023 Compare March 20, 2016 10:34

FernetMenta reviewed Mar 20, 2016
View reviewed changes

xbmc/cores/VideoPlayer/VideoPlayer.cpp

@@ -4391,6 +4418,9 @@ bool CVideoPlayer::OnAction(const CAction &action)

}

else

break;

case ACTION_TOGGLE_COMMSKIP:

ToggleCommSkip();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Commercial skipping notifications and toggle action

1b2c8e0

b-pass force-pushed the comm-skip-toggle branch from 2055023 to 1b2c8e0 Compare March 20, 2016 13:39

FernetMenta added Type: Feature non-breaking change which adds functionality v17 Krypton Component: Video labels Mar 20, 2016

FernetMenta added a commit that referenced this pull request Mar 20, 2016

Merge pull request #9399 from b-pass/comm-skip-toggle

c2afc63

Commercial skipping notifications and toggle action

FernetMenta merged commit c2afc63 into xbmc:master Mar 20, 2016

FernetMenta added this to the Krypton 17.0-alpha1 milestone Mar 20, 2016

nerddtvg mentioned this pull request Nov 29, 2016

Adding a setting to enable or disable prompting for Auto-Skip #11008

Closed

9 tasks

b-pass deleted the comm-skip-toggle branch December 3, 2016 00:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Commercial skipping notifications and toggle action #9399

Commercial skipping notifications and toggle action #9399

b-pass commented Mar 19, 2016

This comment was marked as spam.

b-pass commented Mar 19, 2016

FernetMenta commented Mar 19, 2016

b-pass commented Mar 19, 2016

FernetMenta commented Mar 20, 2016

b-pass commented Mar 20, 2016

FernetMenta commented Mar 20, 2016

This comment was marked as spam.

This comment was marked as spam.

b-pass commented Mar 20, 2016

FernetMenta commented Mar 20, 2016

FernetMenta commented Mar 20, 2016

jpribyl commented Nov 26, 2022

enen92 commented Nov 26, 2022

jpribyl commented Nov 27, 2022

enen92 commented Nov 27, 2022

b-pass commented Nov 27, 2022

enen92 commented Nov 27, 2022

jpribyl commented Nov 27, 2022

calania commented Sep 13, 2023

b-pass commented Sep 13, 2023

calania commented Sep 13, 2023

b-pass commented Sep 15, 2023

calania commented Sep 16, 2023

Commercial skipping notifications and toggle action #9399

Commercial skipping notifications and toggle action #9399

Conversation

b-pass commented Mar 19, 2016

This comment was marked as spam.

b-pass commented Mar 19, 2016

FernetMenta commented Mar 19, 2016

b-pass commented Mar 19, 2016

FernetMenta commented Mar 20, 2016

b-pass commented Mar 20, 2016

FernetMenta commented Mar 20, 2016

This comment was marked as spam.

This comment was marked as spam.

b-pass commented Mar 20, 2016

FernetMenta commented Mar 20, 2016

FernetMenta commented Mar 20, 2016

jpribyl commented Nov 26, 2022

enen92 commented Nov 26, 2022

jpribyl commented Nov 27, 2022

enen92 commented Nov 27, 2022

b-pass commented Nov 27, 2022

enen92 commented Nov 27, 2022

jpribyl commented Nov 27, 2022

calania commented Sep 13, 2023

b-pass commented Sep 13, 2023

calania commented Sep 13, 2023

b-pass commented Sep 15, 2023

calania commented Sep 16, 2023