[FFmpeg] version bump to n1.2 (rev e820e3a) #2554

Merged
merged 4 commits into from Apr 11, 2013

Conversation

Projects
None yet

FFmpeg version bump to n1.2 (rev e820e3a).

This solves a bunch of more or less serious problems that we currently struggle with in version 0.10.2, for instance preventing users to watch certain content types, hangs or causes stuttering video playback. Also, n1.2 is the last ffmpeg version that supports the old api. Essentially, this upgrade contains the following files and directories:

  • dir: lib/ffmpeg
    FFmpeg n1.2 fork (rev e820e3a) including all necessary xbmc add on patches.
  • dir: lib/ffmpeg/patches
    This directory contains all the XBMC add-on patches applied to ffmpeg n1.2.
  • file: lib/ffmpeg/patches/README-patches
    Brief description of the xbmc add on patches.
  • file: lib/ffmpeg/ffmepg/patches/obsolete-patches
    Contains obsolete xbmc custom patches that were NOT applied to n1.2.
  • file: lib/ffmpeg/build_xbmc_win32.sh
    Win32 build script that generates the include file DllPaths_generated_win32_ffmpeg.h and is called from buildmingwlibs.sh.
  • file: lib/DllAvFormat.h
    Deals with the decrepated version av_read_frame_flush -> ff_read_frame_flush
  • file: lib/Makefile.in
    When ffmpeg is configured for osx then remove duplicate log2_tab.o from libavcodec.a, libavformat.a and libswresample.a
  • file: xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecFFmpeg.cpp & xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecFFmpeg.h
    Resolves dllAvFilter dependencies of LibPostProc that previously required LD_LIBRARY_PATH
  • General adjustments to n1.2 (files)
    file: xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecLibMpeg2.cpp
    file: xbmc/cores/dvdplayer/DVDCodecs/Video/DXVA.cpp
    file: xbmc/cores/dvdplayer/DVDCodecs/DVDCodecs.h
    file: xbmc/cores/dvdplayer/DVDAudio.h
    file: xbmc/cores/dvdplayer/DVDDemux.h
    file: xbmc/cores/dvdplayer/DVDPlayerAudio.h
    file: xbmc/cores/dvdplayer/DVDStreamInfo.h
  • file: tools/depends/native/gas-preprocessor-native/gas-preprocessor.pl
    Updated version that manages libavcodec/arm assembler optimizations. Source: github.com/mansr/gas-preprocessor, rev 76a72f00e (Dec 03, 2012).
  • file: project/Win32BuildSetup/BuildSetup.bat
    Win32 build script adjusted to ffmpeg n1.2
  • file: project/Win32BuildSetup/buildmingwlibs.sh
    Win32 build script that generates the include file DllPaths_generated_win32_ffmpeg.h.
  • file: xbmc/DllPaths_win32.h
    DllPaths_win32.h is now modified to include the auto-genrerated file "DllPaths_generated_win32_ffmpeg.h".
Member

wsoltys commented Apr 6, 2013

could you please separate the changes to lib/ffmpeg and the rest to different commits? would make it easier to review the changes to the XBMC core.

Done, commit splitted in two parts.

FlyingRat closed this Apr 6, 2013

FlyingRat reopened this Apr 6, 2013

pushed the close button by accident...

Member

elupus commented Apr 7, 2013

The patches directory would be nice if it was named the same as before. Also i really want the ffmpeg part of the update a separate commit from the addition of the patches.

FlyingRat added some commits Apr 7, 2013

FlyingRat [FFmpeg] version bump to n1.2 (rev e820e3a) - lib/ffmpeg
This commit now contains the original patches sub directory:
  patches 			- Org dir that contains applied xbmc custom patches.
  patches/README-patches	- New README file with info about xbmc patches.
  patches/obsolete-patches	- New dir with obsolete xbmc patches.
