headers
Brenden Bain | 22 Jun 2010 00:40
Picon

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

Hello,

In r20631 x11grab.c was changed to query the current cursor from the
Xserver using the XFixes extension. This caused ffmpeg to segfault on
x11grab.c:265 when connected to a server that does not support this
extension (e.g. the VNC server).

In versions prior to r20631 ffmpeg used to just draw a static "arrow"
cursor. The fix basically brings back this code into x11grab.c and
uses it when the Xserver does not support XFixes.

The user will get a info message of the form "Unable to query cursor
shape. Using fixed mouse cursor." when ffmpeg is connected to a
Xserver without XFixes.

Thanks,
Brenden.
Attachment (x11grab.patch): text/x-patch, 7749 bytes
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
Permalink | Reply |
headers
Carl Eugen Hoyos | 22 Jun 2010 00:58
Picon

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

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

> In r20631 x11grab.c was changed to query the current cursor from the
> Xserver using the XFixes extension. This caused ffmpeg to segfault on
> x11grab.c:265 when connected to a server that does not support this
> extension (e.g. the VNC server).

Is this related to issue 1997?

[...]

> +    static const uint16_t const mousePointerBlack[] =
> +        {
> +            0x0000, 0x0003, 0x0005, 0x0009, 0x0011,

Indentation looks wrong.

[...]

> -    xcim = XFixesGetCursorImage(dpy);;
> +    xcim = XFixesGetCursorImage(dpy);

Applied this hunk, Carl Eugen
Permalink | Reply |
headers
Brenden Bain | 22 Jun 2010 01:46
Picon

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

On 22 June 2010 08:58, Carl Eugen Hoyos <cehoyos <at> ag.or.at> wrote:
> Is this related to issue 1997?

Nope. That looks like a completely different bug. Should I also create
a bug report for this one?

> [...]
>
>> +    static const uint16_t const mousePointerBlack[] =
>> +        {
>> +            0x0000, 0x0003, 0x0005, 0x0009, 0x0011,

This is as the code was formatted in < r20631. Happy to change it and resubmit.

Brenden.
Permalink | Reply |
headers
Brenden Bain | 22 Jun 2010 12:49
Picon

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

Reformatted and changed information message printing when XFixes not
available to "Unable to query cursor shape. Using "arrow" mouse
cursor."

Brenden.

On 22 June 2010 09:46, Brenden Bain <brenden.bain <at> gmail.com> wrote:
> On 22 June 2010 08:58, Carl Eugen Hoyos <cehoyos <at> ag.or.at> wrote:
>> Is this related to issue 1997?
>
> Nope. That looks like a completely different bug. Should I also create
> a bug report for this one?
>
>> [...]
>>
>>> +    static const uint16_t const mousePointerBlack[] =
>>> +        {
>>> +            0x0000, 0x0003, 0x0005, 0x0009, 0x0011,
>
> This is as the code was formatted in < r20631. Happy to change it and resubmit.
>
> Brenden.
>
Attachment (x11grab_cursor_fix.patch): text/x-patch, 7596 bytes
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
(Continue reading)

Permalink | Reply |
headers
Måns Rullgård | 22 Jun 2010 13:08

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

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.

--

-- 
Måns Rullgård
mans <at> mansr.com
Permalink | Reply |
headers
Brenden Bain | 22 Jun 2010 14:41
Picon

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

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. 

Brenden.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
Permalink | Reply |
headers
Michael Niedermayer | 22 Jun 2010 15:00
Picon
Picon

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

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.

[...]
--

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad

_______________________________________________
(Continue reading)

Permalink | Reply |
headers
Måns Rullgård | 22 Jun 2010 15:13

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

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.

The rest can be added separately.  I also think the patch is much
larger than it needs to be.

--

-- 
Måns Rullgård
(Continue reading)

Permalink | Reply |
headers
Brenden Bain | 23 Jun 2010 11:39
Picon

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

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.

Brenden.
(Continue reading)

Permalink | Reply |
headers
Brenden Bain | 23 Jun 2010 11:45
Picon

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

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

Permalink | Reply |
headers
Måns Rullgård | 23 Jun 2010 11:57

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

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

Permalink | Reply |
headers
Brenden Bain | 23 Jun 2010 12:14
Picon

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

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

Makes sense.

Brenden.
Attachment (x11_segfault_fix.patch): text/x-patch, 1756 bytes
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
Permalink | Reply |
headers
Måns Rullgård | 23 Jun 2010 12:52

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

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

>> I'd just set nomouse here and carry on.  If it failed once, it's
>> likely to fail again.
>
> Makes sense.
>
> Brenden.
>
> Index: libavdevice/x11grab.c
> ===================================================================
> --- libavdevice/x11grab.c	(revision 23733)
> +++ libavdevice/x11grab.c	(working copy)
>  <at>  <at>  -67,7 +67,7  <at>  <at> 
>      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. */

Unrelated.

>  };
>  
>  /**
>  <at>  <at>  -91,7 +91,7  <at>  <at> 
>      XImage *image;
>      int x_off = 0;
>      int y_off = 0;
> -    int use_shm;
(Continue reading)

Permalink | Reply |
headers
Brenden Bain | 24 Jun 2010 13:25
Picon

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

2010/6/23 Måns Rullgård <mans <at> mansr.com>:
> Brenden Bain <brenden.bain <at> gmail.com> writes:
>
>>> I'd just set nomouse here and carry on.  If it failed once, it's
>>> likely to fail again.
>>
>> Makes sense.
>>
>> Brenden.
>>
>> Index: libavdevice/x11grab.c
>> ===================================================================
>> --- libavdevice/x11grab.c     (revision 23733)
>> +++ libavdevice/x11grab.c     (working copy)
>>  <at>  <at>  -67,7 +67,7  <at>  <at> 
>>      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. */
>
> Unrelated.

Removed.

>>  };
>>
>>  /**
>>  <at>  <at>  -91,7 +91,7  <at>  <at> 
>>      XImage *image;
(Continue reading)

Permalink | Reply |
headers
Måns Rullgård | 24 Jun 2010 13:44

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

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

> Index: libavdevice/x11grab.c
> ===================================================================
> --- libavdevice/x11grab.c	(revision 23755)
> +++ libavdevice/x11grab.c	(working copy)
>  <at>  <at>  -91,7 +91,7  <at>  <at> 
>      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);
>  <at>  <at>  -115,6 +115,11  <at>  <at> 
>          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);
>  <at>  <at>  -245,8 +250,9  <at>  <at> 
>   *          coordinates
>   *  <at> param x Mouse pointer coordinate
(Continue reading)

Permalink | Reply |
headers
Brenden Bain | 24 Jun 2010 14:12
Picon

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

On Thu, 2010-06-24 at 12:44 +0100, Måns Rullgård wrote:
> Brenden Bain <brenden.bain <at> gmail.com> writes:
> >  <at>  <at>  -388,7 +399,11  <at>  <at> 
> >      }
> >  
> >      if(!s->nomouse){
> > -        paint_mouse_pointer(image, s);
> > +        if (!paint_mouse_pointer(image, s)) {
> > +            /* An error occured and will likely occur again. Just disable mouse capture from now on */
> 
> That comment is superfluous.  Anyone with half a brain can see pointer
> capture is being turned off.

I am trying to document the intent of the line. It was not obvious to me
why one error would turn mouse capture off (though many would argue that
I only have half a brain). Maybe some bad Xserver returns NULL when you
move the cursor off the screen and continues to work later. I just
wanted to document the decision that was made. I am happy to take it out
if it bugs. 

Thanks for your patience and help.

Brenden.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel <at> mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
Permalink | Reply |
headers
Brenden Bain | 25 Jun 2010 01:12
Picon

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

2010/6/24 Måns Rullgård <mans <at> mansr.com>:
> Brenden Bain <brenden.bain <at> gmail.com> writes:
>
>> Index: libavdevice/x11grab.c
>> ===================================================================
>> --- libavdevice/x11grab.c     (revision 23755)
>> +++ libavdevice/x11grab.c     (working copy)
>>  <at>  <at>  -91,7 +91,7  <at>  <at> 
>>      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);
>>  <at>  <at>  -115,6 +115,11  <at>  <at> 
>>          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);
>>  <at>  <at>  -245,8 +250,9  <at>  <at> 
>>   *          coordinates
(Continue reading)

