[MPlayer-advusers] mplayer crash when geometry changes

[Reimar Döffinger]

On Sat, Dec 03, 2011 at 03:31:03PM -0700, David Hollister wrote:
> >>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?
> 
> I would agree.  I will see what I can find out about this.  However,
> it's really a linker issue, not a compiler issue.  I don't recall
> whether I've tried with the GNU linker, but I haven't had a lot of
> luck with it on Solaris in the past.

It is not only a linker issue, since the linker can't know what
instructions exist.
So at the very last it needs cooperation from the assembler.

> >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.
> 
> I tried your patch, and it appears to work as expected.

Good, I'll probably go ahead with that.

> As for a segment of that video, if you'd still like it, I'll see
> what I can do to cut out a snippet for you.  Just can't get to it
> right this moment, though.  If you still want it, let me know.

Yes, it would be good. I think FFmpeg behaviour is not ideal in this
case (it returns an AVFrame with dimensions that contradict the
AVCodecContext width/height).

> >>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.
> 
> If you look closely at the patch, the #if HAVE_AMD3DNOW essentially
> is breaking the if/else into just one "if" in the case where
> HAVE_AMD3DNOW is 0.  I'm not arguing this is the best way to do
> this, or is even correct.  Just wanted to ensure you didn't miss
> that.

The case to consider is
HAVE_AMD3DNOW == 1
gCpuCaps.has3DNow == 0
gCpuCaps.hasMMX == 1
old code executes EMMS, your patched code I just realized won't even
compile if HAVE_AMD3DNOW is 1...