[MPlayer-advusers] mplayer crash when geometry changes

[Reimar Döffinger]

On Sat, Dec 03, 2011 at 01:44:34PM -0700, David Hollister wrote:
> Hi Reimar,
> 
> Thanks for your quick response.
> 
> On 12/ 3/11 11:21 AM, Reimar Döffinger wrote:
> >On Sat, Dec 03, 2011 at 10:50:16AM -0700, David Hollister wrote:
> >>First off, the system is running Solaris 11.  I always have to make
> >>a handful of modifications in order to get it to compile, but I've
> >>never had problems before.  (I can provide all the diffs I have to
> >>make whenever I update the source if anybody really wants to know).
> >
> >Unless they are due to not having switched Solaris to the POSIX
> >environment we would want to fix those issues, yes/
> 
> I am attaching another diff containing the changes I make to get
> things to work for Solaris.  They are actually much smaller now than
> they were a few months ago :)
> 
> A quick description of why:
> 
> mp3lib/dct64_sse.c: As is, I'll get the following errors:
> 
> Assembler: dct64_sse.c
> 	"<stdin>", line 250 : Illegal mnemonic
> 	Near line: "	fistps 512(%esi)"
> 	"<stdin>", line 250 : Syntax error
> ...
> 	"<stdin>", line 258 : Illegal mnemonic
> 	Near line: "	fists  256(%ebx)"
> 	"<stdin>", line 258 : Syntax error
> 	Near line: "	fists  256(%ebx)"
> ...
> 
> This is with the following gcc: gcc (GCC) 4.6.2 20110826
> (prerelease) (also happens with gcc 4.3.3 and 4.5.x).

No surprise since it is a binutils issue, you can change compilers
all you want and it won't make a difference.
But did you test your change? My understanding is that these should
be 16-bit stores, and removing the "s" would make them 32 bit.
Though my tests indicate that "s" seems to be the default here.
Anyway mp3lib is deprecated so just disabling it might be the better
solution.

> libmpcodecs/dec_video.c: This change is not necessary to compile...
> it will compile as-is.  However, it results in the following when I
> try to run:
> 
> $ file mplayer
> mplayer: ELF 32-bit LSB executable, Intel 80386, version 1,
> dynamically linked (uses shared libs), not stripped, uses FPU TSC
> CMOV MMX AMD_3DNow SSE SSE2 SSE3 SSSE3 SSE4.1
> 
> $ ./mplayer
> ld.so.1: mplayer: fatal: mplayer: hardware capability (CA_SUNW_HW_1)
> unsupported: 0x100  [ AMD_3DNow ]
> Killed
> 
> So, I suppose the Solaris ld.so will refuse to run binaries with
> hardware capabilities it knows aren't supported by the current
> processor.  Just FYI, There are a couple of ffmpeg files that
> require similar changes (libavcodec/x86/cavsdsp_mmx.c and
> dsputilenc_mmx.c)

That is just broken, instructions in the binary don't generally have
much to do with instructions needed to execute.
Also, this:
http://ffmpeg.org/pipermail/ffmpeg-cvslog/2011-September/040912.html
seems to indicate this should only happen when compiling with suncc?

> help/help_mp-en.h: As is, this will result in the following compile error:
> 
> libao2/ao_sun.c: In function 'realtime_samplecounter_available':
> libao2/ao_sun.c:161:35: error: 'MSGTR_AO_SUN_RtscWriteFailed'
> undeclared (first use in this function)
> libao2/ao_sun.c:161:35: note: each undeclared identifier is reported
> only once for each function it appears in
> gmake: *** [libao2/ao_sun.o] Error 1

Strange, I am sure that worked at some point.

> >>During my testing, I found that it's only the frame passed in to
> >>draw_image/draw_slice immediately following the geometry change that
> >>has the incorrect width and height.  Subsequent frames were correct.
> >
> >Please provide a sample file.
> >Also say whether this also happens with -noslices and -vo x11 or -vo gl
> >or such.
> 
> Unfortunately, the sample file is about 1.8GB in size.  However, I'm
> pretty sure I can easily generate one that is much smaller.  I'll
> try to do that and will post a link as soon as I can.

You should be able to just cut out a few MB at the transition point with
dd.
I think there's even a command you could bind to a key top make MPlayer
print the current file position in bytes.
get_property stream_pos
I think.

> With "-noslices -vo x11": Still crashes.
> 
> With "-vo gl": Appears to work fine.

Probably just luck that -vo gl works.
Would also appreciate if you could test my patch.

> Index: libmpcodecs/dec_video.c
> ===================================================================
> --- libmpcodecs/dec_video.c	(revision 34378)
> +++ libmpcodecs/dec_video.c	(working copy)
> @@ -423,12 +423,16 @@
>  #if HAVE_MMX
>      // some codecs are broken, and doesn't restore MMX state :(
>      // it happens usually with broken/damaged files.
> +#if HAVE_AMD3DNOW
>      if (gCpuCaps.has3DNow) {
>          __asm__ volatile ("femms\n\t":::"memory");
> -    } else if (gCpuCaps.hasMMX) {
> +    } else 
> +#else
> +    if (gCpuCaps.hasMMX) {
>          __asm__ volatile ("emms\n\t":::"memory");
>      }
>  #endif
> +#endif

That will actually break --enable-runtime-cpudetection builds that
are run on a CPU without 3DNow - neither emms nor femms will be
executed in that case.
Since 3DNow implies MMX the #if should just be split I think.
Might get a bit ugly though.