Incompatibilities beween ffmpeg 2.0.2 and 2.1 exposed via XBMC

[EricV]

I'm building XBMC from source (​https://github.com/FernetMenta/xbmc/commits/master> using external ffmpeg 2.0.2 (deb-multimedia debian package). All works well.

Upgrading ffmpeg to 2.1 (deb-multimedia debian package), breaks XBMC (no more video output, sound OK, no error in XBMC log). Even rebuilding XBMC with new libav* headers does not solve the problem. Same symptoms.

Are there known API inciompatibilities?

[Carl Eugen Hoyos]

Is the same problem reproducible with current git head?
Which change broke the video output?

[EricV]

Fernetmenta git tree today. NB this is the tree used by openelec. It has been merged with upstream today.

Which change in ffmpeg, unfortunately I do not know. I say XBMC binary compiled with external ffmpeg 2.0.2 works, changing only external ffmpeg to 2.1 via installation of binary packages => breaks XBMC => ABI breakage. And even recompiling using correct 2.1 headers produce a non working XBMC. Did the numbering of codec change?

[Carl Eugen Hoyos]

Is the same problem reproducible with current FFmpeg git head?
Please understand that while I consider this a (very) important issue, I don't see how this can be fixed as long as we don't know the change that introduced the regression.

[EricV]

I will build git head tomorrow. I normally don't use it because upstream rendering capabilities are far below the one on fernetmenta tree. NB this occurs using vdpau acceleration on several different PC all using vdpau and being build using different fernetmenta git tree. Plus even if upstream git does not exhibit the problem (usually I find no difference for such problems), the problem still exist and may be seen in other software as well.

Regarding the commit: I have no time to bisect so I hoped you would have an hint.

[EricV]

As expected, xbmc git upstream (official xbmc git today 10h45) behaves exactly the same with ffmpeg 2.1 : sound, no errors in log but dark window (no video out) whereas deb-mutimedia package that also use upstream git works with ffmpeg 2.0.2 (when build using it due to known incompatibilities with 1.x).

[Carl Eugen Hoyos]

Is the same problem reproducible with current FFmpeg git head?

[EricV]

Try yourself!

[Michael Niedermayer]

Ive build FernetMenta/master against FFmpeg 2.1 and it seems working fine.
I needed

diff --git a/lib/DllAvCodec.h b/lib/DllAvCodec.h
index c54f542..866f996 100644
--- a/lib/DllAvCodec.h
+++ b/lib/DllAvCodec.h
@@ -114,7 +115,7 @@ public:
     CSingleLock lock(DllAvCodec::m_critSection);
     return ::avcodec_open2(avctx, codec, options);
   }
-  virtual int avcodec_open2_dont_call(AVCodecContext *avctx, AVCodec *codec, AVDictionary **options) { *(volatile int *)0x0 = 0; return 0; }
+  virtual int avcodec_open2_dont_call(AVCodecContext *avctx, const AVCodec *codec, AVDictionary **options) { *(volatile int *)0x0 = 0; return 0; }
   virtual int avcodec_close_dont_call(AVCodecContext *avctx) { *(volatile int *)0x0 = 0; return 0; }
   virtual AVCodec *avcodec_find_decoder(enum AVCodecID id) { return ::avcodec_find_decoder(id); }
   virtual AVCodec *avcodec_find_encoder(enum AVCodecID id) { return ::avcodec_find_encoder(id); }

also for reasons i dont know yet i had to remove the ubuntu libav libraries as they kept being linked in instead of the more recent ffmpeg ones from /usr/local and caused linking failures. Note though this might be a configuration issue on my box, and unrelated.

So I dont seem to be able to reproduce a problem when xbmc is build and run against ffmpeg 2.1
havnt yet looked at what happens when its build against 2.0 and linked to 2,1 libs

[Carl Eugen Hoyos]

Replying to michael:

Ive build FernetMenta/master against FFmpeg 2.1 and it seems working fine.

Did you test VDPAU decoding?

[EricV]

The patch has been pushed upstream at fernetmenta already... I notivced it yesterday also

Here is my package list:

ii libav-tools 10:2.1-dmo2 amd64 Multimedia player, server, encoder and transco
ii libavcodec-dev:amd64 10:2.1-dmo2 amd64 Library to encode decode multimedia streams -
ii libavcodec54:amd64 10:1.2.4-dmo4 amd64 Library to encode decode multimedia streams -
ii libavcodec55:amd64 10:2.1-dmo2 amd64 Library to encode decode multimedia streams -
ii libavdevice-dev:amd6 10:2.1-dmo2 amd64 Development files for libavdevice.
ii libavdevice55:amd64 10:2.1-dmo2 amd64 FFmpeg device handling library.
ii libavfilter-dev:amd6 10:2.1-dmo2 amd64 Development files for libavfilter.
ii libavfilter3:amd64 10:2.1-dmo2 amd64 FFmpeg filter library.
ii libavformat-dev:amd6 10:2.1-dmo2 amd64 Development files for libavformat.
ii libavformat54:amd64 10:1.2.4-dmo4 amd64 FFmpeg file format library.
ii libavformat55:amd64 10:2.1-dmo2 amd64 FFmpeg file format library.
ii libavutil-dev:amd64 10:2.1-dmo2 amd64 FFmpeg avutil devel files - devel files.
ii libavutil52:amd64 10:2.1-dmo2 amd64 FFmpeg avutil library - runtime files

