Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#3133 closed defect (fixed)

Incompatibilities beween ffmpeg 2.0.2 and 2.1 exposed via XBMC

Reported by: EricV Owned by:
Priority: important Component: avcodec
Version: git-master Keywords: vdpau regression
Cc: ordroid Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

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?

Attachments (2)

revert_ffmpeg_git_2852740.patch (15.8 KB) - added by EricV 5 years ago.
Revert of patch that broke XBMC. Tested with XBMC git and ffplay on various H264 1080p material
0001-vdpau-restore-compatibility-with-deprecated-fields-i.patch (2.9 KB) - added by heleppkes 5 years ago.

Download all attachments as: .zip

Change History (95)

comment:1 Changed 6 years ago by cehoyos

  • Keywords regression added; ABI removed
  • Priority changed from normal to important

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

comment:2 Changed 6 years ago by 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?

comment:3 Changed 6 years ago by cehoyos

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.

comment:4 Changed 6 years ago by 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.

comment:5 Changed 6 years ago by 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).

Last edited 6 years ago by EricV (previous) (diff)

comment:6 Changed 6 years ago by cehoyos

Is the same problem reproducible with current FFmpeg git head?

comment:7 Changed 6 years ago by EricV

Try yourself!

comment:8 follow-ups: Changed 6 years ago by michael

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

comment:9 in reply to: ↑ 8 Changed 6 years ago by cehoyos

Replying to michael:

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

Did you test VDPAU decoding?

comment:10 follow-up: Changed 6 years ago by 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...

Last edited 6 years ago by EricV (previous) (diff)

comment:11 follow-up: Changed 6 years ago by 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.

Last edited 6 years ago by EricV (previous) (diff)

comment:12 in reply to: ↑ 11 Changed 6 years ago by cehoyos

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.)

comment:13 Changed 6 years ago by 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).

comment:14 in reply to: ↑ 8 Changed 6 years ago by michael

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

comment:15 in reply to: ↑ 10 ; follow-up: Changed 6 years ago by 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

comment:16 in reply to: ↑ 15 Changed 6 years ago by 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.

Last edited 6 years ago by EricV (previous) (diff)

comment:17 Changed 6 years ago by 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


comment:18 Changed 6 years ago by EricV

Tested with 2.1.1. Same problem.

comment:19 Changed 6 years ago by cehoyos

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.

comment:20 Changed 6 years ago by EricV

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

comment:21 Changed 6 years ago by cehoyos

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?

comment:22 follow-up: Changed 6 years ago by 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.

comment:23 in reply to: ↑ 22 Changed 6 years ago by ubitux

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.

[...]

comment:24 Changed 5 years ago by 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.

Last edited 5 years ago by ordroid (previous) (diff)

comment:25 Changed 5 years ago by cehoyos

  • Cc ordroid added
  • Version changed from 2.1 to git-master

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

comment:26 Changed 5 years ago by cehoyos

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.

comment:27 Changed 5 years ago by EricV

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

comment:28 follow-up: Changed 5 years ago by 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

comment:29 in reply to: ↑ 28 Changed 5 years ago by cehoyos

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)?

comment:30 Changed 5 years ago by 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).

comment:31 follow-up: Changed 5 years ago by 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
Last edited 5 years ago by ordroid (previous) (diff)

comment:32 in reply to: ↑ 31 ; follow-ups: Changed 5 years ago by cehoyos

  • Keywords vdpau added

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

Last edited 5 years ago by cehoyos (previous) (diff)

comment:33 in reply to: ↑ 32 Changed 5 years ago by cehoyos

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!

Last edited 5 years ago by cehoyos (previous) (diff)

comment:34 in reply to: ↑ 32 ; follow-up: Changed 5 years ago by 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 :)

comment:35 in reply to: ↑ 34 ; follow-up: Changed 5 years ago by 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.

comment:36 in reply to: ↑ 35 ; follow-up: Changed 5 years ago by 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 :)

comment:37 in reply to: ↑ 36 Changed 5 years ago by cehoyos

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.

comment:38 follow-up: Changed 5 years ago by ordroid

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

XBMC tip compiles and runs fine with bf36dc50.

comment:39 in reply to: ↑ 38 Changed 5 years ago by cehoyos

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.

comment:40 follow-up: Changed 5 years ago by 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.

comment:41 in reply to: ↑ 40 ; follow-up: Changed 5 years ago by 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.

