FernetMenta / xbmc

Fork of XBMC Main PVR Development Repository.
http://xbmc.org
Other
55 stars 20 forks source link

XVBA - VC-1 interlaced artefacts #94

Closed JdK28 closed 11 years ago

JdK28 commented 11 years ago

When playing the BluRay files from Planet Earth (also other BBC BluRay series) occasionally scene changes cause blocking artefacts. I've made a sample which you can download here: https://www.dropbox.com/s/1te2zll4exg46q7/Sample%20-%20From%20Pole%20to%20Pole%20-%201080i.BluRay.DTSHD-HR.VC-1.mkv. The blocking artefacts can be seen at the scene change at 52 seconds. I'm working with an AMD Fusion APU with OE 2.0. Sync playback and XVBA acceleration is on and the UMA is set to 512M. To me it seems like the P-frames are being calculated from not the latest I-frame, but from a older one from before the scene change.

I've been trying to understand this issue the last two weeks and made a topic in the OE forum (http://openelec.tv/forum/117-xvba-amd/53373-xvba-stuttering-in-interlaced-vc-1-files).

First I thought it had something to do with the missing B-frames. So I pulled the FFmpeg patches for the missing B-frames from the FFmpeg master branch and compiled OE 2.0 again. This solved the missing B-frames, but the blocking artefacts were still there. The missing B-frames thus have nothing to do with the blocking artefacts and are missing because XBMC/OpenElec is not up-to-date with respect to FFmpeg.

Since MPC-HC (uses FFmpeg) with DXVA2 acceleration does not show the artefacts I thought I was missing a fix that is not in the 0.10 FFmpeg branch (used by XBMC/OE), but is in the FFmpeg master branch. After some work I am now able to build the OpenElec 3.0 branch with the latest master of FFmpeg. The result is that the artefacts are still there. This makes me believe it perhaps has something to do with how the XVBA acceleration is used by XBMC. If I understand correctly the patch that OE uses to add XVBA support to XBMC originates from this Git repository.

If you need any help or need somebody to test patches I am willing to help.

FernetMenta commented 11 years ago

The "problem" is that vc-1 supports field based transport of interlaced material (true interlaced). This was not supported by ffmpeg at the time we upgraded our fork. It may be supported now but I haven't checked. I guess none of our hw decoders can cope with field based interlaced material yet. All have to be adjusted.

JdK28 commented 11 years ago

I think the BBC planet earth blurays are not true interlaced material, but progressive segmented frames. The content is 25p, but since this is not a bluray standard they encode it as 50i. By weaving the fields together the 50i becomes 25p again.

The interlaced VC-1 support is indeed pretty recent in FFmpeg. The finishing commit was done on the 19th of October.

I think you did an excellent job with your hw decoders and that perhaps not everything has to be adjusted because it only happens on certain scene changes. If this bug were to be crushed and with my patch for FFmpeg 0.10.6 in my OpenElec fork for adding the B-frame code, OpenElec would be able to play all the BBC BluRay content.

FernetMenta commented 11 years ago

can you provide a list of required ffmpeg patches?

fritsch commented 11 years ago

@JeroenDeKleijn:

What you don't tell here is, that it even produces artefacts, when you disable XVBA completely and also DXVA on windows shows these issues with xbmc. So the error still seems to be in some underlying ffmpeg infrastructure.

Please post the patches that are needed to get it somewhat working.

JdK28 commented 11 years ago

@FernetMenta