So indeed I have a mix of 2.1 and 1.2.4 but:

1) all dev files are from 2.1
2) they have there library counterpart

ldd /usr/local/lib/xbmc/xbmc.bin | grep libav

libavahi-client.so.3 => /usr/lib/x86_64-linux-gnu/libavahi-client.so.3 (0x00007fdc8dd24000)
libavahi-common.so.3 => /usr/lib/x86_64-linux-gnu/libavahi-common.so.3 (0x00007fdc8db17000)
libavcodec.so.55 => /usr/lib/x86_64-linux-gnu/libavcodec.so.55 (0x00007fdc8564c000)
libavfilter.so.3 => /usr/lib/x86_64-linux-gnu/libavfilter.so.3 (0x00007fdc85344000)
libavformat.so.55 => /usr/lib/x86_64-linux-gnu/libavformat.so.55 (0x00007fdc84fcc000)
libavutil.so.52 => /usr/lib/x86_64-linux-gnu/libavutil.so.52 (0x00007fdc84d7b000)

All build time linked dynamic libraries are linked against the 2.1 libraries. Did not check dlopen...

[EricV]

And regarding the comment, yes I use vdpau decoding. But you gave me an idea: i can dynamically disable it and then yes I have video (that flicker and has others problem due to cpu, but I do have images). So yes its related to vdpau.

[Carl Eugen Hoyos]

Replying to EricV:

And regarding the comment

What I tried to tell Michael was that you did not explain how to reproduce the problem, I guessed that it is VDPAU-related but Michael probably did not.
(I fear Michael has no VDPAU hardware.)

[EricV]

Yes sorry about that. The benefit of fenetmenta tree is hardware decoding in general, vdpau in particular but also Intel (and even xvba but on a dedicated branch and only using internal ffmpeg).

Go in expert mode for setting. I use :

Use VDPAU hardware acceleration
Use VDPAU video mixer
Use audio video sync -> method = resample audio

And I have on system video output (vertical sync always on).

[Michael Niedermayer]

Replying to michael:

also for reasons i dont know yet i had to remove the ubuntu libav libraries as they kept being linked in instead of the more recent ffmpeg ones from /usr/local and caused linking failures. Note though this might be a configuration issue on my box, and unrelated.

i was just missing "LDFLAGS=-L/usr/local/lib", iam not sure why that ubuntu box needs that though but thats unrelated

[Michael Niedermayer]

Replying to EricV:

The patch has been pushed upstream at fernetmenta already... I notivced it yesterday also

There are 2 lines that need changing in the file, i think one was missed

[EricV]

Replying to michael:

Replying to EricV:

The patch has been pushed upstream at fernetmenta already... I notivced it yesterday also

There are 2 lines that need changing in the file, i think one was missed

I see only one line of diff in what you posted... Changing this single line fixes the compilation error for me.

[EricV]

My configure line :

./configure --enable-external-libraries --disable-vaapi --disable-crystalhd --enable-pulse

Resulting config given the packages I have:

---

XBMC Configuration:

---

git Rev.:
Debugging: Yes
Profiling: No
Optimization: Yes
SWIG Available: Yes
JRE Available: Yes
Doxygen Available: Yes
Crosscomp.: No
target ARCH: no
target CPU: no
OpenGL: Yes
ALSA: Yes
DBUS: Yes
VDPAU: Yes
VAAPI: No
CrystalHD: No
VTBDecoder: No
OpenMax: No
Joystick: Yes
XRandR: Yes
Waveform: Yes
Spectrum: Yes
GOOM: No
RSXS: Yes
FishBMC: Yes
ProjectM: Yes
Skin Touched: No
X11: Yes
Wayland: No
Bluray: Yes
TexturePacker:Yes
MID Support: No
ccache: Yes
ALSA Support: Yes
PulseAudio: Yes
HAL Support: Yes
DVDCSS: Yes
Google Test Framework Configured: No
Avahi: Yes
mDNSEmbedded: No
Non-free: Yes
ASAP Codec: No
MySQL: Yes
Webserver: Yes
libssh support: Yes
libRTMP support: Yes
libsmbclient support: Yes
libnfs client support:No
libafpclient support: No
AirPlay support: Yes
AirTunes support: No
UPnP support: Yes
Optical drive: Yes
libudev support: Yes
libusb support: No
libcec support: Yes
libmp3lame support: Yes
libvorbisenc support: Yes
libcap support: Yes
additional players: No
additional codecs: No
External FFmpeg: Yes
PVR add-ons: No
prefix: /usr/local

---

[EricV]

Tested with 2.1.1. Same problem.

