New MMX code for YUV420 conversion causes issues

[pdbailey@…]

The changes introduced in r11013 cause me an intermittent problem with both live TV and recordings. On changing channels, or on skipping through a recorded programme, the video and audio freezes, and the log fills up with:

"NVP: Timed out waiting for free video buffers"

I was unsure as to the exact cause of this problem, but after backing out the changes in this changeset, I am unable to reproduce the problem, so I'm fairly confident that somehow this change is the cause. There were some posts on the dev mailing list about others on Intel Macs suffering from the exact same symptoms.

I have no crashlog to attach (as the app just freezes), but I will attach a verbose log from the frontend when I get home this evening.

[Nigel]

Paul, does the fault occur if you disable "Use Vector-enhanced color space conversion" in the TV Playback settings? This will result in garbage images, but at least it shouldn't change any buffers.

[pdbailey@…]

Nigel,

My recollection is that it did, but I'll be able to check that in about 45 minutes or so.

[pdbailey@…]

Ignore my last comment, I was totally wrong. The video plays without that message appearing (but messed up visually, obviously)

[Nigel]

OK. I will guess that something is overwriting the frame's buffer space and clobbbering videobuffers queue management? Could I get someone to turn the Vector-enhanced setting back on after applying this patch:

% svn diff yuv2rgb.cpp
Index: yuv2rgb.cpp
===================================================================
--- yuv2rgb.cpp (revision 11063)
+++ yuv2rgb.cpp (working copy)
@@ -927,7 +927,7 @@
 #endif
 
 #ifdef MMX
-    return mmx_yuv420_2vuy;
+    //return mmx_yuv420_2vuy;
 #endif
 
     return non_vec_yuv420_2vuy; /* Fallback to C */

which will tell us if QuickTime is screwing up the frontend's data, or if the MMX routine is. And the output from -v playback would be helpful, too.

[pdbailey@…]

OK, after applying that patch. I can't reproduce the issue.

Oddly enough, if I run with logging set to -v playback, I can't seem to reproduce it either...

[Nigel]

1) OK. At least we can easily work around it.
2) Bummer. That seems to indicate more complex problems.
I will try to work out the problem with the routine, but if that doesn't get sorted before 0.20, I will commit something like:

% svn diff yuv2rgb.h yuv2rgb.cpp videoout_quartz.cpp
Index: yuv2rgb.h
===================================================================
--- yuv2rgb.h   (revision 11063)
+++ yuv2rgb.h   (working copy)
@@ -54,6 +54,13 @@
 
 yuv2vuy_fun get_yuv2vuy_conv(void);
 
+extern void non_vec_yuv420_2vuy(uint8_t, uint8_t,
+                                uint8_t, uint8_t,
+                                int, int, int, int, int);
+
+
+// Conversion in the other direction, currently unused
+
 typedef void (* vuy2yuv_fun) (uint8_t * image, uint8_t * py,
                               uint8_t * pu, uint8_t * pv,
                               int h_size, int v_size,
Index: yuv2rgb.cpp
===================================================================
--- yuv2rgb.cpp (revision 11063)
+++ yuv2rgb.cpp (working copy)
@@ -784,10 +784,10 @@
 #endif // MMX
 
 
-static void non_vec_yuv420_2vuy (uint8_t * image, uint8_t * py,
-                                 uint8_t * pu, uint8_t * pv,
-                                 int h_size, int v_size,
-                                 int vuy_stride, int y_stride, int uv_stride)
+void non_vec_yuv420_2vuy (uint8_t * image, uint8_t * py,
+                          uint8_t * pu, uint8_t * pv,
+                          int h_size, int v_size,
+                          int vuy_stride, int y_stride, int uv_stride)
 {
     uint8_t *pi1, *pi2 = image;
     uint8_t *py1, *py2 = py;
Index: videoout_quartz.cpp
===================================================================
--- videoout_quartz.cpp (revision 11063)
+++ videoout_quartz.cpp (working copy)
@@ -1341,7 +1341,13 @@
     if (gContext->GetNumSetting("MacYuvConversion", 1))
         data->yuvConverter = get_yuv2vuy_conv();
     else
+#ifdef WORDS_BIGENDIAN
+        // It is safe on PPC to let QuickTime convert YUV420:
         data->yuvConverter = NULL;
+#else
+        // ... but on Intel it crashes, so we always pre-convert:
+        data->yuvConverter = non_vec_yuv420_2vuy;
+#endif
 
     if (!CreateQuartzBuffers())
     {
@@ -1453,11 +1459,7 @@
     ImageDescription *desc = *data->imgDesc;
 
     desc->idSize = sizeof(ImageDescription);
-#ifdef WORDS_BIGENDIAN
     desc->cType = kYUV420CodecType;
-#else
-    desc->cType = kComponentVideoCodecType;  // Wrong, but prevents Intel crash
-#endif
     desc->version = 1;
     desc->revisionLevel = 0;
     desc->spatialQuality = codecNormalQuality;

[pdbailey@…]

Playback log from attempting to view DVD content

[pdbailey@…]

2nd attempt at playback, after patch applied (patch also attached)

[pdbailey@…]

Attached my buildscript erroneously

[pdbailey@…]

2nd attempt at playback, after patch applied (patch also attached)

[pdbailey@…]

patch applied (tweaked to try to get it to compile from the one on the mythtv-dev mailing list)

[Nigel]

(In [11076]) Add emms() that was missing from new MMX code. Closes #2312 Also added a fallback for incompatible source dimensions, but now realise that it will only work horizontally. If we ever get a vertical resolution that is not even, then things will still fail.