[Approved] Show Comskip Markers in Timeline (1 Viewer)

[mattjcurry]

Sorry for the large number of patches. I am a obsessive about checking in my code as I make changes so I can easily revert.
**2012-04-03: I am deleting the current patches as they are out of date. Please check the binaries.

**2012-04-22: Posting binaries for 1.2.3
**2012-04-22: adding patchfile for anyone working with source.

To use this, you need to edit the skin file and add the new properties for the marker support. An example is here:

    <control>
      <description>TV Progress Bar</description>
      <type>tvcomskipprogress</type>
      <id>1</id>
      <posX>90</posX>
      <posY>484</posY>
      <width>362</width>
      <height>16</height>
      <toptexture>osd_progress_indicator.png</toptexture>
      <TextureOffsetY>17</TextureOffsetY>
      <bottomtexture>-</bottomtexture>
      <texturetick>-</texturetick>
      <lefttexture>-</lefttexture>
      <midtexture>-</midtexture>
      <righttexture>-</righttexture>
      <logotexture>-</logotexture>
      <fillbackgroundtexture>-</fillbackgroundtexture>
      <fillbgxoff>0</fillbgxoff>
      <fillbgyoff>0</fillbgyoff>
      <filltexture1>osd_progress_mid_red.png</filltexture1>
      <filltexture2>osd_progress_mid.png</filltexture2>
      <filltexture3>osd_progress_mid_orange.png</filltexture3>
      <markertexture>osd_progress_mid_orange.png</markertexture>
      <fillheight>16</fillheight>
      <label>#TV.Record.percent1</label>
      <label1>#TV.Record.percent2</label1>
      <label2>#TV.Record.percent3</label2>
      <labelmarkerstarts>#TV.Record.jumppoints</labelmarkerstarts>
      <labelmarkerends>#TV.Record.chapters</labelmarkerends>
      <startlabel />
      <endlabel />
      <toplabel />
      <font>font10</font>
      <textcolor>FFffffff</textcolor>
      <visible>!control.hasfocus(1237)</visible>
    </control>

Important Lines:
<markertexture>osd_progress_mid_orange.png</markertexture>
<labelmarkerstarts>#TV.Record.jumppoints</labelmarkerstarts>
<labelmarkerends>#TV.Record.chapters</labelmarkerends>



File you might want to update in the skin:
TVOSD.xml
common.expanded.tv.osd.xml
mytvFullScreen.xml
---------

UPDATE 4/3/2012
I am attaching the latest binaries, so that it is easy for people to test. Please download the zip file and use the README.txt file as a guide to installing the files.

Please let me know if you have any issues.

 

[mattjcurry]

Does anyone have any feedback, or has anyone tested the patch?

Thanks

 

[DieBagger]

I haven't tried your patch yet (don't have the time atm), but if I understand this correctly your patch will show where the ads are in a recording (not using comskip yet)? That would indeed be awesome...

However I don't think it's a good idea to introduce yet another control (tvcomskipprogress). Instead why not just improve and reuse the existing tvprogress? Also you might get more feedback if you post binaries as well.

thx for your efforts!!!

 

[mattjcurry]

DieBagger,

I have posted binaries here, sorry for cross posting, just not sure how to get traction on this feature:
https://forum.team-mediaportal.com/threads/comskip-timeline.68427/page-3#post-847655

As for your comment about an additional control, I guess my thought was that by making it a separate control, that it would give the skin devs a choice on if they wanted to use the comskip markers. I am happy to make a merge into the existing tvprogresscontrol, but since comskip is not really generic, it seemed like it should be separate.

I am open to any advice anyone has on this feature. Just looking to push it forward and hopefully get it into the main codeline.

the way that the patch works is that it looks at the jump point and chapter information already available to the gui, and it just adds colors where the commercials are based on that information. Since the gui already knows about the chapters, it was fairly easy to just set a different color in the timeline.

 

[DieBagger]

DieBagger,

I have posted binaries here, sorry for cross posting, just not sure how to get traction on this feature:
https://forum.team-mediaportal.com/threads/comskip-timeline.68427/page-3#post-847655

As for your comment about an additional control, I guess my thought was that by making it a separate control, that it would give the skin devs a choice on if they wanted to use the comskip markers. I am happy to make a merge into the existing tvprogresscontrol, but since comskip is not really generic, it seemed like it should be separate.

I am open to any advice anyone has on this feature. Just looking to push it forward and hopefully get it into the main codeline.

the way that the patch works is that it looks at the jump point and chapter information already available to the gui, and it just adds colors where the commercials are based on that information. Since the gui already knows about the chapters, it was fairly easy to just set a different color in the timeline.

My thoughts were that it would be best to add a generic way to show this kind of "sectioned" information on tvprogress. If a skin doesn't want to show it, it could still just not define textures/colors for it. It's always problematic to introduce new controls (which has been done in the past a bit too freely) because it leads to more maintainance work later on. This might also be helpful to show other information (e.g. loaded parts in streams).

We haven't really worked out the procedure for getting in new features from community devs unfortunately, but youshould probably fork MediaPortal and create a branch+pullrequest. Imo we should definately include this in 1.3.0 if the code works as expected...

 

[mattjcurry]

DieBagger, I have forked the repo in Git and created a pull request.

I rebased the Branch last night to the latest master so that it would be up to date. Not sure if that is the best way to do it, but as long as I am operating within my own private github space it seems like it would be unlikely to cause an issue.

If you would like I can create another pull request. I will also look at simply adding the functionality to the timeline in a more generic way. The way I have it programmed is that it uses a tokenized string for the jump points and then another one for the chapters. These are of course the start and end points of the chapters. I could definitely make it generic by just renaming the params to startpoints and endpoints.

There were a few things I noticed from a performance standpoint that I thought could use some improvement. For example, the background image, right image, and left image are rendered every time rather than just on the initial pass. It seems like a waste of resources to re-render elements that are not going to change. I feel the same way about the commercial markers or whatever you want to call them. Seems like it would be better to only render them on the first pass and then update any dynamic elements on every call to render.

Please let me know what you think. I really appreciate your feedback.

 

[tourettes]

Nice to see such feature being worked on - in the past there has been some requests from the users. I'll try to lobby a bit so we could have the feature includeed in 1.3.0 release.

 

[vuego]

Agreed, this is a very nice feature when you have autoskip enabled.
Would the commercial markers also update during playback since comskip supports live commercial detection? Only useful when timeshifting I guess

 

[mattjcurry]

Right now the way the g_player code works is that the jump points and chapters are loaded when the video is loaded, so I think that it would only work for recorded television and not for time shifting.

I know that comskip supports live commercial detection, but does MePo support it? If so, I would think that it would be very easy to just pass the additional parameters to the control. The control is very generic, it works just like the existing tvprogress bar, it can read any parameter out of the skin file.

It sounds like I am going to make some changes though and just integrate this functionality into the existing control.

 

[vuego]

Hey, I'm not sure if MP supports live commercial detection. I've never tried it, sorry