emk / subtitles-rs

Use SRT subtitle files to study foreign languages (in progress)
Apache License 2.0
304 stars 32 forks source link

Vobsub: allow parsing input stream by chunks #41

Closed fengalin closed 6 months ago

fengalin commented 6 years ago

This is a first step toward https://github.com/sdroege/gst-plugin-rs/issues/21

The PR adds a new struct SubtitlesFromChunks which keeps track of the parsing context and allows passing chunks.

Except if I missed something, the API and behaviour for Subtitles is unchanged. I couldn't find a solution to avoid using the Rc<RefCell<>> for SubtitlesContext without breaking the existing API.

Changes were tested using examples in "fixtures" (including for the parse_corpus ignored set).

fengalin commented 6 years ago

It turns out I will need to process demuxed streams which contain only data and control sequences (no header), so I need extra work on vobsub. I'm thinking about doing it in a separate PR though.

emk commented 6 years ago

Thank you very much for this PR! I totally want to integrate any changes needed for GStreamer.

I'll have some time tomlook at this the coming weekend. If you have any questions, please don't hesitate to ask. And if you don't hear from me, please feel free to ping me here.

emk commented 6 years ago

I'm on the road for part of today, but I've pulled your proposed branches locally, and will try to find time to look at them either today or tomorrow.

This looks really great, BTW. I may have a few suggestions, but I like the overall strategy and I'm delighted to have these new features.

(And once the new code is integrated, we'll need to fire up the fuzzer again!)

fengalin commented 6 years ago

Great to read that! Please note I've updated #42 around 12:00 UTC today, so you may need to pull the branch again if you cloned it before that time.

fengalin commented 6 years ago

Hi @emk! Do you think you will get time to review this PR?

emk commented 6 years ago

Sorry! Just after getting back from vacation, our house-buying schedule got made much shorter by outside events, and I'm currently operating on about 5 hours of sleep a night. I absolutely will review your patches soon, but I'm trying to get a few other distractions out of the way first. I can't give you an exact ETA, but I'm hoping to have some time to really look at these in the next week.

My apologies for the delay. :-( These patches are very welcome—and very interesting—and I really want to dig into them.

fengalin commented 6 years ago

No problem! I'm not in a rush anyway.