When it reads STL files, it generates an array of ttiblocks in the cl… #25

inremen · 2020-08-18T08:14:06Z

…ass Subtitles. If we use sync from a stl filel, it changes starttimes in ttiblocks array and it saves the file from ttiblocks instead of Items, mantaining lines properties.

It takes account colour and text position converting from STL to VTT.
If creattion date and modification date of STL are empty, it continues conversion from STL to VTT

…ass Subtitles. If we use sync from a stl filel, it changes starttimes in ttiblocks array and it saves the file from ttiblocks instead of Items, mantaining lines properties. It takes account colour and text position converting from STL to VTT. If creattion date and modification date of STL are empty, it continues conversion from STL to VTT

coveralls · 2020-08-18T08:17:41Z

Coverage decreased (-0.4%) to 76.488% when pulling 356ab75 on inremen:master into 88639b8 on asticode:master.

asticode

I like the general ideas but this PR needs some changes

subtitles.go

stl.go

subtitles.go

teletext.go

… as STLSubtitleGroupNumber, STLExtensionBlockNumber, etc. new function newItemMetadata to intialize - Index attribute moved from Item to ItemMetadata - New types stlVerticalPosition (with function WebVTTPosition) and stlJustification (with function WebVTTLine) code and pointers STLVerticalPosition and STLJustificaationCode of this type in StyleAttributes. This attributes are assigned in function propagateSTLAttributes od subtitles.go - If we save a STL from another STL, for example to sync,we use STL attributtes of ItemMetadata and StyleAttributes to create new TTI blocks

asticode

Thanks for the changes, still some changes needed though

subtitles.go

srt.go

asticode · 2020-08-21T09:43:59Z

stl.go

+			STLCumulativeStatus:     t.cumulativeStatus,
+			STLExtensionBlockNumber: t.extensionBlockNumber,
+			STLSubtitleGroupNumber:  t.subtitleGroupNumber,
+			STLText:                 t.text,


I don't really like this attribute. Does copying []byte makes that much difference ?

Yes. Our subtitle broadcast system uses spaces to center text. So we need to mantain all the bytes when we sync from stl to stl

Besides, colour and size information also goes in this lines

subtitles.go

stl.go

subtitles.go

teletext.go

New attribute Metadata in Item

….stl -t "25s" -o starttime25s.stl or function func (s *Subtitles) ChangeStartTime(starttime time.Duration)

- No read stl subtitles when no lines

teletext.go

webvtt.go

subtitles.go

astisub/main.go

asticode · 2020-08-31T10:03:11Z

stl.go

