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

video playback enhancements for Linux part1: vdpau #870

Closed
wants to merge 11 commits into from

Conversation

FernetMenta
Copy link
Contributor

I have been working on this for quite a while and built dozens of prototypes. Now I think I have something worth for you to review. It is actually an almost complete re-write of vdpau. In order to get the most out of it I had to touch player and rederer as well.

  • Performace improvements
    Separate threads for deinterlacing, opengl interoperation, frame buffering in renderer. This maximizes time for decoding and deinterlacing.
  • Improved deinterlacing
    The advanced methods now use 4 past and 2 future fields, instead of 2 past and 1 future. I have stutter free playback of 1080i@50, temporal/spacial deinterlacing on ION2
  • Fix stuttering caused by overwrite (early reuse) of video surfaces

@elupus
Copy link
Contributor

elupus commented Apr 10, 2012

You really need to split this up in separated changes. It's quite impossible to review like this.

For example changes in renderer like:
Add feature more than 3 buffers
Use feature more than 3 buffers for vdpau

@elupus
Copy link
Contributor

elupus commented Apr 10, 2012

For example, the whole new logic for dropping support should absolutely be a separate commit.

@FernetMenta
Copy link
Contributor Author

I have split the commits.

fields[1][0].id = vdpau->texture[0];
fields[1][1].id = vdpau->texture[2];
fields[2][0].id = vdpau->texture[1];
fields[2][1].id = vdpau->texture[3];

This comment was marked as spam.

This comment was marked as spam.

@FernetMenta FernetMenta mentioned this pull request Sep 16, 2012
@ghost ghost assigned elupus Sep 25, 2012
@FernetMenta
Copy link
Contributor Author

@elupus
I have rebased and updated this PR. The first two commits are from another PR

@ghost
Copy link

ghost commented Oct 7, 2012

ping

@Cyberwizzard
Copy link

Is this the fix for the VDPAU issue where at the start of playback only audio is played and XBMC hangs? If so, does it need additional testing?

@escalade
Copy link

Shouldn't this be integrated soon?

@arnova
Copy link
Member

arnova commented Dec 7, 2012

@elupus : What's the status of this? @FernetMenta : could you please rebase this PR on master?

@davilla
Copy link
Contributor

davilla commented Dec 7, 2012

too late for Frodo, this touches to many areas and beta3 should be the last beta.

@arnova
Copy link
Member

arnova commented Dec 7, 2012

That's a pity although I already suspected that. The reason is that currently using vdpau + h264 + upscaling to 1080p doesn't work properly on Ion (although I assume it's a more general nVidia issue), sometimes the video drops to a very low framerate, especially when dialogs are shown on top of a playing video. This PR fixes that. @elupus @FernetMenta : Is there anything that can be cherrypicked specifically for this issue from this PR perhaps for Frodo?

@davilla
Copy link
Contributor

davilla commented Dec 7, 2012

yes, a pity but last activity on it was a month ago. plenty of time for someone to have handled it then.

@FernetMenta
Copy link
Contributor Author

This is a complete redesign of vdpau, hence I don't think anything can be cherry picked. It's really a pity, I would rather not using XBMC at all on Linux than without those changes.

@Memphiz
Copy link
Member

Memphiz commented Dec 7, 2012

Frodo won't be our last release for sure ;). And that said - i'm using XBMC on linux with VDPAU since ages (without this patch) - so at least its not totally broken as is.

@opdenkamp
Copy link
Member

well, it's broken enough. i haven;t used xbmc on nvidia and amd without @FernetMenta's patches since ages, as that's what makes it usable for pvr, so it's really a shame that this won't be in frodo (although completely understandable)

@fritsch
Copy link
Member

fritsch commented Dec 7, 2012

There is a reason that OpenElec v2.0 shipped with the complete vdpau rewrite. This will also be our basis when going for the OpenElec 3.0 release. If you find some time, read the forum - a huge amount of people is upgrading their default xbmc(buntu) installation to the version with the @FernetMenta patches. That brings btw. other goods, like a XVBA implementation, that bases on the same changes and is therefore strongly coupled ...

@arnova
Copy link
Member

arnova commented Dec 7, 2012

Yeah it's a shame I didn't figure out any time sooner, what the cause of my problems was (and that this PR fixes it). The main reason as that the "scene" recently switched from xvid to h264 for SD releases and I therefor didn't use vdpau acceleration that often... I now also patched my htpc setup, but the users out there that don't have that ability are probably going to b*tch about that...

@wsnipex
Copy link
Member

wsnipex commented Dec 7, 2012

Users already bitch about it all over the forums since Eden was released. Thats why we redirect them to FernetMentas branch(alias Xvba branch)

@arnova
Copy link
Member

arnova commented Dec 7, 2012

Then we might as well only release an ATI/Intel version of xbmcbuntu 12 and redirect them to OpenElec for nVidia....

@fritsch
Copy link
Member

fritsch commented Dec 7, 2012

Erm. ATI does not work at all with mainline xbmc - the xvba-va-driver that is used here is not developed since ages. You get huge artefacts with specific H264 files and some VC1 files don't run at all. Stability is unbelievable bad - also there is to my knowledge no deinterlacing at all, which makes AMD useless for every PVR setup. Nothing can be done about this.

Cause of this @FernetMenta and I started writing XVBA, which replaces the buggy support for AMD via VAAPI. This is also integrated in OpenELEC 2.0 - here some mergendising: http://www.youtube.com/watch?v=jc10em9qQBI

@opdenkamp
Copy link
Member

way to late to do something about this now. next chance will be frodo+1. maybe ati will have fixed some bugs with their driver at that time too :P

@macmenot
Copy link

macmenot commented Dec 7, 2012

well as 99% of Linux users will likely be installing from debian/ubuntu repositories/PPAs, and maybe some Archlinux PKGBUILD, is it worth talking to @amejia1 (e.g.,) about submitting this as a clean debian/patches/vdpau.patch on top of the 12.0 release build, for inclusion in the debian/ubuntu unstable / development repos (where the stable/testing repos would hold 12.0 release) ?

@dteirney
Copy link
Contributor

dteirney commented Jan 6, 2013

@FernetMenta, is it possible for this to be rebased against master? I'd like to try this PR out to see if it helps with the frequent "only audio starts playing and then XBMC locks up when stopping" problem we've been having playing recorded 1080i DVB-T content on Linux using VDPAU. The current PR doesn't cleanly apply to master. Thanks!

@FernetMenta
Copy link
Contributor Author

@dteirney I will do soon. In the meantime can you just try my master?

@arnova
Copy link
Member

arnova commented Jan 6, 2013

Shouldn't the PR's description be made more general? This doesn't only affect vdpau, right?

@FernetMenta
Copy link
Contributor Author

Other decoders can benefit from the buffering but in the PR it is only enabled for vdpau.

@wmyrda
Copy link

wmyrda commented Jan 30, 2013

Seems like the time for merging could be now ;)

