[MPlayer-cvslog] r27550 - in trunk: libmpdemux/demux_mkv.c mpcommon.c

[Reimar Döffinger]

On Mon, Sep 08, 2008 at 11:26:22PM +0200, uau wrote:
> Author: uau
> Date: Mon Sep  8 23:26:22 2008
> New Revision: 27550
> 
> Log:
> Revert bad changes to SSA/ASS subtitle packet format
> 
> The following commits are reverted partially or completely:
> "a valid ASS line contains 9 ',' before actual text"
> "demux_mkv: output correctly formated ASS packets"
> "libass: add a new ass_process_data() to process demuxed subtitle packets"
> 
> These commits converted the internal representation of SSA/ASS
> subtitle packets from the format used by Matroska to a custom format
> where each packet has contents exactly matching one line in complete
> SSA script files. AFAIK no files natively use such a format for muxed
> subtitles. The stated reason for this change was to use a format that
> could in principle be muxed into a maximal number of containers. SSA
> subtitles do not have an implicit duration so both start time and
> duration or end time need to be specified explicitly; the new format
> moved timing information inside the codec packet data so it could be
> muxed without modification into containers that can represent only
> start time at the container level. However such a change is wrong from
> the viewpoint of program architecture. Timing information belongs to
> the demuxer level, but these commits moved not only the duration but
> also the authoritative value of the start time to inside the codec
> data. Additionally the new format lost the value of the Matroska
> ReadOrder field which is used by MPlayer.
> 
> This commit changes the internal packet format back to that used by
> Matroska and makes the internal Matroska demuxer output that format
> again. Libavformat still outputs the "new" format; it could be
> converted back to the Matroska format in demux_lavf.c, but I'm not
> adding that code at least yet. The current lavf code has similar
> problems as the reverted code in MPlayer, and it also currently fails
> to provide any way to access the value of the ReadOrder field. I hope
> that the lavf side will be improved; if it isn't conversion can be
> added later. For now I'll make MPlayer default to the internal Matroska
> demuxer instead of the lavf one in a separate commit.

That is not an acceptable commit message. If it needs such a long
explanation, explain it on the mailing list and reference the mail, but
commit messages are supposed to be messages, not whole reference books.