0e63a81
FlyingRat [FFmpeg] version bump to n1.2 (rev e820e3a) - xbmc files
lib/DllAvFormat.h
lib/Makefile.in
lib/ffmpeg/build_xbmc_win32.sh
project/Win32BuildSetup/BuildSetup.bat
project/Win32BuildSetup/buildmingwlibs.sh
tools/depends/native/gas-preprocessor-native/gas-preprocessor.pl
xbmc/DllPaths_win32.h
xbmc/cores/dvdplayer/DVDAudio.h
xbmc/cores/dvdplayer/DVDCodecs/DVDCodecs.h
xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecFFmpeg.cpp
xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecFFmpeg.h
xbmc/cores/dvdplayer/DVDCodecs/Video/DVDVideoCodecLibMpeg2.cpp
xbmc/cores/dvdplayer/DVDCodecs/Video/DXVA.cpp
xbmc/cores/dvdplayer/DVDDemuxers/DVDDemux.h
xbmc/cores/dvdplayer/DVDPlayerAudio.h
xbmc/cores/dvdplayer/DVDStreamInfo.h
6f653c5
  • bookmarks and TODO:s cleaned and wiped!
  • renamed: XBMC-patches -> patches
  • updated: patches/README-patches
  • renamed: patches/OBSOLTE -> patches/obsolete-patches
  • Commit: "[FFmpeg] version bump to n1.2 (rev e820e3a) - lib/ffmpeg"
    lib/ffmpeg n1.2 including xbmc patches.
  • Commit: "[FFmpeg] version bump to n1.2 (rev e820e3a) - xbmc files"
    Designated xbmc files...

Why did you remove that? Its really difficult to review that due to all the unnecessary changes you did. Afaik all what was needed was to build the mingw libs before the xbmc main project.

so regardless what file we find this check will return true? whats if the above founds no file and checkfiles will get an empty var? will it return true?

Member

wsoltys commented Apr 7, 2013

nice work on the ffmpeg bump. I'm a little unsure about all the batch file changes. most will get lost when switching to jenkins since I already rewrite them but probably I can get some nicer ways to handle it from your scripts.
I have mixed feeling about the automatic generation of the header includes. For the two times in a year (if ever) we bump ffmpeg I'm able to change the files by hand and all the error prone changes wouldn't be needed.

Thanks! The intention was to make the build process work smoother but you are of cource welcome to disregard or/and restructure the any of the win32 build script whichever way you see fit best for the new Jenkins build environment.

If you change this you also need to update lib/xbmc-dll-symbols/DllAvFormat.c
and likely the other references to av_read_frame_flush in that file.

This could probably be a separate commit also

Owner

FlyingRat replied Apr 8, 2013

The suggested changes are not needed (yet) since patch 0024 is still in effect.

This is rather a preparation for a future wipe out of the decrepated av_read_frame_flush but was put temporary on hold since we need to figure out a better way to deal with DllAvFormat.c where av_read_frame_flush is redefined as a local source code copy for shared library use. I would rather see a different solution than copy and paste source from a library that has as many updates as ffmpeg but that's a complete different topic.

Currently, we only have one referens to av_read_frame_flush in:

cores/dvdplayer/DVDDemuxers/DVDDemuxFFmpeg.cpp:
m_dllAvFormat.av_read_frame_flush(m_pFormatContext);

ok but does it work with external ffmpeg after that change?
also, I prefer a local copy of this function over using the ff_ version that might change its abi under our feet

Owner

FlyingRat replied Apr 8, 2013

Well, it "should" work but hasn't been tested yet since I'm not sure about the "xbmc policy" regarding external ffmpeg libraries and what level of compatibility that's required. I'm curious, why (and how) are external ffmpeg libraries needed and is there a separate maintainer for this kind of work?

Regarding a local copy of av_read_frame_flush, it won't be of any further use if for instance AVFormatContext was to be refactored thus you will probably need perform plenty of work to restore the functionality anyways (but it's probably a matter of taste... :)

Btw, as this is not a discussion forum let's move this debate to: http://forum.xbmc.org/showthread.php?tid=156303&action=lastpost . /Thanks in advance, Lars.

Owner

FlyingRat replied Apr 8, 2013

@aballier wrote:

ok but does it work with external ffmpeg after that change?

Hope this will answer your question (get back if not): "Some general questions regarding external ffmpeg libraries"

Not really: If I understood correctly, the change is not needed yet. Moreover, it is only made in the external ffmpeg code path but not in the internal ffmpeg one, hence has almost nothing to do with the current PR.

I'm all for killing av_read_frame_flush usage within xbmc, but so far the current workaround by maintaining a local copy is the best and safest way: as I already wrote, ff_* functions are private within ffmpeg and might change their ABI without warning, meaning weird and hard to debug xbmc crashes if that happens; the above change exposes xbmc to this situation.

Owner

FlyingRat replied Apr 9, 2013

Agreed, let's keep the current solution for now.

Owner

FlyingRat replied Apr 8, 2013

Solves the problem that forced the use of LD_LIBRARY_PATH to load postproc-52 during video playback. This fix affects all linux based platforms that utilize dynamic linking with shared libraries.

