Okay, this may seem a silly question, but why didn't the case 1 block work in the original version? I'm new to the codebase and despite my best grep-fu can't seem to find where seq->enddisp is interpreted by the UI as being exclusive to the frame range rather than inclusive.

Thanks!

In D8841#216951, @Henry Gernhardt (hcgernhardtiii) wrote:

Okay, this may seem a silly question, but why didn't the case 1 block work in the original version? I'm new to the codebase and despite my best grep-fu can't seem to find where seq->enddisp is interpreted by the UI as being exclusive to the frame range rather than inclusive.

Thanks!

Sorry I really don't understand question. if you had more strips on both side of playhead in line you would see better how this selection was wrong before and what this change does.

Sorry I really don't understand question. if you had more strips on both side of playhead in line you would see better how this selection was wrong before and what this change does.

Sorry I wasn't more clear. What I'm seeing looks to me like a potential off-by-one error that may be promulgated elsewhere, and that's what I'm asking about.

Looking at the logic of your fix, I can see that it works: a strip to the left of the playhead (case -1) will end at or before the playhead, and a strip to the right of the playhead (case 1) will begin at or after the playhead. This makes sense logically and semantically based on what I've found in the codebase. In the VSE, however, two strips on the same channel cannot share the same frame as the endpoint of the frame to the left and the start point of the frame to the right. To use the screencap I posted as an example, the left strips have their endpoint at frame 647 whilst the right strips have their start points at frame 648. The playhead is positioned at frame 648.

Assuming that the values stored in the Sequence structue (Sequence.startdisp and Sequence.enddisp, as defined in DNA_sequence_types.h) correspond to the start and end frames shown in the UI, and using the values in the screencap, here's my logic for the original code:

• case -1 (left) should erroneously select the strip to the right, as the right strip has a displayed start frame equal to that of the playhead frame position.
• case 1 (right) should not erroneously select the strip to the left, as the playhead (frame 648) is at a greater position than the displayed end frame of the strip to the left (frame 647)

This implies that, in the Sequence structure, the code is storing an end display frame value that is one greater than that which is displayed in the UI. (in this example, the strips ending at frame 647 would, by this logic, have the enddisp value stored as 648 within the Sequence structure).

This ambiguity is what led me to not provide the solution you did. Not because your solution doesn't work (the logic is sensible as I run through it), but I hadn't the foggiest how these numbers are defined and used within the Blender codebase. I'm unfamiliar enough with the codebase that I don't know where to begin to find the Python bindings or other logic to determine how the Sequence.enddisp and Sequence.startdisp values are either calculated, stored, or interpreted by the UI.

So, the crux of my question: Is there something in the codebase somewhere that interprets Sequence.enddisp as being akin to the position of the \0 at the end of a standard C string (i.e, we don't count it as where the strip actually ends, but one greater)? Again, I'm not confused by your fix. Your fix makes logical and semantic sense and should function appropriately. I'm just wondering why a strip that ends at frame 647 in the VSE would have its internal representation such that seq->enddisp == 648.

In D8841#217247, @Henry Gernhardt (hcgernhardtiii) wrote:

I'm just wondering why a strip that ends at frame 647 in the VSE would have its internal representation such that seq->enddisp == 648.

Yes I see this now. Will have to look, but it looks like a bug on first sight. So value on end handle looks incorrect.

@Richard Antalik (ISS) Somewhat related, 'Select Under Playhead', are you going to do something about that patch too, now you're dealing with selection in relation to the playhead position?
(Imo, it's absurd that it is not possible to select all strips at playhead position through a shortcut key, since you'll have to do that all of the time before splitting video+audio)

I was thinking about it when I was here, but decided not to do it in this patch

@Richard Antalik (ISS), @Peter Fog (tintwotin) —

Somewhat related, 'Select Under Playhead', are you going to do something about that patch too…

I was thinking about it when I was here, but decided not to do it in this patch

Looks to me (from my codebase naïve standpoint) like a relatively simple matter of setting up a python binding for case 0, which should work unmodified for select under playhead. If you can give me a nudge in the correct direction of these bindings, I wouldn't mind playing with it on my end.