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

Updating mlt-xml.dtd #346

Closed
camillem opened this issue Jun 1, 2018 · 6 comments
Closed

Updating mlt-xml.dtd #346

camillem opened this issue Jun 1, 2018 · 6 comments
Labels
Milestone

Comments

@camillem
Copy link
Contributor

camillem commented Jun 1, 2018

Hi,
First, thanks for this most useful piece of software that is MLT!

The dtd file seems a little bit outdated compared to the rest of the code, and it doesn't allow to validate mlt files that I think are conformant: I made simple test with Shotcut and Kdenlive and get the same kind of errors with both applications.
Most problems are missing child elements and attributes.
Would that be relevant that I submit a PR with changes to adapt the dtd to the output of Shotcut?
One trickier point is the ID "main bin" in the file, as in xml, ID can't contain spaces, AFAICT.
Do you think that this could be changed without breaking too many things?
Thanks,
Camille

@ddennedy
Copy link
Member

ddennedy commented Jun 1, 2018

Would that be relevant that I submit a PR with changes to adapt the dtd to the output of Shotcut?

Yes, but it should not contain any "shotcut:" attributes.

Do you think that this could be changed without breaking too many things?

Yes, I would make Shotcut start output as "main_bin" and then make whatever code is looking for "main bin" look for either (backwards compatibility).

@camillem
Copy link
Contributor Author

camillem commented Jun 1, 2018

Thanks for the fastest answer !

@ddennedy
Copy link
Member

ddennedy commented Jun 1, 2018

I have the Shotcut change staged now for testing, but I am busy making new release. I will get to it soon.

@camillem
Copy link
Contributor Author

camillem commented Jun 2, 2018

Thanks very much.
I have a few (like 2) additional changes but I would like to test the whole thing more thoroughly.
Would you have a pointer to a correct-but-not-so-simple mlt file that I could test the DTD on?

ddennedy added a commit to mltframework/shotcut that referenced this issue Jun 4, 2018
@ddennedy
Copy link
Member

ddennedy commented Jun 4, 2018

test.mlt.txt

@camillem
Copy link
Contributor Author

camillem commented Jun 5, 2018

Thanks for the file. It produces no error against the current version of the DTD, so I'll submit a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants