Bug 72518

Summary: Post processing driver does not respect the field flag when bob deinterlacing
Product: libva Reporter: freedesktop.arad85
Component: intelAssignee: haihao <haihao.xiang>
Status: RESOLVED FIXED QA Contact: Sean V Kelley <seanvk>
Severity: major    
Priority: medium CC: freedesktop.arad85, gb.devel
Version: unspecified   
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Possible fix for correct bob deinterlacing
Possible fix (needs rebase)

Description freedesktop.arad85 2013-12-09 12:51:30 UTC
Created attachment 90508 [details]
Possible fix for correct bob deinterlacing

Hi,

xbmc is decoding and displaying a 1080i50 stream onto a display that is running at 50p. To do this, it decodes two interlaces as a single frame (this is how the stream is encoded) and then bob deinterlaces it, by calling the VAAPI PP deinterlace twice, once for the top field and once for the bottom field. This converts the i50 fields input into p50 frames output. 

On Haswell (at least) the BOTTOM_FIELD flag is not respected when VAAPI post processing bob deinterlacing is selected. On input to bob deinterlacing, the buffer contains a decoded frame which is two fields interlaced. The frame has the top field first. To get the top field bob deinterlaced, I'd expect to set:
flags=0

to get the bottom field deinterlaced, I'd expect to set 
flags=VA_DEINTERLACING_BOTTOM_FIELD

In fact, in order to get the bottom field out, I have to set
flags=VA_DEINTERLACING_BOTTOM_FIELD_FIRST

but my understanding of VA_DEINTERLACING_BOTTOM_FIELD_FIRST is that it means the first line in the buffer is from the bottom field, so should only be set when that is true. 

If I have a sequence of data  1tb, 2tb (where 1tb = frame 1, top and bottom fields interlaced, 2tb = frame 2 top & bottom fields interlaced etc...) I'd expect the result of the deinterlacing to be 1T,1B,2T,2B (where 1T = 1t bob deinterlaced, 1B = 1b bob deinterlaced etc...

If I alternately set flags to:
0, VA_DEINTERLACING_BOTTOM_FIELD I get:
1T,1T,2T,2T...

which gives an unnatural output as I have lost half of the temporal information. It looks like a high speed shutter effect (which is effectively what is happening).

If I alternately set flags to:
0, VA_DEINTERLACING_BOTTOM_FIELD_FIRST I get:
1T,1B,2T,2B...

which gives a smoother, more natural output for anything that is moving.

The attached patch appears to fix it for me so I can set VA_DEINTERLACING_BOTTOM_FIELD and get the correct 1T,1B,2T,2B output (although I haven't looked at the whole driver/other deinterlacing methods to know if it is a general patch or just fixes it for me).

Alternatively, if VA_DEINTERLACING_BOTTOM_FIELD_FIRST is the correct way of getting the second field out from an interlaced frame with BOB deinterlacing, can you update the header files to document this correctly

Thanks
Comment 1 Peter Frühberger 2013-12-09 14:59:44 UTC
During VPP development, we discussed with Haihao and he posted us the following patch: https://gist.github.com/BtbN/7873385

This was lost during development. If it makes it work for you, please report - perhaps it's not too late to include it into 1.2.2 libva-driver-intel
Comment 2 Peter Frühberger 2013-12-09 15:03:39 UTC
Created attachment 90516 [details]
Possible fix (needs rebase)

This patch was made by Haihao Xiang while doing VPP development within xbmc
Comment 3 freedesktop.arad85 2013-12-09 15:22:26 UTC
Hi Peter,
(In reply to comment #1)
> During VPP development, we discussed with Haihao and he posted us the
> following patch: https://gist.github.com/BtbN/7873385
> 
> This was lost during development. If it makes it work for you, please report
> - perhaps it's not too late to include it into 1.2.2 libva-driver-intel

Yes, this patch applies and works with the latest git master branch for me (tested on Haswell i3-4330 only).
Comment 4 Gwenole Beauchesne 2013-12-11 05:02:54 UTC
Hi, for reference, the correct way to see the deinterlacing flags is as follows:

VA_DEINTERLACING_BOTTOM_FIELD: this is the only flag that describes the picture structure of the frame that is being processed, i.e. the one passed as the vaBeginPicture().

VA_DEINTERLACING_BOTTOM_FIELD_FIRST and VA_DEINTERLACING_ONE_FIELD are the flags that apply to the frames in the history buffer, i.e. the frames used as reference and available for advanced deinterlacing.

If you think the documentation is not clear enough, then I will update it. :)

Thanks.
Comment 5 Gwenole Beauchesne 2013-12-11 05:36:39 UTC
(In reply to comment #4)
> VA_DEINTERLACING_BOTTOM_FIELD: this is the only flag that describes the
> picture structure of the frame that is being processed, i.e. the one passed
> as the vaBeginPicture().

i.e. this is the way the frame needs to be viewed for processing. If you want to process the top field of the surface, this flag is not set (default). If you want to process the bottom field of the surface, then you set this flag. So, for bob-deinterlacing purposes, this flag is alternatively set to produce the expected result.
Comment 6 freedesktop.arad85 2013-12-11 15:17:02 UTC
Hi Gwenole,

I see you/Haihao have committed the change so I've pulled 1.2.2.pre2 and tested that it works here (i3-4330 only. Not sure on your bug process, but as far as I'm concerned, it's resolved so I'm marking it such. Please let me know if this is the wrong thing to do.

As to documentation - that's an interesting one. I think the "problem" comes from 2 things - engineers sometimes don't read documentation ;) and look at the source code and the source code is using a variable called "dndi_top_first" which sounds like it needs a flag with _FIRST in it, which leads to the confusion. Also, it's not clear (to me!) how to make the doxygen files (but I only have libva and intel-driver here and I'm not sure if that makes a difference) so my only documentation has been header files. 

Having said that, it might be worth adding your explanations from below next time you are visiting the files - extra clarity never hurts!!

Andy

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.