[Bug 58425] New: video/x-h264 alignment=au caps causes artifacts and crashes with nal-aligned streams
http://bugs.winehq.org/show_bug.cgi?id=58425 Bug ID: 58425 Summary: video/x-h264 alignment=au caps causes artifacts and crashes with nal-aligned streams Product: Wine Version: 10.11 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: winegstreamer Assignee: wine-bugs(a)winehq.org Reporter: mkrsym1(a)gmail.com Distribution: --- Multiple applications try to playback nal-aligned streams, but Wine always sets the alignment caps to au. This causes h264parse to work incorrectly, which in turn leads to various unpleasant effects, depending on the decoder used. Some examples: - libav's avdec_h264 is the most tolerant one, and only displays brief image artifacts when au-alignment is broken. - OpenH264 displays major image and temporal artifacts on au-alignment break, sometimes hangs. - NVIDIA's nvh264dec immediately crashes the entire application on au-alignment break. This issue is completely fixed by not setting the alignment caps at all and letting h264parse do it's job, as done in this commit: https://github.com/ValveSoftware/wine/commit/0304a42 Not sure why it's set in the first place. Maybe that patch can be merged into Wine master, or is there something that I'm missing? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 mkrsym1(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mkrsym1(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #1 from Ken Sharp <imwellcushtymelike(a)gmail.com> ---
Multiple applications
Which ones? It's helpful if there is a download available to test against. It also helps to attach the bug to the relevant entries in the AppDB. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #2 from mkrsym1(a)gmail.com --- Mostly CRIWare under Unity. Can't find anything that is both free and public, but there probably is. I'll try to make a test application to show the issue. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #3 from mkrsym1(a)gmail.com --- Upon further investigation, the underlying problem appears to be different from what I first described. Most buffers submitted by CRIWare are actually au-aligned, but sometimes the last nal is incomplete, and it's continuation is submitted in the next buffer, after which following buffers are correctly au-aligned again. Windows can somehow handle this nightmare, but as of now, Wine cannot. Letting h264parse align the buffers by not setting alignment=au makes the behavior match Windows. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 mkrsym1(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|video/x-h264 alignment=au |video/x-h264 alignment=au |caps causes artifacts and |caps causes artifacts and |crashes with nal-aligned |crashes with |streams |"almost-aligned" streams -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #4 from mkrsym1(a)gmail.com --- Created attachment 78875 --> http://bugs.winehq.org/attachment.cgi?id=78875 Test program Here is a test program that demonstrates the problem in a very exaggerated way, by splitting the last NALU in every AU in half. Test steps: 1. Recode a video to Annex B with e.g. `ffmpeg -i input_video -c:v libx264 -x264-params 'annexb=1:aud=1' -f h264 test.h264`. 2. Try decoding the video with `test_align.exe test.h264 out.bin` on Wine 10.11. If using avdec_h264, you will get lots of buffer overread errors. Verify artifacts with `ffplay -f rawvideo -video_size WxH out.bin`. 3. Try decoding the video on Windows in the same way. Verify that there aren't any artifacts with ffplay. 4. Try decoding the video on Wine again, but with the abovementioned patch. Verify that the characteristic artifacts are not there anymore. Sidenote: when decoding on Wine, there often appears a green strip at the bottom of the video. That does not depend on whether the video was "preprocessed", so it's a different bug. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 mkrsym1(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rbernon(a)codeweavers.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 mkrsym1(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bmcgrath(a)codeweavers.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #5 from mkrsym1(a)gmail.com --- CC Rémi Bernon per git blame, Brendan McGrath as commit author. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 mkrsym1(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |testcase -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #6 from Brendan McGrath <bmcgrath(a)codeweavers.com> --- One of the reasons I felt this was safe for Proton but not for upstream was because I know 'h264parse' is always available with Proton. For upstream wine however, it will require the user to install `gst-plugins-bad`. Without 'h264parse' available, removing the 'au' alignment could make things worse (I'll need to double check). But even so, there is probably a way to check if 'h264parse' is missing and then fallback to 'au' alignment in that scenario. I'll look in to it. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 Zeb Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12(a)gmail.com --- Comment #7 from Zeb Figura <z.figura12(a)gmail.com> --- (In reply to Brendan McGrath from comment #6)
One of the reasons I felt this was safe for Proton but not for upstream was because I know 'h264parse' is always available with Proton. For upstream wine however, it will require the user to install `gst-plugins-bad`.
Without 'h264parse' available, removing the 'au' alignment could make things worse (I'll need to double check). But even so, there is probably a way to check if 'h264parse' is missing and then fallback to 'au' alignment in that scenario.
I'll look in to it.
I think depending on h264parse is fine. We need it anyway for other reasons; cf. 5988. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #8 from Brendan McGrath <bmcgrath(a)codeweavers.com> --- I ran the following (on Ubuntu using default repos): $ apt-cache depends libwine | grep gstreamer and got: Depends: libgstreamer-plugins-base1.0-0 Depends: libgstreamer1.0-0 Recommends: gstreamer1.0-plugins-good Suggests: gstreamer1.0-libav Suggests: gstreamer1.0-plugins-bad Suggests: gstreamer1.0-plugins-ugly So I guess h264 decoding wouldn't work with a default install anyway (i.e. without --install-suggests). avdec_h264 requires gstreamer1.0-libav, and the other h264 decoders I looked at needed gstreamer1.0-plugins-bad. What about other downstream projects? Do they all include gstreamer1.0-plugins-bad? Alternatively, I was thinking I could separate the caps into: - known values; and - assumed values Then apply the assumed values only when we don't have a parser. For example, using H.264, assumed values are: gst_caps_set_simple(caps, "stream-format", G_TYPE_STRING, "byte-stream", NULL); gst_caps_set_simple(caps, "alignment", G_TYPE_STRING, "au", NULL); But if a parser is present, we don't have to assume those values (as the parser will determine them for us). -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #9 from Brendan McGrath <bmcgrath(a)codeweavers.com> --- I've raised the following MR: https://gitlab.winehq.org/wine/wine/-/merge_requests/8493 I've just cherry-picked what was in Proton. I couldn't find anything downstream that wasn't at least including the videoparsers from gst-plugins-bad. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #10 from mkrsym1(a)gmail.com --- Looking at the MR, I believe it needs to be said that the stream in question is actually neither nal-aligned, nor au-aligned. A nal-aligned stream would not have NALUs split across buffers, which happens here, and is correctly handled both by Windows, and by h264parse if alignment is not specified. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 mkrsym1(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|video/x-h264 alignment=au |video/x-h264 alignment=au |caps causes artifacts and |caps causes artifacts and |crashes with |crashes with streams that |"almost-aligned" streams |have NALUs split across | |buffers -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #11 from Brendan McGrath <bmcgrath(a)codeweavers.com> --- You are right. Thanks for that. I've updated the commit message to better reflect how it works. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 --- Comment #12 from Brendan McGrath <bmcgrath(a)codeweavers.com> --- Thanks for raising this. This should be fixed with f3de748e53b892533ae4f2a220a1f549bfd6adf3. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 mkrsym1(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|UNCONFIRMED |RESOLVED --- Comment #13 from mkrsym1(a)gmail.com --- Can confirm that the issue is no longer present as of 10.12. Thank you. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 Ken Sharp <imwellcushtymelike(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |f3de748e53b892533ae4f2a220a | |1f549bfd6adf3 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58425 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #14 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 10.13. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla