[MPlayer-dev-eng] [PATCH]chapters in lavf demuxer
Aurelien Jacobs
aurel at gnuage.org
Thu May 29 21:10:05 CEST 2008
Reimar Döffinger wrote:
> On Thu, May 29, 2008 at 07:35:46PM +0200, Aurelien Jacobs wrote:
> > Anton Khirnov wrote:
> > > On Thu, May 29, 2008 at 6:26 PM, Reimar Döffinger
> > > <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> > > > On Thu, May 29, 2008 at 05:02:39PM +0200, Anton Khirnov wrote:
> > > >> Hi,
> > > >> this patch adds support for chapters to lavf demuxer.
> > > >>
> > > >> Anton
> > > >
> > > >> Index: libmpdemux/demux_lavf.c
> > > >> ===================================================================
> > > >> --- libmpdemux/demux_lavf.c (revision 26920)
> > > >> +++ libmpdemux/demux_lavf.c (working copy)
> > > >> @@ -477,6 +477,15 @@
> > > >> // if(avfc->track ) demux_info_add(demuxer, "track" , avfc->track );
> > > >> if(avfc->genre [0]) demux_info_add(demuxer, "genre" , avfc->genre );
> > > >>
> > > >> + if(avfc->nb_chapters) {
> > > >> + uint64_t start, end;
> > > >> + for(i=0; i < avfc->nb_chapters; i++) {
> > > >> + start = avfc->chapters[i]->start * 1000 * avfc->chapters[i]->time_base.num / avfc->chapters[i]->time_base.den;
> > > >> + end = avfc->chapters[i]->end * 1000 * avfc->chapters[i]->time_base.num / avfc->chapters[i]->time_base.den;
> > > >> + demuxer_add_chapter(demuxer, avfc->chapters[i]->title, start, end);
> > > >> + }
> > > >> + }
> > > >
> > > > The "if" is not necessary and the declaration and assignment of start
> > > > and end can be merged.
> > >
> > > ok
> >
> > demuxer_add_chapter() will segfault when avfc->chapters[i]->title is NULL.
>
> Since it then needs another version anyway: IMO it would be nicer to use
> a temporary variable, something like
> AVChapter *c = avfc->chapters + i;
Agree.
> for the other problem, something like this might be good enough:
> demuxer_add_chapter(demuxer, c->title ? c->title : MSGTR_Unknown, start, end);
It may be even better to factorize this inside demuxer_add_chapter().
Aurel
More information about the MPlayer-dev-eng
mailing list