[Carl Eugen Hoyos]

There is not much difference between 2.1 and 2.1.1 and to the best of my knowledge no difference wrt VDPAU.

If you want to help (meaning: if you are interested to get this ticket fixed), please test current FFmpeg git head (with a relevant version of xbmc of your choice) to make sure there actually is an unfixed bug.

[EricV]

Frankly I have no time, and a solution. What is the difference between git and 2.1.1. Why should I know?

[Carl Eugen Hoyos]

You just tested 2.1.1 although this is (nearly) useless and although I already asked you repeatedly to test current git head and although this is the very first thing mentioned on ​http://ffmpeg.org/bugreports.html
Why didn't you test current git head instead (which would bring us one step further)?

And of course: If you don't want to work on this issue (that I, as said, consider important) why do you expect a developer to work on it?

[EricV]

1) Forcing to test with git before reporting a bug is "for me" a non-sense as except developers nobody uses theses versions and only few people are really able to correctly rebuild from git.
2) End user use only packaged version on their system. I hardly see any git version in any distribution,
3) I reported this bug because it will annoy many Linux user when they will discover XBMC breaks because they updated ffmpeg so that you are aware of it. I just need to freeze the ffmpeg package version and will do so until the bug is resolved,
4) ffmpeg binary incompatibilities (even in series which should not allow it) should not be allowed. It did happen in the past and just happened again. This creates packaging nightmare and conflicts between maintainers

This issue is not mine its yours. XBMC packagers will not provide ffmpeg 2.1.x until it is fixed or use internal ffmpeg version if distributions allow it. This means less exposure for new feature, and less quality at the end. I'm more concerned to be able to see my films.

[Clément Bœsch]

Replying to EricV:

1) Forcing to test with git before reporting a bug is "for me" a non-sense as except developers nobody uses theses versions and only few people are really able to correctly rebuild from git.

You are not suggested to use the git version as a workaround for your problem, but to help developers figuring out where is the regression and/or if it's already fixed. Also note that when this is fixed upstream, we might ask you to pull the git/master to try it before the fix is actually backported to stable branches.

[...]

[ordroid]

I just came across this on a nvidia ION2 system. ffmpeg master (as of c122e697) does not improve the situation (I had to patch XBMC master as well).

ffmpeg 2.0.2 works fine indeed, even without recompiling XBMC against it. Let me know if I can provide anything else.

[Carl Eugen Hoyos]

Please confirm that b7fc2693 works fine, that bf36dc50 is broken and please also test a0c6c8e5

[Carl Eugen Hoyos]

And please test (and report) if this reproducible for all of H264, MPEG-4 ASP, VC-1 and MPEG 1/2 Video or only for some of them.

[EricV]

Just kidding : and if you can even fix the bug do not hesitate!

[ordroid]

b7fc2693:
H264 - pass
VC-1 - pass
MPEG2 - pass
MPEG4-2 - pass

bf36dc50:
all pass (checked twice..)

a0c6c8e5:

H264 - fail
VC-1 - fail
MPEG2 - fail
MPEG4-2 - pass

[Carl Eugen Hoyos]

Replying to ordroid:

Thank you for the tests!

Could you confirm that this ticket is about VDPAU, ie decoding with "software decoder" works fine with current git head, decoding with "hardware acceleration (vdpau)" does not work?

bf36dc50:
all pass (checked twice..)

Is it possible that xbmc automatically falls back to software decoding with this version? Perhaps CPU usage could tell you. (Original VDPAU API was removed from this version, so if it works for you, that would a surprise.)

a0c6c8e5:

H264 - fail
VC-1 - fail
MPEG2 - fail
MPEG4-2 - pass

Can you confirm that your ION-2 hardware supports hardware decoding for MPEG4-ASP (and that XBMC actually supports it)?

[EricV]

ION-2 mean low profile hardware (typically netop) that is unable to decode HD without hardware acceleration. So software decoding will probably fail (at least not work correctly but should output some images at least).

[ordroid]

Yes, this is strictly about VDPAU acceleration.

I'm still on bf36dc50 and playing hardware accelerated videos like a champ. With VDPAU disabled in XBMC, a single Atom core can't keep up with 720p H264 (but multithreading works even with 1080p :-).

720p MPEG4-ASP uses about ~30% CPU with and without VDPAU. As far as I can tell with a simple Google search the ION2 is supposed to support MPEG4-ASP, but it's apparently not using VDPAU.

CPU usage with VDPAU enabled is about 11% for H264, VC-1 and MPEG2 (still on bf36dc50). Note that I did not compile XBMC between these runs, I simply swapped the ffmpeg libs and rebooted. So I guess my XBMC(-git) is already using the newer APIs? It's compiled against ffmpeg master as of yesterday.

With ffmpeg head and VDPAU CPU usage is still ~11% but with a black screen.

