After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 488913 - Move to a saner ffmpeg checkout system
Move to a saner ffmpeg checkout system
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal enhancement
: 0.10.4
Assigned To: Edward Hervey
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-10-22 08:30 UTC by Edward Hervey
Modified: 2007-12-17 12:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gst-ffmpeg autogen.sh patch for ffmpeg svn checkout (2.36 KB, patch)
2007-11-02 15:02 UTC, Dejan Sakelšak
none Details | Review
patch for configure.ac (2.10 KB, patch)
2007-11-02 15:04 UTC, Dejan Sakelšak
none Details | Review
Updated patch with fixes to properly build against ffmpeg trunk (r10910) (24.15 KB, patch)
2007-11-03 17:21 UTC, Edward Hervey
none Details | Review
gst-ffmpeg build system and sources patch (36.79 KB, patch)
2007-11-08 09:07 UTC, Dejan Sakelšak
none Details | Review
Fixed linking from previous patch (35.82 KB, patch)
2007-11-08 11:00 UTC, Dejan Sakelšak
none Details | Review
Indentation and common/Makefile.am fix from previous patch (24.88 KB, patch)
2007-11-08 13:55 UTC, Dejan Sakelšak
none Details | Review
Fixed merge problems from previous patches (26.56 KB, patch)
2007-11-09 12:34 UTC, Dejan Sakelšak
none Details | Review
I hope the last one. (25.21 KB, patch)
2007-11-09 13:07 UTC, Dejan Sakelšak
none Details | Review
Adds make dist capability to ffmpeg's checkout (28.17 KB, patch)
2007-11-14 16:54 UTC, Dejan Sakelšak
none Details | Review
Common configuration file for ffmpeg downloading (144 bytes, patch)
2007-11-15 08:27 UTC, Dejan Sakelšak
committed Details | Review
Fixed sed problem from 99096 (27.97 KB, patch)
2007-11-15 08:58 UTC, Dejan Sakelšak
committed Details | Review

Description Edward Hervey 2007-10-22 08:30:42 UTC
Currently gst-ffmpeg uses a patched checkout of ffmpeg stored in a gstreamer cvs module.

The only big advantage (and unfortunately disadvantage) of that mirror system is the autotools patch.
* advantage because it makes it easy to fit into the gst-ffmpeg module build system
* disadvantage because it is the most complex patch to handle when updating the mirror

Since those times, ffmpeg has a build system (not based on autotools) which solves most of the issues we had back in the past. So that autotools patch isn't needed anymore.

The idea is to, instead, store in gst-ffmpeg a ffmpeg svn revision number and modify the build-system so that it:
* Don't store the ffmpeg checkout in svn anymore
* Checkout/update the specific revision of ffmpeg at autogen.sh time
* configure that checkout at configure time
* Copy that checkout when 'make dist'-ing

During devel time (not release), we can then easily do the compile/runtime/regression tests with different checkouts to have a faster/better feeling about various ffmpeg revisions and do the gst-ffmpeg releases with the most recent 'best' ffmpeg revision.

The disadvantage of this system... is that we need to:
* either find another system for patching our checkout
* or push our patches upstream and make sure they get commited ASAP with the help of some gstreamer-sympathizers ffmpeg developers.

After much thinking, this seems to be the smartest way to move for gst-ffmpeg. Comments welcome.
Comment 1 Jens Persson 2007-10-31 22:28:20 UTC
hey man, you weren't kidding you were expecting my whining on the ml. :D i also found this -> http://bugzilla.gnome.org/show_bug.cgi?id=385668

well, as ffmpeg doesn't make filereleases it makes sense to build it from a specified svn revision. then you can make your own patches that applies correctly to that exact revision. it's a little weird though that ffmpeg doesn't make filereleases, can't really see what's stopping them doing that but i guess that it works to grab a specified svn revision too.
Comment 2 Dejan Sakelšak 2007-11-01 02:25:00 UTC
Hi,

I'm working on this issue to help bilboed, but it is more complicated than it should be. I've said many times that i hate ffmpeg. :)

Anyway, the svn checkout stuff works, now i'm trying to patch the current autotools scripts from gstreamer's ffmpeg mirror to compile the new revision.

