Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stuttering on 1080i playblack #15817

Closed
yasij opened this issue Mar 26, 2019 · 35 comments

Comments

@yasij
Copy link
Contributor

commented Mar 26, 2019

Bug report

Describe the bug

Here is a clear and concise description of what the problem is:

I've noticed stuttering on 1080i interlaced content with the i965 VAAPI driver as well. In the "o" player process menu, "Deinterlace" alternates between vaapi-none and vaapi-madi. If I use the iHD intel-media-driver VAAPI driver instead, it's fine and I get no alternation between vaapi-none and vaapi-madi. If I use vaapi-bob as the deinterlacer with the i965 driver, I also don't see the stuttering. VLC and MPV both play this content fine. I've uploaded a 24M sample clip from the 1080i mpegts stream: clip. The kodi log complains at the beginning of playback about the file, but neither VLC or MPV have issues.

Expected Behavior

Here is a clear and concise description of what was expected to happen:

Smooth playback with interlaced content with the i965 driver.

Actual Behavior

Choppy and stuttery playback with interlaced MPEG2 1080i content with the i965 driver.

Possible Fix

To Reproduce

Steps to reproduce the behavior:

  1. Turn on VAAPI decoding of MPEG2.
  2. Play sample clip.
  3. Set the deinterlacer to vaapi-motion adaptive or vaapi-motion compensated
  4. Press "o". Notice the deinterlacer alternates between vaapi-none and vaapi-madi
    Also notice stuttering playback.

Debuglog

The debuglog can be found here: log

Screenshots

Here are some links or screenshots to help explain the problem:

Additional context or screenshots (if appropriate)

Here is some additional context or explanation that might help:

Your Environment

Used Operating system:

  • Android

  • iOS

  • Linux

  • OSX

  • Raspberry-Pi

  • Windows

  • Windows UWP

  • Operating system version/name: Ubuntu 18.10

  • Kodi version: 18.1

note: Once the issue is made we require you to update it with new information or Kodi versions should that be required.
Team Kodi will consider your problem report however, we will not make any promises the problem will be solved.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

@pkerling this is your construction site: 2006969

this strategy only works from deinterlace to none but not the other way round. now when deinterlace flag toggles, the filter gets destroyed and recreated all the time. v17 did cope with this situation.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Yup seen this, it's a PAFF video. mpv by the way copes with this by deinterlacing every frame no matter whether it is marked as interlaced (in default settings) :-)

I see massive artifacts on AMD by the way (in Kodi and mpv). I wonder whether we should disable mpeg2 hw decoding by default? (regardless of this bug)

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Please don't - this worked fine for v17 at least on intel, because their madi/mcdi can cope with that stuff. If there is no other chance for AMD, just disable it for AMD's VAAPI separately.

Btw. mpv also works correctly on my intel vaapi machine with that sample, also without artifacts.

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Reason why mpeg-2 vaapi is useful:

  • Slow machines, like Celeron, even Apollo Lakes can only cope with 1080i50 mpeg2 if they hw decode it
  • Deinterlacing in good quality (vaapi-mcdi, vaapi-madi) can only be achieved with ffmpeg's Deinterlace - this puts additional load on those devices

Therefore: Disabling mpeg-2 acceleration in general is not a good idea.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Fair enough. I just wanted to get around blacklisting/whitelisting manufacturers. But I guess we might have to do that.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

@yasij Do you maybe have some other PAFF (mixed interlaced/progressive frames) video where this is more visible? The difference here is so small that I'm not sure whether I'm just imagining things.

If I use the iHD intel-media-driver VAAPI driver instead, it's fine and I get no alternation between vaapi-none and vaapi-madi

That's strange. As you can gather already, switching the deinterlace mode per-frame is by design actually. So I don't know why you get other results with intel-media-driver.

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

I guess the iHD does not support VPP correctly? A debuglog will tell.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

And if you do submit further debug logs please activate video component logging

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Yes:

2019-03-26 13:01:22.313 T:139929296303872   ERROR: VAAPI::CheckSuccess - Error: the requested VAProfile is not supported(12)
2019-03-26 13:01:22.313 T:139929296303872   ERROR: VAAPI output: Postproc preinit failed
2019-03-26 13:01:22.463 T:139929296303872   ERROR: VAAPI::CheckSuccess - Error: the requested VAProfile is not supported(12)
2019-03-26 13:01:22.463 T:139929296303872   ERROR: VAAPI output: Postproc preinit failed
2019-03-26 13:01:22.545 T:139929296303872   ERROR: VAAPI::CheckSuccess - Error: the requested VAProfile is not supported(12)
2019-03-26 13:01:22.546 T:139929296303872   ERROR: VAAPI output: Postproc preinit failed
2019-03-26 13:01:22.712 T:139929296303872   ERROR: VAAPI::CheckSuccess - Error: the requested VAProfile is not supported(12)
2019-03-26 13:01:22.712 T:139929296303872   ERROR: VAAPI output: Postproc preinit failed
2019-03-26 13:01:22.796 T:139929296303872   ERROR: VAAPI::CheckSuccess - Error: the requested VAProfile is not supported(12)
2019-03-26 13:01:22.796 T:139929296303872   ERROR: VAAPI output: Postproc preinit failed
2019-03-26 13:01:22.878 T:139929296303872   ERROR: VAAPI::CheckSuccess - Error: the requested VAProfile is not supported(12)
2019-03-26 13:01:22.878 T:139929296303872   ERROR: VAAPI output: Postproc preinit failed
@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