ffmpeg version N-55266-gbf36dc5
built on Nov 29 2013 17:28:35 with gcc 4.8.2 (Gentoo 4.8.2 p1.0, pie-0.5.8)
configuration: --prefix=/usr --libdir=/usr/lib64 --shlibdir=/usr/lib64 --mandir=/usr/share/man --enable-shared --cc=x86_64-pc-linux-gnu-gcc --cxx=x86_64-pc-linux-gnu-g++ --ar=x86_64-pc-linux-gnu-ar --optflags='-march=atom -msse2 -msse3 -mssse3 -mcx16 -msahf -O2 -pipe' --extra-cflags='-march=atom -msse2 -msse3 -mssse3 -mcx16 -msahf -O2 -pipe' --extra-cxxflags='-march=atom -msse2 -msse3 -mssse3 -mcx16 -msahf -O2 -pipe' --disable-static --enable-gpl --enable-postproc --enable-avfilter --enable-avresample --disable-stripping --enable-version3 --enable-nonfree --disable-indev=v4l2 --disable-outdev=v4l2 --disable-indev=oss --disable-indev=jack --disable-outdev=oss --enable-bzlib --disable-runtime-cpudetect --disable-debug --disable-doc --enable-gnutls --enable-hardcoded-tables --enable-iconv --enable-network --disable-openssl --enable-ffplay --disable-vaapi --enable-vdpau --enable-zlib --enable-libvo-aacenc --disable-libvo-amrwbenc --enable-libmp3lame --enable-libaacplus --disable-libfaac --enable-libtheora --disable-libtwolame --disable-libwavpack --enable-libx264 --enable-libxvid --disable-libcdio --disable-libiec61883 --disable-libdc1394 --disable-libcaca --disable-openal --disable-libv4l2 --disable-libpulse --enable-x11grab --disable-libflite --disable-frei0r --disable-fontconfig --enable-libass --disable-libfreetype --disable-libsoxr --enable-pthreads --disable-libopencore-amrwb --disable-libopencore-amrnb --disable-libfdk-aac --disable-libopenjpeg --enable-libbluray --disable-libcelt --disable-libgme --disable-libgsm --disable-libmodplug --enable-libopus --disable-libquvi --enable-librtmp --disable-libschroedinger --enable-libspeex --enable-libvorbis --disable-libvpx --disable-amd3dnow --disable-amd3dnowext --disable-altivec --disable-avx --disable-mmxext --disable-vis --disable-neon --cpu=atom
libavutil      52. 40.100 / 52. 40.100
libavcodec     55. 20.100 / 55. 20.100
libavformat    55. 13.102 / 55. 13.102
libavdevice    55.  3.100 / 55.  3.100
libavfilter     3. 82.100 /  3. 82.100
libavresample   1.  1.  0 /  1.  1.  0
libswscale      2.  4.100 /  2.  4.100
libswresample   0. 17.103 /  0. 17.103
libpostproc    52.  3.100 / 52.  3.100

[Carl Eugen Hoyos]

Replying to ordroid:

Yes, this is strictly about VDPAU acceleration.

Thank you.

720p MPEG4-ASP uses about ~30% CPU with and without VDPAU.

So it is safe to say that we can remove ASP from the testing list;-)

CPU usage with VDPAU enabled is about 11% for H264, VC-1 and MPEG2 (still on bf36dc50). Note that I did not compile XBMC between these runs, I simply swapped the ffmpeg libs and rebooted.

So I guess my XBMC is already using the newer APIs?

It would be great if you could verify this because it would indicate a different bug than what I thought so far. The difference in the XBMC source code would be that the new API uses a "hwaccel" and the "h264/vc1/etc. decoder while the new old API was based on specific decoders h264_vdpau, vc1_vdpau etc.

It's compiled against ffmpeg master as of yesterday.

I don't think it is generally ok to use an older shared library instead of the one you compiled against, using a newer one may be ok (bugs are of course possible), ideally you would recompile to get valid results.