For details see: http://forum.xbmc.org/showthread.php?tid=156303&pid=1377375#pid1377375

nice; it could be a separate bugfix commit though

Shouldn't this rather be just adding DllPostProc in class DllAvFilter in DllAvFilter.h, like is done for AvUtil & co there, since this is actually not used by DVDVideoCodecFFmpeg but is just an internal dependency of DllAvFilter?

Owner

FlyingRat replied Apr 9, 2013

No problem, please provide a fix (patch/diff) and I'll bring it in right away.

Just remove it I'd say, and this could be a separate commit :)
See: fd3506a

Owner

FlyingRat replied Apr 8, 2013

Thanks, I'll remove that comment in a upcoming commit where I gather all potential cleaning activities into a single commit.

Contributor

aballier commented Apr 8, 2013

Very nice work. However, I fear ffmpeg 1.2 (as opposed to eg 1.0.6) could break some things by introducing planar audio formats.
I'm not sure that these files will still work:
xbmc/cores/AudioEngine/Encoders/AEEncoderFFmpeg.cpp
xbmc/cdrip/EncoderFFmpeg.cpp

for example, ffmpeg aac encoder now seems to expect planar float which xbmc has no clue about

@aballier, please provide some media samples we can verify with that also can work as a reference if we need to alter stuff. /Thanks in advance, Lars.

popcornmix referenced this pull request in huceke/omxplayer Apr 8, 2013

Closed

MKV video plays at double speed (FFmpeg-1.2) #150

Contributor

aballier commented Apr 8, 2013

well, those are encoders so basically any sample would work ;)
its just a matter of triggering that code path, but note that i'm not saying that based on my experiences but rather based on my reading of the code so it's possible I'm wrong; I'm just saying to be careful about these two encoders :)

@popcornmix, referenced this pull request in huceke/omxplayer: (github.com/huceke/omxplayer/issues/150)

"Issue #150 MKV video plays at double speed (FFmpeg-1.2)"

I'm sorry, but what has OMXplayer and "Built a recently checked out omxplayer against external ffmpeg-1.2" anything to do with this pull request??

Btw, as this is not a discussion forum please move further discussion to: http://forum.xbmc.org/showthread.php?tid=156303&action=lastpost . /Thanks in advance, Lars.

@aballier commented:

well, those are encoders so basically any sample would work ;) its just a matter of triggering that code path,
but note that i'm not saying that based on my experiences but rather based on my reading of the code so
it's possible I'm wrong; I'm just saying to be careful about these two encoders :)

Ok, thanks. Since there are no actual problems reported I'll consider this particular case closed for the moment. You are more then welcome to get back if you find anything specific issues that should be solved but in the mean time let's move more general discussions to: http://forum.xbmc.org/showthread.php?tid=156303&action=lastpost /Thanks, Lars.

Member

wsoltys commented Apr 8, 2013

@flyingrat: please bring back the copy of the pdb file as we need it that jenkins can fetch that too. Then I'm fine with it.

@ghost

ghost commented Apr 8, 2013

@flyingrat, "apologize" to popcornmix, all he did was open an issue on his repo. github is responsible for the noise..

cptspiff commented

@flyingrat, "apologize" to popcornmix, all he did was open an issue on his repo. github is responsible for the >noise..

Well, sorry about that but I had no idea about this "feature". I'll know github is good for many things but... anyhow, now I know.

@popcornmix good 👍 github bad (well, surprises..) 👎

Member

FernetMenta commented Apr 8, 2013

@flyingrat How much testing does this require on Linux with vdpau and vaapi?

@sraue You already run external ffmpeg 1.2 with OE, right? Are there any tweaks you have done which should be considered here?

@fernetmenta commented:

@flyingrat How much testing does this require on Linux with vdpau and vaapi?

I've done quite some testing on my own but only with vmware fusion. There are some other folks on the following thread that I'm quite sure have done some more extensive testing and can probably answer any of your questions:
"[WIP] FFmpeg v1.1.x + XBMC add-on patches."

@sraue You already run external ffmpeg 1.2 with OE, right? Are there any tweaks you have done which
should be considered here?

Check this post: http://forum.xbmc.org/showthread.php?tid=156303&pid=1391318#pid1391318

@wsoltys commented:

@flyingrat: please bring back the copy of the pdb file as we need it that jenkins can fetch that too. Then I'm
fine with it.

Ok, will fix. Want a rebase or will a separate commit do?

Member

sraue commented Apr 8, 2013

@fernetmenta no, we build with external 0.10.6 with all XBMC patches applied which is similar to the included in xbmc-frodo/master (0.10.2 or so)