I guess our error path is not robust enough if we still display madi in the process info :-/

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Here is a full debuglog with ihd driver: http://paste.ubuntu.com/p/NfZPqwnrNh/
It does not support VPP from what I see, so no filter reconfiguration.

@Leatherface75

This comment has been minimized.

Copy link

commented Mar 26, 2019

For me it totally hangs often for 30-60 seconds when switching channels with vaapi-madi.
With other methods and software decoding it works but stuttering with sports etc.
This with Sandy Bridge hardware (Intel HD 2000).

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Which has nothing to do with this bugreport.

@Leatherface75

This comment has been minimized.

Copy link

commented Mar 26, 2019

Well maybe not exactly same but Kodi have problems with some atleast older Intel hardware with deinterlacing. On other hardware and on Haswell it seems to work as it should.

@yasij

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2019

@Leatherface75 The hardware I reported this on is Coffee Lake. I've also seen it on Kaby Lake. Your problems on older hardware have nothing to do with this issue. Open another bug report.

Sorry that the sample is dark. I just grabbed the beginning 22 MiB of the file which unfortunately has a dark section. It's most visible during the spinning Universal Earth at the beginning and after the lights come on toward the end. I have this issue for multiple recordings from the same channel. I will upload some more when I get home.

I will note that I've tried disabling hardware acceleration as well to see if that fixes this. It doesn't. The "Deinterlace" filter (which I believe is Yadif 2x) also stutters. If I switch to a different software deinterlace filter, I get corrupt video. I'll upload logs of this tonight.

In any case, turning the VPP on and off every few frames for content like this doesn't make sense for filters like vaapi-madi and vaapi-mcdi. They're stateful. You're resetting the state every time you do that. It makes sense why vaapi-bob works now since it's not stateful.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

check out what I did with vdpau long time ago: https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoPlayer/DVDCodecs/Video/VDPAU.cpp#L2502

it keeps deinterlacing active once the flag was set.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

I will note that I've tried disabling hardware acceleration as well to see if that fixes this. It doesn't. The "Deinterlace" filter (which I believe is Yadif 2x) also stutters. If I switch to a different software deinterlace filter, I get corrupt video.

"Deinterlace" is yadif, yes. I'm not sure whether this is related or a different bug, we'll see.

In any case, turning the VPP on and off every few frames for content like this doesn't make sense for filters like vaapi-madi and vaapi-mcdi. They're stateful. You're resetting the state every time you do that. It makes sense why vaapi-bob works now since it's not stateful.

I'm not sure the state in the driver is the root reason, but that's also not particularly important. If it can deal with PAFF and progressive content properly, it makes sense to keep the deinterlace postprocessing filter enabled.

@yasij

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

Here's another sample with more obvious stuttering: sample.ts

I have a few logs of playing that file in different contexts:
iHD does support madi, at least on X11: kodi log for iHD. It does not support mcdi. I believe the stuttering is actually still happening for iHD, but is less obvious due to algorithm differences.

kodi log with Video component logging for i965 driver. This stutters badly.

kodi log with software decoding and "Deinterlace". This also stutters badly.

kodi log with software decoding and "Bob" deinterlacer. This plays corrupt video and interjects old frames every few frames.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

With that video I see it clearly, thanks

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

