enc: h264: Implement software encoding. - #169
Conversation
any1
left a comment
There was a problem hiding this comment.
Thanks for the contribution.
I experimented with this some on the weekend, but I implemented an ffmpeg + libx264 encoder as a separate encoding backend. It duplicates some code, but it makes the whole control flow easier to reason about.
However, your approach isn't as bad as I would have thought originally, so I'll want to have think about which I prefer.
I did find the CPU usage to be rather high and all the copying extremely wasteful in regard to memory bandwidth. Did you perform any performance tests of your own?
| } | ||
|
|
||
| int32_t stride = fb->stride * src_fmt.bits_per_pixel / 8; | ||
| if (nvnc_buffer_map(fb->buffer, fb->width, fb->height, &stride) < 0) { |
There was a problem hiding this comment.
There's a wrapper for this in nvnc_frame_map.
There was a problem hiding this comment.
Thanks for the info! I will change my commit accordingly.
| } | ||
|
|
||
| for (int y = 0; y < fb->height; y++) { | ||
| memcpy(frame->data[0] + y * frame->linesize[0], |
There was a problem hiding this comment.
Do you have a good reason for doing a copy here? The format conversion filter should have a fairly regular access pattern, so I doubt that this would be more performant than just letting the filter access the buffer directly.
There was a problem hiding this comment.
I am not sure if I can assume that the stride of the AVFrame is the same as the stride of nvnc_frame.
But I could probably check and only copy if they are different.
There was a problem hiding this comment.
I don't see why the filter would not work with any given source stride. I's probably doing sws_scale under the bonnet which takes source linesizes and destination linesizes as separate arguments.
If I remember correctly, the destination buffer must have a specific stride and some padding after the buffer for SIMD operations not to overrun the buffer, but that constraint does not apply to the source buffer.
Looking at this more closely, I'm left wondering how this worked at all in the first place because you're treating nvnc_fb::stride like it's a byte stride, but it's actually pixel stride.
There was a problem hiding this comment.
I don't see why the filter would not work with any given source stride. I's probably doing
sws_scaleunder the bonnet which takes source linesizes and destination linesizes as separate arguments.If I remember correctly, the destination buffer must have a specific stride and some padding after the buffer for SIMD operations not to overrun the buffer, but that constraint does not apply to the source buffer.
I see. I didn't realize that I can just set the linesize[0] of the AVFrame to our stride. Then no memcpying would be necessary.
Looking at this more closely, I'm left wondering how this worked at all in the first place because you're treating
nvnc_fb::stridelike it's a byte stride, but it's actually pixel stride.
I changed nvnc_frame_map to change the stride to a byte value. In hindsight, that was probably not a good idea as this might interfere with other encodings. I'll undo that.
|
Thank you for the fast reply and sorry for the delay. I was sick in the hospital.
No formal tests, but I have been using this at work for a few weeks now and it seems to works very well. The software encoder has a better visual quality than the hardware encoder of the Intel i9-12900 and it feels faster than other encoding methods, especially when connecting from home. |
|
I have been using this version without the extra memcpy for the past week at work. The maximum CPU usage was around 50% on a single core (around 2% overall). The average CPU usage was much less. Overall, it had no noticeable impact for me.. |
Allow using H.264 software encoding when no GPU is available or the GPU encoder doesn't output sufficient quality.
This addresses any1/wayvnc#374.
I have read and understood CONTRIBUTING.md.