Member

FernetMenta commented Apr 8, 2013

I did a couple of tests on Linux with vdpau. Looks good so far.
Transcoding True HD to AC3 seemed to work as well.

This thread discusses the ffmpeg testing across the various platforms supported by OpenELEC

http://www.openelec.tv/forum/130-video-decoders/62608-xbmc-ffmpeg-1-1-2

What has changed? Is the constant unused or does it come from somewhere else now?

The reason doesn't seem to be mentioned in the commit message (plus I would've probably done it as a separate commit).

Owner

FlyingRat replied Apr 9, 2013

anssih wrote:

What has changed? Is the constant unused or does it come from somewhere else now?

It's in the docs and is also documented in avcodec.h, hint: https://lists.ffmpeg.org/pipermail/ffmpeg-cvslog/2011-December/044803.html

The reason doesn't seem to be mentioned in the commit message (plus I would've probably done it as a separate commit).

Well, like most of the other changes aren't as you probably have noticed. I'm spending a lot of my spare time (mostly late evenings) doing this kind of unpaid work which means I've to focus on the functionality rather than documenting the PR. Regarding a separate commit, yes of course but why? It was done the regular way i.e all changes were rebased into a single commit (but in this case it was split into two parts on request).

I don't see any mention of CODEC_ID_MPEG1VIDEO in the provided link, probably a wrong link?

Contributor

Alphix commented Apr 9, 2013