To reproduce the blocking artefacts you don't need ffmpeg patches, since they have nothing to do with the B-frames. The artefacts are already visible with the current OpenElec. To get the missing B-frames you do need ffmpeg patches. The problem I had with pulling the patches was that between ffmpeg 0.10 and master certain enumerations and function names had changed. In the end I therefore made my own patch which pulled up the vc1 related files close to ffmpeg master while keeping the old enumerations. This patch is here https://github.com/JeroenDeKleijn/OpenELEC.tv/commit/dbc5243b49d059da4f02330c82091ad28caf7947 There have been a lot of commits between ffmpeg 0.10 and master for the vc1 related files, but the important ones to get the B-frames are: https://github.com/FFmpeg/FFmpeg/commit/33f2a4942380184f3a28cbf2a36366c8ed105232#libavcodec/vc1dec.c https://github.com/FFmpeg/FFmpeg/commit/b87ff3449662e4f6b8415471dd4b7c76f06fbcda#libavcodec/vc1dec.c https://github.com/FFmpeg/FFmpeg/commit/d9dcc940fa0c2d7852b01928532ed9231725fc8c#libavcodec/vc1.c https://github.com/FFmpeg/FFmpeg/commit/feaa40020b59e8abb2c70f2944a12cb068ab2850#libavcodec/vc1.c https://github.com/FFmpeg/FFmpeg/commit/7662a532fbb83d5b3e559a7da58254e56cae3157#libavcodec/vc1.c

@fritsch

XBMC on windows with DXVA indeed also does not work, but the artefacts on windows are different and the video gets in a loop while the audio continues. On MPC-HC with DXVA there is no problem. So it seems it can work for DXVA as long as you use a recent version of ffmpeg. Disabling XVBA and using software decoding is not really an option since the B-frame codepath for software decoding is still disabled in ffmpeg master, only for hardware acceleration it is enabled. I do agree with you that it is still possible that it is a problem in ffmpeg. Perhaps for the DXVA ffmpeg case the problem does not occur.

FernetMenta commented 11 years ago

I was looking into this 4 months ago and iirc I tested with a bbc recording. It turned out to be field interlaced. In this case the fields can't be combined into a frame for decoding. We need to allocate video surfaces half size for each field which is a bugger surgery.

JdK28 commented 11 years ago

If you look at the sample I posted above you see it is encoded as 50i, but results in 25p. It is thus not true interlaced, because this would result in 50p after deinterlacing. Meaning in my sample fields can probably be combined.

Can the allocating of the surfaces explain why 99% of the time frames/fields are decoded fine, but only sometimes artefacts occur during scene changes? Allocating more memory than needed for surfaces should in principle not be a problem, it is just not that optimal/pretty. Just half would be used in case of fields.

StrangeNoises commented 11 years ago

i'm pretty sure planet earth for me was vc1-progressive. certainly i never had any problem with playing the raws in xbmc, or for that matter encoding it in handbrake, long (years) before any vc1-interlaced support started appearing in ffmpeg. (handbrake uses ffmpeg).

Is it possible there were two different releases of Planet Earth, mastered differently? A lot of BBC HD material was released as VC1-1080i (i say "was" they seem to have stopped, still releasing AVC-1080i), but this wasn't one of them to my knowledge. Also, now i'm looking at it in makemkv (see below), it turns out my planet earth blurays have a frame rate of 23.976.

Right this minute I don't have any of those rips on my system as i had to delete them for raid-business (don't go there), but i'll re-rip some of it ahead of schedule now so I've got some examples back online. I won't do planet earth again right now as I can clearly see it's 1080p.

FernetMenta commented 11 years ago

If you look at the sample I posted above you see it is encoded as 50i, but results in 25p. It is thus not true interlaced, because this would result in 50p after deinterlacing.

Well, it always results in 50p after de-interlacing.

Can the allocating of the surfaces explain why 99% of the time frames/fields are decoded fine, but only sometimes artefacts occur during scene changes?

If you combine the fields for decoding on true interlaced material you'll get exactly this. The references to adjacent frames are wrong then, best noticeable when scens are changing.

JdK28 commented 11 years ago

@StrangeNoises

You are correct that there are two releases of Planet Earth. The first one was 23.976, because of the american market which had problems with 25p. Later the Special Edition was released which I am talking about. Since the content was originally shot using 25p, the first planet earth release was not optimal. This was later corrected in the Special Edition release which is 1080i50. Another advantage of the special edition release is the higher bitrate, because the first release had quite some artefacts. BBC Life is also 1080i50 VC-1 and I think the latest Frozen Planet series is h264.

@FernetMenta Is interlaced h264 completely supported for XVBA?

