[MPlayer-dev-eng] [PATCH] mkv chapters and linked segments

[compn]

On Wed, 28 May 2008 21:27:51 +0200, "Fredrik Nilsson" <esserex at gmail.com> wrote:
>On Wed, May 28, 2008 at 2:17 PM, compn <tempn at twmi.rr.com> wrote:
>> On Wed, 28 May 2008 11:27:29 +0200, "Fredrik Nilsson" <esserex at gmail.com> wrote:
>>>Hi.
>>>This is a patch that loads mkv chapters from external files.
>>>It does this by loading the extra files into separate tracks and switches to
>>>them when the chapter changes. Changes in mplayer.c and demuxer.c makes
>>>mplayer switch chapters automatically at end of chapter.
>>>
>>>To switch tracks I use a function in mplayer.c to reset decoders,
>>>an ugly hack but I see no simple solution to this.
>>
>> wouldnt this make sense in case the next track/chapter was a different codec?
>
>It does make sense as different codecs/parameters is a common case,
>but calling the function from demux_mkv.c is less pretty.
>>
>>>
>>>I have looked into using libavformat but I see no way of implementing chapters
>>>or decoder change without changing the libavformat interface.
>>>
>>>Thoughts?
>>>
>>>Fredrik
>>
>> huge patch. smaller patches are reviewed more quickly
>>
>
>Yeah, unfortunately I can't think of any spot there I can split the
>patch and still have
>both pieces compile, it really only does one thing already.
>
>> dvd and mkv chapter support was merged a few weeks/months ago
>> do your changes affect dvd chapter support at all? does -chapter 3-5 still work? (play chaps 3-5 then stop)
>
>The code for dvd and mkv are separate so the dvd code should still work.
>I missed the chapter select, it works as before with the exception that if you
>start at a chapter in a different file than the first chapter it crashes. Fixed.
>
>>
>> assuming any of this is accepted, should linked chapters be disabled by default? it looks like you got it working for mencoder, where encoding extra things by default might not be the right/consistent thing to do.
>
>I would like to have it enabled by default, the files are meant to be
>played this way after all.
>I don't think there is a security issue because it is not possible to
>specify any filenames only
>segment ids. It does read the file directory but so does the subtitle
>loader and that is enabled by
>default. It only does this if there actually is linked chapters so it
>shouldn't affect start up performance
>on normal mkv files.
>
>mencoder is a different story though, even if there are no linked
>chapters, chapters can still overlap,
>be out of order or skip parts of a file. Even chapters should probably
>be disabled by default.
>There are no changes to mencoder so currently, it will just go from
>chapter 1 to eof.
>The way things are now this should be completely disabled for mencoder
>as it doesn't follow
>the chapters anyway.
>
>While it would be nice to have it enabled by default the only way I
>see this working right now
>is if it is disabled by default and have an mplayer only option to enable.

oh, just disabled for mencoder then. on by default sounds good for mplayer.

>Should I send a new patch with this or wait until more comments?

you can send an updated patch in this thread. please wait up to two weeks for review, we are short on devels and free time.

i think -chapter in manpage also has to be updated to say DVD and MKV ONLY.

also i notice that -identify lists dvd chapters. would it be possible to do this for mkv too? it might already do it, i just dont have any mkv chapter samples handy.

-compn