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

[Subtitles] fix smi subtitles to use quoted start tags #17749

Merged
merged 1 commit into from Apr 26, 2020
Merged

[Subtitles] fix smi subtitles to use quoted start tags #17749

merged 1 commit into from Apr 26, 2020

Conversation

howie-f
Copy link
Contributor

@howie-f howie-f commented Apr 25, 2020

Description

kodi would not pick up subtitles if tags are quoted in smi-file.

Motivation and Context

closes #17418

How Has This Been Tested?

i've tested this change with both smi-files provided by the reporting user
<SYNC Start=1398403><P Class=KOKRCC></P></SYNC> ... was working before
<SYNC Start="1398403"><P Class="KOKRCC"></P></SYNC> ... now also working

Types of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

@phunkyfish phunkyfish added v19 Matrix Type: Fix non-breaking change which fixes an issue Component: Video labels Apr 25, 2020
@phunkyfish phunkyfish added this to the Matrix 19.0-alpha 1 milestone Apr 25, 2020
@phunkyfish
Copy link
Contributor

phunkyfish commented Apr 25, 2020

The the bug report the working example was:
<SYNC Start=1398403><P Class=KOKRCC></P></SYNC>

and not:

<SYNC Start=1398403><P Class="KOKRCC"></P></SYNC>

Your change handles the quotes for the start attribute. How about the class attribute? Or is that not required?

@howie-f
Copy link
Contributor Author

howie-f commented Apr 25, 2020

c+p error. handling the class is not required. i tested with the delivered files in the zip
EDIT: c+p error corrected ;)

Copy link
Contributor

@phunkyfish phunkyfish left a comment

Choose a reason for hiding this comment

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

Nice

@howie-f
Copy link
Contributor Author

howie-f commented Apr 25, 2020

i checked this pr once again with a smi-sample from wikipedia containing:

notice the manually "quoted classes" and "start attribute"

<!-- Open play menu, choose Captions and Subtiles, On if available -->
<!-- Open tools menu, Security, Show local captions when present -->

<SYNC Start="0">
  <P Class="ENUSCC" ID=Source>The Speaker</P>
  <P Class="ENUSCC">SAMI 0000 text</P>

  <P Class="FRFRCC" ID=Source>Le narrateur</P>
  <P Class="FRFRCC">Texte SAMI 0000</P>
</SYNC>

resulting screenshot (english):
screenshot000

resulting screenshot (french):
screenshot001

so i think we're good to go with this. best we can do atm.

@phunkyfish
Copy link
Contributor

Will merge shortly if you want to create the backport PR?

@phunkyfish phunkyfish merged commit 06bbfc9 into xbmc:master Apr 26, 2020
@phunkyfish phunkyfish mentioned this pull request Apr 26, 2020
7 tasks
@phunkyfish
Copy link
Contributor

phunkyfish commented Apr 26, 2020

@howie-f also, would you like to join our slack, in case you have any queries which are not PR ready?

If interested PM your email address on the kodi forum.

@howie-f howie-f deleted the v19-fix-smi-subtitles branch April 26, 2020 11:57
@howie-f
Copy link
Contributor Author

howie-f commented Apr 26, 2020

@phunkyfish thanks, that was quick.
seems like i cannot pm on the forum, but you can find the valid email in my commits

Maven85 pushed a commit to Maven85/kodi that referenced this pull request Apr 27, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request May 5, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 3, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 4, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 5, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 6, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Aug 7, 2020
[Subtitles] fix smi subtitles to use quoted start tags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport: Done Component: Video Type: Fix non-breaking change which fixes an issue v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

some smi subtitles not appear
3 participants