@yasij please try (just for a quick test, it's not the final solution)

diff --git a/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.cpp b/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.cpp
index c2a1444f52..34c9a7060f 100644
--- a/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.cpp
+++ b/xbmc/cores/VideoPlayer/DVDCodecs/Video/VAAPI.cpp
@@ -2447,14 +2447,12 @@ bool CVppPostproc::Init(EINTERLACEMETHOD method)
 
 bool CVppPostproc::UpdateDeintMethod(EINTERLACEMETHOD method)
 {
-  if (method == m_vppMethod)
+  if (method == m_vppMethod || method == VS_INTERLACEMETHOD_NONE)
   {
     return true;
   }
 
   m_vppMethod = method;
-  m_forwardRefs = 0;
-  m_backwardRefs = 0;
 
   if (m_filter != VA_INVALID_ID)
   {
@@ -2486,6 +2484,13 @@ bool CVppPostproc::UpdateDeintMethod(EINTERLACEMETHOD method)
     return false;
   }
 
+  // Reset references in case we get an error further below.
+  // When switching to VS_INTERLACEMETHOD_NONE, we keep the value from before intentionally.
+  // With PAFF video, there are many switches between interlaced and progressive frames,
+  // and we must not throw away the reference frames at every switch.
+  m_forwardRefs = 0;
+  m_backwardRefs = 0;
+
   VAProcFilterParameterBufferDeinterlacing filterparams;
   filterparams.type = VAProcFilterDeinterlacing;
   filterparams.algorithm = vppMethod;
@@ -2597,7 +2602,7 @@ bool CVppPostproc::Filter(CVaapiProcessedPicture &outPic)
 
   // skip deinterlacing cycle if requested
   if ((m_step == 1) &&
-      ((outPic.DVDPic.iFlags & DVD_CODEC_CTRL_SKIPDEINT) || (m_vppMethod == VS_INTERLACEMETHOD_NONE)))
+      ((outPic.DVDPic.iFlags & DVD_CODEC_CTRL_SKIPDEINT) || !(outPic.DVDPic.iFlags & DVP_FLAG_INTERLACED) || (m_vppMethod == VS_INTERLACEMETHOD_NONE)))
   {
     Advance();
     return false;
@@ -2635,6 +2640,7 @@ bool CVppPostproc::Filter(CVaapiProcessedPicture &outPic)
   pipelineParams->output_region = &outputRegion;
   pipelineParams->surface_region = &inputRegion;
   pipelineParams->output_background_color = 0xff000000;
+  pipelineParams->filter_flags = 0;
 
   VASurfaceID forwardRefs[32];
   VASurfaceID backwardRefs[32];
@@ -2651,33 +2657,35 @@ bool CVppPostproc::Filter(CVaapiProcessedPicture &outPic)
   if (m_vppMethod != VS_INTERLACEMETHOD_NONE)
   {
     unsigned int flags = 0;
-    if (it->DVDPic.iFlags & DVP_FLAG_TOP_FIELD_FIRST)
-      flags = 0;
-    else
-      flags = VA_DEINTERLACING_BOTTOM_FIELD_FIRST | VA_DEINTERLACING_BOTTOM_FIELD;
 
-    if (m_step)
+    if (it->DVDPic.iFlags & DVP_FLAG_INTERLACED)
     {
-      if (flags & VA_DEINTERLACING_BOTTOM_FIELD)
-        flags &= ~VA_DEINTERLACING_BOTTOM_FIELD;
+      if (it->DVDPic.iFlags & DVP_FLAG_TOP_FIELD_FIRST)
+        flags = 0;
       else
-        flags |= VA_DEINTERLACING_BOTTOM_FIELD;
+        flags = VA_DEINTERLACING_BOTTOM_FIELD_FIRST | VA_DEINTERLACING_BOTTOM_FIELD;
+
+      if (m_step)
+      {
+        if (flags & VA_DEINTERLACING_BOTTOM_FIELD)
+          flags &= ~VA_DEINTERLACING_BOTTOM_FIELD;
+        else
+          flags |= VA_DEINTERLACING_BOTTOM_FIELD;
+      }
     }
+
     if (!CheckSuccess(vaMapBuffer(m_config.dpy, m_filter, (void**)&filterParams)))
     {
       return false;
     }
+
     filterParams->flags = flags;
+
     if (!CheckSuccess(vaUnmapBuffer(m_config.dpy, m_filter)))
     {
       return false;
     }
 
-    if (m_vppMethod == VS_INTERLACEMETHOD_VAAPI_BOB)
-      pipelineParams->filter_flags = (flags & VA_DEINTERLACING_BOTTOM_FIELD) ? VA_BOTTOM_FIELD : VA_TOP_FIELD;
-    else
-      pipelineParams->filter_flags = 0;
-
     pipelineParams->filters = &m_filter;
     pipelineParams->num_filters = 1;
   }
@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

kodi log with software decoding and "Deinterlace". This also stutters badly.

I'm not sure this combination makes sense/is supportable. Hw-decoding 1080i and then deinterlacing with yadif at 50fps uses up too much bandwidth in download/upload and CPU time in yadif. You can try with mpv, for me it also stutters badly:

mpv --hwdec=vaapi-copy -vf yadif=1:-1 sample20190315.ts

@yasij

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

I'm building with that patch now.

kodi log with software decoding and "Deinterlace". This also stutters badly.

I'm not sure this combination makes sense/is supportable. Hw-decoding 1080i and then deinterlacing with yadif at 50fps uses up too much bandwidth in download/upload and CPU time in yadif. You can try with mpv, for me it also stutters badly:

mpv --hwdec=vaapi-copy -vf yadif=1:-1 sample20190315.ts

I was doing software decode with yadif. VAAPI was disabled. However, mpv --deinterlace=yes --hwdec=none -vf yadif=1:-1 sample20190315.ts is also quite choppy.

@yasij

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

With the patch, the stutter is much improved. There is a still a little stutter, but it's much less noticeable.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

There is a still a little stutter, but it's much less noticeable

Is it worse than with mpv --hwdec=vaapi --deinterlace?

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

@FernetMenta The VAAPI source says that the sw deinterlacing is still supposed to work with 1080i content - does this apply only to bob deinterlacing or also yadif? Tests suggest that hardware does not seem to be able to deal with that (at least in the way we implement it). Cf.

// copying large surfaces via sse4 is a bit slow
// we just return false here as the primary use case the
// sse4 copy method is deinterlacing of max 1080i content
if (m_config.vidWidth > 1920 || m_config.vidHeight > 1088)
return false;

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

@pkerling I implemented the sw deinterlacing path only for yadif. (render_bob is not sw deint). It might still work, I haven't tried it for a long time. The linked comment only refers to old Intel hw, haswell. Not sure if Intel improved the driver for more recent hw. On the old hw it took ages to copy data from the GPU to system mem. That's why the sse lib is there. The code was taken from some Intel article.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

I implemented the sw deinterlacing path only for yadif. (render_bob is not sw deint)

But for bob we still copy to system memory, correct? which could be a bottleneck

My tests with mpv show heavy stuttering for yadif=1:-1 but it seems to run OK with yadif=0:-1. I guess there's no harm in keeping it in there in case people want to try to use it though.

I don't think you want to do this on AMD

On AMD we don't do vaapi+yadif anyway because the code does not work with drivers only supporting the vaapi2 buffer export (which is the case for the mesa state tracker) and I didn't feel the need to implement it back then - seems like this was a correct decision

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Back at the time, we used a Celeron 1820(T) for vaapi decoding and yadif double. My test-machine was an IVB Celeron 1037U, so a very slow dual core Celeron. Here it worked as well.

1080p24-1024

^^ those were the benchmarks back at that time.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

I found the culprit. While testing with mpv I had Kodi running in background just showing the main menu. Seems that was enough to put the card over the edge :-) With Kodi closed it's running fine now, so I guess it should still be possible.

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Ah :-) perfect.

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

