[MPlayer-dev-eng] [PATCH] packet pts as ASS start time

[Aurelien Jacobs]

Evgeniy Stepanov wrote:

> On Monday 15 September 2008 00:00:13 Aurelien Jacobs wrote:
> > Evgeniy Stepanov wrote:
> > > On Sunday 14 September 2008 21:55:16 Aurelien Jacobs wrote:
> > > > Attached patch adds ability for the demuxers to override the ASS events
> > > > start time with the pts read from the container when sending new ASS
> > > > event with ass_process_data().
> > >
> > > Well, what about duration? Are you going to read start pts from
> > > demux_packet_t::pts and end pts from the packet data?
> >
> > Kind of.
> > To be clear, libass currently don't read end_pts from the packet data.
> > What it read is end_pts-start_pts (IOW: display_duration).
> > My patch override start_pts after duration was calculated.
> > And I provided no way to override duration, because the demuxers don't
> > have to know the duration (which is generally not stored in containers).
> > That was the whole purpose of this controverted change: move the
> > duration information to the packet data level (as for any other codec)
> > instead of keeping it at the container level (which is useless for
> > almost any kind of codec, and thus not possible in almost any container).
> >
> > > Anyway, it is better to return the index of the new event from
> > > ass_process_data and ass_process_chunk, and override any field in the
> > > calling function.
> >
> > That was my first idea. But it has a few problems:
> >  - It forces calling apps to mess-up directly with libass internal
> >    structures, which is somewhat ugly, and can easily lead to
> >    insidious bugs.
> 
> This structures are part of the public interface.

I know this. I just find it a bit ugly, but that's OK.

> They are already being  messed with in ass_mp.c, which is not part
> of libass.

May I suggest to move this file out of the libass directory then ?

> >  - The string passed to ass_process_data() could very well contain
> >    several ASS events lines (especially several events starting at the
> >    same pts). This makes it even more painful to return indexes of the
> >    new events and to fix the timestamp of all those events.
> >
> > Do you want me to attempts such an implementation ? I think it would
> > be ugly, but that's your call.
> 
> Let's assume that new events are always added to the end of the array. Of 
> course this should be mentioned in comments. Then it's enough to return the 
> number of new events. This will even allow defragmentation of the events array 
> in ass_process_data if it is ever needed.

OK. New patch attached, following your advise.

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ass_nb_events.diff
Type: text/x-patch
Size: 2382 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20080916/7d2f4915/attachment.bin>