Skip to content
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

Use the video frame's declared line stride if it is non 0 #8

Merged
merged 2 commits into from
Aug 3, 2024

Conversation

guillaumeriousat
Copy link
Contributor

This fixes #7

There should probably be additional tests for this edge case but the testing code is a bit beyond me for the time I have to invest.

@nocarryr
Copy link
Owner

nocarryr commented Aug 2, 2024

This is interesting. I wouldn't be surprised that I got some of that math wrong.

One thing though:
The _recalc_pack_info method is also used in VideoSendFrame. In that scenario, the frame pointer's line stride will always need to be set (since it's updated from user input on resolution, etc).

Maybe it would be better to add a bint argument to "force" the use of NDIlib_video_frame_v2_t.line_stride_in_bytes for instances of VideoRecvFrame and VideoFrameSync?

@nocarryr
Copy link
Owner

nocarryr commented Aug 2, 2024

Something like

cdef int _recalc_pack_info(self, bint use_ptr_stride=True) except -1 nogil:
    cdef FourCC fcc = self._get_fourcc()
    cdef bint changed = False
    cdef size_t line_stride = 0
    if use_ptr_stride:
        line_stride = self.ptr.line_stride_in_bytes
    if self.pack_info.fourcc != fcc:
        self.pack_info.fourcc = fcc
        changed = True
    if self.ptr.xres != self.pack_info.xres or self.ptr.yres != self.pack_info.yres:
        self.pack_info.xres = self.ptr.xres
        self.pack_info.yres = self.ptr.yres
        changed = True
    if self.pack_info.xres == 0 or self.pack_info.yres == 0:
        return 0
    if changed:
        calc_fourcc_pack_info(&(self.pack_info), line_stride)
        if not use_ptr_stride:
            self.ptr.line_stride_in_bytes = self.pack_info.bytes_per_pixel * self.ptr.xres
    return 0

I think? I could be wrong since I'm just typing this and can't compile right now, but hopefully you get what I mean

And then an override in VideoSendFrame:

cdef int _recalc_pack_info(self, bint use_ptr_stride=True) except -1 nogil:
    return VideoFrame._recalc_pack_info(self, False)

EDIT:

After a bit of testing with your code (and the additions above), it's clear there's an issue. The padding however for the VideoSendFrame won't be corrected with this, but that's for another day.

Ideally, I'd like to fix the calculations in the pack info so both send and receive will be correct, but it's the end of a long work week and I don't think I have the mental capacity for that right now.

If those changes above work on your setup though, I'll merge. Just let me know

Also, thanks!

@guillaumeriousat
Copy link
Contributor Author

Woops, I didn't think I was breaking the VideoSendFrame by doing that...

I made a commit with the changes you suggested and it still works for my use case !

Thanks for the library and the responsivity !

@nocarryr
Copy link
Owner

nocarryr commented Aug 3, 2024

LGTM
Thanks for finding the issue and the fix!

@nocarryr nocarryr merged commit 9e2024b into nocarryr:main Aug 3, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The line stride declared by the NDI video frame is ignored which causes issues for non standard resolutions
2 participants