| Message ID | 20190723.122305.989255413260855558.aran@wwu.edu |
|---|---|
| State | New |
| Headers | show |
On 7/23/19, Aran.Clauson@wwu.edu <Aran.Clauson@wwu.edu> wrote: > From: Chad Fraleigh <chadf@triularity.org> >> >> On 7/22/2019 11:14 AM, Aran.Clauson@wwu.edu wrote: >> >>> +static void rm_shmid(AVFormatContext *s) { >>> + XCBGrabContext *c = s->priv_data; >>> + if(c->shmid != -1) { >>> + shmctl(c->shmid, IPC_RMID, 0); >>> + c->shmid == -1; >> ^^^^^^^^^^^^^^ >> Assignment/compare operator mismatch. >> >> >>> + } >>> +} >>> + > > Well, that's embarrassing. Do not simply concat your patch to rest of mail. Please use git format-patch and then attach it to new mail. Or use git send-email. > > diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c > index b7e689343e..1acf3cdf28 100644 > --- a/libavdevice/xcbgrab.c > +++ b/libavdevice/xcbgrab.c > @@ -75,6 +75,7 @@ typedef struct XCBGrabContext { > const char *framerate; > > int has_shm; > + int shmid; > } XCBGrabContext; > > #define FOLLOW_CENTER -1 > @@ -221,6 +222,14 @@ static int check_shm(xcb_connection_t *conn) > return 0; > } > > +static void rm_shmid(AVFormatContext *s) { > + XCBGrabContext *c = s->priv_data; > + if(c->shmid != -1) { > + shmctl(c->shmid, IPC_RMID, 0); > + c->shmid = -1; > + } > +} > + > static int allocate_shm(AVFormatContext *s) > { > XCBGrabContext *c = s->priv_data; > @@ -230,7 +239,8 @@ static int allocate_shm(AVFormatContext *s) > > if (c->buffer) > return 0; > - id = shmget(IPC_PRIVATE, size, IPC_CREAT | 0777); > + > + id = shmget(IPC_PRIVATE, size, IPC_CREAT | 0666); > if (id == -1) { > char errbuf[1024]; > int err = AVERROR(errno); > @@ -239,15 +249,20 @@ static int allocate_shm(AVFormatContext *s) > size, errbuf); > return err; > } > + > xcb_shm_attach(c->conn, c->segment, id, 0); > data = shmat(id, NULL, 0); > - shmctl(id, IPC_RMID, 0); > - if ((intptr_t)data == -1 || !data) > - return AVERROR(errno); > + > + if ((intptr_t)data == -1 || !data) { > + shmctl(id, IPC_RMID, 0); > + return AVERROR(errno); > + } > c->buffer = data; > + c->shmid = id; > return 0; > } > > + > static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt) > { > XCBGrabContext *c = s->priv_data; > @@ -268,6 +283,8 @@ static int xcbgrab_frame_shm(AVFormatContext *s, > AVPacket *pkt) > > xcb_flush(c->conn); > > + rm_shmid(s); > + > if (e) { > av_log(s, AV_LOG_ERROR, > "Cannot get the image data " > > >
On Tue, Jul 23, 2019 at 19:23:12 +0000, Aran.Clauson@wwu.edu wrote: Some style nits for a first time contributer: > + if(c->shmid != -1) { ^ Please stick to the style "if (" (whitespace). > - id = shmget(IPC_PRIVATE, size, IPC_CREAT | 0777); > + > + id = shmget(IPC_PRIVATE, size, IPC_CREAT | 0666); Is this change required for your fix? And please don't introduce arbitrary new empty lines and such. > } > + > xcb_shm_attach(c->conn, c->segment, id, 0); Needless change, please remove. > } > > + > static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt) Needless change, please remove. Cheers, Moritz
diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c index b7e689343e..1acf3cdf28 100644 --- a/libavdevice/xcbgrab.c +++ b/libavdevice/xcbgrab.c @@ -75,6 +75,7 @@ typedef struct XCBGrabContext { const char *framerate; int has_shm; + int shmid; } XCBGrabContext; #define FOLLOW_CENTER -1 @@ -221,6 +222,14 @@ static int check_shm(xcb_connection_t *conn) return 0; } +static void rm_shmid(AVFormatContext *s) { + XCBGrabContext *c = s->priv_data; + if(c->shmid != -1) { + shmctl(c->shmid, IPC_RMID, 0); + c->shmid = -1; + } +} + static int allocate_shm(AVFormatContext *s) { XCBGrabContext *c = s->priv_data; @@ -230,7 +239,8 @@ static int allocate_shm(AVFormatContext *s) if (c->buffer) return 0; - id = shmget(IPC_PRIVATE, size, IPC_CREAT | 0777); + + id = shmget(IPC_PRIVATE, size, IPC_CREAT | 0666); if (id == -1) { char errbuf[1024]; int err = AVERROR(errno); @@ -239,15 +249,20 @@ static int allocate_shm(AVFormatContext *s) size, errbuf); return err; } + xcb_shm_attach(c->conn, c->segment, id, 0); data = shmat(id, NULL, 0); - shmctl(id, IPC_RMID, 0); - if ((intptr_t)data == -1 || !data) - return AVERROR(errno); + + if ((intptr_t)data == -1 || !data) { + shmctl(id, IPC_RMID, 0); + return AVERROR(errno); + } c->buffer = data; + c->shmid = id; return 0; } + static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt) { XCBGrabContext *c = s->priv_data; @@ -268,6 +283,8 @@ static int xcbgrab_frame_shm(AVFormatContext *s, AVPacket *pkt) xcb_flush(c->conn); + rm_shmid(s); + if (e) { av_log(s, AV_LOG_ERROR, "Cannot get the image data "