[vlc-devel] [PATCH] subtitle: fix AQT subtitle parsing

[Steve Lhomme]

On 2021-01-18 17:05, Alexandre Janniaux wrote:
> Hi,
> 
> LGTM if the nit below is solved, but it probably needs
> comment in the code too, as shown by the comment:
> 
>      /* 1/10th of second ? Frame based ? FIXME */

That line is for PJS (which I didn't fix). It's not related to this patch.

> and:
> 
>      int t; /* Time */
> 
>> -                TextPreviousLine( txt );
> 
> Is the removal of this line voluntary? It seems to remove
> the initial intent in this code specified by the comment
> right above:
> 
>      /* We have been too far: end of the subtitle, begin of next */
> 
> And there is not explanation of this removal in the commit
> message.

That's one of the fix, with the wrong use of frame numbers. There is a 
start time and an end time, as with proper subtitles. The discarding of 
the line with the second time is bogus.

> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> On Mon, Jan 18, 2021 at 03:46:59PM +0100, Steve Lhomme wrote:
>> The numbers we read are frame numbers. There's a start and stop frame number.
>> (based on http://web.archive.org/web/20070210095721/http://www.volny.cz/aberka/czech/aqt.html
>> and libavformat)
>> ---
>>   modules/demux/subtitle.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/modules/demux/subtitle.c b/modules/demux/subtitle.c
>> index 968a356a596..f117bda2b76 100644
>> --- a/modules/demux/subtitle.c
>> +++ b/modules/demux/subtitle.c
>> @@ -1599,18 +1599,16 @@ static int ParseAQT(vlc_object_t *p_obj, subs_properties_t *p_props, text_t *txt
>>           /* Data Lines */
>>           if( sscanf (s, "-->> %d", &t) == 1)
>>           {
>> -            p_subtitle->i_start = (int64_t)t; /* * FPS*/
>> -            p_subtitle->i_stop  = -1;
>> -
>>               /* Starting of a subtitle */
>>               if( i_firstline )
>>               {
>> +                p_subtitle->i_start = t * p_props->i_microsecperframe;
>>                   i_firstline = 0;
>>               }
>>               /* We have been too far: end of the subtitle, begin of next */
>>               else
>>               {
>> -                TextPreviousLine( txt );
>> +                p_subtitle->i_stop  = t * p_props->i_microsecperframe;
>>                   break;
>>               }
>>           }
>> --
>> 2.29.2
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>