After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 766978 - vaapidecode: mpeg-2 don't render with ximagesink
vaapidecode: mpeg-2 don't render with ximagesink
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal critical
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
P2
Depends on: 771382
Blocks:
 
 
Reported: 2016-05-28 23:10 UTC by Tim-Philipp Müller
Modified: 2018-11-03 15:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
debug log (xz) (1.25 MB, application/x-xz)
2016-05-30 11:38 UTC, Tim-Philipp Müller
  Details
videomemory: load also VA image from surface in case of GST_MAP_WRITE flag (1.09 KB, patch)
2016-09-07 09:03 UTC, Hyunjun Ko
needs-work Details | Review
libs: surface: ensure overlay is not bigger (1.27 KB, patch)
2016-09-07 15:38 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
dvbsuboverlay: use GST_MAP_READWRITE instead of only GST_MAP_WRITE when calling gst_video_frame_map (1.18 KB, patch)
2016-09-08 03:07 UTC, Hyunjun Ko
rejected Details | Review

Description Tim-Philipp Müller 2016-05-28 23:10:14 UTC
$ 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.
Comment 1 Víctor Manuel Jáquez Leal 2016-05-30 10:05:52 UTC
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.
Comment 2 Tim-Philipp Müller 2016-05-30 11:30:55 UTC
$ 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
Comment 3 Tim-Philipp Müller 2016-05-30 11:38:17 UTC
Created attachment 328722 [details]
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.
Comment 4 sreerenj 2016-05-30 12:09:02 UTC
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 ???
Comment 5 Tim-Philipp Müller 2016-05-30 12:48:09 UTC
Works with gstreamer-vaapi 1.8.0 (and gst master)
Comment 6 Tim-Philipp Müller 2016-05-30 12:57:39 UTC
git bisect yields:

7baacda91c1e2f66d4618669862b7b6f7dfc4f6c is the first bad commit
commit 7baacda91c1e2f66d4618669862b7b6f7dfc4f6c
Author: Víctor Manuel Jáquez Leal <victorx.jaquez@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
Comment 7 Víctor Manuel Jáquez Leal 2016-05-30 13:12:36 UTC
:( 

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.
Comment 8 Tim-Philipp Müller 2016-05-30 14:03:13 UTC
If I revert just that one commit, it works again, fwiw.
Comment 9 Víctor Manuel Jáquez Leal 2016-05-30 15:11:03 UTC
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.
Comment 10 Tim-Philipp Müller 2016-05-30 15:27:05 UTC
(not directly related, but isn't vaapidecode/postproc supposed to do some deinterlacing here?)
Comment 11 Víctor Manuel Jáquez Leal 2016-05-30 16:04:33 UTC
(In reply to Tim-Philipp Müller from comment #10)
> (not directly related, but isn't vaapidecode/postproc supposed to do some
> deinterlacing here?)

vaapipostproc should do deinterlacing.
Comment 12 Tim-Philipp Müller 2016-06-08 10:59:12 UTC
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.
Comment 13 Víctor Manuel Jáquez Leal 2016-06-08 16:04:30 UTC
(In reply to Tim-Philipp Müller from comment #12)
> 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
Comment 14 Tim-Philipp Müller 2016-06-08 16:21:06 UTC
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.
Comment 15 Scott D Phillips 2016-07-19 22:11:46 UTC
(In reply to Tim-Philipp Müller from comment #14)
> 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.
Comment 16 Víctor Manuel Jáquez Leal 2016-09-05 11:54:32 UTC
(In reply to Tim-Philipp Müller from comment #12)
> 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)
> (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.
Comment 17 Tim-Philipp Müller 2016-09-05 12:21:00 UTC
> > 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?**

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.
Comment 18 sreerenj 2016-09-05 12:43:58 UTC
Just to confirm, is this something only reproducible in HSW or older hw?
Can't see any issue in SKL..
Comment 19 Víctor Manuel Jáquez Leal 2016-09-05 16:50:14 UTC
(In reply to sreerenj from comment #18)
> 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.
Comment 20 Víctor Manuel Jáquez Leal 2016-09-05 16:53:25 UTC
(In reply to Tim-Philipp Müller from comment #17)
> 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

Yep. The user needs to disable the subtitles :(

The same behavior happens with kmssink.

>  - with ximagesink: just aborts (driver assert afaict)

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

>  - 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.
Comment 21 Hyunjun Ko 2016-09-07 09:03:29 UTC
Created attachment 334971 [details] [review]
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.
Comment 22 Tim-Philipp Müller 2016-09-07 09:38:55 UTC
Comment on attachment 334971 [details] [review]
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!
Comment 23 Víctor Manuel Jáquez Leal 2016-09-07 09:46:03 UTC
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
Comment 24 Víctor Manuel Jáquez Leal 2016-09-07 15:38:19 UTC
Created attachment 335005 [details] [review]
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.
Comment 25 Hyunjun Ko 2016-09-08 03:06:04 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #23)
> Comment on attachment 334971 [details] [review] [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.
Comment 26 Hyunjun Ko 2016-09-08 03:07:55 UTC
Created attachment 335056 [details] [review]
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.
Comment 27 Víctor Manuel Jáquez Leal 2016-09-08 11:27:12 UTC
Review of attachment 335056 [details] [review]:

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.
Comment 28 Víctor Manuel Jáquez Leal 2016-09-08 11:28:39 UTC
Review of attachment 334971 [details] [review]:

I would use this

if (((flags & GST_MAP_READWRITE) != GST_MAP_READWRITE) && !ensure_image_is_current (mem))
Comment 29 Víctor Manuel Jáquez Leal 2016-09-08 16:10:31 UTC
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.
Comment 30 Víctor Manuel Jáquez Leal 2016-09-08 16:32:24 UTC
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.
Comment 31 Víctor Manuel Jáquez Leal 2016-09-13 18:00:45 UTC
I have reverted my version of the patch in attachment 334971 [details] [review], 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.
Comment 32 GStreamer system administrator 2018-11-03 15:47:49 UTC
-- 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.