Permalink | Reply |
headers
Michael Niedermayer | 2 Jul 2010 17:00
Picon
Picon

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

On Fri, Jun 25, 2010 at 09:12:50AM +1000, Brenden Bain wrote:
> 2010/6/24 Måns Rullgård <mans <at> mansr.com>:
> > Brenden Bain <brenden.bain <at> gmail.com> writes:
> >
> >> Index: libavdevice/x11grab.c
> >> ===================================================================
> >> --- libavdevice/x11grab.c     (revision 23755)
> >> +++ libavdevice/x11grab.c     (working copy)
> >>  <at>  <at>  -91,7 +91,7  <at>  <at> 
> >>      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);
> >>  <at>  <at>  -115,6 +115,11  <at>  <at> 
> >>          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);
> >>  <at>  <at>  -245,8 +250,9  <at>  <at>
(Continue reading)

Permalink | Reply |
headers
Brenden Bain | 3 Jul 2010 09:20
Picon

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

On Fri, 2010-07-02 at 17:00 +0200, Michael Niedermayer wrote:
> >  x11grab.c |   20 +++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> > 232ef00cd548c0a8ab48d48ec883d687b0e46eff  x11_segfault_fix.patch
> > Index: libavdevice/x11grab.c
> > ===================================================================
> > --- libavdevice/x11grab.c     (revision 23763)
> > +++ libavdevice/x11grab.c     (working copy)
> >  <at>  <at>  -91,7 +91,7  <at>  <at> 
> >      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);
> >  <at>  <at>  -115,6 +115,11  <at>  <at> 
> >          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);
(Continue reading)

Permalink | Reply |
headers
Michael Niedermayer | 3 Jul 2010 19:21
Picon
Picon

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

On Sat, Jul 03, 2010 at 05:20:52PM +1000, Brenden Bain wrote:
> On Fri, 2010-07-02 at 17:00 +0200, Michael Niedermayer wrote:
> > >  x11grab.c |   20 +++++++++++++++++---
> > >  1 file changed, 17 insertions(+), 3 deletions(-)
> > > 232ef00cd548c0a8ab48d48ec883d687b0e46eff  x11_segfault_fix.patch
> > > Index: libavdevice/x11grab.c
> > > ===================================================================
> > > --- libavdevice/x11grab.c     (revision 23763)
> > > +++ libavdevice/x11grab.c     (working copy)
> > >  <at>  <at>  -91,7 +91,7  <at>  <at> 
> > >      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);
> > >  <at>  <at>  -115,6 +115,11  <at>  <at> 
> > >          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) {
(Continue reading)

Permalink | Reply |

Gmane