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

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

Merged
15 commits merged into from
Sep 11, 2019

Conversation

ghost
Copy link

@ghost ghost commented Aug 13, 2019

This checks that, when converted from EBU-TT-Live to EBU-TT-D, the styling > style elements do not contain nested references. Instead the styling information contained in referenced styles should be merged with the parent style, keeping the original style ID.

So if there are 4 style elements like so:

<style id="s1" ... />
<style id="s2" style="s1" ... />
<style id="s3" ... />
<style id="s4" style="s2 s3" ... />

Then the converted document will contain style s4 with all the elements of s3, s2 and s1 with priority to the attributes of s4 > s3 > s2 > s1.

This PR also moves padding defined in <style> elements to their corresponding <region>

Copy link
Collaborator

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

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

Some pretty minor changes requested, this mostly looks good.

testing/bdd/conftest.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

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

I just noticed that when I run make test on this branch the new tests aren't run. I'm not sure why test_ebuttd_required_metadata.py and test_ebuttd_style_references.py are not picked up. My terminal output goes straight past them. Here's an excerpt:

testing/bdd/test_documentMetadata_elements_order.py ..........                                                                                                                                          [ 19%]
testing/bdd/test_elements_active_times.py ..............................................................................................                                                                [ 32%]

@nigelmegitt
Copy link
Collaborator

Sorry, my bad, they're running fine.

Copy link
Collaborator

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

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

Looking really good, sorry I've added a few "down in the weeds" comments to keep things tidy, but this functionally works, so once those last minor tweaks are done this will be good to merge.

…s...

-  and check that only one style/region element exists before checking attributes
Copy link
Collaborator

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the previous review comments @danpope-bbc ! Looking good to squash and merge, I think.

testing/bdd/test_ebuttd_style_references.py Show resolved Hide resolved
@ghost ghost merged commit 8f5d910 into tt3-to-ttd-conversion Sep 11, 2019
@ghost ghost deleted the ebuttd/MSPA-446-styles branch September 11, 2019 11:00
This pull request was closed.
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.

3 participants