@fritsch learn to label your axes properly ;-)

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Nope, not needed in this case. Totally clear that it is ms

@pkerling

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Not really. 1 could also refer to some arbitrary baseline.

@fritsch

This comment has been minimized.

Copy link
Member

commented Mar 28, 2019

Yeah and "ms" in Klingon could mean kilometers per hour ...

pkerling added a commit to pkerling/xbmc that referenced this issue Mar 28, 2019

[vaapi] Always deinterlace when an interlaced frame was encountered once
PAFF video requires to run all frames through the deinterlacing filter
even if they are stored progressive.
After one interlaced frame was seen by the output, do not automatically
set the deinterlace method to NONE which would skip the filter.
Still, deinterlacing must be switched off if the user requests it.

Fixes xbmc#15817

pkerling added a commit to pkerling/xbmc that referenced this issue Mar 28, 2019

[vaapi] Always deinterlace when an interlaced frame was encountered once
PAFF video requires to run all frames through the deinterlacing filter
even if they are stored progressive.
After one interlaced frame was seen by the output, do not automatically
set the deinterlace method to NONE which would skip the filter.
Still, deinterlacing must be switched off if the user requests it.

Fixes xbmc#15817

@fritsch fritsch closed this in 05a4ef5 Apr 5, 2019

@pkerling pkerling added this to the Leia 18.2-rc1 milestone Apr 5, 2019

fuzzard added a commit to fuzzard/xbmc that referenced this issue Apr 13, 2019

[vaapi] Always deinterlace when an interlaced frame was encountered once
PAFF video requires to run all frames through the deinterlacing filter
even if they are stored progressive.
After one interlaced frame was seen by the output, do not automatically
set the deinterlace method to NONE which would skip the filter.
Still, deinterlacing must be switched off if the user requests it.

Fixes xbmc#15817

fuzzard added a commit to fuzzard/xbmc that referenced this issue Apr 15, 2019

[vaapi] Always deinterlace when an interlaced frame was encountered once
PAFF video requires to run all frames through the deinterlacing filter
even if they are stored progressive.
After one interlaced frame was seen by the output, do not automatically
set the deinterlace method to NONE which would skip the filter.
Still, deinterlacing must be switched off if the user requests it.

Fixes xbmc#15817
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.