[FFmpeg-devel] [PATCH] Fix segfault in x11grab when drawing Cursor on Xservers that don't support the XFixes extension

[Måns Rullgård]

Brenden Bain <brenden.bain at gmail.com> writes:

> On 23 June 2010 19:39, Brenden Bain <brenden.bain at gmail.com> wrote:
>> 2010/6/22 M?ns Rullg?rd <mans at mansr.com>:
>>> Michael Niedermayer <michaelni at gmx.at> writes:
>>>
>>>> On Tue, Jun 22, 2010 at 10:41:59PM +1000, Brenden Bain wrote:
>>>>> On Tue, 2010-06-22 at 12:08 +0100, M?ns Rullg?rd wrote:
>>>>> > Brenden Bain <brenden.bain at gmail.com> writes:
>>>>> >
>>>>> > > Reformatted and changed information message printing when XFixes not
>>>>> > > available to "Unable to query cursor shape. Using "arrow" mouse
>>>>> > > cursor."
>>>>> >
>>>>> > I would apply just the checks for the extension being present first,
>>>>> > and add the fallback static image afterwards.
>>>>> >
>>>>>
>>>>> Sorry, its not 100% clean to me what you mean here. Do you mean two
>>>>> patches? If you want two patches the simplest fix is probably to just do
>>>>> a "!= NULL" check.
>>>>
>>>> i dunno about mans but i would like to see the crash fixed and once thats
>>>> done any new features could be considered depending on how widespread the
>>>> need for them is.
>>>
>>> I meant to query the extension and set nomouse if it's not available.
>>> Error checking when fetching the cursor should also be added.
>>>
>>
>> Thanks for clearing things up. I have attached the modified patch.
>>
>>
>
> Sorry. That patch was slightly broken. Lets try this again.
>
> Brenden.
>
> Index: libavdevice/x11grab.c
> ===================================================================
> --- libavdevice/x11grab.c	(revision 23733)
> +++ libavdevice/x11grab.c	(working copy)
> @@ -67,7 +67,8 @@
>      XImage *image;           /**< X11 image holding the grab */
>      int use_shm;             /**< !0 when using XShm extension */
>      XShmSegmentInfo shminfo; /**< When using XShm, keeps track of XShm infos */
> -    int nomouse;
> +    int nomouse;             /**< 0 when the mouse should be recoreded. */
> +    int mouse_warning_shown;
>  };
>  
>  /**
> @@ -91,7 +92,7 @@
>      XImage *image;
>      int x_off = 0;
>      int y_off = 0;
> -    int use_shm;
> +    int use_shm, ignore;
>      char *param, *offset;
>  
>      param = av_strdup(s1->filename);
> @@ -115,6 +116,11 @@
>          return AVERROR(EIO);
>      }
>  
> +    if (!XFixesQueryExtension(dpy, &ignore, &ignore)) {
> +        av_log(s1, AV_LOG_INFO, "Disabling cursor recording. Unable to query cursor shape.\n");
> +        x11grab->nomouse = 1;
> +    }
> +
>      st = av_new_stream(s1, 0);
>      if (!st) {
>          return AVERROR(ENOMEM);
> @@ -247,7 +253,7 @@
>   * @param y Mouse pointer coordinate
>   */
>  static void
> -paint_mouse_pointer(XImage *image, struct x11_grab *s)
> +paint_mouse_pointer(XImage *image, struct x11_grab *s, AVFormatContext *s1)
>  {
>      int x_off = s->x_off;
>      int y_off = s->y_off;
> @@ -262,6 +268,14 @@
>  
>      xcim = XFixesGetCursorImage(dpy);
>  
> +    if (xcim == NULL) {
> +        if (!s->mouse_warning_shown) {
> +            av_log(s1, AV_LOG_INFO, "Couldn't load mouse image. Unable to render cusor in video.\n");
> +            s->mouse_warning_shown = 1;
> +        }
> +        return;

I'd just set nomouse here and carry on.  If it failed once, it's
likely to fail again.

-- 
M?ns Rullg?rd
mans at mansr.com