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

[Evgeniy Stepanov]

On Tuesday 16 September 2008 03:48:36 Aurelien Jacobs wrote:
> 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 ?

Is there a better place? It's more closely related to libass than to anything 
else in mplayer.

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

OK.