Chapter support in Matroska (MKV) video containers?
#31
elupus Wrote:Well, i initially didn't add it that way. Up/Down skips chapters instead of doing large seeks when file supports chapter.

I've now also added so the >| works fine for non dvd's with chapters, which didn't work before.

Is this functionality in the xbox build too, or just the Linux/Mac/Windows builds?
Reply
#32
waldo22 Wrote:Is this functionality in the xbox build too, or just the Linux/Mac/Windows builds?
This chapter code only appears to be in the DVDPlayer core and support for MKV chapters is only in the ffmpeg demuxer, so unless Xbox builds don't use the mplayer any more there's no chance of it being in Xbox.

To change topic: I'm really shaky on how things should be coded but here's a go at a patch to add support for chapter names for mkv files, which can be accessed through skins via the player.chaptername $INFO tag. I welcome comments from developers
Link to diff vs r13974 since sf's tracker is down

Things I'm not sure if I did the XBMC Way:
-- When adding a GUI Info constant (i.e. PLAYER_CHAPTERNAME) should it go with the other constants related to chapter and move the rest down, or just at the end like I did?
-- Should IPlayer::GetChapterName() return a string vs modify a passed one? Other functions in the class seem to work with the passed string, so I copied that.

Tested on the Win32 and Linux builds on DVDs, mkvs with chapters, mkvs with no chapters, mp4s with chapters and without (although chapters do not work at all in mp4), and avi files.
Reply
#33
Looks good to me. The only thing I can think of that could go wrong is checking that GetChapter() returns 0 whenever there is no chapters (i.e. ensuring that chapterIdx is actually in the set [1 .. m_pFormatContext->chapters.size()])

Assuming that is OK, it looks fine. Mind checking?

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#34
Oh absolutely. The DVDDemuxFFmpeg::GetChapters() returns 0 if there's no m_pFormatContext, if m_iCurrentPts is DVD_NOPTS_VALUE, if the AVContext has no chapters, or if the PTS value is outside the range of all the chapters. I rely on GetChapter() to do all the work and return 0 if there's anything wrong, which makes the patch pretty small.

Switching gears slightly, for anyone who doesn't want to wait for this to get properly integrated into skins, here's the change for PM3 I used for testing
Code:
Index: skin/Project Mayhem III/PAL/DialogFullScreenInfo.xml
===================================================================
--- skin/Project Mayhem III/PAL/DialogFullScreenInfo.xml    (revision 13974)
+++ skin/Project Mayhem III/PAL/DialogFullScreenInfo.xml    (working copy)
@@ -268,8 +268,8 @@
                <description>Chapter Pos No</description>
                <width>200</width>
                <posx>680</posx>
-                <posy>80</posy>
-                <label>$INFO[player.chapter,$LOCALIZE[21396] ]$INFO[player.chaptercount, / ]</label>
+                <posy>60</posy>
+                <label>$INFO[player.chaptername][CR]$INFO[player.chapter,$LOCALIZE[21396] ]$INFO[player.chaptercount, / ]</label>
                <visible>player.chaptercount + !Control.HasFocus(2) + !Control.HasFocus(61)</visible>
                <align>right</align>
                <font>font10</font>
Index: skin/Project Mayhem III/PAL/DialogSeekBar.xml
===================================================================
--- skin/Project Mayhem III/PAL/DialogSeekBar.xml    (revision 13974)
+++ skin/Project Mayhem III/PAL/DialogSeekBar.xml    (working copy)
@@ -200,7 +200,7 @@
            <posy>60</posy>
            <align>center</align>
            <aligny>center</aligny>
-            <label>$INFO[player.chapter,$LOCALIZE[21396] ]$INFO[player.chaptercount, / ]</label>
+            <label>$INFO[player.chaptername,,[CR]]$INFO[player.chapter,$LOCALIZE[21396] ]$INFO[player.chaptercount, / ]</label>
            <visible>player.chaptercount</visible>
            <align>right</align>
            <font>font13</font>
Reply
#35
Excellent - thanks for saving me the legwork of looking through the code myself Smile

Will get it in to SVN later tonight if someone else doesn't beat me to it.

Cheers,
Jonathan
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.


Image
Reply
#36
CapnBry Wrote:This chapter code only appears to be in the DVDPlayer core and support for MKV chapters is only in the ffmpeg demuxer, so unless Xbox builds don't use the mplayer any more there's no chance of it being in Xbox.

Capn,

This thread has quickly gone over my head, but could you explain this statement?

Do you mean "DVDPlayer core" as opposed to the DVDPlayer that is in all branches?

Isn't the ffmpeg demuxer used for DVDPlayer for the xbox build as well?

Is it a lot of work to compile this version of DVDPlayer for xbox as well?

Clearly, DVDPlayer is now the player of choice for all platforms.

I can finally play Apple's h.264 product demos on my xboxex perfectly in DVDPlayer, when they jerk like crazy in mplayer.

Thanks for the work you're doing for the other platforms, even if I don't understand :o

-Wes
Reply
#37
Actually waldo22, checking the code I may or may not be completely accurate because the linuxport branch and trunk (which I believe the xbox builds still come from) have different code. I have not built a xbox build since my xbox is experiencing the Autumn of its years and doesn't work so well any more.

Ok so when xbmc wants to play something it needs a player, and that top level object is called the Player Core. XBOX has DVDPlayer, MPlayer, PAPlayer and maybe ModPlayer. By default xbox hardware prefers the mplayer core to the dvdplayer core. And I've just realized that you've already realized that you can override the default core with the "Play using..." context menu option sooooo you know all this already. But to finish the concept for academic purposes: then player core picks a demux, and the demux picks the codecs.

I guess the question is does the >| button chapter skip, right? Yeah it was added to trunk's ffmpeg demux in r13293 (May 28) and DVDPlayer in r13533 (Jun 11). It should be bound to the "skipplus" and "skipminus" actions in your remote map.
Reply
#38
Any update on when this can get into SVN? [url=http://capnbry.net/~bmayland/fi/code/chaptername_support.diff]Link to patch again[url] for chapter name support in MKV files.
Reply
#39
Oh dammit I should have previewed first. This is actually that link
Reply
#40
Maybe you could try to catch elupus on IRC for discussion Wink
http://wiki.xbmc.org/?title=Appendix_D:_...ct_methods
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#41
elupus,

Do you think you'll have time to roll this into the xbox build?

I'd send beer, but I think all the good stuff is already on your side of the Atlantic.

-Wes
Reply
#42
Wink 
CapnBry Wrote:Oh dammit I should have previewed first. This is actually that link
@CapnBry, please create a new patch ticket on Trac for it.
http://trac.xbmc.org

@waldo22, you should also create new ticket on trac for your feature request.
Always read the XBMC online-manual, FAQ and search the forum before posting.
Do not e-mail XBMC-Team members directly asking for support. Read/follow the forum rules.
For troubleshooting and bug reporting please make sure you read this first.
Reply
#43
I'll put a new patch against head in trac tomorrow morning. I would have done it when trac went back up but there was a conflict and I didn't have the time to look at it. I was afraid of making a sloppy patch so I held off.
Reply
#44
Added to trac #4414
Reply
#45
CapnBry Wrote:Added to trac #4414

CapnBry, looks like this is done now. (Thanks Jezz_X!)

Thanks for all your work on this Elupus, Jezz_X, FFMPEG guys, and others.

I'll look for the next T3ch build. (next week maybe).

Wes
Reply

Logout Mark Read Team Forum Stats Members Help
Chapter support in Matroska (MKV) video containers?1