cubicibo / SUPer

HDMV PGS (BD SUP) subtitle encoder compatible with typesetting effects.
GNU General Public License v3.0
21 stars 4 forks source link

Add sort entries option #31

Closed MonoS closed 3 months ago

cubicibo commented 3 months ago

Hi,

Would you mind telling which software generates BDNs with unordered events? Or what would be the use-case?

I know Lemony Pro implements notions of concurrent events that overlap in time, but these are not supported by SUPer, nor by your approach of sorting the events.

MonoS commented 3 months ago

I wanted to merge additional subpic to an already existing sup, the faster way i was able to find was to generate the new subpic with ass2bdnxml, demux the original and then add the new subpic to the demuxed bdn.xml, as the new subtitle where all across the timeline i found it simpler to modify your tool and sort the events afterwards, i was not able to find a different solution, but i didn't search much Here the fine i've generated ita.part_exp.xml.zip

It's probably cursed as a solution, but it worked almost fine

cubicibo commented 3 months ago

Totally forgot that text editors existed.

I think it is acceptable to always sort events, and not clutter the user interface.

Suggestion:

MonoS commented 3 months ago

Do you think this will be sufficient?

event_prec = None
for event in self.events:
    assert TC.tc2f(event.tc_in, self.fps) < TC.tc2f(event.tc_out, self.fps), "An event ends before starting"
    if event_prec is not None:
        assert TC.tc2f(event_prec.tc_out, self.fps) <= TC.tc2f(event.tc_in, self.fps), "An event ends after the next one starting"

    event_prec = event
cubicibo commented 3 months ago

Yes, that ensures the timeline is monotonic. More concise is maybe more readable here:

for k, ev in enumerate(self.events):
    assert TC.tc2f(ev.tc_in, self.fps) < TC.tc2f(ev.tc_out, self.fps), f"Event at InTC={ev.tc_in} has a zero duration."
    assert 0 == k or TC.tc2f(self.events[k-1].tc_out, self.fps) <= TC.tc2f(ev.tc_in, self.fps), f"Two events overlap in time around InTC={ev.tc_in}."

(I really need to get rid of those string timestamps...)

MonoS commented 3 months ago

Ok, pushed the latest changes, python is not my language of choice so that you for the snippet there :)

cubicibo commented 3 months ago

Seems to work nicely, thank you. Please squash the two commits and, in the process, remove the stray whitespaces in sort_events_func.

MonoS commented 3 months ago

I don't see any trailing whitespace in the function immagine

Also i don't know how to squash, i can try searching for some guide and try to do it, but it seems it can be done directly from the github interface when you close the PR, is that feasible?

cubicibo commented 3 months ago

The two lines around self.events.sort(key=...) should have no spaces preceeding the Line Feed. Running git diff before committing changes should highlight said whitespaces like this. whitespaces_filestreams

Sure, I can squash the commits on merge. But please remove the whitespaces in a new commit.

MonoS commented 3 months ago

Ok, i should have succesfully squashed the commits, hope everything is ok now.