MonoS / SupMover

SupMover - Shift timings and Screen Area of PGS/Sup subtitle
GNU Affero General Public License v3.0
39 stars 4 forks source link

Bug in crop mode? #13

Closed YourMJK closed 5 months ago

YourMJK commented 5 months ago

If I understand the code right then the same i, which is actually the index of the wds.windows being iterated, is also used here to index pcs.compositionObject:

https://github.com/MonoS/SupMover/blob/9646a10a0f7aac351f9f18fad99f86e469ce7bf5/main.cpp#L973-L980

I don't know much about the PGS spec but this can't be right?

This should only work if a Display Set contains exactly one composition object/image per window (spec says can also be two) and their order is the same (not sure if that's guaranteed). Besides that, my example .sup files have Display Sets with pcs.numCompositionObject == 0 but wds.numberOfWindows > 0, so uninitialized t_compositionObjects are being manipulated in that case.

I supposed it should be something like this instead:

if (corrVer != 0 || corrHor != 0) {
    for (int j = 0; j < pcs.numCompositionObject; j++) {
        if (pcs.compositionObject[j].windowID != wds.windows[i].windowID) continue;
        pcs.compositionObject[j].objectVerPos -= corrVer;
        pcs.compositionObject[j].objectHorPos -= corrHor;
    }
    fixPCS = true;
}
MonoS commented 5 months ago

I think you are completely right, I've probably missed the windowID field when implementing the corrVer/Hor handling and worked fine as it is on my test case.

Please make a pull request if you want, I'll merge it immediately :)