| Summary: | SDL doesn't report error when requesting YUV overlay bigger than Xv overlay | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Martin Pulec <pulec> |
| Component: | video | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED ENDOFLIFE | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P1 | ||
| Version: | 1.2.14 | ||
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: |
sample solution
patch corrected version of previous |
||
|
Description
Martin Pulec
2011-12-23 04:18:26 UTC
Thanks for the report. Have you investigated and have a patch for this issue? (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 (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); ; } (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. 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 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;
}
}
Created attachment 745 [details]
patch
here is the patch
Created attachment 746 [details]
corrected version of previous
corrected minor errors (braces + freeing XvEncodingInfo)
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. 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? (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). 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. 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). 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. |