-
Notifications
You must be signed in to change notification settings - Fork 1
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
Error thrown for unsupported segment type #10
Comments
Just a note on the sample file. It came from 3D source in Over Under format, so the subtitles would be displayed at the top and the bottom - which is to be expected. Just in case you wonder why. |
Hi @sssanwar, bdsup2subpp report:
libpgs-js reports:
The size of the ODS segment in the 11th set is set to Best I can do is adding a way to ignore critical errors and abort reading. But you won't be able to read past the error (10th display set). |
Not sure what app was used to generate that. Got it from the internet. :) My guess is they just chopped the file without paying attention to the display set boundaries. With bdsup2sub, I was able to retrieve 3 frames without crashing. I think discarding the rest with errors is OK. At least it's not crashing, just doing best effort. |
Just FYI, I made small changes to the 0.4.0 code and I was able to retrieve 17 cues from the same sample file. I haven't tested it with the latest PR#11. Rather than throwing error in displaySet, I set it to continue. if (magicNumber != 0x5047) continue which advances the reader position by 1 while skipping the error part. Then I put a try-catch block at the internal renderer: while (reader.position < reader.length) {
const displaySet = new DisplaySet()
try {
displaySet.read(reader, true)
this.displaySets.push(displaySet)
this.updateTimestamps.push(displaySet.presentationTimestamp)
} catch (e: any) {
errorMessages.push(e.message)
}
} Perhaps not the most elegant way, but it might give you some idea. |
The suggested fix just brute-forces through the stream until it finds a valid magic number. Also it skips two bytes for every magic number read, so you could accidentally skip PGS data if you are offset by one byte. Since the output is the same as in SubtitleEdit, FFmpeg and bdsup2subpp I guess we can say that the file itself is broken and non-valid PGS data. The library shouldn't be dealing with fixing corrupted subtitle files. Accounting every possible error costs a lot of operations and a lot of guess work at runtime. I try to make it as performant as possible for low end devices. You could create a tool that scans for the PGS magic number and tries to restores the segments and write a valid PGS file afterwards. But this shouldn't happen every time at runtime. |
That's true. As mentioned before, it's best effort with no guarantee of recovering from errors.
I think it is a difference between a fault-tolerant system vs a system with little fault tolerance. It boils down to our own system design philosophy. I'd probably make the same argument as yours if I were you. :)
Of course this is not possible. The method I used was actually very simple, and doesn't require extra overhead. The lib is still required to read the file byte by byte regardless of whether there is corrupted data or not. Thanks again for making this great library, and making it open source. I can modify it according to my needs. |
I ran the lib against the attached sample file and it threw an error. Is it possible to skip the invalid segments or display sets with errors and continue on? I tested with bdsup2sub, and it could handle the errors and continue on to the end. Last but not least, thanks for the great work.
sample.sup.zip
The text was updated successfully, but these errors were encountered: