Add tests (and fix implementation for) EBU-TT-D style conversion

ebu_tt_live/bindings/__init__.py

@@ -152,7 +152,7 @@ def ordered_styles(self, dataset):
                 return self._ordered_styles
             ordered_styles = [self]
             if self.style is not None:
-                for style_id in self.style:
+                for style_id in self.style[::-1]: # Reverse style references: last reference should take precedence
                     try:
                         style_elem = dataset['tt_element'].get_element_by_id(elem_id=style_id, elem_type=style_type)
                         cascading_styles = style_elem.ordered_styles(dataset=dataset)

ebu_tt_live/bindings/converters/ebutt3_ebuttd.py

@@ -192,6 +192,17 @@ def convert_region(self, region_in, dataset):
         if extent is not None:
             if isinstance(extent, ebuttdt.cellExtentType):
                 extent = ebuttdt.convert_cell_region_to_percentage(extent, dataset['cellResolution'])
+        if region_in.padding == None:
+            region_validated_styles = [style for style in region_in.validated_styles if style.id in region_in.style]
+            for region_style in region_validated_styles:
+                parent_styles = region_style.ordered_styles(dataset)
+                if parent_styles:
+                    for parent_style in parent_styles:
+                        if parent_style.padding:
+                            region_in.padding = parent_style.padding
+                else:
+                 if region_style.padding:
+                    region_in.padding = region_style.padding
         new_elem = d_region_type(
             *self.convert_children(region_in, dataset),
             id=region_in.id,
@@ -216,32 +227,36 @@ def convert_styling(self, styling_in, dataset):
         return new_elem
 
     def convert_style(self, style_in, dataset):
-        color = style_in.color
+        ordered_styles = style_in.ordered_styles(dataset)
+        computed_style = style_type(id=style_in.id)
+        for s in ordered_styles:
+            computed_style.add(s)
+        color = computed_style.color
         if color is not None:
             if isinstance(color, ebuttdt.namedColorType):
                 color = ebuttdt.named_color_to_rgba(color)
-        backgroundColor = style_in.backgroundColor
+        backgroundColor = computed_style.backgroundColor
         if backgroundColor is not None:
             if isinstance(backgroundColor, ebuttdt.namedColorType):
                 backgroundColor = ebuttdt.named_color_to_rgba(backgroundColor)
         new_elem = d_style_type(
-            *self.convert_children(style_in, dataset),
-            id=style_in.id,
-            style=style_in.style,  # there is no ordering requirement in styling so too soon to deconflict here
-            direction=style_in.direction,
-            fontFamily=style_in.fontFamily,
+            *self.convert_children(computed_style, dataset),
+            id=computed_style.id,
+            style=computed_style.style,  # there is no ordering requirement in styling so too soon to deconflict here
+            direction=computed_style.direction,
+            fontFamily=computed_style.fontFamily,
             fontSize=None,  # This will be regenerated in separate style. This is necessary due to % fontSize conversions
             lineHeight=None,  # lineHeight also receives the fontSize treatment
-            textAlign=style_in.textAlign,
+            textAlign=computed_style.textAlign,
             color=color,
             backgroundColor=backgroundColor,
-            fontStyle=style_in.fontStyle,
-            fontWeight=style_in.fontWeight,
-            textDecoration=style_in.textDecoration,
-            unicodeBidi=style_in.unicodeBidi,
-            wrapOption=style_in.wrapOption,
-            padding=style_in.padding,
-            linePadding=style_in.linePadding,
+            fontStyle=computed_style.fontStyle,
+            fontWeight=computed_style.fontWeight,
+            textDecoration=computed_style.textDecoration,
+            unicodeBidi=computed_style.unicodeBidi,
+            wrapOption=computed_style.wrapOption,
+            padding=computed_style.padding,
+            linePadding=computed_style.linePadding,
             _strict_keywords=False
         )
         return new_elem

testing/bdd/features/metadata/ebuttd_required_metadata.feature

@@ -1,3 +1,4 @@
+@metadata @document @ebuttd_conversion 
 Feature: Converted EBUTTD file contains required metadata elements
 
   Scenario: It adds two conformsToStandard elements to the EBU-TT-D metadata

testing/bdd/features/styles/ebuttd_style_references.feature

@@ -0,0 +1,89 @@
+@styles @document @ebuttd_conversion 
+Feature: Remove style elements that refer to other style elements
+
+  Examples:
+    | xml_file                     |
+    | style_element_references.xml |
+
+
+  Scenario: Convert style element with reference to another style to a presentationally equivalent one
+    Given an xml file <xml_file>
+    When  it contains style "s_default"
+    And   style "s_default" has attribute "color" set to "white"
+    And   style "s_default" has attribute "backgroundColor" set to "black"
+    And   it contains style "s_font_reith"
+    And   style "s_font_reith" has attribute "style" set to "s_default"
+    And   style "s_font_reith" has attribute "fontFamily" set to "reith"
+    And   it contains style "s_font_monospace"
+    And   style "s_font_monospace" has attribute "fontFamily" set to "monospace"
+    And   it contains style "s_top"
+    And   style "s_top" has attribute "style" set to "s_font_reith s_font_monospace"
+    And   style "s_top" has attribute "color" set to "yellow"
+    And   it contains some text with style "s_top"
+    When  the document is generated
+    And   the EBU-TT-Live document is converted to EBU-TT-D
+    Then  the ebu_tt_d document contains style "s_top" with attribute "fontFamily" set to "monospace"
+    And   the ebu_tt_d document contains style "s_top" with attribute "color" set to "#ffff00ff"
+    And   the ebu_tt_d document contains style "s_top" with attribute "backgroundColor" set to "#000000ff"
+
+
+  Scenario: Convert padding specifed on style applied to region to padding specified only on region when padding is on both style and region
+    Given an xml file <xml_file>
+    When  it contains style "s1"
+    And   style "s1" has attribute "padding" set to "10px"
+    And   it contains region "r1"
+    And   region "r1" has attribute "style" set to "s1"
+    And   region "r1" has attribute "padding" set to "5px"
+    And   it contains some text with region "r1"
+    When  the document is generated
+    And   the EBU-TT-Live document is converted to EBU-TT-D
+    Then  EBUTTD document is valid
+    And   the ebu_tt_d document contains style "s1" without a "padding" attribute
+    And   the ebu_tt_d document contains region "r1" with attribute "padding" set to "5px"
+
+
+  Scenario: Convert padding specifed on style applied to region to padding specified only on region when padding is only on style
+    Given an xml file <xml_file>
+    When  it contains style "s1"
+    And   style "s1" has attribute "padding" set to "10px"
+    And   it contains region "r1"
+    And   region "r1" has attribute "style" set to "s1"
+    And   it contains some text with region "r1"
+    When  the document is generated
+    And   the EBU-TT-Live document is converted to EBU-TT-D
+    Then  EBUTTD document is valid
+    And   the ebu_tt_d document contains style "s1" without a "padding" attribute
+    And   the ebu_tt_d document contains region "r1" with attribute "padding" set to "10px"
+
+
+  Scenario: Convert padding specifed on 2 styles applied to a region to padding specified only on region when padding is only on style
+    Given an xml file <xml_file>
+    When  it contains style "s1"
+    And   style "s1" has attribute "padding" set to "10px"
+    When  it contains style "s2"
+    And   style "s2" has attribute "padding" set to "5px"
+    And   it contains region "r1"
+    And   region "r1" has attribute "style" set to "s1 s2"
+    And   it contains some text with region "r1"
+    When  the document is generated
+    And   the EBU-TT-Live document is converted to EBU-TT-D
+    Then  EBUTTD document is valid
+    And   the ebu_tt_d document contains region "r1" with attribute "padding" set to "5px"
+
+
+  Scenario: Convert padding specifed on 2 styles where one inherits from another applied to a region to padding specified only on region when padding is only on style
+    Given an xml file <xml_file>
+    When  it contains style "s1"
+    And   style "s1" has attribute "padding" set to "10px"
+    When  it contains style "s2"
+    And   style "s2" has attribute "padding" set to "5px"
+    When  it contains style "s3"
+    And   style "s3" has attribute "color" set to "white"
+    And   style "s3" has attribute "style" set to "s2"
+    And   it contains region "r1"
+    And   region "r1" has attribute "style" set to "s1 s3"
+    And   it contains some text with region "r1"
+    When  the document is generated
+    And   the EBU-TT-Live document is converted to EBU-TT-D
+    Then  EBUTTD document is valid
+    And   the ebu_tt_d document contains region "r1" with attribute "padding" set to "5px"

testing/bdd/features/styles/style_attribute_inherited.feature

@@ -2,15 +2,15 @@
 Feature: Compute style attribute on a single EBU-TT Live element
 
   Examples:
-  | xml_file                      | cell_resolution | extent      |
-  | style_attribute_inherited.xml | 32 15           | 320px 150px |
+  | xml_file                      |
+  | style_attribute_inherited.xml |
 
 
   # Inheritance: region (S1) > div (S2) > p (S3) > span (S4)
   Scenario: Inheritable style attributes
     Given an xml file <xml_file>
-    When it has a cell resolution of <cell_resolution>
-    And it has extent of <extent>
+    When it has a cell resolution of "32 15"
+    And it has extent of "320px 150px"
     And it contains style S1 with <style_attribute> value <S1_value>
     And it contains style S2 with <style_attribute> value <S2_value>
     And it contains style S3 with <style_attribute> value <S3_value>
@@ -42,3 +42,13 @@ Feature: Compute style attribute on a single EBU-TT Live element
     |           |          |                |          | tts:wrapOption       | span1   | wrap           |
 
 
+  Scenario: Circular style references should fail
+    Given an xml file <xml_file>
+    When it contains style S1 with <style_attribute> value <S1_value>
+    And it contains style S2 with <style_attribute> value <S2_value>
+    And it contains style S3 with <style_attribute> value <S3_value>
+    Then document is invalid
+
+    Examples:
+    | S1_value | S2_value | S3_value | style_attribute |
+    | S2       | S3       | S1       | style           |

testing/bdd/templates/style_element_references.xml

@@ -0,0 +1,46 @@
+<?xml version="1.0" ?>
+<tt:tt
+        ebuttp:sequenceIdentifier="TestSequence"
+        ebuttp:sequenceNumber="1"
+        ttp:timeBase="media"
+        xml:lang="en-GB"
+        xmlns:ebuttm="urn:ebu:tt:metadata"
+        xmlns:ebuttp="urn:ebu:tt:parameters"
+        xmlns:tt="http://www.w3.org/ns/ttml"
+        xmlns:ttp="http://www.w3.org/ns/ttml#parameter"
+        xmlns:tts="http://www.w3.org/ns/ttml#styling"
+        xmlns:xml="http://www.w3.org/XML/1998/namespace"
+        xmlns:f="urn:foreign">
+  <tt:head>
+    <tt:metadata>
+      <ebuttm:documentMetadata/>
+      <f:foreignMetadata>Fake metadata</f:foreignMetadata>
+    </tt:metadata>
+    <tt:styling>
+      {% for style in styles %}
+        <tt:style xml:id="{{ style.id }}" {% if style.style %} style="{{ style.style }}" {% endif %}
+        {% if style.color %} tts:color="{{ style.color }}" {% endif %}
+        {% if style.backgroundColor %} tts:backgroundColor="{{ style.backgroundColor }}" {% endif %}
+        {% if style.fontFamily %} tts:fontFamily="{{ style.fontFamily }}" {% endif %} 
+        {% if style.padding %} tts:padding="{{style.padding}}" {% endif %}  />
+        {% endfor %}
+    </tt:styling>
+    <tt:layout>
+       {%if regions%}
+        {% for region in regions%}
+          <tt:region xml:id="{{region.id}}" style="{{region.style}}" {% if region.padding %} tts:padding="{{region.padding}}" {% endif %} tts:displayAlign="after" tts:extent="71.25% 24%" tts:origin="14.375% 60%" tts:overflow="visible" tts:writingMode="lrtb" />
+        {% endfor %}
+      {% endif %}
+    </tt:layout>
+  </tt:head>
+  <tt:body begin="500ms" dur="00:00:05">
+      <tt:div>
+        <tt:p begin="500ms" end="3420ms" xml:id="ID006" {% if text_region %} region="{{text_region}}" {% endif %}>
+          <tt:span {% if text_style %} style="{{text_style}}" {% endif %} >Some example text...</tt:span>
+          <tt:br/>
+          <tt:span>And another line</tt:span>
+        </tt:p>
+      </tt:div>
+  </tt:body>
+</tt:tt>
+

testing/bdd/test_ebuttd_style_references.py

@@ -0,0 +1,75 @@
+from pytest_bdd import scenarios, when, then, given, parsers
+import xml.etree.ElementTree as ET
+
+scenarios('features/styles/ebuttd_style_references.feature')
+
+
+@when(parsers.parse('it contains style "{style_name}"'))
+def when_it_contains_style(test_context, template_dict, style_name):
+    if 'styles' not in template_dict:
+        template_dict['styles'] = list()
+    style = {"id": style_name}
+    template_dict['styles'].append(style)
+    test_context[style_name] = style
+
+
+@when(parsers.parse('style "{style_name}" has attribute "{attribute}" set to "{value}"'))
+def when_style_has_attribute(test_context, style_name, attribute, value):
+    test_context[style_name][attribute] = value
+
+
+@when(parsers.parse('it contains some text with style "{style_name}"'))
+def when_text_has_style(template_dict, style_name):
+    template_dict['text_style'] = style_name
+
+
+@when(parsers.parse('it contains some text with region "{region_name}"'))
+def when_text_has_region(template_dict, region_name):
+    template_dict['text_region'] = region_name
+
+
+@when(parsers.parse('it contains region "{region_id}"'))
+def when_it_contains_region(test_context, template_dict, region_id):
+    if 'regions' not in template_dict:
+        template_dict['regions'] = list()
+    region = {"id": region_id}
+    template_dict['regions'].append(region)
+    test_context[region_id] = region
+
+
+@when(parsers.parse('region "{region_id}" has attribute "{attribute}" set to "{value}"'))
+def when_region_has_attribute(test_context, region_id, attribute, value):
+    test_context[region_id][attribute] = value
+
+
+@then(parsers.parse('the ebu_tt_d document contains style "{style_name}" with attribute "{attribute}" set to "{value}"'))
+def then_converted_document_has_style(test_context, style_name, attribute, value):
+    ebuttd_document = test_context['ebuttd_document']
+    tree = ET.fromstring(ebuttd_document.get_xml())
+    elements = tree.findall('{http://www.w3.org/ns/ttml}head/'
+                            '{http://www.w3.org/ns/ttml}styling/'
+                            '{http://www.w3.org/ns/ttml}style[@{http://www.w3.org/XML/1998/namespace}id="%s"]' % style_name)
+    assert len(elements) == 1
+    assert elements[0].get('{http://www.w3.org/ns/ttml#styling}%s' % attribute) == value
+
+
+@then(parsers.parse('the ebu_tt_d document contains region "{region_id}" with attribute "{attribute}" set to "{value}"'))
+def then_converted_document_has_region_with_styling_attribute(test_context, region_id, attribute, value):
+    ebuttd_document = test_context['ebuttd_document']
+    tree = ET.fromstring(ebuttd_document.get_xml())
+    elements = tree.findall('{http://www.w3.org/ns/ttml}head/'
+                            '{http://www.w3.org/ns/ttml}layout/'
+                            '{http://www.w3.org/ns/ttml}region[@{http://www.w3.org/XML/1998/namespace}id="%s"]' % region_id)
+    assert len(elements) == 1
+    assert elements[0].get('{http://www.w3.org/ns/ttml#styling}%s' % attribute) == value
+
+
+@then(parsers.parse('the ebu_tt_d document contains style "{style_id}" without a "{attribute}" attribute'))
+def then_converted_document_has_style_without_attribute(test_context, style_id, attribute):
+    ebuttd_document = test_context['ebuttd_document']
+    tree = ET.fromstring(ebuttd_document.get_xml())
+    elements = tree.findall('{http://www.w3.org/ns/ttml}head/'
+                            '{http://www.w3.org/ns/ttml}styling/'
+                            '{http://www.w3.org/ns/ttml}style[@{http://www.w3.org/XML/1998/namespace}id="%s"]' % style_id)
+    assert len(elements) == 1
+    assert elements[0].get('{http://www.w3.org/ns/ttml#styling}%s' % attribute) == None

testing/bdd/test_style.py

@@ -1,4 +1,4 @@
-from pytest_bdd import when, scenarios, then
+from pytest_bdd import when, scenarios, then, parsers
 from ebu_tt_live.clocks.media import MediaClock
 from ebu_tt_live.bindings import ebuttdt
 from ebu_tt_live.documents.converters import EBUTT3EBUTTDConverter
@@ -13,11 +13,13 @@
 
 
 @when('it has a cell resolution of <cell_resolution>')
+@when(parsers.parse('it has a cell resolution of "{cell_resolution}"'))
 def when_cell_resolution(template_dict, cell_resolution):
     template_dict['cell_resolution'] = cell_resolution
 
 
 @when('it has extent of <extent>')
+@when(parsers.parse('it has extent of "{extent}"'))
 def when_extent(template_dict, extent):
     template_dict['extent'] = extent