<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta http-equiv="Content-Style-Type" content="text/css" />
<meta name="generator" content="pandoc" />
<title></title>
<style type="text/css">
code{white-space: pre-wrap;}
span.smallcaps{font-variant: small-caps;}
span.underline{text-decoration: underline;}
div.column{display: inline-block; vertical-align: top; width: 50%;}
</style>
</head>
<body>
<p>Hi Zhao,</p>
<p>On 2018-09-04 14:57, Zhao Zhili wrote:</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> ---
A test sample:
https://drive.google.com/open?id=11kQP2OrbFyPnmbH9BPS__kJzbsXKmAQT
I totally understand it may not worth to support such broken files. In
this case, it looks like a small effort. The time takes by Fix() is
less than 200 us on my PC with the sample.
modules/demux/subtitle.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/modules/demux/subtitle.c b/modules/demux/subtitle.c
index 9af186e..5e64ba4 100644
--- a/modules/demux/subtitle.c
+++ b/modules/demux/subtitle.c
@@ -696,7 +696,10 @@ static int Open ( vlc_object_t *p_this )
fmt.subs.cc.i_reorder_depth = -1;
}
else
+ {
+ Fix( p_demux );
es_format_Init( &fmt, SPU_ES, VLC_CODEC_SUBT );
+ }</code></pre>
</blockquote>
<p>I don’t know how many samples there are in the wild where the order of individual subtitles are not in increasing timestamp order, and theoretically sorting the parsed entities is not a big thing (as you have mentioned), but the location for this fix hurts my eyes a little.</p>
<p>As it is definitely possible to detect out-of-order subtitles during parsing, <em>if</em> we want such an explicit invocation of <code>Fix</code> I vote that one flags the necessity of that during parsing, and then invoke the function later if that is needed.</p>
<p>Adding a call to <code>Fix</code> where your patch does also results in a rather weird looking <em>if-else-tree</em> (given that a call to <code>Fix</code> already exists in there).</p>
<p>While you are at it, you could also fix the issue of <code>p_sys->i_length</code> being inaccurate, as it is populated with the timestamp of the last subtitle, even if said input is later <code>Fix</code>ed (potentially resulting in the last subtitle not at all being the last subtitle).</p>
<blockquote style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;color:#500050">
<pre><code> /* Stupid language detection in the filename */
char * psz_language = get_language_from_filename( p_demux->psz_filepath );</code></pre>
</blockquote>
<p>Best Regards, Filip</p>
</body>
</html>