While this is quite certainly unrelated, your configure line looks very broken;-(
Perhaps testing with ./configure --enable-shared && make wouldn't hurt...

[Carl Eugen Hoyos]

Replying to cehoyos:

While this is quite certainly unrelated, your configure line looks very broken;-(
Perhaps testing with ./configure --enable-shared && make wouldn't hurt...

Or actually something like:

./configure --prefix=/usr --libdir=/usr/lib64 --shlibdir=/usr/lib64 --enable-shared --disable-static --enable-gpl --enable-avresample --enable-version3 --enable-nonfree --disable-doc --enable-gnutls --enable-libmp3lame --enable-libaacplus --enable-libtheora --enable-libx264 --enable-libxvid --enable-libass --enable-libbluray --enable-libopus --enable-librtmp --enable-libspeex --enable-libvorbis --cpu=atom

But since it is unrelated, please just ignore.

If there really is a problem with the dev's, please report it though!

[ordroid]

It would be great if you could verify this because it would indicate a different bug than what I thought so far. The difference in the XBMC source code would be that the new API uses a "hwaccel" and the "h264/vc1/etc. decoder while the new API was based on specific decoders h264_vdpau, vc1_vdpau etc.

The bundled ffmpeg in XBMC is version 1.2 and states "VDPAU hardware acceleration through normal hwaccel". Not sure how I'd go about verifying the shared behaviour.

It's compiled against ffmpeg master as of yesterday.

I don't think it is generally ok to use an older shared library instead of the one you compiled against, using a newer one may be ok (bugs are of course possible), ideally you would recompile to get valid results.

Are there any combinations I could try to help isolate this problem?

While this is quite certainly unrelated, your configure line looks very broken;-(
Perhaps testing with ./configure --enable-shared && make wouldn't hurt...

I'm using the Gentoo git ebuild and actually had to remove some flags to compile these revisions. Incidentally all those enable flags map to the libraries I have installed through portages USE flags. But for simplicitys sake I'll do --enable-shared :)

[Carl Eugen Hoyos]

Replying to ordroid:

But for simplicitys sake I'll do --enable-shared :)

Careful, because this may break other applications which expect libavcodec to be linked against certain libraries.
I am primarily (always) surprised about the --extra-cflags: I suspect they hurt performance, if they don't hurt performance, our configure script should be improved. Additionally, many options are the default (--enable-iconv) or make no sense on your architecture (--disable-altivec) or look contradictory to me (--disable-stripping --disable-debug). But as said all this is unrelated and it is much more failsafe if you continue using ebuild.

[ordroid]

Replying to cehoyos:

Replying to ordroid:

But for simplicitys sake I'll do --enable-shared :)

Careful, because this may break other applications which expect libavcodec to be linked against certain libraries.
I am primarily (always) surprised about the --extra-cflags: I suspect they hurt performance, if they don't hurt performance, our configure script should be improved. Additionally, many options are the default (--enable-iconv) or make no sense on your architecture (--disable-altivec) or look contradictory to me (--disable-stripping --disable-debug). But as said all this is unrelated and it is much more failsafe if you continue using ebuild.

Noted. On a further unrelated note, I've noticed that ffmpeg disables SSE and SSSE3 (from configure output) even though the CPU is capable of it (altivec and sse are USE flags and --disabled unless activated globally). Apparently it's slow on Atom - so not sure of the impact of those --extra compiler flags. I'll try removing them.

But this is certainly unrelated, and I'm not sure how to progress. If you don't have any specific code points, I can start a binary search between bf36dc50 and a0c6c8e5 and try to nail down the commit that introduced it. Might take a while though :)

[Carl Eugen Hoyos]

Replying to ordroid:

If you don't have any specific code points, I can start a binary search between bf36dc50 and a0c6c8e5 and try to nail down the commit that introduced it. Might take a while though :)

Just test the commit before - 0a141b0e - I would expect it to "work" (if not we at least know what to search for) but as said, replacing a newer shared library with an older is not supposed to work so please compile bf36dc50 - if it really works with that one, we know that this is an issue with the new API.

[ordroid]

I built ffmpeg 0a141b0e and XBMC against it, but VDPAU video went black again.

XBMC tip compiles and runs fine with bf36dc50.

[Carl Eugen Hoyos]

Replying to ordroid:

I built ffmpeg 0a141b0e and XBMC against it, but VDPAU video went black again.

XBMC tip compiles and runs fine with bf36dc50.

This is quite unexpected, please run git bisect, I don't have any better ideas. Sorry for the time you spent on testing the changes I guessed were responsible.

[EricV]

If you look at comment 5, the fact that rebuilding from source doesn't solve the problem was already clear. I would like to thank ordroid for his patience.

[Timothy Gu]

Replying to EricV:

If you look at comment 5, the fact that rebuilding from source doesn't solve the problem was already clear. I would like to thank ordroid for his patience.

If you can solve the bug, there you go.
If you can't solve it but can help in the solution of the bug, nice.
If you just want to troll around, shut up and find somewhere else.

[EricV]

Replying to Timothy_Gu:

Replying to EricV:

If you look at comment 5, the fact that rebuilding from source doesn't solve the problem was already clear. I would like to thank ordroid for his patience.

If you can solve the bug, nice.
If you can't solve it but can help in the solution of the bug, nice.
If you just want to troll around, shut up and find somewhere else.

If you do not help either shut up. At least I reported the bug and gave enough information to ffmpeg dev team to reproduce it. But the was not enough. Unfortunately, except ordroid nobody seems to have tried himself to reproduce the bug.

VDPAU is the hardware acceleration API for Nvidia and now also for AMD and nobody seems to care enough except affected users. Fine. I understand why XBMC will probably use internal lib forever preventing most users to find bug in ffmpeg and enhance it.

[Carl Eugen Hoyos]

Replying to EricV:

If you do not help either shut up.

Please allow me to inform you that you are very close to being banned from this tracker.
I would suggest you do not comment for the moment here, thank you.

[Timothy Gu]

Replying to EricV:

At least I [...] gave enough information to ffmpeg dev team to reproduce it.

Really? I don't think so.

Unfortunately, except ordroid nobody seems to have tried himself to reproduce the bug.

Because we either cannot or don't have the hardware.

VDPAU is the hardware acceleration API for Nvidia and now also for AMD and nobody seems to care enough except affected users.

This is because of somebody's snotty behaviors and unwillingness to provide any additional info regarding this bug, and because of the lack of resource that we have in order to reproduce the bug.

You want this bug to be fixed, you contribute because we can't.

[EricV]

Replying to Timothy_Gu:

Replying to EricV:

At least I [...] gave enough information to ffmpeg dev team to reproduce it.

Really? I don't think so.

Honestly what was missing concretely? I gave you the way to reproduce the XBMC binary exhibiting the bug, the compilation options, the settings. For ffmpeg, I'm sure you know better than me.

You want this bug to be fixed, you contribute because we can't.

This makes me wonder how you can test/verify/regress test vdpau changes before accepting them?

[ordroid]

Short update: I've bisected the problem down to this range (five commits). 50fb8c is known good and 66056f is bad. Will continue the search, but may take a couple of days before I have time again. Maybe someone can spot the error in this 650 line VDPAU diff :)

50fb8c1114b9c2b7d299cbc17a18a457d12069a8..66056f74a1e9c1baa910841e0c966cb3b74f0634

[Carl Eugen Hoyos]

Thank you!

You only have to do one more test to know the offending commit:
If you test 9547e3ee or c3b29023, we will know which commit introduced the regression.
(I usually never guess, I did it in this frame because of the continuous flaming and you know how it ended...)

[Carl Eugen Hoyos]

Just test c3b29023 - the other commits are mostly cosmetic.

[ordroid]

c3b29023 is bad and git bisect claims 2852740 as the culprit.

Will stay on 549294fbb or thereabout until further notice :)

