[vlc-commits] [Git][videolan/vlc][master] 4 commits: demux: subtitle: add VLC_TICK_0 to timings
Jean-Baptiste Kempf (@jbk)
gitlab at videolan.org
Mon Aug 28 11:54:42 UTC 2023
Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC
Commits:
f97cfab4 by Alexandre Janniaux at 2023-08-28T11:32:14+00:00
demux: subtitle: add VLC_TICK_0 to timings
Currently, 0 is mapped to VLC_TICK_INVALID. Since the timings will be
reported as-is to the core afterwards, they need to start at VLC_TICK_0.
It won't change the duration, given that i_start - i_stop removes the
constant VLC_TICK_0 offset.
- - - - -
6f6c52d7 by Alexandre Janniaux at 2023-08-28T11:32:14+00:00
demux: subtitle: add subtitle_plugin_test
The test is embedded into the subtitle.c file because it tests some
internal functions parsing the input subtitle format. For now, only
VLC_TICK_0 origin is tested, no specific example, but invalid strings
are also tested.
- - - - -
c76b8a42 by Alexandre Janniaux at 2023-08-28T11:32:14+00:00
demux: subtitle: refactor timing parsing
Refactor the parsing of timing values so that it doesn't allocate
memory. Indeed, we just need to parse the input string to locate where
the delimiter is, and call the parsing of timing values on each side of
the arrow.
We also use a length validation defensively, despite the current
impossibility to parse characters like `-->` in the sscanf given the
current format, to ensure that future addition here would not try to
parse additional characters and to ensure that future usage of the
function on strings that were not splitted by such character before
(eg. 0:0:0, 0 limited on 6 characters) don't impact the parsing.
- - - - -
c74180ad by Alexandre Janniaux at 2023-08-28T11:32:14+00:00
demux: subtitle: improve parsing of sized string
As mentioned in previous commit, it's possible for the value parsing
function to be called on a shorter string of a longer valid format,
which should not be parsed in that case.
The problem didn't arise previously because the function could only be
called on null-terminated strings and those strings were only built from
the split of ` --> ` into two strings.
- - - - -
2 changed files:
- modules/demux/Makefile.am
- modules/demux/subtitle.c
Changes:
=====================================
modules/demux/Makefile.am
=====================================
@@ -71,6 +71,12 @@ libsubtitle_plugin_la_SOURCES = demux/subtitle.c
libsubtitle_plugin_la_LIBADD = $(LIBM)
demux_LTLIBRARIES += libsubtitle_plugin.la
+subtitle_plugin_test_SOURCES = demux/subtitle.c
+subtitle_plugin_test_CPPFLAGS = $(AM_CPPFLAGS) -DENABLE_TEST
+subtitle_plugin_test_LDADD = $(LIBM)
+check_PROGRAMS += subtitle_plugin_test
+TESTS += subtitle_plugin_test
+
libty_plugin_la_SOURCES = demux/ty.c codec/cc.h \
demux/mpeg/pes.h demux/mpeg/timestamps.h
demux_LTLIBRARIES += libty_plugin.la
=====================================
modules/demux/subtitle.c
=====================================
@@ -2,10 +2,12 @@
* subtitle.c: Demux for subtitle text files.
*****************************************************************************
* Copyright (C) 1999-2007 VLC authors and VideoLAN
+ * Copyright (C) 2023 Videolabs
*
* Authors: Laurent Aimar <fenrir at via.ecp.fr>
* Derk-Jan Hartman <hartman at videolan dot org>
* Jean-Baptiste Kempf <jb at videolan.org>
+ * Alexandre Janniaux <ajanni at videolabs.io>
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published by
@@ -1119,24 +1121,33 @@ static int ParseSubRipSubViewer( vlc_object_t *p_obj, subs_properties_t *p_props
* Parses SubRip timing value.
*/
static int subtitle_ParseSubRipTimingValue(vlc_tick_t *timing_value,
- const char *s)
+ const char *s, size_t length)
{
int h1, m1, s1, d1 = 0;
- if ( sscanf( s, "%d:%d:%d,%d",
- &h1, &m1, &s1, &d1 ) == 4 ||
- sscanf( s, "%d:%d:%d.%d",
- &h1, &m1, &s1, &d1 ) == 4 ||
- sscanf( s, "%d:%d:%d",
- &h1, &m1, &s1) == 3 )
- {
- (*timing_value) = vlc_tick_from_sec( h1 * 3600 + m1 * 60 + s1) +
- VLC_TICK_FROM_MS( d1 );
+ int count;
+ if (sscanf(s, "%d:%d:%d,%d%n", &h1, &m1, &s1, &d1, &count) == 4
+ && (size_t)count <= length)
+ goto success;
- return VLC_SUCCESS;
- }
+ if (sscanf(s, "%d:%d:%d.%d%n", &h1, &m1, &s1, &d1, &count) == 4
+ && (size_t)count <= length)
+ goto success;
+
+ d1 = 0;
+ if (sscanf(s, "%d:%d:%d%n", &h1, &m1, &s1, &count) == 3
+ && (size_t)count <= length)
+ goto success;
return VLC_EGENERIC;
+
+success:
+ (*timing_value) = VLC_TICK_0
+ + vlc_tick_from_sec(h1 * 3600 + m1 * 60 + s1)
+ + VLC_TICK_FROM_MS(d1);
+
+ return VLC_SUCCESS;
+
}
/* subtitle_ParseSubRipTiming
@@ -1145,23 +1156,18 @@ static int subtitle_ParseSubRipTimingValue(vlc_tick_t *timing_value,
static int subtitle_ParseSubRipTiming( subtitle_t *p_subtitle,
const char *s )
{
- int i_result = VLC_EGENERIC;
- char *psz_start, *psz_stop;
- psz_start = malloc( strlen(s) + 1 );
- psz_stop = malloc( strlen(s) + 1 );
-
- if( sscanf( s, "%s --> %s", psz_start, psz_stop) == 2 &&
- subtitle_ParseSubRipTimingValue( &p_subtitle->i_start, psz_start ) == VLC_SUCCESS &&
- subtitle_ParseSubRipTimingValue( &p_subtitle->i_stop, psz_stop ) == VLC_SUCCESS )
- {
- i_result = VLC_SUCCESS;
- }
+ const char *delimiter = strstr(s, " --> ");
+ if (delimiter == NULL || delimiter == s)
+ return VLC_EGENERIC;
- free(psz_start);
- free(psz_stop);
+ int ret = subtitle_ParseSubRipTimingValue(&p_subtitle->i_start, s, (size_t)(delimiter - s));
+ if (ret != VLC_SUCCESS)
+ return ret;
- return i_result;
+ const char *right = delimiter + strlen(" --> ");
+ return subtitle_ParseSubRipTimingValue(&p_subtitle->i_stop, right, strlen(right));
}
+
/* ParseSubRip
*/
static int ParseSubRip( vlc_object_t *p_obj, subs_properties_t *p_props,
@@ -2472,3 +2478,155 @@ static char * get_language_from_filename( const char * psz_sub_file )
free( psz_work );
return psz_ret;
}
+
+#ifdef ENABLE_TEST
+static void test_subtitle_ParseSubRipTimingValue(void)
+{
+ fprintf(stderr, "\n# %s:\n", __func__);
+
+ struct test_timing_value
+ {
+ const char *str;
+ vlc_tick_t value;
+ };
+
+ static const struct test_timing_value timing_values_success[] =
+ {
+ { "0:0:0,0", VLC_TICK_0 },
+ { "0:0:0.0", VLC_TICK_0 },
+ { "0:0:0", VLC_TICK_0 },
+ };
+
+ struct test_sized_timing_value
+ {
+ const char *str;
+ vlc_tick_t value;
+ size_t length;
+ };
+
+ static const struct test_sized_timing_value sized_timing_values_success[] =
+ {
+ { "0:0:0,1", VLC_TICK_0, strlen("0:0:0") },
+ { "0:0:0.1", VLC_TICK_0, strlen("0:0:0") },
+ };
+
+ static const char *timing_values_fail[] =
+ {
+ "0:0",
+ "0",
+ };
+
+ for (size_t i=0; i<ARRAY_SIZE(timing_values_success); ++i)
+ {
+ fprintf(stderr, "Checking that %s parses into %" PRId64 "\n",
+ timing_values_success[i].str, timing_values_success[i].value);
+
+ vlc_tick_t value;
+ int ret = subtitle_ParseSubRipTimingValue(&value,
+ timing_values_success[i].str,
+ strlen(timing_values_success[i].str));
+ fprintf(stderr, " -> %" PRId64 "\n", value);
+ assert(ret == VLC_SUCCESS);
+ assert(value == timing_values_success[i].value);
+ }
+
+ for (size_t i=0; i<ARRAY_SIZE(sized_timing_values_success); ++i)
+ {
+ fprintf(stderr, "Checking that %s (length=%zu) parses into %" PRId64 "\n",
+ sized_timing_values_success[i].str,
+ sized_timing_values_success[i].length,
+ sized_timing_values_success[i].value);
+
+ vlc_tick_t value;
+ int ret = subtitle_ParseSubRipTimingValue(&value,
+ sized_timing_values_success[i].str,
+ sized_timing_values_success[i].length);
+ assert(ret == VLC_SUCCESS);
+ fprintf(stderr, " -> %" PRId64 "\n", value);
+ assert(value == sized_timing_values_success[i].value);
+ }
+
+ for (size_t i=0; i<ARRAY_SIZE(timing_values_fail); ++i)
+ {
+ fprintf(stderr, "Checking that %s fails to parse\n",
+ timing_values_fail[i]);
+ vlc_tick_t value;
+ int ret = subtitle_ParseSubRipTimingValue(&value,
+ timing_values_fail[i], strlen(timing_values_fail[i]));
+ (void)value;
+ assert(ret != VLC_SUCCESS);
+ }
+
+ for (size_t i=0; i<ARRAY_SIZE(timing_values_fail); ++i)
+ {
+ fprintf(stderr, "Checking that %s fails to parse\n",
+ timing_values_fail[i]);
+ vlc_tick_t value;
+ int ret = subtitle_ParseSubRipTimingValue(&value,
+ timing_values_fail[i], strlen(timing_values_fail[i]));
+ (void)value;
+ assert(ret != VLC_SUCCESS);
+ }
+}
+
+static void test_subtitle_ParseSubRipTiming(void)
+{
+ fprintf(stderr, "\n# %s:\n", __func__);
+
+ struct test_timing_value
+ {
+ const char *str;
+ vlc_tick_t left;
+ vlc_tick_t right;
+ };
+
+ static const struct test_timing_value timing_values_success[] =
+ {
+ { "0:0:0,0 --> 0:0:0,0", VLC_TICK_0, VLC_TICK_0 },
+ { "0:0:0.0 --> 0:0:0.0", VLC_TICK_0, VLC_TICK_0 },
+ { "0:0:0 --> 0:0:0", VLC_TICK_0, VLC_TICK_0 },
+ };
+
+ static const char *timing_values_fail[] =
+ {
+ "0:0 --> 0:0",
+ "0:0 --> 0:0:0,0",
+ "0:0:0,0 --> 0:0",
+ "0 -> 0",
+ };
+
+ for (size_t i=0; i<ARRAY_SIZE(timing_values_success); ++i)
+ {
+ fprintf(stderr, "Checking that %s parses into %" PRId64 " --> %" PRId64 "\n",
+ timing_values_success[i].str,
+ timing_values_success[i].left,
+ timing_values_success[i].right);
+
+ subtitle_t sub = { .i_start = VLC_TICK_INVALID, .i_stop = VLC_TICK_INVALID };
+ int ret = subtitle_ParseSubRipTiming(&sub, timing_values_success[i].str);
+ fprintf(stderr, " -> %" PRId64 " --> %" PRId64 "\n", sub.i_start, sub.i_stop);
+ assert(ret == VLC_SUCCESS);
+ assert(sub.i_start == timing_values_success[i].left);
+ assert(sub.i_stop == timing_values_success[i].right);
+ }
+
+ for (size_t i=0; i<ARRAY_SIZE(timing_values_fail); ++i)
+ {
+ fprintf(stderr, "Checking that %s fails to parse\n",
+ timing_values_fail[i]);
+ subtitle_t sub = { .i_start = VLC_TICK_INVALID, .i_stop = VLC_TICK_INVALID };
+ int ret = subtitle_ParseSubRipTiming(&sub, timing_values_fail[i]);
+ (void)sub;
+ assert(ret != VLC_SUCCESS);
+ }
+}
+
+int main(int argc, char **argv)
+{
+ (void)argc; (void)argv;
+ test_subtitle_ParseSubRipTimingValue();
+ test_subtitle_ParseSubRipTiming();
+
+ return 0;
+}
+#endif
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/a32dac2b63e16ae2916eab55f2e2e48018d01f0d...c74180ad548326e2c7537c0e0d0df19aecdde3af
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/a32dac2b63e16ae2916eab55f2e2e48018d01f0d...c74180ad548326e2c7537c0e0d0df19aecdde3af
You're receiving this email because of your account on code.videolan.org.
VideoLAN code repository instance
More information about the vlc-commits
mailing list