We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 1350 - SDL doesn't report error when requesting YUV overlay bigger than Xv overlay
Summary: SDL doesn't report error when requesting YUV overlay bigger than Xv overlay
Status: RESOLVED ENDOFLIFE
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: 1.2.14
Hardware: x86_64 Linux
: P1 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-23 04:18 UTC by Martin Pulec
Modified: 2015-08-25 09:38 UTC (History)
0 users

See Also:


Attachments
sample solution (15.95 KB, patch)
2011-12-29 11:09 UTC, Martin Pulec
Details | Diff
patch (2.25 KB, patch)
2011-12-29 11:11 UTC, Martin Pulec
Details | Diff
corrected version of previous (2.44 KB, patch)
2011-12-29 11:35 UTC, Martin Pulec
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Pulec 2011-12-23 04:18:26 UTC
Hi, I have following problem:

I create display fullscreen surface with SDL_SetVideoMode and create YUV_Overlay over it. The problem arises when I request YUV overlay that is larger than maximal XvImage size (reported with xvinfo) - SDL returns a valid pointer to a valid SDL_Overlay struct. If I write to the buffer then, my program fails with SEGV.

Thereof, I am afraid I have no possibility to get know that there is some probem.Even SDL_GetError doesn't report anything.

In conclusion, I think that there should be some feedback from SDL to inform the program that it has requested larger overlay then possible, rather than letting it fail with segfault.

