-
Notifications
You must be signed in to change notification settings - Fork 6k
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
EIA608 Styling #1312
EIA608 Styling #1312
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
/* | ||
* Copyright (C) 2014 The Android Open Source Project | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package com.google.android.exoplayer.text.eia608; | ||
|
||
import android.text.Layout; | ||
import android.text.SpannableStringBuilder; | ||
import com.google.android.exoplayer.text.Cue; | ||
|
||
public class Eia608CueBuilder { | ||
private SpannableStringBuilder text; | ||
private float position; | ||
private float line; | ||
private int rowIndex; | ||
|
||
public Eia608CueBuilder() { | ||
reset(); | ||
} | ||
|
||
public Eia608CueBuilder(Eia608CueBuilder other) { | ||
text = new SpannableStringBuilder(other.text); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have no idea what changed, but this line started throwing exception with the same content if other.text is null. So an extra check is probably necessary here :) |
||
position = other.position; | ||
line = other.line; | ||
rowIndex = other.rowIndex; | ||
} | ||
|
||
public void reset() { | ||
text = null; | ||
position = Cue.DIMEN_UNSET; | ||
line = Cue.DIMEN_UNSET; | ||
rowIndex = 15; // bottom row is the default | ||
} | ||
|
||
public Cue build() { | ||
return new Cue(text, Layout.Alignment.ALIGN_NORMAL, | ||
line, Cue.LINE_TYPE_FRACTION, Cue.ANCHOR_TYPE_START, | ||
position, Cue.TYPE_UNSET, Cue.DIMEN_UNSET); | ||
} | ||
|
||
public boolean isEmpty() { | ||
return (text == null) || (text.length() == 0); | ||
} | ||
|
||
public Eia608CueBuilder setText(SpannableStringBuilder aText) { | ||
text = aText; | ||
return this; | ||
} | ||
|
||
public void setRow(int rowIdx) { | ||
if ((rowIdx < 1) || (15 < rowIdx)) { | ||
this.line = 0.9f; | ||
return; | ||
} | ||
|
||
rowIndex = rowIdx; // saved for roll-up feature | ||
|
||
// 10% of screen is left of for safety reasons (analog overscan) | ||
// the leftover 80% is divided into 15 equal rows. The problem is that the font size | ||
// must match the row line height for this, so at the moment, I scale to 90% instead, to avoid | ||
// overlap of the borders around the rows. | ||
// -1 as the row and column indices are 1 based in the spec | ||
this.line = (rowIdx - 1) / 15f * 0.9f + 0.05f; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs minor updates to get closer to the original standard. To fit 15 lines into the safe area, previously I set 15 points (position is the top left corner of a line) evenly in the middle 80% of the screen. But positioning the bottom line exactly to 80% would cause the text of that last line go under its top left corner - effectively outside the intended safe area. So let's distribute "80% - font size" evenly:
But this pulls all the lines closer to each other, and the default subtitle rendering might cause overlapping edges with the default font and subtitle background setting. I solved this by reordering the calls in cuePainter:
|
||
} | ||
|
||
/** | ||
* Decrease the current row and reposition the captions to the new location | ||
* @return true if rolling was possible | ||
*/ | ||
public boolean rollUp() { | ||
if (rowIndex <= 1) { | ||
return false; | ||
} | ||
|
||
setRow(rowIndex - 1); | ||
return true; | ||
} | ||
|
||
public void setColumn(int columnIdx, int additionalTabs) { | ||
// the original standard defines 32 columns for the safe area of the screen (middle 80%) | ||
// but it also mentions that widescreen displays should use 42 columns. As the incoming data | ||
// does not know about the display size, maybe if the content uses widescreen aspect ratio, | ||
// than it will contain 42 columns. I never met such, but we should not forget about it... | ||
if (columnIdx < 1 || 32 < columnIdx) { | ||
this.position = 0.1f; | ||
return; | ||
} | ||
|
||
// -1 as the row and column indices are 1 based in the spec | ||
this.position = (((columnIdx - 1 + additionalTabs) / 32f) * 0.8f) + 0.1f; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove all the debug stuff from committed code? Feel free to keep it locally, but it's probably >50% of the whole change to this file! Note that getIndentation is also not used anywhere, so that method can go or be deferred until a subsequent change also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added getIndentation() while I was trying to figure out the meaning of the bits of the stream. Although it is not useful without full positioning, it might help anyone how is attempting to update the code.
Now, when you stop at a break point, Android Studio is able to write out every piece of information about the incoming Commands instead of a single HEX value. Shall I remove it? (Either ways I will add most of it back with the next change set implementing correct positioning).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be preferable to remove it until it's needed, yes. It makes it easier to find "the change that added positioning" if its contained within a single change, as opposed to some of it arriving first :).