[Carl Eugen Hoyos]

Replying to ordroid:

Will stay on 549294fbb or thereabout until further notice :)

A significantly less buggy version is for example 50fb8c11

[ordroid]

Already on 50fb8c11 actually. Decided to just go for the last known good commit instead of fighting with the build (as with some of the bisecting points..).

Considering my work here done for now, but I'm happy to test any patches.

[Carl Eugen Hoyos]

Was this fixed in xbmc? Or did you return to the old (tested) vdpau code?

The reason I ask is that a new 2.1 release will be made and maybe the offending commit 2852740e should be reverted in the release branch? Since I cannot test, you will have to provide the patch.

[EricV]

This is not fixed in XBMC AFAIK. I will test it with yesterday version. I propose you do the revert (I have no clue how to do it) and I test it.

EDITED: confirmed not working with latest fernetmenta git tree.

[Carl Eugen Hoyos]

Did you ever report the problem to Fernetmenta? It may be easier for him to simply revert to the old VDPAU code...

[EricV]

No I did not. Do you have a way for me to look at the commit that did break? Before doing this I need to evalaute if it is indeed a break in the API. Reporting bug in XBMC for ffmpeg is usually useless as you only are advised to use internal version... But distrib like debian dislike it.

EDITED: I have contacted rainer asking that it looks at this thread. Hope he will have some time to do it.

[EricV]

Side comment, now that ATI via UVD also use VDPAU for hardware video decoding, you have 80% of the graphical card market on linux that will use vdpau for hardware decoding. I tested the open source drivers on a zotac AD02 and it works well but the bug is there too => I need to use the internal version. So it breaks NVIDIA and ATI/AMD with open source drivers. As anyway XBMC support for XVBA is dead it means that peole will soon be affected for AMD card.

See ​http://forum.xbmc.org/showthread.php?tid=174854

[Carl Eugen Hoyos]

Replying to EricV:

I need to use the internal version.

Could you elaborate? I think I don't understand...

[EricV]

XBMC has its own outdated version of ffmpeg. You can configure XBMC to use either external ffmpeg (dynamic libraries) or use the internal version of the libraries. As the internal version is a pre 2.0 one, it works. Unfortunately this is not the way packager do usually build XBMC.

./configure --enable-external-libraries --disable-vaapi --disable-crystalhd => Ko

./configure --enable-external-libraries --disable-external-ffmpeg --disable-vaapi --disable-crystalhd => OK

[FernetMenta]

Replying to cehoyos:

Did you ever report the problem to Fernetmenta? It may be easier for him to simply revert to the old VDPAU code...

We are short final for 13.0 release and can't do such changes in that stage. With last ffmpeg upgrade we changed vdpau to use hwaccel context. The mentioned commit moved required parameters from the hw context to a new picture context. why?

XBMC depends heavily on ffmpeg, this is why we have our own fork. Currently this fork is embedded into XBMC's tree which is not ideal. We intend to move it out of the tree which also makes upgrade easier. We can pull a release directly from ffmpeg repo during build.

From XBMC's perspective breaking the API is not a big deal. Like Eric said, we don't really support using "external" ffmpeg. On debian this even means libav which no dev runs tests against.

[EricV]

That's the developpers view and surely not the various distribution maintainers views. Most packages are build using external dependencies and external ffmpeg. As a side note, anybody wanting serious multimedia in debian use marillat deb-multimedia repo that build ffmpeg 2.1, xbmc git, vlc handbrake, ... XBMC from marillat tree was build using external dependencies for example...