FernetMenta commented 11 years ago

Is interlaced h264 completely supported for XVBA?

yes

StrangeNoises commented 11 years ago

Have just built ffmpeg master from source and tried playing the (just re-ripped) first episode of The Story of India, which is definitely vc1-1080i@50.

It plays a lot better than any previous version; as in, it actually plays rather than hanging up. But there are indeed a lot of blocking and combing artefacts. Not occasionally; consistently. That was ffplay with no options; I don't know if xbmc would do more with that while deinterlacing. I have an earlier upload of an opening excerpt of this here: http://merrow.strangenoises.org/~rachel/xbmctesting/deinterlace-testing/1080i-50-vc1i-001.mkv (which I expect fernetmenta and fritsch have already seen).

JdK28 commented 11 years ago

@StrangeNoises I did some quick tests. On Windows XBMC Frodo beta 1 hangs in a video loop similar to my sample. With MPC-HC using DXVA acceleration your sample plays fine, no blocking artefacts or combing artefacts. Your sample is indeed similar to the one I posted.

On which platform did you run ffplay and with which gpu acceleration. If it was software decoding the artefacts are not that strange because the vc-1 software decoding is not finished yet in ffmpeg

FernetMenta commented 11 years ago

Yes, this is the sample I debugged into ffmpeg and iirc it was field interlaced.

StrangeNoises commented 11 years ago

i played it on an ubuntu quantal 64-bit; it has an nvidia gt520 on it but i don't suppose ffplay with no options made use of vdpau, going by what you're saying. Now i actually have to read that ffplay --help output. :-)

StrangeNoises commented 11 years ago

if there's a way of forcing it to use vc1_vdpau i can't find it. either that or it's possible it's already doing it, and those artefacts are just what i see. Basically there's no difference if i use "-codec vc1_vdpau" from if i leave that out, but also, while playing, there's no discernible spike in cpu usage at all, meanwhile the powermizer "performance level" in nvidia settings goes up to its maximum while playback is ongoing. So although i'm not seeing it definitely say so i suspect it is using vdpau and it just isn't working very well. :-)

JdK28 commented 11 years ago

I also tested it on my OpenElec 3.0 with ffmpeg master using XVBA acceleration. The sample plays roughly correct half the time without any artefacts nicely at 25fps. The other half it is full of blocking artefacts similar to my sample. Overall no combing artefacts so only blocking artefacts.

fritsch commented 11 years ago

if you press stop at such a position and resume at the very same position - are the artefacts gone?

2012/11/25 JeroenDeKleijn notifications@github.com

I also tested it on my OpenElec 3.0 with ffmpeg master using XVBA acceleration. The sample plays roughly correct half the time without any artefacts nicely at 25fps. The other half it is full of blocking artefacts similar to my sample. Overall no combing artefacts so only blocking artefacts.

— Reply to this email directly or view it on GitHubhttps://github.com/FernetMenta/xbmc/issues/94#issuecomment-10693530.

               Key-ID:     0x1A995A9B
               keyserver: pgp.mit.edu

Fingerprint: 4606 DA19 EC2E 9A0B 0157 C81B DA07 CF63 1A99 5A9B

JdK28 commented 11 years ago

@fritsch

I tried what you say, but if I press stop and again play it always starts from the beginning. It does not offer me the option to continue from where I left. Pressing pause and play has no effect.

The blocking artefacts are nicely moving with the content indicating to me the wrong reference I-frame is used for compensation.

fritsch commented 11 years ago

Last time I tried, it was enough to "empty the whole decoder" and start fresh with the next reframe, but obviously this is not a good idea. Finding the reason is better.

Try to print out some pure ptr adresses of the Reframes :-)

FernetMenta commented 11 years ago

Insert some debug logs into ffmpeg. I bet you'll see it taking the field interlace path.

JdK28 commented 11 years ago

@FernetMenta

