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

[Alexandre Janniaux]

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 */

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.

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