@flyingrat The mkv embedded subtitle issue you've hit upon is something I've already fixed in my pull request (#2525)

@Alphix, I have applied your fix and subtitles are working again! Thanks

@Alphix commented

The mkv embedded subtitle issue you've hit upon is something I've already fixed in my pull request (#2525)

Excellent work dude! 👍 👍 👍 Saved me a lot of headache, thank you!

@cptspiff: is there a way to create a dependency to another PR (like this one: #2525) or how is this supposed to work?

Contributor

davilla commented Apr 11, 2013

the window is open to commit this to mainline, @elupus and all, green button time ?

Member

Voyager1 commented Apr 11, 2013

I'm in favor, the sooner the better re. testing exposure.

FlyingRat added some commits Apr 11, 2013

FlyingRat [WIN32] fixed BuildSetup.bat: copy the pdb file to the build setup di…
…rectory.

Since Jenkins is dependent of the "pdb" file, it is copied to the setup directory with the new name "XBMCSetup-%GIT_REV%-%target%.pdb"
a6cb8db
FlyingRat Removed "dead" (commented out) line (// pic->age = 256*256*256*64; //…
… decrepated)
e85eb14

@wsoltys commented:

@flyingrat: please bring back the copy of the pdb file as we need it that jenkins can fetch that too. Then I'm
fine with it.

It's in as https://github.com/FlyingRat/xbmc-ffmpeg-version-bump/commit/a6cb8db11bd2f7657dd7e537014e5c622d7534cf

Tested some binaries yesterday that included Pull Request #2525 and they all appear to work as expected:

http://forum.xbmc.org/showthread.php?tid=156303&pid=1393277#pid1393277
http://forum.xbmc.org/showthread.php?tid=156303&pid=1393505#pid1393505

Member

wsoltys commented Apr 11, 2013

Pull it in.

Owner

MartijnKaijser commented Apr 11, 2013

@wsoltys
You can adjust the Jenkins builder after merging because it fails now?

Member

wsoltys commented Apr 11, 2013

Hope so ;)

Am 11.04.2013 um 21:00 schrieb Martijn Kaijser notifications@github.com:

@wsoltys
You can adjust the Jenkins builder after merging because it fails now?


Reply to this email directly or view it on GitHub.

Contributor

davilla commented Apr 11, 2013

ping @elupus :)

Member

wsoltys commented Apr 11, 2013

just a short update after trying to build this revision.

  1. the buildsetup script is still missing some things which are currently in like setting the shell -> I can fix that later
  2. the buildsetup script won't work and throws a syntax error on Martijn's and my setup -> I was able to fix that somehow
  3. the ffmpeg libs won't build at all. the make breaks with
common.mak:138: *** missing separator.  Stop.

This message comes already before configure starts but configure runs fine so the error might be somewhere else but I couldn't figure it out yet.

So I wouldn't stop the merge as we don't have a working buildbot anyways. But please help to figure out where the build problem with ffmpeg is.

Member

elupus commented Apr 11, 2013

Have 0028-fixed-dvd-still-frames-ended-up-in-internal-lavf.patch been verified to not a problem anymore?

Member

elupus commented Apr 11, 2013

We can merge it anyway. That is of no major concern since we are likely to break dvd menu's anyway in some way. Don't think we've managed a ffmpeg update yet without doing it. (we really should use another demuxer there :) )

davilla merged commit 9ad1743 into xbmc:master Apr 11, 2013

@wsoltys

  1. the ffmpeg libs won't build at all. the make breaks with

common.mak:138: *** missing separator. Stop.
This message comes already before configure starts but configure runs fine so the error might be somewhere else > but I couldn't figure it out yet.

That error may indicate there are spaces instead of a leading tab or crlf problems on the "command" part in a make section. I don't get that error in my current build env that was a made out of clean clone from today so it's hard to tell the reason behind.

common.mak:138: *** missing separator. Stop.

I got the same error some time ago and found this error report: https://ffmpeg.org/trac/ffmpeg/ticket/1209. I think this made it to work again: git config --global core.autocrlf false

Owner

MartijnKaijser commented Apr 11, 2013

Git docs tell to set it true on windows if working together with other platforrms http://git-scm.com/book/ch7-1.html

Git docs tell to set it true on windows if working together with other platforrms http://git-scm.com/book/ch7-1.html

Yeah i know, but it actually helped in the ffmpeg case (I dont remember the root cause thou). One could argue about the scope regarding the "--global" flag...

@elupus commented

Have 0028-fixed-dvd-still-frames-ended-up-in-internal-lavf.patch been verified to not a problem anymore?

Well, you might be lucky this time since 0028 is nowadays included upstreams: https://github.com/FFmpeg/FFmpeg/blob/n1.2/libavformat/utils.c#L341

Member

wsoltys commented Apr 12, 2013

As Martijn said we can't set autoctrl to false. This is a no go in our multi platform env so we have to find another solution. It worked before so where is the difference?
How did the build setup error slipped thru? As said this happens on Martijn's and my setup.

Am 11.04.2013 um 23:29 schrieb FlyingRat notifications@github.com:

common.mak:138: *** missing separator. Stop.

I got the same error some time ago and found this error report: https://ffmpeg.org/trac/ffmpeg/ticket/1209. I think this made it to work again: git config --global core.autocrlf false


Reply to this email directly or view it on GitHub.

I agree, it's very odd but I'll dig into it...

Contributor

BtbN commented Apr 12, 2013

This totaly breaks vaapi(xbmc just crashes), because of a bug in ffmpeg-1.2.
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=b066d90211072c7532e17c0c54d8475f10fc97ad
This is the fix for it.

Contributor

aballier commented Apr 12, 2013

well, this has been merged as such when there were pending improvements on the PR, what should be done now?

Owner

MartijnKaijser commented Apr 12, 2013

if there needs to be fixes/improvements on current master they should just be done through PR

Contributor

BtbN commented Apr 12, 2013

It's not only that. ffmpeg-1.2 and even latest master has massive problems with vaapi. I can say for sure that decoding mpeg2 just crashes. VLC has the same problem, so it's a problem in libavcodec.
ffmpeg-1.2 release version is without the patch i posted completely unusable for vaapi. It just crashes on the first frame.

Owner

MartijnKaijser commented Apr 12, 2013

@BtbN
Let me repeat myself again. If something needs fixing do a PR.

@aballier commented
some hunks such as:
FlyingRat@6f653c5#lib-dllavformat-h-P6
just need to be reverted

What has changed in regards to the previous discussion on this topic? Trifle or not, please motivate and explain why. /Thanks in advance, Lars.

My findings regarding the common.mak issue:

  • Brief summary:
    Currently Mingw-Make does only work when ffmpeg is checked out with core.autocrlf=false under win32 (as most already may have guessed!) It's quite odd that mingw-make can't cope with crlf endings since I guess this was not an issue with the old 0.10.2 ffmpeg.

    Are we missing a "miniwg" settings file or something like that?

  • Background:
    The build env that was used is based on a ffmeg repo cloned from the ffmpeg n1.2 fork. Since it was a git repo it was also possible to utilizes local config settings like core.autocrlf=false. As soon as the PR made it into the xbmc master repo, ffmpeg just became a regular directory which makes it impossible to use local git config settings.

  • Temporary workaround:
    A very temporary solution I've tested is to re-checkout ffmpeg with core.autocrlf=false but that is obviously a rather ugly solution!

  • Possible solutions?
    Does it exist any platform settings (local or global) in the minigw environment that enables Mingw-Make to accept crlf instead of just lf? Can someone with deeper skills in this area please explain if anything like that is possible?

    Other ideas?

I'll continue to dig deeper into this issue and see if i may find anything in the old 0.10.2 environment. Any type of feedback or ideas is of course welcome...

Contributor

aballier commented Apr 12, 2013

@flyingrat commented
What has changed in regards to the previous discussion on this topic? Trifle or not, please motivate and explain why. /Thanks in advance, Lars.

I should rather ask you this question. You agreed that this hunk shouldn't be merged, but it has been merged and you deleted the branch and all the comments with it...

FYI, again: the dllavformat changes merged with this PR makes xbmc use an internal ffmpeg symbol that may change its ABI (so that xbmc will crash) or even be removed (so that xbmc wont work) for absolutely no reason nor gain. Everything was safe before.

Regarding buildsetup.bat

I had the same oddity with buildsetup.bat that behaved very strangely in the same way as reported in the forums but I couldn't locate any defects when i debugged it. When a renamed it and checked out the file again, everything worked as normal(!) That made me suspect that buildsetup.bat somehow was checked in with the wrong line endings.

I made a successful build of master (rev c01c97d) on win32 by first re-check out buildsetup.bat and then re-check lib/ffmpeg with git core.autocrlf=false (only used for ffmpeg)

Uploaded: XBMCSetup-20130412-c01c97d-dx.exe to https://www.dropbox.com/sh/a7tsbe6o80rrv2a/S2NszQXp7H

@aballier: it might be a misunderstanding but pls let me get back to you when we have solved win32 problems. /Thank you, Lars.

Worth a try: #2604

Member

FernetMenta commented Apr 12, 2013

here is a quick fix for AE transcoding: FernetMenta/xbmc@ca16827

I leave it to @fritsch to make this nice :)

