[FFmpeg-devel] OS/2?

[Diego Biurrun]

On Wed, Jul 04, 2007 at 10:52:07PM +0200, Michael Niedermayer wrote:
> 
> On Wed, Jul 04, 2007 at 04:11:10PM -0300, Ramiro Ribeiro Polla wrote:
> > 
> > Should we remove first, and then announce it on ffmpeg-user, or the 
> > other way around? It's unlikely that someone that reads ffmpeg-user but 
> > not ffmpeg-devel might help here...
> > 
> > Is any version bump necessary?
> > Patch attached. To commit along with svn rm libavcodec/os2thread.c. Ok 
> > to apply?
> 
> iam not objecting removing dirty hacks related to os/2 but removing clean
> os2 related code seems like it might harm a possible future attempt to get
> os2 support working again
> 
> i mean if we do some functions renaming or other API redesign
> removed code will not get updated ...
> 
> > --- configure	(revision 9470)
> > +++ configure	(working copy)
> > @@ -71,7 +71,6 @@
> >    echo "  --enable-pp              enable GPLed postprocessing support [default=no]"
> >    echo "  --enable-swscaler        software scaler support [default=no]"
> >    echo "  --enable-beosthreads     use BeOS threads [default=no]"
> > -  echo "  --enable-os2threads      use OS/2 threads [default=no]"
> >    echo "  --enable-pthreads        use pthreads [default=no]"
> >    echo "  --enable-w32threads      use Win32 threads [default=no]"
> >    echo "  --enable-x11grab         enable X11 grabbing [default=no]"
> > @@ -612,7 +611,6 @@
> >  
> >  THREADS_LIST='
> >      beosthreads
> > -    os2threads
> >      pthreads
> >      w32threads
> >  '
> > @@ -674,7 +672,6 @@
> >      malloc_h
> >      memalign
> >      mlib
> > -    os2
> >      ppc64
> >      sdl
> >      sdl_video_size
> > @@ -1184,27 +1181,6 @@
> >      targetos=irix
> >      ranlib="echo ignoring ranlib"
> >      ;;
> > -  os/2)
> > -    TMPE=$TMPE".exe"
> > -    ar="emxomfar -p128"
> > -    ranlib="echo ignoring ranlib"
> > -    strip="echo ignoring strip"
> > -    add_cflags "-Zomf"
> > -    FFLDFLAGS="-Zomf -Zstack 16384 -s"
> > -    SHFLAGS="-Zdll -Zomf"
> > -    FFSERVERLDFLAGS=""
> > -    LIBPREF=""
> > -    LIBSUF=".lib"
> > -    SLIBPREF=""
> > -    SLIBSUF=".dll"
> > -    EXESUF=".exe"
> > -    osextralibs=""
> > -    pkg_requires=""
> > -    dv1394="no"
> > -    ffserver="no"
> > -    vhook="no"
> > -    os2="yes"
> > -    ;;
> >    *)
> >      targetos="${targetos}-UNKNOWN"
> >      ;;
> 
> this looks fairly clean to me so IMHO it could be kept, but its mans decission

Clean or not, it is cruft.  I'm against keeping cruft around.

> > --- Changelog	(revision 9470)
> > +++ Changelog	(working copy)
> > @@ -89,6 +89,7 @@
> >  - codebook generator
> >  - RoQ video encoder
> >  - QTRLE encoder
> > +- OS/2 support removed
> >  
> > --- libavcodec/Makefile	(revision 9470)
> > +++ libavcodec/Makefile	(working copy)
> > @@ -318,7 +318,6 @@
> >  
> >  OBJS-$(HAVE_PTHREADS)                  += pthread.o
> >  OBJS-$(HAVE_W32THREADS)                += w32thread.o
> > -OBJS-$(HAVE_OS2THREADS)                += os2thread.o
> >  OBJS-$(HAVE_BEOSTHREADS)               += beosthread.o
> >  
> >  OBJS-$(HAVE_XVMC_ACCEL)                += xvmcvideo.o
> 
> iam against removing os2thread.c
> the file does no harm IMHO
> removing it might cause some poor guy to waste his time reimplementing
> it

IMO it is extremely unlikely that somebody will do that and furthermore
even if we keep the file lying around, does that really reduce the
chance that work will be duplicated?  The file does not merely have to
exist, people need to be aware of its existence.  A notice in the
changelog that states this fact should be much more effective.

> > --- libpostproc/mangle.h	(revision 9470)
> > +++ libpostproc/mangle.h	(working copy)
> > @@ -26,7 +26,7 @@
> >  
> >  /* Feel free to add more to the list, eg. a.out IMO */
> >  /* Use rip-relative addressing if compiling PIC code on x86-64. */
> > -#if defined(__CYGWIN__) || defined(__MINGW32__) || defined(__OS2__) || \
> > +#if defined(__CYGWIN__) || defined(__MINGW32__) || \
> >     (defined(__OpenBSD__) && !defined(__ELF__))
> >  #if defined(ARCH_X86_64) && defined(PIC)
> >  #define MANGLE(a) "_" #a"(%%rip)"
> 
> this looks duplicated from libavutil/internal.h

Indeed it is.  This needs to be addressed.

Diego