[Bug 766978] New: mpeg-2 decoding broken

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Bug ID: 766978
Summary: mpeg-2 decoding broken
Classification: Platform
Product: GStreamer
Version: git master
OS: Linux
Status: NEW
Severity: critical
Priority: Normal
Component: gstreamer-vaapi
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@zen.co.uk
QA Contact: gstreamer-***@lists.freedesktop.org
CC: ***@gmail.com, ***@igalia.com
GNOME version: ---

$ gst-play-1.0 https://people.freedesktop.org/~tpm/samples/bbcnews.m2t

Doesn't work at all for me with gstreamer-vaapi (i.e. if vaapisink is picked as
sink which it is by default). Lots of criticals and a pink image. Works with
glimagesink and xvimagesink.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Severity|critical |blocker

--- Comment #6 from Tim-Philipp Müller <***@zen.co.uk> ---
git bisect yields:

7baacda91c1e2f66d4618669862b7b6f7dfc4f6c is the first bad commit
commit 7baacda91c1e2f66d4618669862b7b6f7dfc4f6c
Author: Víctor Manuel Jáquez Leal <***@intel.com>
Date: Thu May 5 18:23:10 2016 +0200

vaapipostproc: negotiate frame size fixation

Refactor _fixate_frame_size(). Now, instead of fixating the frame size only
using the sink caps, also it use the next capsfilter.

This code is a shameless copy of gst_video_scale_fixate_caps() from

https://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst/videoscale/gstvideoscale.c?id=1.8.1#n634

https://bugzilla.gnome.org/show_bug.cgi?id=758548

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #10 from Tim-Philipp Müller <***@zen.co.uk> ---
(not directly related, but isn't vaapidecode/postproc supposed to do some
deinterlacing here?)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |NEEDINFO

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #1 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Hi Tim,

