diff --git a/docs/changelog.rst b/docs/changelog.rst index b18fec13..2e4bc71a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,15 @@ Changelog dropped, ``CaptionReadNoCaptions`` is raised. With ``ignore_timing_errors=False``, clamped cues no longer fail the start-ordering check. Zero and positive shifts are unchanged. + - Fix ``WebVTTReader`` not closing a cue with no text at the blank line + that ends it, so the next block was read as that cue's text: a following + cue's ID appeared above its caption, and a ``NOTE`` comment, ``STYLE`` + block or ``REGION`` block became a caption at the empty cue's time; these + blocks are now skipped. Empty cues are still dropped; the + following ID now counts towards the duplicate-ID warning. Text after an + empty cue that is not part of a cue is now ignored, so a file whose only + text sits there raises ``CaptionReadNoCaptions``. Files without empty + cues are unchanged. 2.3.13 ^^^^^^ diff --git a/pycaption/webvtt/reader.py b/pycaption/webvtt/reader.py index e49843d3..08f2237a 100644 --- a/pycaption/webvtt/reader.py +++ b/pycaption/webvtt/reader.py @@ -258,12 +258,10 @@ def _parse_line(self, line, line_index, state, captions): state.pending_id = None return - if line == "" and state.found_timing and state.nodes: + if line == "" and state.found_timing: self._finalize_cue(state, captions) return - if not state.found_timing or line == "": - return if state.nodes: state.nodes.append(CaptionNode.create_break()) state.nodes.extend(self._parse_cue_text(line, state.open_tags)) @@ -317,10 +315,10 @@ def _consume_pending_id(state, line_index): state.seen_ids.add(cue_id) def _finalize_cue(self, state, captions): - """Close the current cue, append it to captions, and reset state - for the next cue.""" + """Close the current cue, append it to captions unless it is dropped + or empty, and reset state for the next cue.""" state.found_timing = False - if not state.drop_cue: + if not state.drop_cue and state.nodes: captions.append( self._build_cue( state.nodes, diff --git a/tests/conftest.py b/tests/conftest.py index 022f4de1..8f790960 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -219,7 +219,16 @@ sample_webvtt_double_br, sample_webvtt_empty, sample_webvtt_empty_cue, + sample_webvtt_empty_cue_multiple_blanks, + sample_webvtt_empty_cue_only_orphan_text, sample_webvtt_empty_cue_output, + sample_webvtt_empty_cue_then_duplicate_id, + sample_webvtt_empty_cue_then_id, + sample_webvtt_empty_cue_then_note, + sample_webvtt_empty_cue_then_orphan_text, + sample_webvtt_empty_cue_then_region, + sample_webvtt_empty_cue_then_style, + sample_webvtt_empty_last_cue, sample_webvtt_from_dfxp, sample_webvtt_from_dfxp_with_positioning_and_style, sample_webvtt_from_dfxp_with_style, diff --git a/tests/fixtures/webvtt.py b/tests/fixtures/webvtt.py index 3183fa0f..17e8b6be 100644 --- a/tests/fixtures/webvtt.py +++ b/tests/fixtures/webvtt.py @@ -343,6 +343,124 @@ def sample_webvtt_empty_cue(): """ +@pytest.fixture(scope="session") +def sample_webvtt_empty_cue_then_id(): + return """WEBVTT + +cue-1 +00:00.000 --> 00:02.000 + +cue-2 +00:04.000 --> 00:05.000 +C +""" + + +@pytest.fixture(scope="session") +def sample_webvtt_empty_cue_then_note(): + return """WEBVTT + +00:00.000 --> 00:02.000 + +NOTE hello + +00:04.000 --> 00:05.000 +C +""" + + +@pytest.fixture(scope="session") +def sample_webvtt_empty_cue_then_style(): + return """WEBVTT + +00:00.000 --> 00:02.000 + +STYLE +::cue { color: red } + +00:04.000 --> 00:05.000 +C +""" + + +@pytest.fixture(scope="session") +def sample_webvtt_empty_cue_then_region(): + return """WEBVTT + +00:00.000 --> 00:02.000 + +REGION +id:r1 + +00:04.000 --> 00:05.000 +C +""" + + +@pytest.fixture(scope="session") +def sample_webvtt_empty_cue_multiple_blanks(): + return """WEBVTT + +00:00.000 --> 00:02.000 + + + +cue-2 +00:04.000 --> 00:05.000 +C +""" + + +@pytest.fixture(scope="session") +def sample_webvtt_empty_last_cue(): + return """WEBVTT + +cue-1 +00:00.000 --> 00:02.000 +A + +cue-2 +00:03.000 --> 00:04.000 +""" + + +@pytest.fixture(scope="session") +def sample_webvtt_empty_cue_then_duplicate_id(): + return """WEBVTT + +cue-1 +00:00.000 --> 00:02.000 + +cue-1 +00:04.000 --> 00:05.000 +C +""" + + +@pytest.fixture(scope="session") +def sample_webvtt_empty_cue_then_orphan_text(): + return """WEBVTT + +00:00.000 --> 00:02.000 + +hello +world + +00:04.000 --> 00:05.000 +C +""" + + +@pytest.fixture(scope="session") +def sample_webvtt_empty_cue_only_orphan_text(): + return """WEBVTT + +00:00.000 --> 00:02.000 + +hello +""" + + @pytest.fixture(scope="session") def sample_webvtt_multi_lang_en(): return """WEBVTT diff --git a/tests/test_webvtt.py b/tests/test_webvtt.py index 92b5e784..5f1d232b 100644 --- a/tests/test_webvtt.py +++ b/tests/test_webvtt.py @@ -298,6 +298,75 @@ def test_zero_start(self, sample_webvtt_last_cue_zero_start): def test_webvtt_empty_cue(self, sample_webvtt_empty_cue): assert 1 == len(self.reader.read(sample_webvtt_empty_cue).get_captions("en-US")) + @pytest.mark.parametrize("ignore_timing_errors", [True, False]) + @pytest.mark.parametrize( + "fixture_name", + [ + "sample_webvtt_empty_cue_then_id", + "sample_webvtt_empty_cue_then_note", + "sample_webvtt_empty_cue_then_style", + "sample_webvtt_empty_cue_then_region", + "sample_webvtt_empty_cue_multiple_blanks", + ], + ) + def test_blank_line_closes_empty_cue( + self, request, fixture_name, ignore_timing_errors + ): + captions = WebVTTReader(ignore_timing_errors=ignore_timing_errors).read( + request.getfixturevalue(fixture_name) + ) + + assert [ + (c.start, c.end, c.get_text()) for c in captions.get_captions("en-US") + ] == [(4000000, 5000000, "C")] + + def test_empty_last_cue_is_dropped(self, sample_webvtt_empty_last_cue): + captions = self.reader.read(sample_webvtt_empty_last_cue) + + assert [ + (c.start, c.end, c.get_text()) for c in captions.get_captions("en-US") + ] == [(0, 2000000, "A")] + + @pytest.mark.parametrize("ignore_timing_errors", [True, False]) + def test_empty_cue_id_counts_for_duplicate_warning( + self, sample_webvtt_empty_cue_then_duplicate_id, ignore_timing_errors + ): + reader = WebVTTReader(ignore_timing_errors=ignore_timing_errors) + with pytest.warns(CaptionReadWarning, match="Duplicate cue identifier 'cue-1'"): + captions = reader.read(sample_webvtt_empty_cue_then_duplicate_id) + + assert [c.get_text() for c in captions.get_captions("en-US")] == ["C"] + + def test_dropped_empty_cue_before_zero_is_closed(self): + captions = WebVTTReader(time_shift_milliseconds=-2000).read( + "WEBVTT\n\n00:00.000 --> 00:01.000\n\n" + "cue-2\n00:03.000 --> 00:04.000\nC\n" + ) + + assert [ + (c.start, c.end, c.get_text()) for c in captions.get_captions("en-US") + ] == [(1000000, 2000000, "C")] + + @pytest.mark.parametrize("ignore_timing_errors", [True, False]) + def test_orphan_text_after_empty_cue_is_ignored( + self, sample_webvtt_empty_cue_then_orphan_text, ignore_timing_errors + ): + captions = WebVTTReader(ignore_timing_errors=ignore_timing_errors).read( + sample_webvtt_empty_cue_then_orphan_text + ) + + assert [ + (c.start, c.end, c.get_text()) for c in captions.get_captions("en-US") + ] == [(4000000, 5000000, "C")] + + @pytest.mark.parametrize("ignore_timing_errors", [True, False]) + def test_only_orphan_text_after_empty_cue_raises_no_captions( + self, sample_webvtt_empty_cue_only_orphan_text, ignore_timing_errors + ): + reader = WebVTTReader(ignore_timing_errors=ignore_timing_errors) + with pytest.raises(CaptionReadNoCaptions): + reader.read(sample_webvtt_empty_cue_only_orphan_text) + class TestWebVTTWriter: def setup_method(self): diff --git a/tests/test_webvtt_conversion.py b/tests/test_webvtt_conversion.py index ae261611..3fcf134c 100644 --- a/tests/test_webvtt_conversion.py +++ b/tests/test_webvtt_conversion.py @@ -110,6 +110,30 @@ def test_output_timestamps(self, sample_webvtt_timestamps): assert re.match(expected_timestamp_line_pattern, results[5]) +class TestWebVTTEmptyCueConversion: + @pytest.mark.parametrize( + "writer", [WebVTTWriter, SRTWriter, DFXPWriter, SAMIWriter] + ) + @pytest.mark.parametrize("ignore_timing_errors", [True, False]) + @pytest.mark.parametrize( + "fixture_name, leaked_text", + [ + ("sample_webvtt_empty_cue_then_id", "cue-2"), + ("sample_webvtt_empty_cue_then_note", "hello"), + ("sample_webvtt_empty_cue_multiple_blanks", "cue-2"), + ], + ) + def test_no_leak_after_empty_cue( + self, request, fixture_name, leaked_text, ignore_timing_errors, writer + ): + caption_set = WebVTTReader(ignore_timing_errors=ignore_timing_errors).read( + request.getfixturevalue(fixture_name) + ) + results = writer().write(caption_set) + + assert leaked_text not in results + + # # TODO: Write a test that includes a WebVTT file with style tags # # That will fail because the styles used in the cues are not tracked.