+	o = append(o, t.justificationCode)                                   // Justification code
+	o = append(o, t.commentFlag)                                         // Comment flag
+	//if text has 112 characters, copy directly
+	if len(t.text) == 112 {


Remove this micro-optimization as you need to compare len(encodeTextSTL(string(t.text)) anyway

If the text is from STL to STL, we dont need to encode. We need to mantain spaces, colour, height, font style that comes in text field

What you're saying makes sense, however I don't like basing the "not encoding" part on the length of t.text.

Out of simplicity, I'd rather:

use encodeTextSTL here (that way, if t.text comes from i.Metadata.STLText it's never encoded and your spaces, colour, height, etc. are maintained)

remove the if len(t.text) == 112 and partially revert to

o = append(o, astikit.BytesPad(t.text), '\x8f', 112, astikit.PadRight, astikit.PadCut)...)

I'm testing this and the codification changes if we use encodeTextSTL.
Here the example:

This doesn't surprise me since there's this TODO.

However that doesn't change anything for you, if you move encodeTextSTL here in your case encodeTextSTL will never get called since you're using i.Metadata.STLText and therefore codification won't change.

I think this is best for this PR to make this change, and removing encodeTextSTL in another PR.

I don't understand where I should move encodeTextSTL

stl.go

…opagateWebVTTAttribues in line 194 of webvtt.go

astisub/main.go

stl.go

subtitles.go

teletext.go

asticode · 2020-09-01T08:26:19Z

stl.go

+	o = append(o, t.justificationCode)                                   // Justification code
+	o = append(o, t.commentFlag)                                         // Comment flag
+	//if text has 112 characters, copy directly
+	if len(t.text) == 112 {


What you're saying makes sense, however I don't like basing the "not encoding" part on the length of t.text.

Out of simplicity, I'd rather:

use encodeTextSTL here (that way, if t.text comes from i.Metadata.STLText it's never encoded and your spaces, colour, height, etc. are maintained)

remove the if len(t.text) == 112 and partially revert to

o = append(o, astikit.BytesPad(t.text), '\x8f', 112, astikit.PadRight, astikit.PadCut)...)

- Sync calculation code lines simplified.

asticode · 2020-09-01T10:16:13Z

stl.go

+	o = append(o, t.justificationCode)                                   // Justification code
+	o = append(o, t.commentFlag)                                         // Comment flag
+	//if text has 112 characters, copy directly
+	if len(t.text) == 112 {


This doesn't surprise me since there's this TODO.

However that doesn't change anything for you, if you move encodeTextSTL here in your case encodeTextSTL will never get called since you're using i.Metadata.STLText and therefore codification won't change.

I think this is best for this PR to make this change, and removing encodeTextSTL in another PR.

asticode · 2020-09-01T12:09:31Z

stl.go

+		for _, l := range i.Lines {
+			lines = append(lines, l.String())
+		}
+		t.text = []byte(strings.Join(lines, "\n"))


Replace this line by

t.text = []byte(encodeTextSTL(strings.Join(lines, "\n")))

and check tests still pass

Thanks. done

asticode · 2020-09-02T09:54:56Z

Tests are failing :

teletext_test.go:34 : you need to add Metadata: &ItemMetadata{}, line 40 in teletext_test.go
teletext_test.go:78 : you need to add proper WebVTTColor values in each LineItem since you've implemented it
stl_test.go:38 : you need to split the one Write test
a) create a testdata/example-out-copy.stl file that is going to be the output when copying directly from STLText (which is what's happening in the test now) and test it
b) then reset each STLText here before testing testdata/example-out.stl

That way we test both copy and encode

asticode requested changes Aug 19, 2020

View reviewed changes

inremen requested a review from asticode August 21, 2020 06:18

asticode requested changes Aug 21, 2020

View reviewed changes

inremen added 2 commits August 24, 2020 13:37

Owned requested changes:

bedd003

New attribute Metadata in Item

change starttime option. Example: ./astisub.exe starttime -i original…

212b882

….stl -t "25s" -o starttime25s.stl or function func (s *Subtitles) ChangeStartTime(starttime time.Duration)

inremen requested a review from asticode August 24, 2020 12:12

- webvtt verticalposition and justification corrected.

9714440

- No read stl subtitles when no lines

asticode requested changes Aug 31, 2020

View reviewed changes

SyncTime option with -t. Reverted started in teletext.go. Reverted pr…

43c7d8f

…opagateWebVTTAttribues in line 194 of webvtt.go

inremen requested a review from asticode September 1, 2020 07:40

asticode requested changes Sep 1, 2020

View reviewed changes

- Removed line feeds.

6be60ec

- Sync calculation code lines simplified.

inremen requested a review from asticode September 1, 2020 10:05

asticode requested changes Sep 1, 2020

View reviewed changes

Not use encodeTextSTL

41f947b

inremen requested a review from asticode September 1, 2020 11:36

asticode requested changes Sep 1, 2020

View reviewed changes

encodeTextSTL before filling t.text in ttiblock

356ab75

inremen requested a review from asticode September 1, 2020 13:09

asticode force-pushed the master branch 6 times, most recently from 1a6c252 to 012fd3a Compare July 27, 2023 09:40

asticode force-pushed the master branch from 012fd3a to 04eba85 Compare August 8, 2023 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

When it reads STL files, it generates an array of ttiblocks in the cl… #25

When it reads STL files, it generates an array of ttiblocks in the cl… #25

inremen commented Aug 18, 2020

coveralls commented Aug 18, 2020 •

edited

asticode left a comment

asticode left a comment

asticode Aug 21, 2020

inremen Aug 24, 2020

inremen Aug 24, 2020

asticode Aug 31, 2020

inremen Sep 1, 2020

asticode Sep 1, 2020

inremen Sep 1, 2020

asticode Sep 1, 2020

inremen Sep 1, 2020

asticode Sep 1, 2020

asticode Sep 1, 2020

asticode Sep 1, 2020

inremen Sep 1, 2020

asticode commented Sep 2, 2020

When it reads STL files, it generates an array of ttiblocks in the cl… #25

Are you sure you want to change the base?

When it reads STL files, it generates an array of ttiblocks in the cl… #25

Conversation

inremen commented Aug 18, 2020

coveralls commented Aug 18, 2020 • edited

asticode left a comment

Choose a reason for hiding this comment

asticode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asticode commented Sep 2, 2020

coveralls commented Aug 18, 2020 •

edited