It is indeed taking the interlace path. When trying to get the B-frames to work I have added log messages to all the error paths in vc1dec.c. However at the moments the blocking artefacts show I don't see any error occuring from the logging messages (of course with debug mode enabled). Is it possible to get more logging from the XVBA patched part of XBMC? Or is this already enabled with the debug option?

FernetMenta commented 11 years ago

There isn't more logging from XVBA.

JdK28 commented 11 years ago

small update:

I build the latest OpenElec master, with xbmc master and ffmpeg 1.2. I also included db8403d04afc025278f611d43a91429bbfa44026 from ffmpeg git, which adds the missing B-frame code in the case of software decoding. The result is that with software decoding the artefacts are gone and the only problem is that the fusion platform is not fast enough to decode 1080psf50 vc1 content. However, with XVBA acceleration the artefacts remain. The funny thing is that in the past when using software decoding it would go wrong at the exact same time position. Somehow it got fixed for software decoding, but XVBA still goes wrong.

FernetMenta commented 11 years ago

Could you log out FCM of this sample? Is it interlace-field mode?

JdK28 commented 11 years ago

What does FCM stand for? I think you mean the v->fcm in vc1dec.c? field/frame coding method or something similar? I could add logging to the vc1_decode_frame function. It think this way I could get this fcm value for each frame.

FernetMenta commented 11 years ago

FCM = frame coding mode. It should not change on every frame: Here is the spec: http://multimedia.cx/mirror/s421m.pdf

JdK28 commented 11 years ago

Ok I finally had some time to debug this issue further like you asked. What I did was dump a lot of the vc1context variables for each frame to the xbmc.log. This gives me the following for the troubling planet earth vc1 content:

14:32:27 T:140291365373696 ERROR: ffmpeg[20FF8700]: [vc1] ---New Frame---,fcm=1,level=3,postprocflag=0,broadcast=1,interlace=1,tfcntrflag=0,refdist_flag=1,extended_dmv=0,psf=0,tfcntr=0,rptfrm=0,tff=1,rff=0,postproc=0,field_mode=0,fptype=0,second_field=0,refdist=0,numref=0,reffield=0,cur_field_type=0,p_frame_skipped=0,bi_type=0
14:32:51 T:140291365373696 ERROR: Previous line repeats 588 times.
14:32:51 T:140291365373696 ERROR: ffmpeg[20FF8700]: [vc1] ---New Frame---,fcm=2,level=3,postprocflag=0,broadcast=1,interlace=1,tfcntrflag=0,refdist_flag=1,extended_dmv=0,psf=0,tfcntr=0,rptfrm=0,tff=1,rff=0,postproc=0,field_mode=1,fptype=1,second_field=0,refdist=2,numref=0,reffield=0,cur_field_type=0,p_frame_skipped=0,bi_type=0
14:32:51 T:140291365373696 ERROR: ffmpeg[20FF8700]: [vc1] ---New Frame---,fcm=2,level=3,postprocflag=0,broadcast=1,interlace=1,tfcntrflag=0,refdist_flag=1,extended_dmv=0,psf=0,tfcntr=0,rptfrm=0,tff=1,rff=0,postproc=0,field_mode=1,fptype=1,second_field=1,refdist=2,numref=0,reffield=0,cur_field_type=1,p_frame_skipped=0,bi_type=0
14:32:51 T:140291365373696 ERROR: ffmpeg[20FF8700]: [vc1] ---New Frame---,fcm=2,level=3,postprocflag=0,broadcast=1,interlace=1,tfcntrflag=0,refdist_flag=1,extended_dmv=0,psf=0,tfcntr=0,rptfrm=0,tff=1,rff=0,postproc=0,field_mode=1,fptype=4,second_field=0,refdist=2,numref=1,reffield=0,cur_field_type=0,p_frame_skipped=0,bi_type=0
14:32:51 T:140291365373696 ERROR: ffmpeg[20FF8700]: [vc1] ---New Frame---,fcm=2,level=3,postprocflag=0,broadcast=1,interlace=1,tfcntrflag=0,refdist_flag=1,extended_dmv=0,psf=0,tfcntr=0,rptfrm=0,tff=1,rff=0,postproc=0,field_mode=1,fptype=4,second_field=1,refdist=2,numref=1,reffield=0,cur_field_type=1,p_frame_skipped=0,bi_type=0
14:32:51 T:140291365373696 ERROR: ffmpeg[20FF8700]: [vc1] ---New Frame---,fcm=2,level=3,postprocflag=0,broadcast=1,interlace=1,tfcntrflag=0,refdist_flag=1,extended_dmv=0,psf=0,tfcntr=0,rptfrm=0,tff=1,rff=0,postproc=0,field_mode=1,fptype=4,second_field=0,refdist=2,numref=1,reffield=0,cur_field_type=0,p_frame_skipped=0,bi_type=0
14:32:51 T:140291365373696 ERROR: ffmpeg[20FF8700]: [vc1] ---New Frame---,fcm=2,level=3,postprocflag=0,broadcast=1,interlace=1,tfcntrflag=0,refdist_flag=1,extended_dmv=0,psf=0,tfcntr=0,rptfrm=0,tff=1,rff=0,postproc=0,field_mode=1,fptype=4,second_field=1,refdist=2,numref=1,reffield=0,cur_field_type=1,p_frame_skipped=0,bi_type=0
14:32:51 T:140291365373696 ERROR: ffmpeg[20FF8700]: [vc1] ---New Frame---,fcm=1,level=3,postprocflag=0,broadcast=1,interlace=1,tfcntrflag=0,refdist_flag=1,extended_dmv=0,psf=0,tfcntr=0,rptfrm=0,tff=1,rff=0,postproc=0,field_mode=0,fptype=4,second_field=0,refdist=2,numref=0,reffield=0,cur_field_type=1,p_frame_skipped=0,bi_type=0
14:32:51 T:140292945905472 ERROR: Previous line repeats 11 times.