From a XBMC perspective, lauching a new version that does not support up-todate ffmpeg is a shame. From ffmpeg perspective it means it will receice less testing because maintainers dislike API breakage and may be reluctant to package 2.1.x.

[Carl Eugen Hoyos]

Just a few comments not strictly related to this problem:
MPlayer uses internal FFmpeg since forever, binaries with external FFmpeg are not supported afaict (or as far as I am concerned), but MPlayer always uses current FFmpeg git head (meaning it receives not only new features but also bug fixes - including security relevant ones)

Random old snapshots are bound (and known) to contain bugs (including regressions and security relevant issues), if you cannot use current git head at the time of a xbmc release (why not?) then I strongly suggest to use at least a current release series (2.1 and very soon 2.2), releases get regression and security fixes later but they get them.

Note that if you have local patches that you need in FFmpeg, please propose them on ffmpeg-devel (yes, the policy has changed).

There will be no further releases in the 2.0 branch, note that it does contain known bugs.

I don't know why the "new" vdpau code was changed, I only feel responsible for the old code (which was extensively tested with tools that we do not have access to and could not be used when the "new" code was committed!), both work with MPlayer, both are tested regularly with MPlayer.

Strictly related to this ticket:
I would suggest that you locally revert the offending commit in the release/2.1 branch and test, I will commit such a change to the FFmpeg repository.

[EricV]

Gime some time: as commit have pilled up on the faulty one, git revert cause conflicts that need to be handled manually... Have no time ATM. Maybe at home tonight.

[EricV]

Requested patch attached. No clue how to send it via git..

EDITED : This first version of the patch was broken because "git diff" after "git revert" does not list all modified files

[Hendrik]

That change only touches internals inside the hwaccel, not the API, considering it still works in other applications, it might be more interesting what XBMC does to get broken by it?

Blindly reverting things may fix the immediate problem, but if it wasn't certain what the problem actually is, it may re-appear again.

[EricV]

If you want to look its mainly there:

​https://github.com/xbmc/xbmc/blob/master/xbmc/cores/dvdplayer/DVDCodecs/Video/VDPAU.cpp
​https://github.com/xbmc/xbmc/blob/master/xbmc/cores/dvdplayer/DVDCodecs/Video/VDPAU.hpp

Probably FernetMenta already knows.

[EricV]

Revert of patch that broke XBMC. Tested with XBMC git and ffplay on various H264 1080p material

[EricV]

Due to lack of git practice, I used git diff to generate the patch but apparently after a git reverse, some file are lacking! Re-attached the patch after a commit via a show.

[Hendrik]

IMHO the problem is here, in line 1043.
​https://github.com/xbmc/xbmc/blob/master/xbmc/cores/dvdplayer/DVDCodecs/Video/VDPAU.cpp#L1043

All the fields in m_hwContext are no longer set since the patch, so the XBMC code can't read them.
The original patch in itself makes total sense, since you want to store per-picture information with the picture.

I am however confused how this is implemented in XBMC, or why it was done this way.
How i read the current VDPAU function API, you would put such an implementation inside the "render" or "render2" callback in the "m_hwContext" (how the variable is called in XBMC)

So unless I'm missing something, XBMCs implementation is kinda...wrong.
What they should do is merge their "FFDrawSlice" into their "Render" function, and stop accessing m_hwContext.info/bitstream*.

I may be missing a bit of information on the why this was done, but this seems to be the current state from my point of view.

[EricV]

Fields that were set and are no more set in a structure is clearly a form of very nasty API change for me (same as if you change the order of a structure passed via pointers). Does the numbering of the ffmpeg version allow such changes and is there ways to detect it at compile time/run time?

I do not care if ffmpeg is fixed or XBMC is fixed and who is wrong at the end. My goal is to make sure XBMC git continues to work with ffmpeg git and this before the soon to be released XBMC gotham.

[Hendrik]

I lack the means to properly test right now, so no guarantees whatsoever for this patch, but the theory is sound - it simply sets those deprecated fields again.

If you could test it, that would be great.

[FernetMenta]

Replying to heleppkes:

IMHO the problem is here, in line 1043.
​https://github.com/xbmc/xbmc/blob/master/xbmc/cores/dvdplayer/DVDCodecs/Video/VDPAU.cpp#L1043

All the fields in m_hwContext are no longer set since the patch, so the XBMC code can't read them.
The original patch in itself makes total sense, since you want to store per-picture information with the picture.

I am however confused how this is implemented in XBMC, or why it was done this way.
How i read the current VDPAU function API, you would put such an implementation inside the "render" or "render2" callback in the "m_hwContext" (how the variable is called in XBMC)

So unless I'm missing something, XBMCs implementation is kinda...wrong.
What they should do is merge their "FFDrawSlice" into their "Render" function, and stop accessing m_hwContext.info/bitstream*.