I hope some of you that understands ffmpeg in a better way would help me with the port to the "new" API.
Comment 3 Dejan Sakelšak 2007-11-02 15:02:01 UTC
Created attachment 98381 [details] [review]
gst-ffmpeg autogen.sh patch for ffmpeg svn checkout

- apply to: gst-ffmpeg/autogen.sh
- adds ffmpeg svn revison checkout
- removes all no more needed stuff
Comment 4 Dejan Sakelšak 2007-11-02 15:04:17 UTC
Created attachment 98382 [details] [review]
patch for configure.ac 

- apply to gst-ffmpeg/configure.ac
- links default static .a libs instead of libtool .la
- adds additional switches for ffmpeg configure script
Comment 5 Dejan Sakelšak 2007-11-02 15:06:56 UTC
After applying these patches comes the time to update the used ffmpeg API.
Comment 6 Edward Hervey 2007-11-03 17:21:14 UTC
Created attachment 98461 [details] [review]
Updated patch with fixes to properly build against ffmpeg trunk (r10910)

This is the full patch I used to properly build gst-ffmpeg with latest svn checkout.

I fixed some stuff in configure.ac/autogen.sh for properly building.

One of the things this doesn't solve... is 'make dist' , I have no idea how to properly do this without patching the ffmpeg checkout (which beats the whole point of doing it this way).

comments welcome
Comment 7 Dejan Sakelšak 2007-11-05 08:26:19 UTC
And how do you propose to fix 'make dist'? 
Why do we need it anyway?
If somebody wants the ffmpeg source, it could be found in the ffmpeg svn.
Everything else, could be just directly packed with the gst-ffmpeg source.
We don't need to distribute it in a tarball, or do we?

Comment 8 Edward Hervey 2007-11-06 07:07:25 UTC
make dist (and most importantly make distcheck) allows us to:
* Generate a tarball with all the required files for a release, which IS distributed as a tarball.
* using distcheck, it will copy over all the files that would go into the tarball into a separate directory, build it, run the tests in order to make sure that everything required to build the release will be included in the tarball.

This is NOT optional, there will be NO release if we can't easily (1) make tarballs of all required files including the ffmpeg checkout and (2) check if all required files are included in the release.

One idea might be to create a specific dist make option in gst-libs/ext/Makefile.am that checks out a clean checkout of the specified revision so that the top-level 'make dist(check)' gets all those files.
Comment 9 Dejan Sakelšak 2007-11-07 11:02:25 UTC
The last patch doesn't seem to work for me. 

i386/dsputil_mmx.c: In function ‘dsputil_init_mmx’:
i386/dsputil_mmx.c:3748: warning: assignment from incompatible pointer type
i386/dsputil_mmx.c:3756: warning: assignment from incompatible pointer type
i386/dsputil_mmx.c: In function ‘flac_compute_autocorr_sse2’:
i386/dsputil_mmx.c:3028: error: can't find a register in class ‘GENERAL_REGS’ while reloading ‘asm’
i386/dsputil_mmx.c:3003: error: ‘asm’ operand has impossible constraints
i386/dsputil_mmx.c:3005: error: ‘asm’ operand has impossible constraints
i386/dsputil_mmx.c:3028: error: ‘asm’ operand has impossible constraints
i386/dsputil_mmx.c:3057: error: ‘asm’ operand has impossible constraints
make[6]: *** [i386/dsputil_mmx.o] Error 1

...

i managed to build and link everything already, and it seems to work except for some deprecated ffmpeg API stuff, that fails in runtime.

i will try to merge your fixed patch with the changes i made
Comment 10 Dejan Sakelšak 2007-11-08 09:07:00 UTC
Created attachment 98756 [details] [review]
gst-ffmpeg build system and sources patch

This patch links gst-ffmpeg with all the static libs needed, but it still needs some polishing on that in configure.ac.
Comment 11 Dejan Sakelšak 2007-11-08 11:00:04 UTC
Created attachment 98761 [details] [review]
Fixed linking from previous patch

