GNOME Bugzilla – Bug 790087
videodecoder: performance regression with libav decoders
Last modified: 2018-11-03 12:01:18 UTC
Commit ae8d0cf3acfa (videodecoder: Make sure we have an actually writable buffer when modifying metadata) introduces a performance regression when using libav based decoders with a downstream pool. More specifically, in the following use case the buffer copy is actually not shallow, the whole memory is copied: gst-launch-1.0 <source> ! avdec_xxx ! waylandsink In this case waylandsink allocates a buffer pool with SHM memory (fd allocator), but the data is copied on the call to gst_buffer_make_writable() in the offending commit. This also prevents reuse of the wl_buffer objects, since those are now attached on a transient buffer and not on the buffer from the pool. I think the only way to fix this properly would be to remove the reference on the buffer that libav has, however I can't find where this reference is kept and used. Any idea ?
The commit is fine, ffmpeg direct rendering is really weird. There is a entire new set of function for decoding in ffmpeg, the answer might be in there.
This is probably related to https://bugzilla.gnome.org/show_bug.cgi?id=740222
I'm sure it is. But would a remap to read would really help here ? What the patch do is basically enforce that the base class is owner, which is incompatible with keeping reference frames.
Yes, it's not going to solve this specific issue. If a shallow copy of a buffer to make its metadata writable is not possible, that's problematic in general
Ah, I see, indeed it will be shallow instead of hard copy. But then we need to chase and remove all code the enable zero copy in sinks by checking: buffer->pool == my_pool And check that the memory objects are correct, that the VideoMeta is still supported etc. It's pretty hard and there is no generic code to check this. For waylandsink, it will copy in a more creative way: wlbuffer = gst_buffer_get_wl_buffer (buffer); Which grabs a cached wlbuffer from the original buffer quark, which won't be copied by a shallow copy. So no, it won't fix it for waylandsink.
I'm more concerned about the video memory copy than the issue of recreating wl_buffers, which is not really a problem; it's a light operation. Would it be possible to attach a WlBufferMeta to the buffers from the pool instead of using a data quark ? This meta would be copied by a shallow copy, right ? This would fix the wl_buffer related issue.
In fact, you might avoid the copy through: if (gst_is_fd_memory (mem)) { wbuf = gst_wl_shm_memory_construct_wl_buffer (mem, sink->display, &sink->video_info); ... Which was put in place to pass DMABuf that are not otherwise supported. Still, it's far from ideal. Creating the wl_buffer is more expensive then you think.
Actually that's the path I'm using with the avdec_xxx ! waylandsink pipeline. The copy still happens when gst_buffer_make_writable is called in gstvideodecoder.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/397.