Skip to content

Conversation

Copy link

Copilot AI commented Jul 9, 2025

This PR adds a new include_positioning boolean parameter to the BaseWriter class that allows users to control whether positioning information is included in caption output files.

Problem

Users had no way to write caption files while ignoring positioning information. All writers would always process and include positioning data when present, even when users wanted clean output with default positioning only.

Solution

Added include_positioning=True parameter to BaseWriter.__init__() that:

  • When True (default): Preserves existing behavior - positioning information is processed and included
  • When False: Ignores all positioning information and uses default positioning

Implementation Details

BaseWriter (pycaption/base.py)

  • Added include_positioning parameter with default True for backward compatibility
  • Modified _relativize_and_fit_to_screen() to return None when positioning should be ignored
  • Updated docstring with parameter documentation

WebVTTWriter (pycaption/webvtt.py)

  • Modified _convert_positioning() to check self.include_positioning and return empty string when disabled
  • Ensures WebVTT respects positioning control like other writers

DFXPWriter (pycaption/dfxp/base.py)

  • Enhanced layout processing to handle CaptionSet-level and language-level layout_info through _relativize_and_fit_to_screen()
  • Ensures region creation respects the include_positioning setting

Example Usage

from pycaption import DFXPWriter, WebVTTWriter

# Default behavior (positioning included)
writer = DFXPWriter(video_width=640, video_height=360)
output_with_positioning = writer.write(caption_set)

# New feature (positioning ignored)
writer = DFXPWriter(include_positioning=False, video_width=640, video_height=360) 
output_without_positioning = writer.write(caption_set)

Results

DFXP Output Comparison:

<!-- With positioning -->
<region tts:origin="25% 75%" tts:extent="65% 20%" xml:id="r0"/>

<!-- Without positioning -->
<region tts:textAlign="start" xml:id="bottom"/>

WebVTT Output Comparison:

<!-- With positioning -->
00:00.000 --> 00:03.000 align:start position:25% line:75% size:65%

<!-- Without positioning -->  
00:00.000 --> 00:03.000

Backward Compatibility

✅ All existing code continues to work unchanged
✅ Default behavior preserved (include_positioning=True)
✅ All existing tests pass
✅ Works with all writers that inherit from BaseWriter

Testing

  • Added comprehensive unit tests for the new parameter
  • Integration tests verify correct behavior across DFXP, WebVTT, and SAMI writers
  • Backward compatibility tests ensure no regressions

Fixes #109.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits July 9, 2025 08:26
Copilot AI changed the title [WIP] Users cannot write caption files ignoring positioning Add include_positioning parameter to BaseWriter to control positioning output Jul 9, 2025
Copilot AI requested a review from lorandvarga July 9, 2025 08:31
Copilot finished work on behalf of lorandvarga July 9, 2025 08:31
@lorandvarga lorandvarga changed the title Add include_positioning parameter to BaseWriter to control positioning output [Experiment, ignore] Add include_positioning parameter to BaseWriter to control positioning output Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Users cannot write caption files ignoring positioning

2 participants