As you hopefully can see (formatting is a little messed up by pasting it here) the FCM is constant for a lot of frames (see line that says repeated 588 times) in Interlaced frame mode and then for a couple of frames switches to Interlaced field mode. This switch seems to trigger the blocking artefacts. This switch seems to mess things up in the xvba case (not in software decoding mode). Even when after a view frames the fcm goes to interlaced frame mode again the artefacts remain and they seem to disappear when the next I-frame arrives.

FernetMenta commented 11 years ago

after this change: https://github.com/FFmpeg/FFmpeg/commit/b87ff3449662e4f6b8415471dd4b7c76f06fbcda they call decoder twice, one time for top field, another time for second field. I am not sure but I think we need accumulate picture and slice buffers for both fields and call xvba decoder only once with all data.

JdK28 commented 11 years ago

If that is true why does the xvba interface have the second_field option? This would only be usefull if you would supply the fields one by one I think.

FernetMenta commented 11 years ago

XVBA allows to submit more than one picture descriptor and slice data between start/end decoding.

JdK28 commented 11 years ago

I think I have found one thing that is not correctly passed to xvba for field mode. The 'picture_structure' in xvba_vc1.c is directly passed to xvba, but if you study vc1dec.c of how the picture_structure is set you see the picture_structure=1 for a top_field and picture_structure=2 bottomfield and picture_structure=3 for a frame. If I read the XVBA spec pdf it states the value of picture_structure should be 0,1,3 being top, bottom, frame respectively. We thus pass the wrong value in field mode. We should map the correct value in xvba_vc1.c.

edit: we should have something like this in xvba_vc1: if (s->picture_structure & PICT_TOP_FIELD) pic_descriptor->picture_structure = 0; if (s->picture_structure & PICT_BOTTOM_FIELD) pic_descriptor->picture_structure = 1; if (s->picture_structure & PICT_FRAME) pic_descriptor->picture_structure = 3;

FernetMenta commented 11 years ago

You may be right but this won't solve the problem. The decoder needs slice data for the entire frame, in case of field interlace mode for both fields. vdpau has the same issue. Currently first field is decoded to a video surface, then decoding of the second field starts a new cycle on the same surface and ruins the first field.

JdK28 commented 11 years ago

