Created attachment 282811 [details] [review]
vaapipostproc: fix heuristic for detecting discontinuity

Hi, I believe the following patch should help. I foresee additional fixlets or workarounds to appear next in VA driver/gst-vaapi but this one should at least be determinant. Could you please give it a try? Thanks.

I applied the patch on top of Simon's last RPM build and it changes the visual problems but does not fix them.

Do I need to work with git-of-the-day?

Barry

Hi,

(In reply to comment #2)
> I applied the patch on top of Simon's last RPM build and it changes the visual
> problems but does not fix them.
> 
> Do I need to work with git-of-the-day?

Yes, please try against the current git master trees. Though, I know Simon usually keeps his branch up-to-date, so I don't really expect additional changes on your side.

We are already at current.

Simon speculated that the problem may be an issue with keeping track of the reference frames for the deinterlacing. Apparently Haswell uses more reference frames then Ivybridge and the driver may not be tracking the additional frames that Haswell algorithms use?

Barry

Hi,

(In reply to comment #4)
> We are already at current.
> 
> Simon speculated that the problem may be an issue with keeping track of the
> reference frames for the deinterlacing. Apparently Haswell uses more reference
> frames then Ivybridge and the driver may not be tracking the additional frames
> that Haswell algorithms use?

There are multiple issues around.

1. The way "discontinuity" (e.g. lost frames) was tracked in gstreamer-vaapi was wrong. This is fixed with the attached patch.

2. The VA driver doesn't program the VEBOX engine, used for advanced deinterlacing.
a. field ordering in the input sequence is checked against the incorrect VA-API flag ;
b. commands are not strictly submitted in the expected order ;
c. the second field is improperly deinterlaced, this requires one future frame to be supplied, and handled in the driver. Otherwise, we are always deinterlacing the first field I think ;
d. "companion" frames to the deinterlacing process are incorrectly rolled around

As of now, patches for (1), (2.a), (2.b) are ready. Patches for (2.c) and (2.d) are somewhat more involved to get the correct behaviour implemented precisely.

I placed the current WIP code on Github:

* gstreamer-vaapi:
  <https://github.com/gbeauchesne/gstreamer-vaapi> ("vpp.deinterlacing" branch)

* libva-intel-driver:
  <https://github.com/gbeauchesne/libva-intel-driver> ("17.vpp.vebox" branch)

That doesn't provide good quality yet, but it shouldn't be worse than the code for Ivybridge(?). Could you please give it a try? Thanks.

Without your changes I see a DVB-T stream (BBC NEWS) flash green
and play frames out of time sequence.

With the patches I see the green flashes, approx 4 green flash a second.
But it looks like the frames are playing out in correct time sequence.
And content looks deinterlaced.

Without your changes I see DVB-T stream BBC one playing frames out of time sequence and rare flash of green.

With your changes I see very rare green flashes. The frames are played out in time sequnence, but inter-cut with an old frame.

When I test with the CrossSpin.mpeg file. I see fewer green flashs
but the cross does not rotate smoothly like it does on older hardware/software.

My uneducated guess is that the green flashes and the showing of an old frame are related algorithm bugs. The green flash being a uninitialise or zeroed frame maybe. Is this evidence of not managing the frames for de-interlacing?
As I say I'm guessing here.

Barry

It occurs to me that testing with DVB-T streams may introduce another source
of error from the streams handling.

I have uploaded a set of test files to

http://90.155.96.198/bscott/video/

Using for as an example:

Slices MPEG2 PAL 1080i 25.ts.003.mpeg

I see green flashes and frames played out of time. The text at the bottom should scroll smoothly.

Can you play these back with good quality?

Barry

Hi,

(In reply to comment #8)
> It occurs to me that testing with DVB-T streams may introduce another source
> of error from the streams handling.
> 
> I have uploaded a set of test files to
> 
> http://90.155.96.198/bscott/video/
> 
> Using for as an example:
> 
> Slices MPEG2 PAL 1080i 25.ts.003.mpeg
> 
> I see green flashes and frames played out of time. The text at the bottom
> should scroll smoothly.
> 
> Can you play these back with good quality?
> 
> Barry

I have pushed additional fixes to the VA intel-driver (17.vpp.vebox). It's not perfect yet, but it should improve things.

The green flashes were due to the fact that the internal helper functions were called with reversed argument when converting from VEBOX internal format to the user expected format.

There would still be an initial green flash, but this one would be fixed with a more itnensive rework of the VEBOX code.

Added dependency on bug #734386 as this further perturbs mpeg2ts streams.

The green flashes are gone from my test cases.

What see on all tests is a frame played out ahead of the main deinterlaced stream of frames.

The Fields.mpeg file shows this very clearly. The movie has a vertical bar moving left them right. There is a second vertical bar that blinks on ahead of the main bar.

Does this match what you see?

Barry

A side question, to double check: isn't the CrossSpin test supposed to be presented bottom-field-first? Even with bob, on a frame per frame basis, it seems to me that the parity of the fields is reversed. Though, PictureExtension.top_field_first is set in the bitstream.

(In reply to comment #11)
> The green flashes are gone from my test cases.
> 
> What see on all tests is a frame played out ahead of the main deinterlaced
> stream of frames.
> 
> The Fields.mpeg file shows this very clearly. The movie has a vertical bar
> moving left them right. There is a second vertical bar that blinks on ahead of
> the main bar.
> 
> Does this match what you see?

I have temporarily disabled the "discontinuity" tracker. That was supposed to work if the sink pad buffer timestamps are correctly set. However, this turns out to *not* be the case.

I have amended my gstreamer-vaapi "vpp.deinterlacing" branch for it @ github.com.

I have picked up the disabled discontinuity change and that has made no difference.

I still still the ghost line in front of the main line in Fields.mpeg.

Are you still seeing the bad playback matching what I am describing?

re: CrosssSpin. I'm not knowledgeable enough about encoding to say if the file is wrong.

Hi Barray,

(In reply to comment #14)
> I have picked up the disabled discontinuity change and that has made no
> difference.
> 
> I still still the ghost line in front of the main line in Fields.mpeg.
> 
> Are you still seeing the bad playback matching what I am describing?

I did see that bad behaviour, but it disappeared after the patch for discontinuity detection was totally disabled.

Let me confirm the software that I need to be testing with.

libva-intel-driver branch 17.vpp.vebox
got log shows at 0ec9fa6158c3f52288c43ad030caeca684038657

gstreamer-vaapi branch vpp.deinterlacing
git log shows at 8edcc8f81744246bdedb5fbe7ba2ee05e5e9966f

To get up to date I did for each repo:

git pull
make clean
make
DESTDIS=/tmp/dest make install

then made a tar ball of /tmp/dest that I moved to the target hardware
and installed. Finally rebooted. After the system came back did the tests.

Hi,

(In reply to comment #14)
> I have picked up the disabled discontinuity change and that has made no
> difference.
> 
> I still still the ghost line in front of the main line in Fields.mpeg.
> 
> Are you still seeing the bad playback matching what I am describing?

I am not sure this is what you call "ghost" line, but the following pipeline would produce something not comfortable to my eyes:

$ gst-launch-1.0 -v filesrc location=/home/gb/Videos/interlaced/Fields.mpeg ! mpegpsdemux ! mpegvideoparse ! vaapidecode ! vaapipostproc deinterlace-method=motion-adaptive ! vaapisink

unlike the following pipeline, where everything looks smoother:

gst-launch-1.0 -v filesrc location=/home/gb/Videos/interlaced/Fields.mpeg ! mpegpsdemux ! mpegvideoparse ! vaapidecode ! vaapipostproc deinterlace-method=motion-adaptive ! vaapisink sync=false

This could mean that the PTS are still incorrectly generated. Either from vaapipostproc, or upstream elements? Or are pixels actually incorrect interpolated? On the other hand, at field granularity, I don't notice artifacts.

I am still experimenting with an alternate VEBOX code.

(In reply to comment #17)
> Hi,
> 
> (In reply to comment #14)
> > I have picked up the disabled discontinuity change and that has made no
> > difference.
> > 
> > I still still the ghost line in front of the main line in Fields.mpeg.
> > 
> > Are you still seeing the bad playback matching what I am describing?
> 
> I am not sure this is what you call "ghost" line, but the following pipeline
> would produce something not comfortable to my eyes:
> 
> $ gst-launch-1.0 -v filesrc location=/home/gb/Videos/interlaced/Fields.mpeg !
> mpegpsdemux ! mpegvideoparse ! vaapidecode ! vaapipostproc
> deinterlace-method=motion-adaptive ! vaapisink
> 
> unlike the following pipeline, where everything looks smoother:
> 
> gst-launch-1.0 -v filesrc location=/home/gb/Videos/interlaced/Fields.mpeg !
> mpegpsdemux ! mpegvideoparse ! vaapidecode ! vaapipostproc
> deinterlace-method=motion-adaptive ! vaapisink sync=false
> 
> This could mean that the PTS are still incorrectly generated. Either from
> vaapipostproc, or upstream elements? Or are pixels actually incorrect
> interpolated? On the other hand, at field granularity, I don't notice
> artifacts.
> 
> I am still experimenting with an alternate VEBOX code.

A degree of judder is not unexpected - the material is 50Hz, while most screens are 60Hz.

http://90.155.96.198/sfarnsworth/CrossSpin.mpeg is native 60Hz material, I'm seeing ghosting/juddering as-if the field order is wrong.

(In reply to comment #19)
> http://90.155.96.198/sfarnsworth/CrossSpin.mpeg is native 60Hz material, I'm
> seeing ghosting/juddering as-if the field order is wrong.

Yes, the stream reports TFF, based on what field comes first at the beginning, but I believe it should be BFF order instead. Note: I have not examined the SEI messages either, maybe they contradict the slice_hdr info?

hmm, no that's mpeg2 encoded, but my conclusion is the same. :)

I've used a pad probe to look at PTS values, and I think you're handling them correctly.

For Fields.mpeg, I see frames go in with 40 ms difference to the previous PTS, and two frames come out, each at 20 ms difference to the previous PTS. The juddering is likely to be because we're displaying a new frame every 16 ms, but generating new content every 20 ms.

For CrossSpin.mpeg, I see frames go in with 33.366667 ms difference to the previous PTS, and two frames come out, first 16.683333 ms after the previous, and the second 16.683334 ms. I also see some frames go in with 33.366666 ms difference to the previous PTS; these come out as two frames, both 16.683333 ms after the previous frame in the output chain. As this is 29.97 Hz material, that's what I'd expect.

I've been diving deeper into PTS/DTS, using the Python bindings (I'll attach the script I used), and I've found an issue. It looks like PTS handling goes wrong whenever mpegpsdemux gives vaapidecode a buffer with a DTS but no PTS:

pts probe          mpegvideoparse output pts_diff       160000000 pts     39629455555 dts_diff        40000000 dts     39509455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39389455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39409455555
pts probe             vaapidecode output pts_diff        40000000 pts     39429455555 dts_diff        40000000 dts     39429455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39429455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39449455555
pts probe             vaapidecode output pts_diff        40000000 pts     39469455555 dts_diff        40000000 dts     39469455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39469455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39489455555
pts probe          mpegvideoparse output pts_diff       -80000000 pts     39549455555 dts_diff        40000000 dts     39549455555

So far, normal output as expected. mpegvideoparse PTS going backwards is expected - frames are in decode order at this point.

pts probe          mpegvideoparse output                                              dts_diff        40000000 dts     39589455555

Now we get a buffer from mpegvideoparse with DTS but no PTS.

pts probe             vaapidecode output pts_diff        40000000 pts     39509455555 dts_diff        40000000 dts     39509455555
pts probe          mpegvideoparse output pts_diff               0 pts     39749455555 dts_diff        40000000 dts     39629455555

PTS is correct for this buffer from mpegvideoparse and is advancing normally.

pts probe           vaapipostproc output pts_diff        20000000 pts     39509455555
Async Message GstMessageTag contents <GstTagList at 0x7f6a0006f400> from <__main__.GstVaapiSink object at 0x7f6a0f959730 (GstVaapiSink at 0x8beea0)>
pts probe           vaapipostproc output pts_diff        20000000 pts     39529455555
pts probe             vaapidecode output pts_diff        40000000 pts     39549455555 dts_diff        40000000 dts     39549455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39549455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39569455555
pts probe             vaapidecode output pts_diff       120000000 pts     39669455555 dts_diff       120000000 dts     39669455555

And now vaapidecode goes wild. One output frame 3 frame times into the future.

pts probe           vaapipostproc output pts_diff       100000000 pts     39669455555
pts probe          mpegvideoparse output pts_diff       -80000000 pts     39669455555 dts_diff        40000000 dts     39669455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39689455555
pts probe          mpegvideoparse output pts_diff        40000000 pts     39709455555 dts_diff        40000000 dts     39709455555
pts probe             vaapidecode output pts_diff               0 pts     39669455555 dts_diff               0 dts     39669455555
pts probe           vaapipostproc output pts_diff       -20000000 pts     39669455555

Then an output frame with the same PTS as the previous frame, which causes vaapipostproc to output a frame that's gone backwards in time.

pts probe          mpegvideoparse output pts_diff       160000000 pts     39869455555 dts_diff        40000000 dts     39749455555
pts probe           vaapipostproc output pts_diff        20000000 pts     39689455555
pts probe             vaapidecode output pts_diff               0 pts     39669455555 dts_diff               0 dts     39669455555
pts probe           vaapipostproc output pts_diff       -20000000 pts     39669455555

And again. We now have three frames, so we've at least got a consistent frame count/PTS time again.

WARNING from <__main__.GstVaapiSink object at 0x7f6a0f959730 (GstVaapiSink at 0x8beea0)>: GError('A lot of buffers are being dropped.',) debug 'gstbasesink.c(2791): gst_base_sink_is_too_late (): /GstPipeline:PTS Check/GstVaapiSink:vaapisink0:\nThere may be a timestamping problem, or this computer is too slow.'
Async Message GstMessageWarning contents (GError('A lot of buffers are being dropped.',), 'gstbasesink.c(2791): gst_base_sink_is_too_late (): /GstPipeline:PTS Check/GstVaapiSink:vaapisink0:\nThere may be a timestamping problem, or this computer is too slow.') from <__main__.GstVaapiSink object at 0x7f6a0f959730 (GstVaapiSink at 0x8beea0)>
pts probe           vaapipostproc output pts_diff        20000000 pts     39689455555
Async Message GstMessageQOS contents 'live_stream True running_time 39340000000L stream_time 39689444444L timestamp 39689455555L duration 20000000L jitter 43594540L proportion 0.0695047768729755 quality 1000000 format <enum GST_FORMAT_BUFFERS of type GstFormat> processed 1966L dropped 2L' from <__main__.GstVaapiSink object at 0x7f6a0f959730 (GstVaapiSink at 0x8beea0)>

And vaapisink tells us it's dropped two frames - probably the two where PTS went backwards.

Created attachment 285397 [details]
Python code to show the PTS and DTS values of buffers

This bit of Python sets up a filesrc ! mpegpsdemux ! mpegvideoparse ! vaapidecode ! vaapipostproc deinterlace-method=motion-adaptive ! vaapisink pipeline, where filesrc reads from the file given as a parameter to the script.

It dumps out the majority of pipeline messages, plus PTS and DTS values for every interesting buffer in a form that's easy to scan by eye.

Should be easy to modify if it's giving useless information - I'm happy for it to be treated as BSD/MIT licensed, or GPLv2 or later.

Moving to Product:GStreamer, Component:gstreamer-vaapi

-- 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/20.