Which hardware are you using? Because I tested it, as is, in my Haswell and I
don't see those artifacts neither the criticals.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #11 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
(In reply to Tim-Philipp Müller from comment #10)

Post by "GStreamer" (GNOME Bugzilla)
(not directly related, but isn't vaapidecode/postproc supposed to do some
deinterlacing here?)

vaapipostproc should do deinterlacing.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEEDINFO |NEW

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #9 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Great!

Though I don't get those errors, what I found in this particular sample is
resized, from 720x576 to 1024x576, because of the display-aspect-ratio.

But also, glimagesink and ximagesink handle well that resize, but vaapisink
doesn't.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #3 from Tim-Philipp Müller <***@zen.co.uk> ---
Created attachment 328722
--> https://bugzilla.gnome.org/attachment.cgi?id=328722&action=edit
debug log (xz)

0:00:01.949885021 6898 0x7f5094002b20 ERROR vaapivideomemory
gstvaapivideomemory.c:259:gst_video_meta_map_vaapi_memory: failed to make image
current
0:00:01.949913223 6898 0x7f5094002b20 ERROR default
video-frame.c:161:gst_video_frame_map_id: failed to map video frame plane 0
WARNING Internal GStreamer error: code not implemented. Please file a bug at
http://bugzilla.gnome.org/enter_bug.cgi?product=GStreamer.
WARNING debug information: gstvideofilter.c(292): gst_video_filter_transform
():
/GstPlayBin:playbin/GstPlaySink:playsink/GstBin:vdbin/GstVideoConvert:vdconv:
invalid video buffer received

Full log attached, compressed with xz.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #8 from Tim-Philipp Müller <***@zen.co.uk> ---
If I revert just that one commit, it works again, fwiw.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #5 from Tim-Philipp Müller <***@zen.co.uk> ---
Works with gstreamer-vaapi 1.8.0 (and gst master)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #7 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
:(

I don't understand. That commit only get the frame resize in downstream caps.

I have all gstreamer repos in master and I can't reproduce the issue. Perhaps I
should make a clean setup of everything.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #4 from sreerenj <***@gmail.com> ---
Just a quick guess.
I too can't reproduce the issue. But my gstreamer-environment in not the
latest. So could be something related with recent changes in upstream ???

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #2 from Tim-Philipp Müller <***@zen.co.uk> ---
$ vainfo
libva info: VA-API version 0.39.0
libva info: va_getDriverName() returns 0
libva info: Trying to open /usr/lib/x86_64-linux-gnu/dri/i965_drv_video.so
libva info: Found init function __vaDriverInit_0_39
libva info: va_openDriver() returns 0
vainfo: VA-API version: 0.39 (libva 1.7.0)
vainfo: Driver version: Intel i965 driver for Intel(R) Haswell Mobile - 1.7.0
vainfo: Supported profile and entrypoints
VAProfileMPEG2Simple : VAEntrypointVLD
VAProfileMPEG2Simple : VAEntrypointEncSlice
VAProfileMPEG2Main : VAEntrypointVLD
VAProfileMPEG2Main : VAEntrypointEncSlice
VAProfileH264ConstrainedBaseline: VAEntrypointVLD
VAProfileH264ConstrainedBaseline: VAEntrypointEncSlice
VAProfileH264Main : VAEntrypointVLD
VAProfileH264Main : VAEntrypointEncSlice
VAProfileH264High : VAEntrypointVLD
VAProfileH264High : VAEntrypointEncSlice
VAProfileH264MultiviewHigh : VAEntrypointVLD
VAProfileH264MultiviewHigh : VAEntrypointEncSlice
VAProfileH264StereoHigh : VAEntrypointVLD
VAProfileH264StereoHigh : VAEntrypointEncSlice
VAProfileVC1Simple : VAEntrypointVLD
VAProfileVC1Main : VAEntrypointVLD
VAProfileVC1Advanced : VAEntrypointVLD
VAProfileNone : VAEntrypointVideoProc
VAProfileJPEGBaseline : VAEntrypointVLD

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #12 from Tim-Philipp Müller <***@zen.co.uk> ---
Seems to work again now for me with git master and vaapisink or glimagesink.

I have problems with xvimagesink now though. The bccnews clip just flashes
green frames after a few seconds, and with another H264-HD-in-TS clip I get
green macroblock-height rows but the pic is also distorted and wrong colours.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #13 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
(In reply to Tim-Philipp Müller from comment #12)

Post by "GStreamer" (GNOME Bugzilla)
I have problems with xvimagesink now though. The bccnews clip just flashes
green frames after a few seconds, and with another H264-HD-in-TS clip I get
green macroblock-height rows but the pic is also distorted and wrong colours.

Just as a side comment, with bbcnews clip, the green frames start to show up
when the CC is rendered.

By the way, I had send a patch to libva-intel-driver to fix the crash of
bbcnews with ximagesink:
https://lists.freedesktop.org/archives/libva/2016-June/004026.html

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #14 from Tim-Philipp Müller <***@zen.co.uk> ---
I also noticed that the subtitles are stretched horizontally with vaapi
compared to software rendering, perhaps that's related?

The other clip I tested was MPEG-2 after all actually, not H264, but without
subs.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Scott D Phillips <***@intel.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@intel.com

--- Comment #15 from Scott D Phillips <***@intel.com> ---
(In reply to Tim-Philipp Müller from comment #14)

Post by "GStreamer" (GNOME Bugzilla)
I also noticed that the subtitles are stretched horizontally with vaapi
compared to software rendering, perhaps that's related?
The other clip I tested was MPEG-2 after all actually, not H264, but without
subs.

The vaapidecodebin within the decodebin scales the video for 1/1 p-a-r and
deinterlaces the video. I think the scaling is interfering with the later
subtitle overlay in the playbin. I'm not yet familiar enough with dvb subtitles
to know what the right thing to do is, but I'll look into it.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Holger Kaelberer <***@getslash.de> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@getslash.de

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #16 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
(In reply to Tim-Philipp Müller from comment #12)

Post by "GStreamer" (GNOME Bugzilla)
Seems to work again now for me with git master and vaapisink or glimagesink.
I have problems with xvimagesink now though. The bccnews clip just flashes
green frames after a few seconds, and with another H264-HD-in-TS clip I get
green macroblock-height rows but the pic is also distorted and wrong colours.

Yes, confirmed. Somehow vaapipostproc pushes empty buffers or xvimagesink
renders empty buffers

**@Tim, just wondering, is this still a blocker for 1.10?**



(In reply to Scott D Phillips from comment #15)

Post by "GStreamer" (GNOME Bugzilla)
(In reply to Tim-Philipp Müller from comment #14)
The vaapidecodebin within the decodebin scales the video for 1/1 p-a-r and
deinterlaces the video. I think the scaling is interfering with the later
subtitle overlay in the playbin. I'm not yet familiar enough with dvb
subtitles to know what the right thing to do is, but I'll look into it.

Setting vaapidecodebin as GST_RANK_NONE, vaapimpeg2dec is used (haswell) and
works fine with xvimagesink (deinterlacing by software).

Also, running
gst-play-1.0 ~/patterns/bug766978.m2t --videosink=xvimagesink --flags=0x613

(disabling subtitles) the play -with vaapidecodebin- is OK.

This empty buffers are result of scaling + deinterlacing + subtitle rendering,
as you found.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Post by "GStreamer" (GNOME Bugzilla)

Post by "GStreamer" (GNOME Bugzilla)
I have problems with xvimagesink now though. The bccnews clip just flashes
green frames after a few seconds, and with another H264-HD-in-TS clip I get
green macroblock-height rows but the pic is also distorted and wrong colours.

Yes, confirmed. Somehow vaapipostproc pushes empty buffers or xvimagesink
renders empty buffers

Well, it's less of a blocker than it was before, but the overall situation with
gstreamer-vaapi or this decoder/postproc combination is far from great:

- with xvimagesink: green frames/flashes, unusuable
- with ximagesink: just aborts (driver assert afaict)
- with glimagesink: works, even subs rendered right
- with vaapisink: works, subs not scaled right

We're having a decoder with primary rank that's pretty much unusable unless the
application picks vaapisink or glimagesink, which is not great.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
Severity|blocker |critical

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #18 from sreerenj <***@gmail.com> ---
Just to confirm, is this something only reproducible in HSW or older hw?
Can't see any issue in SKL..

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #19 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
(In reply to sreerenj from comment #18)

Post by "GStreamer" (GNOME Bugzilla)
Just to confirm, is this something only reproducible in HSW or older hw?
Can't see any issue in SKL..

I had to update my setups for SKL and KBL and in both the green frames appear
when using xvimagesink.

It is something related with mapping deinterlaced frames and composite them
with subtitles.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #20 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
(In reply to Tim-Philipp Müller from comment #17)

Post by "GStreamer" (GNOME Bugzilla)
Well, it's less of a blocker than it was before, but the overall situation
- with xvimagesink: green frames/flashes, unusuable

Yep. The user needs to disable the subtitles :(

The same behavior happens with kmssink.

Post by "GStreamer" (GNOME Bugzilla)
- with ximagesink: just aborts (driver assert afaict)

Using the HEAD of intel-driver it doesn't abort, but only black frames are
shown.

Post by "GStreamer" (GNOME Bugzilla)
- with glimagesink: works, even subs rendered right
- with vaapisink: works, subs not scaled right
We're having a decoder with primary rank that's pretty much unusable unless
the application picks vaapisink or glimagesink, which is not great.

Agree.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Hyunjun Ko <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@igalia.com

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #21 from Hyunjun Ko <***@igalia.com> ---
Created attachment 334971
--> https://bugzilla.gnome.org/attachment.cgi?id=334971&action=edit
videomemory: load also VA image from surface in case of GST_MAP_WRITE flag

The problem with xvimagesink is caused by not loading valid image in vaapi
video map function.

I also tested with ximagesink based on the latest intel driver, but it's still
showing black frames even without dvbsuboverlay.
Probably, this is driver's issue.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #335056|none |rejected
status| |

--- Comment #27 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 335056:
--> (https://bugzilla.gnome.org/review?bug=766978&attachment=335056)

Nope. I didn't mean that.

The patch for vaapi mapping is correct, the issue is that we have to forbid the
GST_MAP_READWRITE flag, but we can handel GST_MAP_READ or GST_MAP_WRITE... not
both at the same time.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #334971|none |needs-work
status| |

--- Comment #28 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Review of attachment 334971:
--> (https://bugzilla.gnome.org/review?bug=766978&attachment=334971)

I would use this

if (((flags & GST_MAP_READWRITE) != GST_MAP_READWRITE) &&
!ensure_image_is_current (mem))

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #22 from Tim-Philipp Müller <***@zen.co.uk> ---
Comment on attachment 334971
--> https://bugzilla.gnome.org/attachment.cgi?id=334971
videomemory: load also VA image from surface in case of GST_MAP_WRITE flag

This patch fixes the issue with xvimagesink for me as well, thanks!

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #25 from Hyunjun Ko <***@igalia.com> ---
(In reply to Víctor Manuel Jáquez Leal from comment #23)

Comment on attachment 334971 [details] [review]
videomemory: load also VA image from surface in case of GST_MAP_WRITE flag
This a long time issue. Look at bug 704078 for the debate.
Perhaps we should check for GST_MAP_READ or GST_MAP_WRITE, but no
GST_MAP_READWRITE

Thanks for letting me know about history of this code. I was wondering why :)
Discussion on bug 704083 helps me understanding a lot.
IIUC, dvbsuboverlay should change the map flag.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Hyunjun Ko <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #334971|0 |1
is obsolete| |

--- Comment #26 from Hyunjun Ko <***@igalia.com> ---
Created attachment 335056
--> https://bugzilla.gnome.org/attachment.cgi?id=335056&action=edit
dvbsuboverlay: use GST_MAP_READWRITE instead of only GST_MAP_WRITE when calling
gst_video_frame_map

To work with some HW decoders correctly, it needs to specify video map flag
clearly.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #24 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Created attachment 335005
--> https://bugzilla.gnome.org/attachment.cgi?id=335005&action=edit
libs: surface: ensure overlay is not bigger

Ensure that the composition overlay rectangle (subtitles) is not bigger than
the surface where it is going to be composite and render.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #23 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Comment on attachment 334971
--> https://bugzilla.gnome.org/attachment.cgi?id=334971
videomemory: load also VA image from surface in case of GST_MAP_WRITE flag

This a long time issue. Look at bug 704078 for the debate.

Perhaps we should check for GST_MAP_READ or GST_MAP_WRITE, but no
GST_MAP_READWRITE

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #335056|rejected |committed
status| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #335005|none |committed
status| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #29 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
I have committed both patches with the asked changes.

I won't close the bug because we still need to figure out what's happening with
ximagesink.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Summary|mpeg-2 decoding broken |vaapidecode: mpeg-2 don't
| |render with ximagesink

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #30 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Aaaggrr!!!

The patch for mapping the memory for writing breaks the use case of H264
decoding + external subtitles + xvimagesink

I won't revert it now. I'll try to find a proper fix tomorrow.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Depends on| |771382

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #335056|committed |rejected
status| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #334971|1 |0
is obsolete| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

--- Comment #31 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
I have reverted my version of the patch in attachment 334971, and created the
bug 771382.

Hyunjun was right, but I didn't realize why until I checked that
gst_video_overlay_composition_blend() reads and writes the frame. So, in order
to fix this bug for xvimagesink, we need to merge that patch.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978
Bug 766978 depends on bug 771382, which changed state.

Bug 771382 Summary: dvbsuboverlay: map with READWRITE for overlay blend
https://bugzilla.gnome.org/show_bug.cgi?id=771382

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

Víctor Manuel Jáquez Leal <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Whiteboard| |P2

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=766978

GStreamer system administrator <***@gstreamer.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |OBSOLETE

--- Comment #32 from GStreamer system administrator <***@gstreamer.net> ---
-- 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/gstreamer-vaapi/issues/39.