Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDL doesn't report error when requesting YUV overlay bigger than Xv overlay #649

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Labels
endoflife Bug might be valid, but SDL-1.2 is EOL.

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 1.2.14
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2011-12-23 04:18:26 +0000, Martin Pulec wrote:

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

On 2011-12-29 01:58:45 +0000, Sam Lantinga wrote:

Thanks for the report. Have you investigated and have a patch for this issue?

On 2011-12-29 05:59:02 +0000, Martin Pulec wrote:

(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

On 2011-12-29 10:04:34 +0000, Martin Pulec wrote:

(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);

;
}

On 2011-12-29 10:11:54 +0000, Martin Pulec wrote:

(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.

On 2011-12-29 11:09:06 +0000, Martin Pulec wrote:

Created attachment 744
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.

On 2011-12-29 11:10:38 +0000, Martin Pulec wrote:

Comment on attachment 744
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;
    					}
    				}
    

On 2011-12-29 11:11:56 +0000, Martin Pulec wrote:

Created attachment 745
patch

here is the patch

On 2011-12-29 11:35:03 +0000, Martin Pulec wrote:

Created attachment 746
corrected version of previous

corrected minor errors (braces + freeing XvEncodingInfo)

On 2011-12-30 01:25:52 +0000, Ryan C. Gordon wrote:

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.

On 2011-12-30 03:08:15 +0000, Sam Lantinga wrote:

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?

On 2011-12-30 03:30:07 +0000, Martin Pulec wrote:

(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).

On 2012-02-02 14:27:23 +0000, Martin Pulec wrote:

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.

On 2012-02-02 14:32:43 +0000, Martin Pulec wrote:

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).

On 2015-08-25 09:38:23 +0000, Ryan C. Gordon wrote:

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.

@SDLBugzilla SDLBugzilla added bug endoflife Bug might be valid, but SDL-1.2 is EOL. labels Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
endoflife Bug might be valid, but SDL-1.2 is EOL.
Projects
None yet
Development

No branches or pull requests

1 participant