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

[Aurelien Jacobs]

Uoti Urpala wrote:

> On Sat, 2008-09-06 at 00:55 +0200, Aurelien Jacobs wrote:
> > 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.
> 
> But it SHOULD get out of the container. That is information stored in
> the file, and the application using the demuxer should be able to access
> it. If it can't that is a deficiency in the demuxer.

Why ???
There are many more information read by the demuxer that are never
exported to the outside world. Because many information are dedicated
to the demuxer itself, so that it can properly extract or relate other
data.
And ReadOrder is exactly one of those information dedicated to the
demuxer itself. Its goal is to allow the demuxer to re-order sub
packets in their original order. This is only useful for specific
demuxers aiming at extracting a bit-exact copy of the original ass
file. But this is never useful for decoding or re-muxing, and honestly,
I have no idea why one would want to produce a bit-exact copy of
original file.

BTW, if you really think this information should get out of the
demuxer, could you propose a way to do it ?
I don't think that putting it in a separate field in AVPacket is an
acceptable idea, because this is a matroska specific field.
Another possibility would be to write this in the AVPacket.data
itself, thus producing a new kind of bitstream which is *not* ASS,
so it should get a new codec name, a new CODEC_ID, and be decoded
by an appropriate decoder (ie. not a standard ASS decoder). Sounds
like overkill for no functional improvement.
Any other idea ?

Anyway, even if the matroska demuxer would output ReadOrder, this
wouldn't change the fact that libass must be able to decode ASS
packets coming from a demuxer, without using this ReadOrder
information. Because other containers (nut, etc...) just won't
store ReadOrder when muxing standard ASS.

> > 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.
> 
> There is, they can be needed after seeking. Keeping them in memory
> ensures that you see all subtitles if you seek within an already-played
> file part.

Which is a very inconsistent behavior. Because it won't work the same
depending on whether you seek to an already played part or not.
My patch fix this inconsistency.

> >  The demuxer will send them again as soon as they are needed.
> 
> No it won't if the packet occurred before the seeked-to position.

Sure, but libass buffering won't change this when seeking to a
not-already-seen part, and it also won't change this when not using
libass to display subs.

If you think getting the sub which should be displayed just after
a seek is really important, then the only way to fix it is to
ensure the demuxer output the relevant subtitle packets after
a seek (patch welcome). No kind of buffering can fix this.

Moreover, if you think that some buffering could have any use,
it should be done outside of libass, so that it benefit every
subtitle decoders, and more importantly, it shouldn't have unbounded
growing memory usage.
Yes, right now, libass uses unbounded memory size. If libass is used
to render a continuous network stream doing heavy use of subtitles
and running for months, it would certainly exhaust memory...
My patch also fixes this.

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

Hopefully, you're not libass maintainer.

Aurel