In xvba_vc1.c is also the following line: pic_descriptor->avc_reference = (s->current_picture_ptr->f.reference & 3) ? 1 : 0;

Is f.reference ever set by the vc1 ffmpeg code? I cannot find the location where it is set. It seems this line was copied from xvba_h264.c. In h264.c f.reference is set.

I wonder if the driver even uses it, since it has 'avc' in the name. However, according to the XVBA spec pdf it is used. So for VC1 content when should this flag be true? For I-frames and P-frames?

fritsch commented 11 years ago

From ffmpeg:

     * The values for this are the same as the MpegEncContext.picture_structure
     * variable, that is 1->top field, 2->bottom field, 3->frame/both fields.
     * Set to 4 for delayed, non-reference frames.

That way we check the bits 01 10 11 if one results in 1, we set it.

JdK28 commented 11 years ago

But we only set avc_reference to 1 if both bits are 1?

Furthermore, nowhere in vc1.c or vc1dec.c is the f.reference value set. In h264.c it is set. So how can we use f.reference in xvba_vc1.c if it is nowhere set?

fritsch commented 11 years ago

No. check what & does.

e.g. 0000 & 0011 = 0 0010 & 0011 = 1 0001 & 0011 = 1 0011 & 0011 = 1

It is bit Operation gets 1 if something hits.

JdK28 commented 11 years ago

you're completely right. I must still be asleep :)

Still for the vc-1 case I cannot find in the code where the 's->current_picture_ptr->f.reference' is set.

fritsch commented 11 years ago

Yes, you are right. I looked through vc1dec.cpp and nothing is done here concerning f.reference. So perhaps we should check it as with the picture structure:

pic_descriptor->avc_reference = s->picture_structure & (PICT_BOTTOM_FIELD | PICT_TOP_FIELD | PICT_FRAME) ? 1 : 0;

Does this change something?

JdK28 commented 11 years ago

I added some debug code to see what the value of avc_refence is set to and it turns out it equals one when it is a I-frame/field or P-frame/field and thus zero for B-frames/fields. To me this seems correct as B-frames are not used as a reference usually. However, modern video encoding techniques sometimes do use the B-frame as a reference. I have no idea if VC-1 does this.

Another strange things I noticed is that ff_mpeg_draw_horiz_band internally still uses s->first_field, which is always zero for vc-1. Somehow even ffmpeg with only software decoding does not suffer from this.

I guess I will wait for when the surfaces are handled correctly for interlaced content.

FernetMenta commented 11 years ago

should be fixed in master now. interestingly the doc was wrong about pic_structure, top bottom field ware mixed up. it also should honor psf: it won't do any deinterlacing if psf is set.

JdK28 commented 11 years ago

Thanks, I will build OpenElec with your changes and test if it fixes the planet earth content.

Just out of interest what does the sdf=0x0c in your changeset exactly do? I thought you were thinking fields had to be sent to xvba together instead of separate to make field interlaced content work, but I cannot believe this change does that?

FernetMenta commented 11 years ago

My assumption of sending both fields together was not correct. 0x0C is the start code suffix for a field, 0x0D for a frame. ffmpeg eats the 4 byte start code, hence hw accelerators which require it, have to add it back to the bit stream.

JdK28 commented 11 years ago

I tested the Planet Earth content and the blocking artefacts are gone :D.

I just notice that the deinterlacer reduces the image quality in scenes which are actually 25p. I think the BBC should have set the psf flag for these scenes, because now I think there is no way to tell that no deinterlacing should be done. Or does the videostream contain presentation timestamps for each field?

Is the XVBA deinterlacer inside the driver or are you using some of the xvba properties to make your own deinterlacer? It seems the XVBA deinterlacer is pretty basic, let's say BOB like. Basically, I am asking can we improve the deinterlacing or are we stuck with what XVBA provides?

fritsch commented 11 years ago

Concerning XVBA we are stuck, there is just nothing else.

There is "always" the method of transfering the data back and do something advanced there - but that won't really work out on Fusion hardware. Bob can be done also externally.