Member

fritsch commented Apr 12, 2013

@fernetmenta:
We don't have that many options, as AE internally works completely on AE_FMT_FLOAT as a basis for encoding. Changing the internal format would be the other option :-)

So we have to die one cpy death. PR it if you please.

Member

wsoltys commented Apr 13, 2013

@flyingrat: short update. ffmpeg compiles with gnu make 3.82 (already committed to master). unfortunately libdvdnav and libdvdread won't compile anymore.
Whereas gnu make 3.81 seems to work fine with VPATH gnu make 3.82 just won't find the source files anymore. if I adapt the src paths in the Makefile directly libdvdread will compile but libdvdnav won't due to a missing rule.
Does any Makefile expert has an idea if this is a bug in gnu make or if there's something wrong with the libdvd makefiles?

2013/4/13 wsoltys notifications@github.com

@flyingrat https://github.com/FlyingRat: short update. ffmpeg compiles
with gnu make 3.82 (already committed to master). unfortunately libdvdnav
and libdvdread won't compile anymore.
Whereas gnu make 3.81 seems to work fine with VPATH gnu make 3.82 just
won't find the source files anymore. if I adapt the src paths in the
Makefile directly libdvdread will compile but libdvdnav won't due to a
missing rule.
Does any Makefile expert has an idea if this is a bug in gnu make or if
there's something wrong with the libdvd makefiles?

Morning! I've probably built 12-14 win32 envs the last couple of days and
everyone has worked with the "built in" msys-make that is downloaded to
BuildDependencies i.e:

k:\src\xbmc\project\BuildDependencies\msys\mingw\bin\make.exe -v
GNU Make 3.82
Built for i386-pc-mingw32
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html

This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Btw, how is v3.81 involved in this case, i mean do you have your own local
msys/mingw installed somewhere that may conflict with the other? This is
just speculations, but you don't think this happens to be another
core.autocrlf symptom as
well?

Member

wsoltys commented Apr 13, 2013

v3.81 was our current make until today when I update make to 3.82. and again I try to build ffmpeg with autoctrl=true as like I already explained we can't use false.

Sorry, forgot to ask if you read my other mail. You'll hopefully reconsider
your opinion regarding autoctrl because I'm quite convinced that the
correct (and default) repository setting is after all
core.autoctrl=false. I rest my case (or at least until proven wrong) :D

ce8efc8#commitcomment-3005705

2013/4/13 wsoltys notifications@github.com

v3.81 was our current make until today when I update make to 3.82. and
again I try to build ffmpeg with autoctrl=true as like I already explained
we can't use false.


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/2554#issuecomment-16332782
.

Member

Voyager1 commented Apr 13, 2013

