Implement subtitles schema [WIP]#805
Conversation
|
Similar to |
|
A usage example could be to write an adapter that can use the Subtitle Schema and write out a sub format that ffmpeg supports. I don't think ffmpeg supports TTML2 so converting to srt or something with an adapter might be a good test. |
|
Yes, I was thinking of an srt adapter. Was waiting to hear others' thoughts on the parameters I've included and if we need to implement any other functionality. I'll write a simple srt adapter and get back to you. |
|
@apetrynet @reinecke I've implemented an srt adapter. The tests pass for python3, but there are some extra carriage returns added to the strings with python2. Any idea why this is happening? |
| class SRTTest(unittest.TestCase): | ||
| def setUp(self): | ||
| fd, self.tmp_path = tempfile.mkstemp(suffix=".srt", text=True) |
There was a problem hiding this comment.
Hi @KarthikRIyer!
It might be a long shot, but I recognized an error when comparing multi-lined strings. Try adding maxDiff = None as a class variablle in SRTTest
| class SRTTest(unittest.TestCase): | |
| def setUp(self): | |
| fd, self.tmp_path = tempfile.mkstemp(suffix=".srt", text=True) | |
| class SRTTest(unittest.TestCase): | |
| maxDiff = None | |
| def setUp(self): | |
| fd, self.tmp_path = tempfile.mkstemp(suffix=".srt", text=True) |
There was a problem hiding this comment.
This did't work.
maxDiff is just the max length of Diff for the compare function right?
There was a problem hiding this comment.
Yes. I've had to use it in other tests to make them pass. The error looked similar.
Codecov Report
@@ Coverage Diff @@
## master #805 +/- ##
==========================================
- Coverage 84.02% 82.42% -1.61%
==========================================
Files 74 80 +6
Lines 3124 3266 +142
==========================================
+ Hits 2625 2692 +67
- Misses 499 574 +75
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
I changed os.open() to io.open() and it worked. But I have no idea why :/ |
| self.assertMultiLineEqual(baseline_data, test_data) | ||
|
|
||
| def test_otio(self): | ||
| st = otio.adapters.read_from_file(SRT_EXAMPLE_PATH) |
There was a problem hiding this comment.
Shouldn't this be reading SRT_OTIO_EXAMPLE_PATH instead?
There was a problem hiding this comment.
Oh, yes. I must've copied the SRT to OTIO test and forgotten to change it to test OTIO to SRT. Thanks for pointing it out. I'll fix it.
If I have something like this and I do: |
7 participants
Link the Issue(s) this Pull Request is related to.
Closes #62
Summarize your change.
This PR adds basic support for Subtitles. I've based it on what I understood from TTML2 and from my experience with Premiere Pro. I've summarized the changes in the issue (#62). Here's an image summarizing the changes (Note change:
SerializableObjectinstead ofSerializableObjectWithMetadata):Some concerns:
I think storing a
Styleobject with eachTimedTextis wasteful, but decided to have this approach because we don't have id system in otio yet. In TTML2 there's a section to specify all styles and then refer then in each TimedText. With the current approach conversion from otio to say TTML2 won't be so direct because we don't have a single set of styles. One other thing I thought of was to have a list of styles insideSubtitleand then have a string id in eachTimedText. But then eachTimedTextobject will have no style meaning independently/outside the Subtitle. Thoughts?What are
Markersused for? Each marker has a markedRange. When we trim anItemor change the sourceRange shouldn't the markedRange of the markers at the start or end of theItemalso change? I couldn't find anything that does something like this. Or is this handled in another way? Why I'm asking is if we trim theSubtitleor change its sourceRange, the ranges ofTimedTextsat the start or end might change.Reference associated tests.
Will be added later.