[MPlayer-dev-eng] [PATCH] Chapter support improvement

[Andrew Calkin]

2007/12/11, Ulion <ulion2002 at gmail.com>:
>
> By now I keep the property named 'chapter', it make me feel better.
> About what you said in the menu directly use chapter property, using
> property is a little complex than directly use the seek_chapter
> command, so I keep to use the command.
>
> Here's the updated patches:
> demuxer: add a functiond demuxer_chapter_time to get chapter start and
> end time if available. And a little chapter name format adjust.
> property & command: property chapter use same cmd id with command
> seek_chapter and handle all requests to command seek_chapter, old
> command handler for MP_CMD_SEEK_CHAPTER removed.
> libmenu: add a keybinding entry for chapsel in menu.conf. support
> display start time info in chapter menu and little custom format
> supported.

I had a quick look through:

osdmenu_chapter3.diff:

in libmenu/menu_chapsel.c, is:

+#include "stream/stream.h"

required anymore? It doesn't seem you use it, so probably legacy from
when stream was accessed directly.

In fmt_replace(), why is p declared "const char *p"? I cannot see why
it is declared const, and also not sure if it is ok to call it const?
I guess someone more knowledgable can help with an explanation.

Minor nit, in fill_menu(), you ensure that start_time >=0, so
timestr[9] should suffice, right? Perhaps a warning if hour > 99, as
it would indicate a bizarre file where the chapter information it
returns for the menu is bogus.

In read_cmd(), even though cmdbuf[26] is quite large still no overrun
checking is performed. Perhaps snprintf(cmdbuf, 25...) and
cmdbuf[25]='\0'; to ensure that a bogus file chapter index doesn't
cause a buffer overrun. I didn't trace all the way through the code,
so I'm not sure if it's an issue or not.

//Andrew