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

Output segment title from EXTINF #158

Merged
merged 1 commit into from
Jul 7, 2023

Conversation

Genteure
Copy link
Contributor

@Genteure Genteure commented Oct 3, 2022

Currently segment titles are parsed but not returned.

Close #90

@@ -19,7 +19,8 @@ module.exports = {
},
duration: 10,
timeline: 0,
uri: 'hls_450k_video.ts'
uri: 'hls_450k_video.ts',
title: ';asljasdfii11)))00,'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Source text:

#EXTINF:;asljasdfii11)))00,

I think if we were to strictly follow the spec this should fail, since ;asljasdfii11)))00 is not a decimal-floating-point nor decimal-integer.
But I just went with the easiest route to make the tests pass.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, missed your comment, yeah, I think this is probably incorrect but that's likely an issue with the parsing/regex in

match = (/^#EXTINF:?([0-9\.]*)?,?(.*)?$/).exec(newLine);
if (match) {
event = {
type: 'tag',
tagType: 'inf'
};
if (match[1]) {
event.duration = parseFloat(match[1]);
}
if (match[2]) {
event.title = match[2];
}
this.trigger('data', event);
return;
}

and shouldn't be a blocker for this PR.

Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -19,7 +19,8 @@ module.exports = {
},
duration: 10,
timeline: 0,
uri: 'hls_450k_video.ts'
uri: 'hls_450k_video.ts',
title: ';asljasdfii11)))00,'
Copy link
Member

Choose a reason for hiding this comment

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

this doesn't seem right. Though, that's down to how it's being parsed and not a blocker for this PR.

@arthurbolsoni
Copy link

arthurbolsoni commented Apr 2, 2023

pls merge this pr

@elvezpablo
Copy link

Would love to be able to use the title data in my time lapse project.

@adrums86 adrums86 merged commit 4adaa2c into videojs:main Jul 7, 2023
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.

parse title from extinf
6 participants