List Info

Thread: chapters in lavf demuxer
chapters in lavf demuxer
user name
 2008-05-29 10:02:39 
Hi,
this patch adds support for chapters to lavf demuxer.

Anton

_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
  
Re: chapters in lavf demuxer
user name
 2008-05-29 11:26:56 
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.
_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
Re: chapters in lavf demuxer
user name
 2008-05-29 11:47:14 
On Thu, May 29, 2008 at 6:26 PM, Reimar Döffinger
<Reimar.Doeffingerstud.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

Anton
_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
  
Re: chapters in lavf demuxer
country flaguser name
France		 2008-05-29 12:35:46 
Anton Khirnov wrote:

> On Thu, May 29, 2008 at 6:26 PM, Reimar Döffinger
> <Reimar.Doeffingerstud.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.

Aurel
_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
Re: chapters in lavf demuxer
user name
 2008-05-29 13:10:59 
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.Doeffingerstud.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;

for the other problem, something like this might be good
enough:
demuxer_add_chapter(demuxer, c->title ? c->title :
MSGTR_Unknown, start, end);

Greetings,
Reimar Döffinger
_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
Re: chapters in lavf demuxer
country flaguser name
France		 2008-05-29 14:10:05 
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.Doeffingerstud.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
_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
Re: chapters in lavf demuxer
user name
 2008-05-29 15:35:44 
On Thu, May 29, 2008 at 9:10 PM, Aurelien Jacobs
<aurelgnuage.org> wrote:
>
> It may be even better to factorize this inside
demuxer_add_chapter().
>
> Aurel
>

I thought demuxer_add_chapter will check for NULL itself.
well
nevermind, it does now.

Anton

_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
  
  
Re: chapters in lavf demuxer
country flaguser name
Austria		 2008-05-29 16:50:15 
On Thu, May 29, 2008 at 10:35:44PM +0200, Anton Khirnov
wrote:
> On Thu, May 29, 2008 at 9:10 PM, Aurelien Jacobs
<aurelgnuage.org> wrote:
> >
> > It may be even better to factorize this inside
demuxer_add_chapter().
> >
> > Aurel
> >
> 
> I thought demuxer_add_chapter will check for NULL
itself. well
> nevermind, it does now.
> 
> Anton

> Index: libmpdemux/demux_lavf.c
>
============================================================
=======
> --- libmpdemux/demux_lavf.c	(revision 26920)
> +++ libmpdemux/demux_lavf.c	(working copy)
>  -477,6 +477,13 
>  //    if(avfc->track       )
demux_info_add(demuxer, "track"    ,
avfc->track    );
>      if(avfc->genre    [0]) demux_info_add(demuxer,
"genre"    , avfc->genre    );
>  
> +    for(i=0; i < avfc->nb_chapters; i++) {
> +        AVChapter *c = avfc->chapters[i];

> +        uint64_t start = c->start * 1000 *
c->time_base.num / c->time_base.den;
> +        uint64_t end = c->end * 1000 *
c->time_base.num / c->time_base.den;

This should use av_rescale_q() so it wont overflow.
Also it could be vertically aligned for beauty:
        uint64_t start = c->start * 1000 *
c->time_base.num / c->time_base.den;
        uint64_t end   = c->end   * 1000 *
c->time_base.num / c->time_base.den;

and except that iam fine with the patch if everyone else is
...

[...]
-- 
Michael     GnuPG fingerprint:
9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve
life. Can you give
it to them? Then do not be too eager to deal out death in
judgement. For
even the very wise cannot see all ends. -- Gandalf

_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
Re: chapters in lavf demuxer
user name
 2008-05-30 01:30:35 
On Thu, May 29, 2008 at 11:50 PM, Michael Niedermayer
<michaelnigmx.at> wrote:
> On Thu, May 29, 2008 at 10:35:44PM +0200, Anton Khirnov
wrote:
>> On Thu, May 29, 2008 at 9:10 PM, Aurelien Jacobs
<aurelgnuage.org> wrote:
>> >
>> > It may be even better to factorize this inside
demuxer_add_chapter().
>> >
>> > Aurel
>> >
>>
>> I thought demuxer_add_chapter will check for NULL
itself. well
>> nevermind, it does now.
>>
>> Anton
>
>> Index: libmpdemux/demux_lavf.c
>>
============================================================
=======
>> --- libmpdemux/demux_lavf.c   (revision 26920)
>> +++ libmpdemux/demux_lavf.c   (working copy)
>>  -477,6 +477,13 
>>  //    if(avfc->track       )
demux_info_add(demuxer, "track"    ,
avfc->track    );
>>      if(avfc->genre    [0])
demux_info_add(demuxer, "genre"    ,
avfc->genre    );
>>
>> +    for(i=0; i < avfc->nb_chapters; i++) {
>> +        AVChapter *c = avfc->chapters[i];
>
>> +        uint64_t start = c->start * 1000 *
c->time_base.num / c->time_base.den;
>> +        uint64_t end = c->end * 1000 *
c->time_base.num / c->time_base.den;
>
> This should use av_rescale_q() so it wont overflow.
> Also it could be vertically aligned for beauty:
>        uint64_t start = c->start * 1000 *
c->time_base.num / c->time_base.den;
>        uint64_t end   = c->end   * 1000 *
c->time_base.num / c->time_base.den;
>
> and except that iam fine with the patch if everyone
else is ...
>

done

Anton

_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
  
  
Re: chapters in lavf demuxer
user name
 2008-06-04 13:57:30 
Ping? anybody there?

Anton
_______________________________________________
MPlayer-dev-eng mailing list
MPlayer-dev-engmplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-d
ev-eng
[1-10] [11-12]

about | contact  Other archives ( Real Estate discussion Medical topics )