@elupus @flyingrat
Patch 0028 is still needed. With great thanks to @wsoltys I could finally get ffmpeg compiled on Win32, and immediately tested the dvds with still menus + audio. Same issue as before that patch.
Line 709 of libavformat/utils.c needs to be patched as follows. They've also wrapped AVPROBE_SCORE_MAX/4 as a new define...

C:\DATA\git\xbmc\lib\ffmpeg\libavformat\avformat.h (1 hit)
    Line 345: #define AVPROBE_SCORE_RETRY (AVPROBE_SCORE_MAX/4)
C:\DATA\git\xbmc\lib\ffmpeg\libavformat\utils.c (4 hits)
    Line 709:             if(    (st->codec->codec_id != AV_CODEC_ID_NONE && score > AVPROBE_SCORE_RETRY-1)
Member

wsoltys commented Apr 13, 2013

@flyingrat : the autoctrl thing was a team decision that helps us working in a mixed environment (win/Linux). Switching would lead to huge problems which was the trigger to switch to autoctrl=true on windows systems.

Owner

MartijnKaijser commented Apr 13, 2013

@flyingrat
core.autoctrl=false is a no go end of story. Period!

@Voyager1
What did you else change to get it compiled?

Member

Voyager1 commented Apr 13, 2013

@MartijnKaijser - I just redownloaded & updated the mingw environment, using DownloadMingwBuildEnv.bat. After wsoltys' update it now includes gmake 3.82. This one works ok with the windows line endings. The patch 0028 story is unrelated to getting it compiled (it is related to DVD with menus)

Ok, i hear and understand what you say. But here are the facts:
core.autoctrl=false is the default repository setting and works without any
problems. If you don't believe me, please read .git/config and tell me what
you see.

Bottom line: regardless if it was the team decision to alter this flag for
Jenkins you need to find a another solution to deal with this problem.
Please take my advice regarding Jenkins, don't start altering code and
other configs to satisfy things but rather fix the Jenkins setup instead
with for example plugins, etc. But do whatever you like, i'm just trying to
make a point :)

2013/4/13 wsoltys notifications@github.com

@flyingrat https://github.com/FlyingRat : the autoctrl thing was a team
decision that helps us working in a mixed environment (win/Linux).
Switching would lead to huge problems which was the trigger to switch to
autoctrl=true on windows systems.


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/2554#issuecomment-16333254
.

Owner

MartijnKaijser commented Apr 13, 2013

this has nothing to do with Jenkings!
your point is not valid and I don't believe you. core.autoctrl=false won't happen.

Member

Voyager1 commented Apr 13, 2013

@flyingrat - I may be wrong but you seem to have built the ffmpeg libs 10-12x with gmake 3.82 and it worked fine. It works fine with 3.82 on my set up too, with windows CRLF line endings! so I think you're chasing the wrong problem here...

Hey Martijn, did you actually read my mail reading the git config?? Well, 3.82
might be a lifesaver for win32 after all... but here are the facts (again):

$ type .git\config
[core]
repositoryformatversion = 0
filemode = false
bare = false
logallrefupdates = true
symlinks = false
ignorecase = true
hideDotFiles = dotGitOnly
autocrlf = false
[remote "origin"]
url = git://github.com/xbmc/xbmc.git
fetch = +refs/heads/:refs/remotes/origin/
[branch "master"]
remote = origin
merge = refs/heads/master
[branch "Frodo"]
remote = origin
merge = refs/heads/Frodo
[remote "upstream"]
url = git://github.com/xbmc/xbmc.git

fetch = +refs/heads/:refs/remotes/upstream/

Please stop blaming ffmpeg or anything else since this obviously a pure
CRLF problem that you brought in yourselves when
you started to override the default setting with core.autocrl=true.

2013/4/13 Martijn Kaijser notifications@github.com

@flyingrat https://github.com/FlyingRat
core.autoctrl=false is a no go end of story. Period!

@Voyager1 https://github.com/Voyager1
What did you else change to get it compiled?


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/2554#issuecomment-16333262
.

Owner

MartijnKaijser commented Apr 13, 2013

no i am ignoring that false statement

Edit:
and since you obviously did not pay attention due to chasing some ghost we already have ffmpeg build but are now stuck at libdvdread

Well, ignore or override, the end result just became the same. OO for now...

2013/4/13 Martijn Kaijser notifications@github.com

no i am ignoring that false statement


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/2554#issuecomment-16333918
.

and since you obviously did not pay attention due to chasing some ghost we already have ffmpeg build
but are now stuck at libdvdread