cc2cf05

Please take a look at this patch as like it have been said here time and again there is plenty of those that would welcome the patch in the official tree :)

@FernetMenta
Copy link
Contributor Author

@Fneufneu hmm, I have not identified this problem. According to your backtrace it crashes when releasing a lock in CVdpau::Release. Do you think you can get the line in this method where this happens?

@Fneufneu
Copy link
Member

@FernetMenta i will !
i tried today on another desktop on FreeBSD and no crash ...
so difference between two systems are: gcc vs clang, boost libs, ..

@Fneufneu
Copy link
Member

@FernetMenta CDecoder::Release() finish completly
and a CLog::Log() after the SAFE_RELEASE(m_pHardware) show nothing :(

@Fneufneu
Copy link
Member

and the most interesting information: it crash only when built with clang !
no problem with gcc 4.6 or 4.7
clang: 3.2 and 3.3 r174891

@FernetMenta
Copy link
Contributor Author

Do you think the issue is caused by clang or this pr? Any ideas how to track this further down? I am not familiar with clang.

@Fneufneu
Copy link
Member

can someone with clang try this PR plz ?
i need to know if it's a FreeBSD problem.

@davilla
Copy link
Contributor

davilla commented Feb 27, 2013

iOS/OSX has clang when building on 10.8 but no vdpau so this does not help

@macmenot
Copy link

@Fneufneu what compiler flags / optimization level did you use? I could build XBMC 12.0 + this PR using clang on debian, but if the issue is due to clang optimization flags (e.g.,) I wouldn't see it.

@macmenot
Copy link

Interestingly I notice that Eden wasn't previously able to compile successfully on clang at all?
http://buildd-clang.debian.net/package.php?p=xbmc&suite=sid

@Fneufneu
Copy link
Member

@macmenot see PR #1535 for this problem
i need it for FreeBSD but i think OSX doesn't

@FernetMenta
Copy link
Contributor Author

@Fneufneu As mentioned I am not familiar with llvm but at least the name implies that it does or can do some kind of garbage collection. Is it possible to disable this? It looks like the decoder object is released though it is still ref counted. It is ref counted by structures 2 levels below the same object. Maybe the gc, if any, does not see this.

@Fneufneu
Copy link
Member

Fneufneu commented Mar 3, 2013

so maybe we hit a clang bug here, i'll request some help to debug this.
but does this bug also hit OSX ?

@davilla
Copy link
Contributor

davilla commented Mar 3, 2013

It can't hit osx in the same place, no vdpau on osx.

@FernetMenta FernetMenta closed this Aug 2, 2013
@FernetMenta FernetMenta mentioned this pull request Aug 2, 2013
tru pushed a commit to plexinc/plex-home-theater-public that referenced this pull request Nov 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet