[MPlayer-dev-eng] Improve bluray chapter support

[Olivier Rolland]

2013/4/24 Reimar Döffinger <Reimar.Doeffinger at gmx.de>:
> On Wed, Apr 24, 2013 at 07:18:03PM +0200, Olivier Rolland wrote:
>> Hello,
>>
>> Here are some patches to improve chapter support for bluray.
>
> I think you got confused by old code that was forgotten to be removed.
> -chapter should work fine with bluray.

Except that end chapter is not respected, start/end chapters are not
respected with -demuxer lavf and the -chapter option is available only
if dvdread support is enabled. In this context, "fine" seems quite
relative!

>> The 2 following patches fix issues in the current code.
>>
>> 0001-Fix-chapter-selection.patch: According to the documentation,
>> -bluray-chapter indicates the chapter to start playing from, the
>> default being 1. However, manually specifying -bluray-chapter 1 starts
>> playing from chapter 2 (-bluray-chapter 0 starts playing from chapter
>> 1). Moreover, specifying a chapter number greater than or equal to the
>> number of chapters starts playing at position -1 because the chapter
>> number is not clamped correctly. This patch fixes all that.
>
> Which obsoletes this one since the code should simply be removed.

See answer above.

>> 0002-Fix-current-chapter.patch: The current chapter is always the
>> start chapter. With this patch, it is indeed the chapter currently
>> playing.
>
> This one seems correct, assuming the function called is fast enough
> that it doesn't cause issues (if done like this it might end up
> getting called 60 times per second).

It's mainly a loop on all chapters.

>> The 3 following patches add support for a chapter to stop playing at.
>> For that, it is necessary to make stream_bluray.c use the same
>> variables for chapters as stream_dvd.c, namely dvd_chapter and
>> dvd_last_chapter. It then has to use the same function to parse
>> chapter ranges.
>>
>> 0003-Extract-chapter-parsing.patch: This patch moves the chapter
>> parsing function from stream_dvd.c/h to stream.c/h.
>
> This makes little sense without at the same time moving the parameter
> outside the ifdef.

It was in an ifdef, it's still in the ifdef.

>> 0004-Add-last-chapter-support.patch: This patch replaces bluray
>> specific chapter variables with dvd variables, makes use of the same
>> command line option for dvd and bluray chapter selection, keeps the
>> bluray specific command line option for compatibility sake and adds
>> full support for end chapter of bluray. It also updates the man page.
>
> That should be obsolete. The man page is wrong, the end chapter
> is supported for everything already, including dvd, bluray, matroska...
>
>
>> 0005-Remove-bluray-chapter.patch: This patch removes the bluray
>> specific command line option for chapter selection from the code and
>> the man page.
>
> I do not see any man-page change. Also it would be better to at least
> make it print an error message pointing out that -chapter should be used
> instead of just failing.

New patch attached.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-Remove-bluray-chapter.patch
Type: application/octet-stream
Size: 1559 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20130424/f2b66aa1/attachment-0001.obj>