Since i'm not using a build server like Jenkins (where core.autocrlf=true) i don't have this problem. Like i said, i'm using the standard git setting that comes with the xbmc repo and it works perfectly well because the effective default setting of core.autocrlff is false which compiles everything without any problems (with default gmake 3.81).

So I'm sorry, but you are on your own on this one.

OO & AWFK - definitely this time since my wife is shouting now! :)

Owner

Montellese commented Apr 13, 2013

@flyingrat: You should read the "Git Usage" article on our wiki: http://wiki.xbmc.org/index.php?title=Git_Usage
It clearly states that you have to set core.autocrlf to true. It doesn't matter whether you use jenkins or anything else, every dev committing something to XBMC needs to set core.autocrlf to true because otherwise you run into problems when people from different platforms (Windows vs. linux/osx) commit/pull stuff. Even github has a page stating that anyone using git on Windows should set core.autocrlf to true.

Yes i know, but that's not the recommended settings for ffmpeg and libav on
Windows. I've also posted about this issue on the forums:
http://forum.xbmc.org/showthread.php?tid=156303&pid=1363840#pid1363840. But
you are probably correct, and the following guys are obviously wrong and
don't know what they are talking about regarding core.autocrlf ;-) [?]

http://stackoverflow.com/questions/2825428/why-should-i-use-core-autocrlf-true-in-git
http://ffmpeg.org/git-howto.html
http://libav.org/download.html

http://libav-users.943685.n4.nabble.com/Libav-user-makefile-problem-on-win32-MinGW-td4657083.html

But when/if you find an alternative solution you could do them a favour and
post that solution on their forums. Need get back to the family again.
Later dudes!

2013/4/13 Sascha Montellese notifications@github.com

@flyingrat https://github.com/FlyingRat: You should read the "Git
Usage" article on our wiki: http://wiki.xbmc.org/index.php?title=Git_Usage
It clearly states that you have to set core.autocrlf to true. It doesn't
matter whether you use jenkins or anything else, every dev committing
something to XBMC needs to set core.autocrlf to true because otherwise you
run into problems when people from different platforms (Windows vs.
linux/osx) commit stuff. Even github has a page stating that anyone using
git on Windows should set core.autocrlf to true.


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/2554#issuecomment-16334816
.

Member

elupus commented Apr 13, 2013

Sadly you are missing the point, the auto setting has nada zilch to do with
ffmpeg. It's xbmc as a whole.

It may very well be that it's bad for ffmpeg to have it on, but that is
beside the point. It's needed for the rest of xbmc.

We have gone through a whole lot of grief regarding this already.

That said, it's obviously not your fault that we have issues now, nor your
job to fix it... , unless you have a magic wand that solves it magically :-)

@ghost

ghost commented Apr 13, 2013

it isnt very hard to see the problem here. usually windows wants to be a type writer. this logic breaks down when windows wants to pretend it is unix. either the tools need to find some common ground, which was the problem that caused the autocrlf in the first place or you deal with this through some variant of (conditional) prebuild patchery. likely not as a patch but through a call to dos2unix.

Member

Voyager1 commented Apr 13, 2013

I think prebuild patchery à la dos2unix is a bad idea as this would cause modification of source files. I believe that where we landed now, using a gnu make that deals with it properly is much better. It seems that for now, this issue is solved, and we can refocus on getting ffmpeg 1.2 a good round of testing...

@ghost

ghost commented Apr 13, 2013

it is no more changing the source than what git does in the first place.. that is after all what the setting discussed inbhere does... anyways i agree a make that handles it is better. what i didnt get was all the confusion that seemed to be around what was happening.

magnyld commented Apr 14, 2013

Wouldn't this be easily fixed by adding text eol=lf for ffmpeg in the .gitattributes? Instead of depending on a make that hopefully handles crlf correctly on windows.

I need to supply a video sample for a patch submitted upstreams as a rfc to
ffmpeg. Does anyone know where to find the video file "brokenStream.mpg"
(or similar) that is related to patch 0015 below?

https://github.com/xbmc/xbmc/blob/master/lib/ffmpeg/patches/0015-fixed-memleak-in-mpegts-demuxer-on-some-malformed-mp.patch

From b83c9a2505338cdf021dd499c26686e82bcbc066 Mon Sep 17 00:00:00 2001
From: Joakim Plate elupus@ecce.se
Date: Fri, 26 Nov 2010 20:56:48 +0000
Subject: [PATCH 15/24] fixed: memleak in mpegts demuxer on some malformed
(??) mpegts files with too large pes packets

at-visions sample file brokenStream.mpg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment