h264: Use consistent size of scaling buffer #1785
Conversation
any1
force-pushed
the
h264-buffer-size-fix
branch
from
f08e649
to
27d0051
Compare
any1
force-pushed
the
h264-buffer-size-fix
branch
from
27d0051
to
9e36dba
Compare
Sure, but I don't use Windows and I do not have any computer set up with Windows, so I'm not your guy for that. |
|
Windows needs to be figured out, as I'm not ready to claim compatibility with neatvnc if it doesn't work on all platforms. :/ @mmozeiko, @xornet-sl, do you think you can have a look? There looks to be some cropping code already in place? |
That code is manually applying the cropping region specified by the SPS. It does not seem to take into account the size of the rect. Edit: Actually, the windows code looks right to me. It should work fine as is and should not crash, no matter how badly the server behaves. |
|
Come to think of it, and looking at the linux kernel implementation of the Thus, this is really just a matter of proper alignment and padding, and not misbehaving encoders, although they should definitely be guarded against. |
any1
force-pushed
the
h264-buffer-size-fix
branch
from
9e36dba
to
65b37cc
Compare
|
@CendioOssman what is the next step to merge the PR ? I think this will benefit for some users (I have PiKVM in mind now) |
|
It's not currently building properly, so that would need to be fixed. Otherwise, I don't think there are any open issues. |
We can disable h264 depending on whether |
|
I thought we already checked for swscale? Or is that some feature that requires a very new ffmpeg? |
|
it requires one that have swscale_frame (which was added in Sep2021 in ffmpeg) . However I can we can rewrite that .. if we do not want to depend on a newer ffmpeg |
|
That sounds like it might not be included in EPEL 8 and Ubuntu 20.04. So if it's possible to support those as well, then it would be appreciated. |
|
This feature was added in version 1.13.0, so I suppose that this fix should be backported to 1.13. Jammy has 1.12.0: https://launchpad.net/ubuntu/jammy/+package/tigervnc-viewer Older version of Ubuntu are bound to have older releases. The Ubuntu versions on which the build fails are not going to receive this update unless a user wants to compile manually. I don't see the point in chasing old software, but if that's how you want things, I can make this work with older ffmpeg. Some time might pass until I find the time to do so. |
|
We don't do bugfix releases for older branches, so you don't need to worry about those. We also do our Ubuntu packages, which we do for all currently active Ubuntu versions. It is not an absolute requirement that every feature should be available on all of them, but we should try. |
This ensures that the buffer is allocated with the correct alignment and padding for use with sws_scale. This fixes out-of-bounds writes which would in some cases cause segmentation faults and/or heap corruption.
This fixes a memory leak
any1
force-pushed
the
h264-buffer-size-fix
branch
from
3b32ca3
to
3596646
Compare
|
It should compile on those old Ubuntus now. I also have a colour space fix lined up for this, but I can make a separate PR for that when this is merged. |
|
Looks good to me. |
This fixes out-of-bounds writes when there is a mismatch between the pixel buffer's stride and the rect's width or between the frame's height and the rect's height.