- Proper merge of configure.ac with fixes
Comment 12 Dejan Sakelšak 2007-11-08 13:55:27 UTC
Created attachment 98771 [details] [review]
Indentation and common/Makefile.am fix from previous patch
Comment 13 Dejan Sakelšak 2007-11-09 12:34:47 UTC
Created attachment 98813 [details] [review]
Fixed merge problems from previous patches
Comment 14 Dejan Sakelšak 2007-11-09 13:07:37 UTC
Created attachment 98815 [details] [review]
I hope the last one.
Comment 15 Dejan Sakelšak 2007-11-14 16:54:07 UTC
Created attachment 99096 [details] [review]
Adds make dist capability to ffmpeg's checkout

BLOCKER - FIXME: a sed line does not work, to be fixed ASAP
Comment 16 Dejan Sakelšak 2007-11-15 08:27:51 UTC
Created attachment 99128 [details] [review]
Common configuration file for ffmpeg downloading

- should be put in source_root after previous patch is applied
Comment 17 Dejan Sakelšak 2007-11-15 08:58:26 UTC
Created attachment 99130 [details] [review]
Fixed sed problem from 99096
Comment 18 Edward Hervey 2007-11-15 10:04:56 UTC
GST_FFMPEG_NO_MIRROR branch is now created with all current code. Please try it and report issues.

2007-11-15  Edward Hervey  <bilboed@bilboed.com>

	* Makefile.am:
	* autogen.sh:
	* configure.ac:
	* ext/ffmpeg/Makefile.am:
	* ffmpegrev:
	* gst-libs/ext/Makefile.am:
	Initial patch of the new mirror-less build-system for gst-ffmpeg using
	specific revisions of ffmpeg svn instead.
	Might still have some issues, we need people to try this.
	Help by : Dejan Sakelšak  <sakdean at gmail dot com>
	* ext/ffmpeg/gstffmpeg.c: (plugin_init):
	* ext/ffmpeg/gstffmpegcodecmap.c: (gst_ffmpeg_codecid_to_caps),
	(gst_ffmpeg_caps_with_codecid), (gst_ffmpeg_caps_to_codecid),
	(gst_ffmpeg_get_codecid_longname):
	* ext/ffmpeg/gstffmpegdec.c: (gst_ffmpegdec_base_init),
	(gst_ffmpegdec_get_buffer), (gst_ffmpegdec_audio_frame),
	(gst_ffmpegdec_register):
	* ext/ffmpeg/gstffmpegdemux.c: (gst_ffmpegdemux_averror),
	(gst_ffmpegdemux_register):
	* ext/ffmpeg/gstffmpegenc.c: (gst_ffmpegenc_register):
	* ext/ffmpeg/gstffmpegmux.c: (gst_ffmpegmux_register):
	Update code for new ffmpeg API.

Comment 19 Edward Hervey 2007-11-15 10:20:47 UTC
Currently we compile gst-ffmpeg with -Wno-deprecated-declarations because of two things that need to be fixed:
_ img_resample (used in ffvideoscale) => switch to libswscale
_ AVPalette is now deprecated. I have NO clue what we should use instead.
Comment 20 Dejan Sakelšak 2007-11-15 13:51:09 UTC
AVPaletteControl is deprecated and i took a look to ffmpeg's source, where in the comment above the structure definition is in other words written "this is a crap, use AVPacket insted". :)

I was looking into the definitions, but i'm rather new to ffmpeg (as a library),
so until i don't understand how must something be done, i can't do a lot with the port of the current api to the new one.
Comment 21 Edward Hervey 2007-12-17 12:45:36 UTC
CVS Root:       /cvs/gstreamer
Module:         gst-ffmpeg
Changes by:     bilboed
Date:           Mon Dec 17 2007  12:43:20 UTC

Log message:
Merging GST_FFMPEG_NO_MIRROR branch to trunk

Modified files:
    .               : ChangeLog Makefile.am autogen.sh configure.ac
                      ffmpegrev
    ext/ffmpeg      : Makefile.am gstffmpeg.c gstffmpeg.h
                      gstffmpegaudioresample.c gstffmpegcfg.c
                      gstffmpegcodecmap.c gstffmpegdec.c
                      gstffmpegdemux.c gstffmpegenc.c gstffmpegmux.c
    gst-libs/ext    : Makefile.am