Hi Filip,<br><br>On 9/20/18 8:59 PM, Filip Ros¨¦en wrote:<br>> Hi Zhao,<br>> <br>> On 2018-09-04 14:57, Zhao Zhili wrote:<br>> <br>>     |--- A test sample:<br>>     https://drive.google.com/open?id=11kQP2OrbFyPnmbH9BPS__kJzbsXKmAQT I<br>>     totally understand it may not worth to support such broken files. In<br>>     this case, it looks like a small effort. The time takes by Fix() is<br>>     less than 200 us on my PC with the sample. modules/demux/subtitle.c<br>>     | 3 +++ 1 file changed, 3 insertions(+) diff --git<br>>     a/modules/demux/subtitle.c b/modules/demux/subtitle.c index<br>>     9af186e..5e64ba4 100644 --- a/modules/demux/subtitle.c +++<br>>     b/modules/demux/subtitle.c @@ -696,7 +696,10 @@ static int Open (<br>>     vlc_object_t *p_this ) fmt.subs.cc.i_reorder_depth = -1; } else + {<br>>     + Fix( p_demux ); es_format_Init( &fmt, SPU_ES, VLC_CODEC_SUBT ); + }|<br>> <br>> I don¡¯t know how many samples there are in the wild where the order of <br>> individual subtitles are not in increasing timestamp order, and <br>> theoretically sorting the parsed entities is not a big thing (as you <br>> have mentioned), but the location for this fix hurts my eyes a little.<br>> <br>> As it is definitely possible to detect out-of-order subtitles during <br>> parsing, /if/ we want such an explicit invocation of |Fix| I vote that <br>> one flags the necessity of that during parsing, and then invoke the <br>> function later if that is needed.<br>> <br>> Adding a call to |Fix| where your patch does also results in a rather <br>> weird looking /if-else-tree/ (given that a call to |Fix| already exists <br>> in there).<br>> <br><br>I contacted with the authors of the sample. They made .srt subtitles <br>from .ass with a tool which I think is unpopular. They prefer .ass <br>format and care little about .srt format. So never mind.<br><br>> While you are at it, you could also fix the issue of |p_sys->i_length| <br>> being inaccurate, as it is populated with the timestamp of the last <br>> subtitle, even if said input is later |Fix|ed (potentially resulting in <br>> the last subtitle not at all being the last subtitle).<br><br>Done.<br>https://mailman.videolan.org/pipermail/vlc-devel/2018-September/121236.html<br><br>> <br>>     |/* Stupid language detection in the filename */ char * psz_language<br>>     = get_language_from_filename( p_demux->psz_filepath );|<br>> <br>> Best Regards, Filip<br>> <br>> <br>> _______________________________________________<br>> vlc-devel mailing list<br>> To unsubscribe or modify your subscription options:<br>> https://mailman.videolan.org/listinfo/vlc-devel<br>> <br><br>