I may be missing a bit of information on the why this was done, but this seems to be the current state from my point of view.

The issue with the Render function is that it does not provide private data like AVCodecContext.opaque. IMO it's useless to pass VdpDecoder, it may be invalid at the time Render is called (due to display lost).

[Hendrik]

There is a "render2" which includes a AVCodecContext, since someone apparently noticed the problem. It is however only available in FFmpeg 2.1, not 2.0

[FernetMenta]

This is great, so we will use render2 when upgrading to 2.1. I expect this short after release of XBMC 13.

[EricV]

Replying to heleppkes:

I lack the means to properly test right now, so no guarantees whatsoever for this patch, but the theory is sound - it simply sets those deprecated fields again.

If you could test it, that would be great.

Thanks for the patch. I tested but XBMC still fails. Either, FF_API_BUFS_VDPAU is not set given the compilation flags used by marillat when building debian packages but libavcodec/version.h has it or there might be other API breakage...

[EricV]

Just to be sure : I reverted your patch, applied mine and without even recompiling XBMC just ffmpeg, it works.

[Hendrik]

Turns out there was a second spot specific to H.264. Can you try again?

[EricV]

Bingo! I'm always amazed how internet and good willing/knowledgeable people can achieve.

Thanks for your help.

[Hendrik]

Great, i submitted the patch to the mailing list, and advised to also include it in the 2.1 branch.

[Carl Eugen Hoyos]

Fxied by Hendrik=-)

Please test the following snapshot just for safety:
​http://git.videolan.org/?p=ffmpeg.git;a=snapshot;h=release/2.1;sf=tgz

[Hendrik]

Anyone should keep in mind that these fields may go away in the future. They are deprecated now, so future developments should use "render2", and all the required data is passed to the render2 callback.

FernetMenta already expressed intention to use render2 after the next XBMC release, so that should cover it, i guess. :)

[EricV]

I don't mind you depreciate API as indeed they should evolve to better fit new hardware, better efficiency, new arch, ... But deprecating does not mean breaking it immediately.

Anyway thanks. Verified fixed!

[FernetMenta]

I am about to upgrade ffmpeg for XBMC's next version. The picture context allocates and destructs the bitstream buffers on every new frame. Mpeg2 can have > 200 slices, not very good to do that many reallocs.

[Carl Eugen Hoyos]

Replying to FernetMenta:

The picture context allocates and destructs the bitstream buffers on every new frame. Mpeg2 can have > 200 slices, not very good to do that many reallocs.

Any reason why you don't go back to the original VDPAU code?

[FernetMenta]

I thought the old api is depreciated. iirc it allocated too many buffers as well. A single set of buffers located in the context is enough.
btw: we face the same issue on dxva so there is a general issue with the idea of that picture context.

[Hendrik]

IMHO you really shouldn't worry about a few allocs a few hundred bytes each.

[EricV]

Replying to heleppkes:

IMHO you really shouldn't worry about a few allocs a few hundred bytes each.

Considering VDPAU is essential for HD viewing on low performance CPU, anything that may impact CPU consumption/latency/video decoding should be really taken into account IMHO. I have an old hardware design with an atom 330 and a ion1 chipset, and on this hardware there just enought CPU for doing vdpau bob deinterlace. Anything that slightly hits the cpu makes the system unsuitable for watching HD TV at 1080i as I start losing frame.

[Hendrik]

I think you are overrating the impact of a few tiny allocations.
Unless you can actually prove a negative impact on playback, the clean separation of frame and global data makes much sense.

[FernetMenta]

Replying to heleppkes:

IMHO you really shouldn't worry about a few allocs a few hundred bytes each.

we are talking about 200 re-allocations each frame. imo this does not make much sense.

why the separation? the bitstream buffers do belong to the decoder context and are only needed once.

[FernetMenta]

and is has negative impact. one of our ffmpeg patches has cured this a bit for dxva: ​https://github.com/FernetMenta/FFmpeg/commit/17e304d1f07093e1a592c8b04cc9cf562463fda0

[Hendrik]

upstream dxva allocates a fixed-size slice list and doesn't do it dynamically. the re-allocation was added in a XBMC patch, and you cure it in another XBMC patch ... good job? :D

[FernetMenta]

the fixed size is another issue of dxva upstream because it only allocates 170 buffers. the dynamic allocation followed the example of vaapi, then the author noticed this issue. vaapi still has the codec context. dxva and vdpau do have this issue.

[Hendrik]

Should've just increased the MAX_SLICES define in the first place. Memory is cheap, and allocating say 1024 slices may waste a few bytes, but only in one allocation.

In any case, the important fact is that it shall not be part of the public user-facing struct, neither for VDPAU or DXVA. If a decoder needs an internal global state for something, maybe it should get one, instead of polluting the user-struct and breaking ABI everytime something is changed? :)

[FernetMenta]

I am fine with bumping MAX_SLICES to 1024. then I can drop two patches from our fork and we get closer to a vanilla version which is the goal.

what about vdpau? can you pre-alloc a larger chunk of buffers as well?