GNOME Bugzilla – Bug 754353
avidemux/qtdemux: Output raw video in unaligned buffers causing crashing in ORC video conversion code
Last modified: 2018-11-03 15:03:40 UTC
+++ This bug was initially created as a clone of Bug #736965 +++ See discussions there
I was thinking about that one, there's no way we can always guaranty an alignment. The video decoder should check the amount of unaligned bytes at the beginning and end of each row and special case it.
There is no video decoder here. I believe we have fixed other elements, e.g. matroskademux, to copy raw video frames to ensure sane alignment if needed.
And generally we assume that raw audio and video buffers are properly aligned, everywhere. Producers of such buffers have to ensure that, it's as simple as that.
I meant the video converter should check, sorry for the confusion. All elements should check and adapt or copy (doing late copies / last resort). Otherwise the is no room for improvement. I agree that in case we don't know (demuxer) and that we have to copy anyway, choosing a large alignment is probably best. But I don't agree that all elements should enforce 32bytes alignment.
The minimum alignment is always the "unit size" of the content (32 bit raw audio samples => 4 bytes), or the alignment requirement for any SIMD instructions on the target platform, whichever is bigger. 32 bytes covers that for now, except for 512 bit AVX :)
For audio it's a good rationale, for video it is not. We can't define something that depends on the possible use of hardware specific SIMD and blindly say we fixed the issue. Good examples, you have no control over what v4l devices will aligned to. It might be very high bitrate, and SIMD might never be used with it. Are we going to copy in the source, all the time ? That's basically what this tentative rule is suggesting. We should fix our converters to special case the non aligned portions, we already do in many cases. This is the same thing inside memcpy.
Alignment requirements might be a case for caps negotiation?
They are partially in the allocation query, which our current design cannot be performed by non-threaded demuxers.
Also, they are not really meant to be authoritative. The reason is that it moves the copy upstream, in places where one will not intuitively think it's needed. There's is a benefit to handle it upstream when supported, because it could save an extra copy of upstream is already making a copy in some form.
-- 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-good/issues/216.