From 9592b0af2a84f0e64c30225bc80013cbc814e159 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Thu, 13 Jun 2024 10:11:29 +0200 Subject: [PATCH] examples: fix mapoffset in examples The mapoffset should be using in the mmap call as the offset. Mapping the whole memory before the offset and then ignoring the part before it seems like it can work but it actually has some problems: 1. some drivers (v4l2) use the mapoffset to calculate the buffer to map 2. we waste resources for mapped but unused pages. The problem with the mapoffset is that it needs to be page aligned and that used to be a problem in the past. Nowadays PipeWire no longer set the mapoffset for any of the memory that it allocates. --- spa/examples/local-libcamera.c | 8 ++++---- spa/examples/local-v4l2.c | 8 ++++---- src/examples/export-sink.c | 8 ++++---- src/examples/export-source.c | 5 ++--- src/examples/local-v4l2.c | 8 ++++---- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/spa/examples/local-libcamera.c b/spa/examples/local-libcamera.c index f91d2db68..2a4d8283d 100644 --- a/spa/examples/local-libcamera.c +++ b/spa/examples/local-libcamera.c @@ -190,11 +190,11 @@ static int on_source_ready(void *_data, int status) sdata = datas[0].data; if (datas[0].type == SPA_DATA_MemFd || datas[0].type == SPA_DATA_DmaBuf) { - map = mmap(NULL, datas[0].maxsize + datas[0].mapoffset, PROT_READ, - MAP_PRIVATE, datas[0].fd, 0); + map = mmap(NULL, datas[0].maxsize, PROT_READ, + MAP_PRIVATE, datas[0].fd, datas[0].mapoffset); if (map == MAP_FAILED) return -errno; - sdata = SPA_PTROFF(map, datas[0].mapoffset, uint8_t); + sdata = map; } else if (datas[0].type == SPA_DATA_MemPtr) { map = NULL; sdata = datas[0].data; @@ -215,7 +215,7 @@ static int on_source_ready(void *_data, int status) SDL_RenderPresent(data->renderer); if (map) - munmap(map, datas[0].maxsize + datas[0].mapoffset); + munmap(map, datas[0].maxsize); } if ((res = spa_node_process(data->source)) < 0) diff --git a/spa/examples/local-v4l2.c b/spa/examples/local-v4l2.c index 7febebb4f..4e7000da3 100644 --- a/spa/examples/local-v4l2.c +++ b/spa/examples/local-v4l2.c @@ -185,11 +185,11 @@ static int on_source_ready(void *_data, int status) sdata = datas[0].data; if (datas[0].type == SPA_DATA_MemFd || datas[0].type == SPA_DATA_DmaBuf) { - map = mmap(NULL, datas[0].maxsize + datas[0].mapoffset, PROT_READ, - MAP_PRIVATE, datas[0].fd, 0); + map = mmap(NULL, datas[0].maxsize, PROT_READ, + MAP_PRIVATE, datas[0].fd, datas[0].mapoffset); if (map == MAP_FAILED) return -errno; - sdata = SPA_PTROFF(map, datas[0].mapoffset, uint8_t); + sdata = map; } else if (datas[0].type == SPA_DATA_MemPtr) { map = NULL; sdata = datas[0].data; @@ -210,7 +210,7 @@ static int on_source_ready(void *_data, int status) SDL_RenderPresent(data->renderer); if (map) - munmap(map, datas[0].maxsize + datas[0].mapoffset); + munmap(map, datas[0].maxsize); } if ((res = spa_node_process(data->source)) < 0) diff --git a/src/examples/export-sink.c b/src/examples/export-sink.c index 76420861f..be1e39689 100644 --- a/src/examples/export-sink.c +++ b/src/examples/export-sink.c @@ -369,9 +369,9 @@ static int do_render(struct spa_loop *loop, bool async, uint32_t seq, if (buf->datas[0].type == SPA_DATA_MemFd || buf->datas[0].type == SPA_DATA_DmaBuf) { - map = mmap(NULL, buf->datas[0].maxsize + buf->datas[0].mapoffset, PROT_READ, - MAP_PRIVATE, buf->datas[0].fd, 0); - sdata = SPA_PTROFF(map, buf->datas[0].mapoffset, uint8_t); + map = mmap(NULL, buf->datas[0].maxsize, PROT_READ, + MAP_PRIVATE, buf->datas[0].fd, buf->datas[0].mapoffset); + sdata = map; } else if (buf->datas[0].type == SPA_DATA_MemPtr) { map = NULL; sdata = buf->datas[0].data; @@ -413,7 +413,7 @@ static int do_render(struct spa_loop *loop, bool async, uint32_t seq, SDL_RenderPresent(d->renderer); if (map) - munmap(map, buf->datas[0].maxsize + buf->datas[0].mapoffset); + munmap(map, buf->datas[0].maxsize); return 0; } diff --git a/src/examples/export-source.c b/src/examples/export-source.c index b266a53c8..54e1fc332 100644 --- a/src/examples/export-source.c +++ b/src/examples/export-source.c @@ -319,14 +319,13 @@ static int impl_port_use_buffers(void *object, } else if (datas[0].type == SPA_DATA_MemFd || datas[0].type == SPA_DATA_DmaBuf) { - b->ptr = mmap(NULL, datas[0].maxsize + datas[0].mapoffset, PROT_WRITE, - MAP_SHARED, datas[0].fd, 0); + b->ptr = mmap(NULL, datas[0].maxsize, PROT_WRITE, + MAP_SHARED, datas[0].fd, datas[0].mapoffset); if (b->ptr == MAP_FAILED) { pw_log_error("failed to buffer mem"); return -errno; } - b->ptr = SPA_PTROFF(b->ptr, datas[0].mapoffset, void); b->mapped = true; } else { diff --git a/src/examples/local-v4l2.c b/src/examples/local-v4l2.c index db8bda0e3..b8ce37fa6 100644 --- a/src/examples/local-v4l2.c +++ b/src/examples/local-v4l2.c @@ -269,9 +269,9 @@ static int do_render(struct spa_loop *loop, bool async, uint32_t seq, if (buf->datas[0].type == SPA_DATA_MemFd || buf->datas[0].type == SPA_DATA_DmaBuf) { - map = mmap(NULL, buf->datas[0].maxsize + buf->datas[0].mapoffset, PROT_READ, - MAP_PRIVATE, buf->datas[0].fd, 0); - sdata = SPA_PTROFF(map, buf->datas[0].mapoffset, uint8_t); + map = mmap(NULL, buf->datas[0].maxsize, PROT_READ, + MAP_PRIVATE, buf->datas[0].fd, buf->datas[0].mapoffset); + sdata = map; } else if (buf->datas[0].type == SPA_DATA_MemPtr) { map = NULL; sdata = buf->datas[0].data; @@ -299,7 +299,7 @@ static int do_render(struct spa_loop *loop, bool async, uint32_t seq, SDL_RenderPresent(d->renderer); if (map) - munmap(map, buf->datas[0].maxsize + buf->datas[0].mapoffset); + munmap(map, buf->datas[0].maxsize); return 0; }