[MPlayer-dev-eng] Improve bluray chapter support

[Reimar Döffinger]

On Wed, Apr 24, 2013 at 09:32:26PM +0200, Olivier Rolland wrote:
> 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

Can you give an example? The code certainly is there!

> and the -chapter option is available only
> if dvdread support is enabled.

Yes, that definitely is an issue. However part of my complaint is
that you change it to be available only for DVD and bluray, which
is almost still as wrong...

> In this context, "fine" seems quite
> relative!

Working "fine" as in the design, it is definitely quite buggy in the
implementation.
But I very much think the solution to a buggy implementation is
to fix the implementation, not to introduce a second one/fix a secondary
one.

> >> 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.

My point is the change doesn't serve any purpose unless and until
you also move the -chapter option, or in other words this just
should be done in the same patch.
And that change makes a lot of sense irrespective of the bluray
issues.
Of course I can modify the patch accordingly myself if you don't feel
like sending a new patch for such minor "complaints".