<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [r600g][GLAMOR] Crash when viewing a movie full screen with mplayer"
href="https://bugs.freedesktop.org/show_bug.cgi?id=69683#c16">Comment # 16</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [r600g][GLAMOR] Crash when viewing a movie full screen with mplayer"
href="https://bugs.freedesktop.org/show_bug.cgi?id=69683">bug 69683</a>
from <span class="vcard"><a class="email" href="mailto:zhigang.gong@gmail.com" title="Zhigang Gong <zhigang.gong@gmail.com>"> <span class="fn">Zhigang Gong</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=69683#c14">comment #14</a>)
<span class="quote">> (In reply to <a href="show_bug.cgi?id=69683#c11">comment #11</a>)
> > Did you also get the following log information:
> >
> > [ 71790.287] (II) fail to get matched format for 7893d260
> > [ 71790.287] (II) fail to get matched format for 7893d260
> >
>
> Yes.
>
> > From the back trace log, glamor fail to download a valid sub_pixmap for
> > the input 1x1 pixmap. Please see code in glamor_get_sub_pixmap().
> >
> > ...
> > data = glamor_download_sub_pixmap_to_cpu(pixmap, x, y, w, h,
> > sub_pixmap->devKind,
> > data, pbo, access);
> > if (pbo) {
> > //we should check whether the data is NULL.
> > // if the data is NULL, we should destroy the sub pixmap and
> > return a NULL pointer. And we should also back trace why we get a NULL
> > pointer.
> > assert(sub_pixmap->devPrivate.ptr == NULL);
> > sub_pixmap->devPrivate.ptr = data;
> > sub_pixmap_priv->base.fbo->pbo_valid = 1;
> > }
> > ...
> >
>
> I have made another patch to glamor now.
>
> diff --git a/src/glamor_pixmap.c b/src/glamor_pixmap.c
> index 9bbc989..52c3c64 100644
> --- a/src/glamor_pixmap.c
> +++ b/src/glamor_pixmap.c
> @@ -1377,6 +1377,10 @@ glamor_get_sub_pixmap(PixmapPtr pixmap, int x, int y,
> int w, int h, glamor_acces
>
> data = glamor_download_sub_pixmap_to_cpu(pixmap, x, y, w, h,
> sub_pixmap->devKind,
> data, pbo, access);
> + if(data == NULL) {
> + fbDestroyPixmap(sub_pixmap);
> + return NULL;
> + }
> if (pbo) {
> assert(sub_pixmap->devPrivate.ptr == NULL);
> sub_pixmap->devPrivate.ptr = data;
>
> It seems to fix the bug. From the code, I have saw it:</span >
Good.
<span class="quote">>
> /*
> * Download a sub region of pixmap to a specified memory region.
> * The pixmap must have a valid FBO, otherwise return a NULL.
> * */
>
> static void *
> _glamor_download_sub_pixmap_to_cpu(PixmapPtr pixmap, GLenum format,
>
> It is expected that the function should return NULL if the pixmap has not a
> valid FBO.
> So the code is wrong when calls the function and doesn't check for NULL.
> Or the pixmap should always had a valid FBO??</span >
I
<span class="quote">> I have another question. The data will be used only in if(pob){ }.
> Why not do this:
>
> if (pbo) {
> assert(sub_pixmap->devPrivate.ptr == NULL);
> data = glamor_download_sub_pixmap_to_cpu(pixmap, x, y, w, h,
> sub_pixmap->devKind,
> data, pbo, access);
> sub_pixmap->devPrivate.ptr = data;
> sub_pixmap_priv->base.fbo->pbo_valid = 1;
> }</span >
No, You can't change it that way, as for non-pbo mode, we pass in a null data
to the glamor_download_sub_pixmap_to_cpu, and then that function will use the
devPrivate.ptr directly and download required content to that memory region.
If pass in Null data pointer, that function will do a map to the pbo, and
return
the mapped data pointer. So with or without pbo we both need to call that
function.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>