Can we upgrade ffmpeg?

[mirkwood]

While the memory leak is first and foremost our biggest problem right now, I'd like to see if we can upgrade to a new ffmpeg revision. The revision we're using is r6324 from the ffmpeg Subversion trunk.

I tried using the latest revision of ffmpeg, r7433, and this fixes problems I saw with ffmpeg 2-pass encoding in the MinGW/Windows and Linux builds.

In short, we need the newest ffmpeg for Windows and Linux for them to work with 2-pass ffmpeg, but we need building and encoding testing on the MacOSX side to make sure there's no regressions.

Thoughts?

[jbrjake]

we need building and encoding testing on the MacOSX side to make sure there's no regressions.

Sorry, been focusing on the mem leak this evening so I haven't had a chance to test it out yet.

[mirkwood]

we need building and encoding testing on the MacOSX side to make sure there's no regressions.

Sorry, been focusing on the mem leak this evening so I haven't had a chance to test it out yet.

No problem, the mem leak is more urgent.

I just noticed that when running the MinGW build there's a warning that ffmpeg prints out saying that "the compiler did not align the stack variables and libavcodec may run slow or crash", so this needs more testing on MinGW!

At the moment though, given all the other problems I have with MinGW, I'm going to try again with the Cygwin build with the new ffmpeg and see how that goes. Maybe x264 encoding kept crashing on the Cygwin build because of the memory leak since I was always using .mp4 output.

[mirkwood]

Hrm, I'm also seeing the alignment warning message on the Cygwin build with the latest ffmpeg, so maybe the Cygwin and MinGW gcc compilers don't support the memory alignment compiler directive used in the ffmpeg source code. May or may not be a problem...

[prigaux]

Did you try to add this in the jamfile

FFMPEG_OPTIONS = --enable-memalign-hack ;

This is used for the macintel part and is perhaps needed too for cygwin ?


Philippe

[mirkwood]

Cygwin has memalign(), so it doesn't need the hack. However, MinGW does need the hack since it doesn't provide memalign(), but the alignment problem happens with both Cygwin and MinGW, so I don't really think it's the real memalign vs memalign-hack.

It basically looks like gcc on Cygwin/MinGW doesn't support the alignment directive ffmpeg is using, though there may be a compiler flag that could get this to work, I just haven't done much testing here yet.

[prigaux]

I upgraded ffmpeg to revision 7444 in the svn.

Remove your ffmpeg.tar.gz file from your contrib folder to use it.

Philippe

[jbrjake]

I upgraded ffmpeg to revision 7444 in the svn.

Remove your ffmpeg.tar.gz file from your contrib folder to use it.

In the future, could you please run svn commits by everyone else before you change a major component *while* we're trying to fix a major memory leak *with* said component? Especially because this upgrade doesn't fix the memory leak, and, as mirkwood explained earlier today, it needs more testing for win32 before we can be sure that fixing 2-pass doesn't break something else.

Like any other change (newer x264, johnallen's threading modification, the new GUI, etc) library upgrades should have been held off until after 0.7.2, per *the consensus we all came to together in the roadmap to 0.7.2 thread*...

[s55]

Compiling 104 on Windows now.

Yes, not a great idea putting a new build in when we have a sodding mem leak.

[s55]

Aside from improved encoding performance, its still going to heck with this caching issue.

Encoding starts fast, Slows down as memory usage increases.

Otherwise i give it the OK on the windows side.

[prigaux]

Humm, OK, I created a thread about theses updates, nobody replied but jbrjake.

If you don't want this update, it's easy to remove.

You really prefer to search bugs in outdated lib versions?

I continue to work on HB on my side, contact me if you really need help to do something.

Philippe

[johnallen]

All, let's not get in the habit of [Censored] off good developers. prigauxs' update to the newest ffmpg appears sound to me. The latest version could have fixed the memory leak. After all it's a leak in ffmpegs' libs, not ours.

As he states, we can goback fairly easily.

prigaux, don't leave us now. We need all the help we can get.

[prigaux]

I do not leave, I'm upgrading on my side xvid and libdvdread perhaps will it run better on sony releases..

Philippe

[johnallen]

I've taken the liberty of creating a branch for development of the future release, 0.7.3 of HB. I suggest we try to fix the ffmpeg issue within the trunk, then release 0.7.2. All other development can be commited to the 0.7.3 branch. Once we release 0.7.2, we can merge 0.7.3 back into the trunk.

prigaux, I would like to have your work on using the latest contrib libs. So can you commit these changes to the branch? I'd also like to see if we can make use of some of the work titer did in his branch. Personally I would like to use the fix where he builds everything from within XCode. This makes it easier for us to debug libhb from within XCode.

We also need to be adamant about getting 0.7.2 released. Not everyone can play in the new branch. We need some people tracking down this ffmpeg issue for the release.

[prigaux]

Yes of course, but can you explain me how I do to work in a specific branch using rapidsvn ?

I'm affraid to break something..

Philippe

[prigaux]

OK, found by myself

I commited the new libdvdread 0.9.7

Philippe

[mirkwood]

I would say the Windows side seems ok, even with the alignment problem. New code was added since our last revision of ffmpeg to actually check the alignment during runtime, so I have a feeling our old ffmpeg probably has the same alignment problem with Cygwin or MinGW gcc.

What I may do is add a patch to our contrib folder to get rid of that alignment warning message that the ffmpeg libavcodec prints out, otherwise the HandBrake forums will be flooded with Windows users asking what the deal with that warning message is.

[johnallen]

I updated the 0.7.3 branch to use r618 of x264. I also removed (by commenting out) the patch for intel macs and windows platforms. If someone realizes we still need the patch, feel free to uncomment the lines int the Jamfile.

[mirkwood]

I updated the 0.7.3 branch to use r618 of x264. I also removed (by commenting out) the patch for intel macs and windows platforms. If someone realizes we still need the patch, feel free to uncomment the lines int the Jamfile.

The Cygwin/Windows patch is needed to build correctly. I'll generate a new patch for this version and uncomment the patch for Cygwin in the Jamfile.

[prigaux]

Cool I have been able to compile a x264 universal on may intel mac

Finally, I will perhaps be able to cook a universal jamfile (when compiled on intel)

Philippe

[johnallen]

prigaux, do we still need the libdvdread patch:

patch-libdvdread.patch?

[prigaux]

Well, I don't know, it seems to work with or without it, the patch don't need to be upgraded.

I think you can try to comment it in jamfile as well..

Philippe

[mirkwood]

The libdvdread patch is needed to get rid of that annoying ASSERT message that will be printed a zillion times.

If you compile and build without the patch and it doesn't print the error, then I guess we can comment out the patch for now, but otherwise please keep the patch updated.

And PLEASE, folks, don't just willy nilly upgrade stuff without going through the patches and making sure other people can get the things to compile on other platforms besides OSX.

[mirkwood]

I just compiled rev 113 of the 0.7.3 branch on Windows with Cygwin, and running HBTest to scan the titles on a DVD I don't see the assert message popping up, so looks good.

[johnallen]

That's the idea of the branch. The objective is to get our contribs updated to the latest version. Every dev will need to confirm that their platforms compile is still working.