[MPlayer-dev-eng] [PATCH] libass: fix parsing of tracks extracted from containers

[Aurelien Jacobs]

Evgeniy Stepanov wrote:

> On Friday 05 September 2008 19:05:59 Aurelien Jacobs wrote:
> > Evgeniy Stepanov wrote:
> > > On Friday 05 September 2008 03:49:48 Aurelien Jacobs wrote:
> > > > Hi,
> > > >
> > > > Currently, libass has a ass_process_chunk() function which is used to
> > > > process packets containing one subtitle line, extracted from the
> > > > subtitle track of a container. This function was designed to parse
> > > > non-compliant, butchered ASS line comming out of the old, deprecated
> > > > matroska demuxer. This is the only demuxer producing such broken
> > > > ASS packet.
> > > > The attached patch adds a new ass_process_data() which will process
> > > > valid ASS data, and uses it instead of ass_process_chunk(). This will
> > > > fix playback of ASS tracks comming from lavf.
> > > > After this patch is applied I guess ass_process_chunk() could be
> > > > removed.
> > > > Test sample: http://samples.mplayerhq.hu/Matroska/haruhi.mkv
> > > >
> > > > Is this OK with you, Evgeniy ?
> > >
> > > It's fine. A small change: i've copied a debug line from
> > > ass_process_chunk to the new function.
> >
> > OK. Applied.
> 
> Sorry, I did not think (and test) enough. Try playing over some subtitle 
> several times, it will multiply.

Dohhh ! I wonder how I did to not notice this before... sorry...

> To prevent this, subtitles in mkv have a field 
> named ReadOrder. It's value is lost somewhere in lavf mkv demuxer.

The ReadOrder field is not part of any ASS spec. It's only part of
Matroska internal format, probably won't be present when muxed in
any other kind of container.
Moreover, the only purpose of this ReadOrder field is to be able
to reconstruct the exact original ASS file out of the Matroska file,
by re-ordering the ASS events line in their original order.
Now the matroska demuxer correctly output a spec compliant ASS
event line, which means the ReadOrder don't get out of the
matroska container.

Anyway, this is quite easy to solve. Currently, libass stores all
the events line it receives, forever. When the events are received
out of demuxed packets, there's no reason to keep them in memory
forever. The demuxer will send them again as soon as they are needed.
So the simple attached patch will just free every events as soon
as they are not displayed anymore (if they come from a demuxed packet).
Is this OK ?

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