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

Odd heights #349

Open
rakslice opened this issue Jan 13, 2025 · 2 comments
Open

Odd heights #349

rakslice opened this issue Jan 13, 2025 · 2 comments

Comments

@rakslice
Copy link
Contributor

rakslice commented Jan 13, 2025

I don't know how much this really matters, since maybe everyone just avoids this case already for now by just not calling with height values that obviously don't work, but for posterity:

When the height of the video is an odd number, we get some lines of chroma error at the top, and a single line of it at the bottom.

image

Also for some odd heights but not others it seems like the bottom line is truncated in the pre-scaling video (this is easier to see at lower resolutions with something like this caption with a gap underneath at the bottom):

image

I suspect playing back videos where the resolution is not a multiple of the subsampling is probably a bad time generally because of all the instances in the code of stuff like:
height >> p[i].ss.y
and
((width * height) >> (p[i].ss.x + p[i].ss.y))
i.e. right shifting by the subsampling bits, in other words dividing by the subsampling multiple rounding down, when you need to round up -- the actual plane obviously has to have another line to cover the last less-than-a-full-subsample's-worth lines of the overall image.

But just fixing instances of those that I could find isn't enough on its own. There's something that needs the extra full subsample worth of lines in the output image, I guess. The hack of just doing e.g.:

    height = ROUND_UP(height, 2);

at the top of nvCreateSurface2() and

    picture_height = ROUND_UP(picture_height, 2);

at the top of nvCreateContext() does make all the problems go away, at least testing from mpv.

@elFarto
Copy link
Owner

elFarto commented Jan 13, 2025

I think that would be fine to add those ROUND_UPs in. It has no effect on 'normal' evenly sized videos so it shouldn't be an issue.

@thesword53
Copy link
Contributor

The issue is in CUVIDDECODECREATEINFO where ulTargetWidth and ulTargetHeight should be multiple of 2. In ffmpeg ulTargetWidth and ulTargetHeight are rounded up to 2.

cuviddec.c:167 FFmpeg n7.1

    // target width/height need to be multiples of two
    cuinfo.ulTargetWidth = avctx->width = (avctx->width + 1) & ~1;
    cuinfo.ulTargetHeight = avctx->height = (avctx->height + 1) & ~1;

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

No branches or pull requests

3 participants