Last edited 5 years ago by Timothy_Gu (previous) (diff)

comment:42 in reply to: ↑ 41 ; follow-ups: Changed 5 years ago by 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.

comment:43 in reply to: ↑ 42 Changed 5 years ago by cehoyos

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.

comment:44 in reply to: ↑ 42 ; follow-up: Changed 5 years ago by 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.

comment:45 in reply to: ↑ 44 Changed 5 years ago by 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?

comment:46 Changed 5 years ago by 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

comment:47 Changed 5 years ago by cehoyos

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...)

comment:48 Changed 5 years ago by cehoyos

Just test c3b29023 - the other commits are mostly cosmetic.

comment:49 follow-up: Changed 5 years ago by ordroid

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

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

comment:50 Changed 5 years ago by cehoyos

  • Component changed from undetermined to avcodec
  • Status changed from new to open

comment:51 in reply to: ↑ 49 Changed 5 years ago by cehoyos

Replying to ordroid:

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

A significantly less buggy version is for example 50fb8c11

comment:52 Changed 5 years ago by 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.

comment:53 Changed 5 years ago by cehoyos

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.

comment:54 Changed 5 years ago by 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.

Last edited 5 years ago by EricV (previous) (diff)

comment:55 follow-up: Changed 5 years ago by cehoyos

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

comment:56 Changed 5 years ago by 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.

Last edited 5 years ago by EricV (previous) (diff)

comment:57 follow-up: Changed 5 years ago by 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

Last edited 5 years ago by EricV (previous) (diff)

comment:58 in reply to: ↑ 57 Changed 5 years ago by cehoyos

Replying to EricV:

I need to use the internal version.

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

comment:59 Changed 5 years ago by 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

Last edited 5 years ago by EricV (previous) (diff)

comment:60 in reply to: ↑ 55 Changed 5 years ago by 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.

comment:61 Changed 5 years ago by 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.

Last edited 5 years ago by EricV (previous) (diff)

comment:62 Changed 5 years ago by cehoyos

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.

comment:63 Changed 5 years ago by 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.

comment:64 Changed 5 years ago by 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

Last edited 5 years ago by EricV (previous) (diff)

comment:65 Changed 5 years ago by heleppkes

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.

Last edited 5 years ago by heleppkes (previous) (diff)

Changed 5 years ago by EricV

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

comment:67 Changed 5 years ago by 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.

comment:68 follow-up: Changed 5 years ago by 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.

comment:69 Changed 5 years ago by 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.

Last edited 5 years ago by EricV (previous) (diff)

comment:70 follow-up: Changed 5 years ago by 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.

comment:71 in reply to: ↑ 68 Changed 5 years ago by 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).

comment:72 Changed 5 years ago by heleppkes

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

comment:73 Changed 5 years ago by FernetMenta

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

comment:74 in reply to: ↑ 70 Changed 5 years ago by 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...

Last edited 5 years ago by EricV (previous) (diff)

comment:75 Changed 5 years ago by EricV

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

comment:76 Changed 5 years ago by heleppkes

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

comment:77 Changed 5 years ago by EricV

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

Thanks for your help.

comment:78 Changed 5 years ago by heleppkes

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

comment:79 Changed 5 years ago by cehoyos

  • Resolution set to fixed
  • Status changed from open to closed

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

comment:80 Changed 5 years ago by heleppkes

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. :)

comment:81 Changed 5 years ago by 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!

Last edited 5 years ago by EricV (previous) (diff)

comment:82 follow-up: Changed 5 years ago by 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.

comment:83 in reply to: ↑ 82 Changed 5 years ago by cehoyos

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?

comment:84 Changed 5 years ago by 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.

comment:85 follow-ups: Changed 5 years ago by heleppkes

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

comment:86 in reply to: ↑ 85 Changed 5 years ago by 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.

comment:87 Changed 5 years ago by heleppkes

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.

Last edited 5 years ago by heleppkes (previous) (diff)

comment:88 in reply to: ↑ 85 Changed 5 years ago by 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.

comment:89 Changed 5 years ago by 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

comment:90 Changed 5 years ago by heleppkes

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

comment:91 Changed 5 years ago by 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.

comment:92 Changed 5 years ago by heleppkes

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? :)

comment:93 Changed 5 years ago by 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?

Note: See TracTickets for help on using tickets.