Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement tick duration support for TTML subtitles #26

Merged
merged 3 commits into from
Aug 31, 2020

Conversation

rrooij
Copy link
Contributor

@rrooij rrooij commented Aug 27, 2020

I tested it with some examples and it should work. For instance, you can take the example Netflix subtitles:

<?xml version="1.0" encoding="UTF-8"?>
<tt xmlns="http://www.w3.org/ns/ttml" xmlns:tt="http://www.w3.org/ns/ttml" xmlns:ttm="http://www.w3.org/ns/ttml#metadata" xmlns:ttp="http://www.w3.org/ns/ttml#parameter" xmlns:tts="http://www.w3.org/ns/ttml#styling" ttp:tickRate="10000000" ttp:version="2" xml:lang="ja">
 <head>
  <styling>
   <initial tts:backgroundColor="transparent" tts:color="white" tts:fontSize="6.000vh"/>
   <style xml:id="style0" tts:textAlign="center"/>
   <style xml:id="style1" tts:textAlign="start"/>
   <style xml:id="style2" tts:ruby="container" tts:rubyPosition="auto"/>
   <style xml:id="style3" tts:ruby="base"/>
   <style xml:id="style4" tts:ruby="text"/>
   <style xml:id="style5" tts:ruby="text"/>
  </styling>
  <layout>
   <region xml:id="region0" tts:displayAlign="after"/>
  </layout>
  </head>
 <body xml:space="preserve">
  <div>
   <p xml:id="subtitle1" begin="18637368750t" end="18676157500t" region="region0" style="style0"><span style="style1">テソプ<span style="style2"><span style="style3">の所だ</span><span style="style4">カン食ン</span></span>食おう<br/><span style="style2"><span style="style3">江陵</span><span style="style5">カンヌン</span></span>で刺身でも食おう</span></p>
  </div>
 </body>
</tt>

(Found here: https://netflixtechblog.com/implementing-japanese-subtitles-on-netflix-c165fbe61989 )

@coveralls
Copy link

coveralls commented Aug 27, 2020

Coverage Status

Coverage increased (+0.2%) to 77.023% when pulling e69b116 on rrooij:ttml_ticks into 88639b8 on asticode:master.

Copy link
Owner

@asticode asticode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice PR.

Few changes needed though.

Also, could you add tests here ?

ttml.go Outdated Show resolved Hide resolved
@@ -288,6 +289,10 @@ func (d TTMLInDuration) duration() time.Duration {
if d.framerate > 0 {
return d.d + time.Duration(float64(d.frames)/float64(d.framerate)*1e9)*time.Nanosecond
}
if d.tickrate > 0 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not compute the value once and for all here ? Is the tickrate value not populated at that time ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it is not populated at that time.

ttml.go Outdated Show resolved Hide resolved
@rrooij
Copy link
Contributor Author

rrooij commented Aug 28, 2020

I created the tests and fixed the styling issues (the alphabetical order).

Unfortunately the proper tick duration is still calculated in the duration function. But, I do think this is the only solution. Unmarshalltext only has access to the duration text itself and the tickrate is not known at that time. (I actually wonder if the framerate is accessible as well in that function).

Unmarshalltext should in my opinion only be responsible for extracting the text to a proper representation, not for calculating it correctly already.

Edit:

We need additional work to completely confirm to the TTMLv2 standard though:

If not specified, then if a frame rate is specified, the tick rate must be considered to be the effective frame rate multiplied by the sub-frame rate (i.e., ticks are interpreted as sub-frames); or, if no frame rate is specified, the tick rate must be considered to be one (1) tick per second of media time. If specified, then the tick rate must not be zero (0).

https://w3c.github.io/ttml2/#parameter-attribute-tickRate

@asticode asticode merged commit e7a6a4e into asticode:master Aug 31, 2020
@asticode
Copy link
Owner

I do agree with you regarding UnmarshalText. I'll see what I can do.

FYI I've created a v0.5.0 tag.

Cheers

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.

None yet

3 participants