TT1 to TT3/MSPA 704 sequence identifier #42
Conversation
…seding class on document creation
MSPA-728 ebu-tt-3 to ebu-tt-d conversion
…seding class on document creation
Does not handle time conversion. Sets `ebuttp:sequenceIdentifier` to the value of `tt/head/metadata/documentMetadata/documentIdentifier` if present, otherwise uses "TestConverter". Resets the `conformsToStandard` to say it is EBU-TT-3 conformant. Sets the `timeBase` to `media` whether you like it or not, but doesn't do any other conversions.
nigelmegitt
force-pushed
the
tt1-to-tt3/mspa-704-sequence
branch
from
b347f48 to
2bda6a8
Compare
|
Thanks @mm326 I realised overnight that the node config override settings of the sequence identifier aren't being carried through. I think that's a minor addition though, so I propose to merge this and open a separate issue/pr to fix that. |
| ebutt3_bindings = converter.convert_document(ebutt1_in.binding) | ||
| doc_xml = ebutt1_in.get_xml() | ||
| ebutt1_doc = EBUTT1Document.create_from_xml(doc_xml) | ||
| ebutt3_bindings = converter.convert_document(ebutt1_doc.binding) |
There was a problem hiding this comment.
This bit confuses me. Here we're taking an EBUTT1Document object, getting its XML, reading that back into an EBUTT1Document object, then converting it to EBUTT3.
The XML-and-back-again conversion seems unnecessary. Any particular reason for this?
There was a problem hiding this comment.
It's consistent with the ebutt3_to_ebuttd function, but I agree it isn't clear why this approach is taken in either place. Also it looks to me as though neither function is ever even called.
| _semantic_dataset = None | ||
| _sequenceIdentifier = None | ||
|
|
||
| def __init__(self, sequence_id): |
There was a problem hiding this comment.
By adding this extra sequence_id parameter, the converter's usage in converters.py won't work. Can you set a default value here?
Alternatively set a value in converters.py
There was a problem hiding this comment.
Yes, a value should be set in converters.py but I'm now wondering why that file even exists!
There was a problem hiding this comment.
I was thinking we'd use those helper functions in Distiller - they seem pretty handy to me.
There was a problem hiding this comment.
OK we haven't discussed exactly how we're going to instantiate the classes and run them in Distiller yet, so we should do that. My thinking was that we would be in closer control within the Distiller component of exactly how we do the conversion. For example wiring in the Denester etc.
|
Also worth mentioning that when this does get merged, it should be squashed. There are all sorts of duplicated commit messages that appear to change whitespace, and these should be removed from the history. |
Also allows for a setting that specifies whether or not to use `ebuttm:documentIdentifier` element value in the input as the `ebuttp:sequenceIdentifier` attribute value in the output. Adds a test for this.
Address some warnings and add a page for EBU-TT 1 to EBU-TT 3 conversion. WIP.
Should have been committed with previous commit.
* Fix validation error messages for unexpected attributes so it doesn't say they are missing. * Fix cloning of unknown element, and conversion of metadata * Make EBUTT1Document instantiatable by including required attributes and elements in the constructor * Add unit test cases for programmatical construction with smpte (skipped) and media timebase * Add unit test cases for from-document construction with smpte (skipped) and media timebase
Pass pep8 wherever we can.
Helps distinguish test EBU-TT 1 files from test EBU-TT 3 files.
Add EBU-TT 1 to EBU-TT 3 conversion
Does not handle time conversion.
Sets
ebuttp:sequenceIdentifierto the value oftt/head/metadata/documentMetadata/documentIdentifierif present, otherwise uses "TestConverter".Resets the
conformsToStandardto say it is EBU-TT-3 conformant.Sets the
timeBasetomediawhether you like it or not, but doesn't do any other conversions.Tests for valid EBU-TT-3 document on output.