Thanks,
Martin Pulec
Comment 1 Sam Lantinga 2011-12-29 01:58:45 UTC
Thanks for the report.  Have you investigated and have a patch for this issue?
Comment 2 Martin Pulec 2011-12-29 05:59:02 UTC
(In reply to comment #1)
> Thanks for the report.  Have you investigated and have a patch for this issue?

Not yet but I can look at soon. Basically, I expect that it shouldn't be so difficult to obtain an error message from XVideo. I will write as soon as I find out sth.

Martin
Comment 3 Martin Pulec 2011-12-29 10:04:34 UTC
(In reply to comment #2)
> I will write as soon as I
> find out sth.

Hi,
I am afraid I cannot post complete patch since I got stuck with prefixing Xv calls with SDL_. Anyway I found out that basically no call gives information about exceeding maximal texture size, so it is needed to figure it out manually.

in file src/video/x11/SDL_x11yuv.c there would suffice add this check:
unsigned int nencode, nadaptors;
XvEncodingInfo *encodings;
XvQueryEncodings(GFX_Display, ainfo[j].base_id, &nencode, &encodings);
break;

to this block:
                                                        if ( Success == SDL_NAME(XvGrabPort)(GFX_Display, ainfo[i].base_id+k, CurrentTime) ) {
                                                                xv_port = ainfo[i].base_id+k;
                                                                unsigned int nencode, nadaptors;
                                                                XvEncodingInfo *encodings;

                                                                XvQueryEncodings(GFX_Display, ainfo[j].base_id, &nencode, &encodings);

;
}
Comment 4 Martin Pulec 2011-12-29 10:11:54 UTC
(In reply to comment #3)
> (In reply to comment #2)

Sorry for previous post, I didn't finished it:

Hi,
I am afraid I cannot post complete patch since I got stuck with prefixing Xv
calls with SDL_. Anyway I found out that basically no call gives information
about exceeding maximal texture size, so it is needed to figure it out
manually.

in file src/video/x11/SDL_x11yuv.c there would suffice add this check:

#include <X11/extensions/Xvlib.h>


int n;
unsigned int nencode, nadaptors;
XvEncodingInfo *encodings;
XvQueryEncodings(GFX_Display, ainfo[j].base_id, &nencode, &encodings);
for(n = 0; n < nencode; n++) {
    if(!strcmp(encodings[n].name, "XV_IMAGE")) {
        if(width > encodings[n].width || height > encodings[n].height)
            /* return error, eg NULL */
        else
            break;
    }
}


to this block:
if ( Success == SDL_NAME(XvGrabPort)(GFX_Display, ainfo[i].base_id+k, CurrentTime) ) {
    xv_port = ainfo[i].base_id+k;
}

I'll try to figure out, how does that SDL_Xv prefixing work, if I succeed, I will post complete patch.
Comment 5 Martin Pulec 2011-12-29 11:09:06 UTC
Created attachment 744 [details]
sample solution

This is a sample patch I proposed. It works very well for me - SDL_CreateYUVOverlay on one hand doesn't return NULL, but I guess that SW_OVERLAY is returned instead? Because with this change I am able to operate properly then.

Only strange thing is that the encoding name should be "XV_IMAGE", I guess, but I get "XV_IMAG".

Patch is obtained against current Ubuntu (11.10) sources.
Comment 6 Martin Pulec 2011-12-29 11:10:38 UTC
Comment on attachment 744 [details]
sample solution

--- a/libsdl1.2-1.2.14/src/video/x11/SDL_x11yuv.c	2009-10-13 01:07:15.000000000 +0200
+++ a/libsdl1.2-1.2.14/src/video/x11/SDL_x11yuv.c	2011-12-29 20:02:29.705555433 +0100
@@ -33,6 +33,7 @@
 #endif
 #include "../Xext/extensions/Xvlib.h"
 
+#include <X11/extensions/Xvlib.h>
 #include "SDL_x11yuv_c.h"
 #include "../SDL_yuvfuncs.h"
 
@@ -220,7 +221,26 @@
 					if ( (Uint32)formats[j].id == format ) {
 						for ( k=0; k < ainfo[i].num_ports; ++k ) {
 							if ( Success == SDL_NAME(XvGrabPort)(GFX_Display, ainfo[i].base_id+k, CurrentTime) ) {
+                                                                int n;
+                                                                unsigned int nencode, nadaptors;
+                                                                SDL_NAME(XvEncodingInfo) *encodings;
+
 								xv_port = ainfo[i].base_id+k;
+
+                                                                SDL_NAME(XvQueryEncodings)(GFX_Display, xv_port, &nencode, &encodings);
+                                                                for(n = 0; n < nencode; n++) {
+                                                                    if(!strncmp(encodings[n].name, "XV_IMAG", sizeof("XV_IMAG"))) {
+                                                                            if(width > encodings[n].width || height > encodings[n].height)
+                                                                                    XFree(formats);
+                                                                                    SDL_NAME(XvFreeAdaptorInfo)(ainfo);
+                                                                                    SDL_NAME(XvFreeEncodingInfo)(encodings);
+                                                                                    SDL_NAME(XvUngrabPort)(GFX_Display, xv_port, CurrentTime);
+                                                                                    return(NULL);
+                                                                            else
+                                                                                break;
+                                                                    }
+                                                                }
+
 								break;
 							}
 						}
Comment 7 Martin Pulec 2011-12-29 11:11:56 UTC
Created attachment 745 [details]
patch

here is the patch
Comment 8 Martin Pulec 2011-12-29 11:35:03 UTC
Created attachment 746 [details]
corrected version of previous

corrected minor errors (braces + freeing XvEncodingInfo)
Comment 9 Ryan C. Gordon 2011-12-30 01:25:52 UTC
Bumping priority on a few bugs that I would like examined more closely before 1.2.15 is finalized. This is not a promise that a bug will be fixed. We may close it with WONTFIX or WORKSFORME or something, but I just want to make sure attention is paid.

--ryan.
Comment 10 Sam Lantinga 2011-12-30 03:08:15 UTC
If you're getting a bad pointer, it seems like we should be able to catch the error later in a more complete way.

Can you disable your fix and add debug statements to find out what code path it's executing, and print out hwdata->image->data at various points along the way?
Comment 11 Martin Pulec 2011-12-30 03:30:07 UTC
(In reply to comment #10)
> If you're getting a bad pointer, it seems like we should be able to catch the
> error later in a more complete way.

Well, I don't think that the pointer is wrong. Eg. I now use Intel SNB graphic, which reports maximal XV image 2048x2048. If I request 2100x300 window (YUV surface), I get valid memory pointer (don't receive segfault while writing to it, but I haven't investigated further). But of course, if I write to it, the displayed picture is completely scrambled - just like interpreted as 2048 px width image.

I think that SEGFAULT occurs when I requested surface that has overall size grater then 4K - I got only 4K texture. But I haven't tested it yet if it is true.

> Can you disable your fix and add debug statements to find out what code path
> it's executing, and print out hwdata->image->data at various points along the
> way?

Sure, I'll look at it in the evening (CET).
Comment 12 Martin Pulec 2012-02-02 14:27:23 UTC
I am really sorry for replaying a month latter but I am quite 
(In reply to comment #10)
> If you're getting a bad pointer, it seems like we should be able to catch the
> error later in a more complete way.
> 
> Can you disable your fix and add debug statements to find out what code path
> it's executing, and print out hwdata->image->data at various points along the
> way?

I am really sorry for replaying so late but I have been quite busy these days. So my observation is following (I have tested with maximal XVImage size 2048x2048 with an older Intel X11 driver, newer one has 8Kx8K but it should be similar):

1) everything up to 2048x2048 is ok
2) increasing rows or columns (when letting other value 2048) gives normal writable pointer, but writing the whole picture fails with segfault. Anyway, writing lower 8 MB (2K width * 2K height * 2bps) doesn't result in SEGFAULT, either. This leads me to result that the pointer is OK in general, but only those lower 8 MB are actually bind/allocated.
Comment 13 Martin Pulec 2012-02-02 14:32:43 UTC
Should I create minimal working example exposing that "problem"?

Anyway, I rather think that this behavior should be replicable (some of our users had this problem also with NVidia, so this is perhaps not Intel specific).
Comment 14 Ryan C. Gordon 2015-08-25 09:38:23 UTC
Hello, and sorry if you're getting several copies of this message by email, since we are closing many bugs at once here.

We have decided to mark all SDL 1.2-related bugs as RESOLVED ENDOFLIFE, as we don't intend to work on SDL 1.2 any further, but didn't want to mark a large quantity of bugs as RESOLVED WONTFIX, to clearly show what was left unattended to and make it easily searchable.

Our current focus is on SDL 2.0.

If you are still having problems with an ENDOFLIFE bug, your absolute best option is to move your program to SDL2, as it will likely fix the problem by default, and give you access to modern platforms and tons of super-cool new features.

Failing that, we _will_ accept small patches to fix these issues, and put them in revision control, although we do not intend to do any further official 1.2 releases.

Failing that, please feel free to contact me directly by email (icculus@icculus.org) and we'll try to find some way to help you out of your situation.

Thank you,
--ryan.