April 2022 - Wine-Devel - list.winehq.org

[PATCH v3 0/9] MR6: Avoid performance degradation due to vDSO unmapping (BZ#52313)

by Jinoh Kang (＠iamahuman)

Commit f558741fabc116534fa598aa890ffed683a7153b removes vDSO if it conflicts with reserved ranges: > Remove the AT_SYSINFO and AT_SYSINFO_EHDR values if the sysinfo page > is in one of our reserved ranges. However, missing vDSO leads to performance issues on some syscalls (e.g. clock_gettime, gettimeofday) and may even lead to crash when run with some ancient C libraries that does not supply a custom signal restorer. vDSO pages can clash with reserved ranges especially in a 32-bit address space with address space layout randomization (ASLR) turned on. Recent versions of the Linux kernel introduced support for mremap()-ping vDSO pages, partly in an effort to support checkpoint restore in userspace (CRIU). Special programs that require specific memory layout constraints (such as Wine preloader) can take advantage of this support to modify the address space to meet its requirements. The following test script has been used to test each changes (use with `git rebase --exec=...`): ```sh set -e make -C ../wine64-build -j5 make -C ../wine32-build -j5 cd ../wine64-build export WINEPRELOADREMAPSTACK export WINEPRELOADREMAPVDSO for WINEPRELOADREMAPSTACK in skip never always force auto on-demand '' do for WINEPRELOADREMAPVDSO in skip never always force auto on-demand '' do ./loader/wine64 wineboot ./loader/wine wineboot done done ``` -- v3: loader: Switch stack if the old stack address is in reserved range. loader: Relocate sigpage on conflict with reserved ranges in ARM. loader: Relocate vDSO on conflict with reserved ranges. loader: Fix return type of get_auxiliary(). loader: Don't clobber existing memory mappings when reserving addresses. loader: Explicitly munmap() the preloader's ELF EHDR. loader: Generalise is_addr_reserved to find overlapping address ranges. loader: Refactor number parsing to own function. loader: Refactor argv/envp/auxv management. https://gitlab.winehq.org/wine/wine/-/merge_requests/6

April 30, 2022

Re: [PATCH] msadpm: Stop decoding instead of crashing for invalid adpcm

by Eric Pouech

Le 30/04/2022 à 05:28, Moore, Brandon A. a écrit : Hi Brandon, two points: - the lines around your patch don't use tabs, please don't introduce them - I don't see why you need to test src+1? (il you copied it from the other routine, it's for stereo handling, while the one you patched is for mono handling, hence a single adpcm coeff) A+

April 30, 2022

[PATCH] msadpm: Stop decoding instead of crashing for invalid adpcm

by Moore, Brandon A.

April 30, 2022

[PATCH] msadpm: Stop decoding instead of crashing for invalid adpcm data.

by Moore, Brandon A.

Apply the same patch from 72528be84fdc for adpcm data sent to mono destinations in addition to stereo destinations. Signed-off-by: Brandon Moore <moore.3071(a)osu.edu> --- dlls/msadp32.acm/msadp32.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dlls/msadp32.acm/msadp32.c b/dlls/msadp32.acm/msadp32.c index 2dc11b9239a..a68b13d58f6 100644 --- a/dlls/msadp32.acm/msadp32.c +++ b/dlls/msadp32.acm/msadp32.c @@ -319,7 +319,14 @@ static void cvtMMms16K(const ACMDRVSTREAMINSTANCE *adsi, { const unsigned char* in_src = src; - assert(*src <= 6); + /* Catch a problem from Lord of the Rings War of the Ring where it + * passes invalid data. */ + if (*src > 6 || *(src + 1) > 6) + { + *ndst -= nblock * nsamp_blk * adsi->pwfxDst->nBlockAlign; + WARN("Invalid ADPCM data, stopping conversion\n"); + break; + } coeff = MSADPCM_CoeffSet[*src++]; idelta = R16(src); src += 2; -- 2.35.1

April 30, 2022

[PATCH] wined3d: Do not use vkCmdClearColorImage() to clear compressed images.

by Zebediah Figura

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52922 Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/wined3d/texture.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 5fd38b49132..49650f2839e 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -5257,9 +5257,10 @@ static void wined3d_texture_vk_download_data(struct wined3d_context *context, } } -static void wined3d_texture_vk_clear(struct wined3d_texture_vk *texture_vk, +static bool wined3d_texture_vk_clear(struct wined3d_texture_vk *texture_vk, unsigned int sub_resource_idx, struct wined3d_context *context) { + struct wined3d_texture_sub_resource *sub_resource = &texture_vk->t.sub_resources[sub_resource_idx]; struct wined3d_context_vk *context_vk = wined3d_context_vk(context); const struct wined3d_format *format = texture_vk->t.resource.format; const struct wined3d_vk_info *vk_info = context_vk->vk_info; @@ -5270,12 +5271,24 @@ static void wined3d_texture_vk_clear(struct wined3d_texture_vk *texture_vk, VkImageAspectFlags aspect_mask; VkImage vk_image; + if (texture_vk->t.resource.format_flags & WINED3DFMT_FLAG_COMPRESSED) + { + struct wined3d_bo_address addr; + + if (!wined3d_texture_prepare_location(&texture_vk->t, sub_resource_idx, context, WINED3D_LOCATION_SYSMEM)) + return false; + wined3d_texture_get_bo_address(&texture_vk->t, sub_resource_idx, &addr, WINED3D_LOCATION_SYSMEM); + memset(addr.addr, 0, sub_resource->size); + wined3d_texture_validate_location(&texture_vk->t, sub_resource_idx, WINED3D_LOCATION_SYSMEM); + return true; + } + vk_image = texture_vk->image.vk_image; if (!(vk_command_buffer = wined3d_context_vk_get_command_buffer(context_vk))) { ERR("Failed to get command buffer.\n"); - return; + return false; } aspect_mask = vk_aspect_mask_from_format(format); @@ -5305,6 +5318,9 @@ static void wined3d_texture_vk_clear(struct wined3d_texture_vk *texture_vk, VK_ACCESS_TRANSFER_WRITE_BIT, vk_access_mask_from_bind_flags(texture_vk->t.resource.bind_flags), VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, texture_vk->layout, vk_image, &vk_range); wined3d_context_vk_reference_texture(context_vk, texture_vk); + + wined3d_texture_validate_location(&texture_vk->t, sub_resource_idx, WINED3D_LOCATION_TEXTURE_RGB); + return true; } static BOOL wined3d_texture_vk_load_texture(struct wined3d_texture_vk *texture_vk, @@ -5319,8 +5335,11 @@ static BOOL wined3d_texture_vk_load_texture(struct wined3d_texture_vk *texture_v if (sub_resource->locations & WINED3D_LOCATION_CLEARED) { - wined3d_texture_vk_clear(texture_vk, sub_resource_idx, context); - return TRUE; + if (!wined3d_texture_vk_clear(texture_vk, sub_resource_idx, context)) + return FALSE; + + if (sub_resource->locations & WINED3D_LOCATION_TEXTURE_RGB) + return TRUE; } if (!(sub_resource->locations & wined3d_texture_sysmem_locations)) -- 2.35.1

April 29, 2022

Re: [PATCH v7 3/3] shell32: Partially implement IShellItemImageFactory (icon only, no thumbnail).

by Jinoh Kang

On 4/29/22 23:44, Nikolay Sivov wrote: > I think this needs a lot of cleanup. > > On 4/26/22 21:17, Jinoh Kang wrote: >> >> >> + >> typedef struct _ShellItem { >> IShellItem2 IShellItem2_iface; >> LONG ref; >> @@ -565,17 +568,205 @@ static ULONG WINAPI ShellItem_IShellItemImageFactory_Release(IShellItemImageFact >> return IShellItem2_Release(&This->IShellItem2_iface); >> } >> +static HRESULT ShellItem_get_icons(ShellItem *This, SIZE size, HICON *big_icon, HICON *small_icon) > > You don't really need two icons, for SIIGBF_BIGGERSIZEOK. SIIGBF_BIGGERSIZEOK does not appear to signify that the size parameter can be entirely disregarded. Windows still takes the size argument as a hint. Maybe it needs more testing, but the impression I got is that it would choose the image that will experience the *least* distortion should it be eventually resized to the given size. It would then actually perform the resize if SIIGBF_RESIZETOFIT is specified; otherwise, the application is supposed to do the resize as needed. Although returning an image of arbitrary size isn't *contractually wrong* per se, it may choose a worse candidate for shrinking: for example, icons tend to have the same text size in both versions; thus, resizing a bigger icon would render the text unreadable. Also, it doesn't match Windows behavior anyway. >> +{ >> + HRESULT hr; >> + IBindCtx *pbc; >> + IExtractIconW *ei; >> + WCHAR icon_file[MAX_PATH]; >> + INT source_index; >> + UINT gil_in_flags = 0, gil_out_flags; >> + INT iconsize; >> + >> + iconsize = min(size.cx, size.cy); >> + if (iconsize <= 0 || iconsize > 0x7fff) >> + iconsize = 0x7fff; >> + >> + hr = CreateBindCtx(0, &pbc); >> + if (FAILED(hr)) goto done; >> + >> + hr = IShellItem2_BindToHandler(&This->IShellItem2_iface, pbc, &BHID_SFUIObject, >> + &IID_IExtractIconW, (void **)&ei); >> + IBindCtx_Release(pbc); >> + if (FAILED(hr)) goto done; >> + >> + hr = IExtractIconW_GetIconLocation(ei, gil_in_flags, icon_file, MAX_PATH, &source_index, &gil_out_flags); >> + if (FAILED(hr)) goto free_ei; > > You probably can get rid of gil_in_flags and pbc. For gil_in_flags: ACK. I just wanted to clarify the purpose of the parameter; maybe it wasn't a good idea to unnecessarily make a constant-as-of-now a variable after all. For pbc: While the IBindCtx is currently ignored, wouldn't it make sense to pass something valid as required by the BindToHandler's interface definition anyway? > >> + >> + if (!(gil_out_flags & GIL_NOTFILENAME)) >> + { >> + UINT ei_res; >> + >> + if (source_index == -1) >> + source_index = 0; /* special case for some control panel applications */ >> + >> + FIXME("%s %d\n", debugstr_w(icon_file), source_index); >> + ei_res = ExtractIconExW(icon_file, source_index, big_icon, small_icon, 1); >> + if (!ei_res || ei_res == (UINT)-1) >> + { >> + WARN("Failed to extract icon.\n"); >> + hr = E_FAIL; >> + } > > Instead of this you can probably do SHGetFileInfo(SHGFI_SYSICONINDEX), and then pick appropriate imagelist. That would preclude GIL_NOTFILENAME in Wine's current implementation; maybe it doesn't matter that much anyway... >> + } >> + else >> + { >> + hr = IExtractIconW_Extract(ei, icon_file, source_index, big_icon, small_icon, MAKELONG(iconsize, iconsize)); >> + } >> + >> +free_ei: >> + IExtractIconW_Release(ei); >> +done: >> + return hr; >> +} >> + >> +static HICON choose_best_icon(HICON *icons, UINT count, SIZE size_limit, SIZE *out_size) >> +{ >> + HICON best_icon = NULL; >> + SIZE best_size = {0, 0}; >> + UINT i; >> + >> + for (i = 0; i < count; i++) >> + { >> + ICONINFO iinfo; >> + BITMAP bm; >> + SIZE size; >> + BOOL is_color, ret; >> + >> + if (!icons[i] || !GetIconInfo(icons[i], &iinfo)) continue; >> + >> + is_color = iinfo.hbmColor != NULL; >> + ret = GetObjectW(is_color ? iinfo.hbmColor : iinfo.hbmMask, sizeof(bm), &bm); >> + DeleteObject(iinfo.hbmColor); >> + DeleteObject(iinfo.hbmMask); >> + if (!ret) continue; >> + >> + size.cx = bm.bmWidth; >> + size.cy = is_color ? abs(bm.bmHeight) : abs(bm.bmHeight) / 2; >> + >> + if (!best_icon || (best_size.cx < size.cx && size.cx <= size_limit.cx && >> + best_size.cy < size.cy && size.cy <= size_limit.cy)) >> + { >> + best_icon = icons[i]; >> + best_size = size; >> + } >> + } >> + >> + *out_size = best_size; >> + return best_icon; >> +} > > This looks too complicated. System imaglists come in few sizes. Not all icons belong to the system imagelist, are they? > Desired size is know on GetImage(), I think it make sense to look for exact match first, and load that. I don't see how that would simplify the algorithm. An exact match may not always exist, but only subpar candidates. > For non-file based case Extract() has size argument on its own, should that do something to pick "the best" size? My intention was not to rely on Extract() to actually do what we want, even as we provide the hint. > >> + >> +static HRESULT ShellItem_get_icon_bitmap(ShellItem *This, IWICImagingFactory *imgfactory, >> + SIZE size, SIIGBF flags, IWICBitmap **bitmap) >> +{ >> + HRESULT hr; >> + HICON icons[2] = { NULL, NULL }, best_icon; >> + SIZE best_icon_size; >> + UINT i; >> + >> + *bitmap = NULL; >> + >> + hr = ShellItem_get_icons(This, size, &icons[0], &icons[1]); >> + if (FAILED(hr)) return hr; >> + >> + best_icon = choose_best_icon(icons, ARRAY_SIZE(icons), size, &best_icon_size); >> + for (i = 0; i < ARRAY_SIZE(icons); i++) >> + if (icons[i] && icons[i] != best_icon) DeleteObject(icons[i]); >> + >> + if (!best_icon) return E_FAIL; >> + >> + hr = IWICImagingFactory_CreateBitmapFromHICON(imgfactory, best_icon, bitmap); >> + DeleteObject(best_icon); >> + return hr; >> +} >> + >> +static HRESULT convert_wicbitmapsource_to_gdi(IWICImagingFactory *imgfactory, >> + IWICBitmapSource *bitmapsource, HBITMAP *gdibitmap) >> +{ >> + BITMAPINFOHEADER bmi; >> + HRESULT hr; >> + UINT width, height; >> + IWICBitmapSource *newsrc; >> + HDC dc; >> + HBITMAP bm; >> + void *bits; >> + >> + *gdibitmap = NULL; >> + >> + hr = WICConvertBitmapSource(&GUID_WICPixelFormat32bppBGRA, bitmapsource, &newsrc); >> + if (FAILED(hr)) goto done; >> + >> + hr = IWICBitmapSource_GetSize(newsrc, &width, &height); >> + if (FAILED(hr)) goto free_newsrc; >> + >> + dc = CreateCompatibleDC(NULL); >> + if (!dc) >> + { >> + hr = E_FAIL; >> + goto free_newsrc; >> + } >> + >> + memset(&bmi, 0, sizeof(bmi)); >> + bmi.biSize = sizeof(bmi); >> + bmi.biWidth = width; >> + bmi.biHeight = -height; >> + bmi.biPlanes = 1; >> + bmi.biBitCount = 32; >> + bmi.biCompression = BI_RGB; >> + >> + bm = CreateDIBSection(dc, (const BITMAPINFO *)&bmi, DIB_RGB_COLORS, &bits, NULL, 0); >> + DeleteDC(dc); > I don't think you need a device context for this. ACK. TIL that CreateDIBSection(NULL, ..., DIB_RGB_COLORS, ...) is legal. > >> + if (!bm) >> + { >> + WARN("Cannot create bitmap.\n"); >> + hr = E_FAIL; >> + goto free_newsrc; >> + } >> + >> + hr = IWICBitmapSource_CopyPixels(newsrc, NULL, width * 4, width * height * 4, bits); >> + if (FAILED(hr)) >> + { >> + DeleteObject(bm); >> + goto free_newsrc; >> + } >> + >> + hr = S_OK; >> + *gdibitmap = bm; >> + >> +free_newsrc: >> + IWICBitmapSource_Release(newsrc); >> +done: >> + return hr; >> +} >> + >> static HRESULT WINAPI ShellItem_IShellItemImageFactory_GetImage(IShellItemImageFactory *iface, >> SIZE size, SIIGBF flags, HBITMAP *phbm) >> { >> ShellItem *This = impl_from_IShellItemImageFactory(iface); >> + HRESULT hr; >> + IWICImagingFactory *imgfactory; >> + IWICBitmap *bitmap = NULL; >> static int once; >> if (!once++) >> - FIXME("%p ({%lu, %lu} %d %p): stub\n", This, size.cx, size.cy, flags, phbm); >> + FIXME("%p ({%lu, %lu} %d %p): partial stub\n", This, size.cx, size.cy, flags, phbm); >> *phbm = NULL; >> - return E_NOTIMPL; >> + >> + if (flags != SIIGBF_BIGGERSIZEOK) return E_NOTIMPL; >> + >> + hr = WICCreateImagingFactory_Proxy(WINCODEC_SDK_VERSION, &imgfactory); >> + if (SUCCEEDED(hr)) >> + { >> + hr = ShellItem_get_icon_bitmap(This, imgfactory, size, flags, &bitmap); >> + if (SUCCEEDED(hr)) >> + { >> + hr = convert_wicbitmapsource_to_gdi(imgfactory, (IWICBitmapSource *)bitmap, phbm); >> + IWICBitmap_Release(bitmap); >> + } >> + IWICImagingFactory_Release(imgfactory); >> + } >> + >> + return hr; >> } > -- Sincerely, Jinoh Kang

April 29, 2022

Re: [PATCH v7 3/3] shell32: Partially implement IShellItemImageFactory (icon only, no thumbnail).

by Nikolay Sivov

I think this needs a lot of cleanup. On 4/26/22 21:17, Jinoh Kang wrote: > > > + > typedef struct _ShellItem { > IShellItem2 IShellItem2_iface; > LONG ref; > @@ -565,17 +568,205 @@ static ULONG WINAPI ShellItem_IShellItemImageFactory_Release(IShellItemImageFact > return IShellItem2_Release(&This->IShellItem2_iface); > } > > +static HRESULT ShellItem_get_icons(ShellItem *This, SIZE size, HICON *big_icon, HICON *small_icon) You don't really need two icons, for SIIGBF_BIGGERSIZEOK. > +{ > + HRESULT hr; > + IBindCtx *pbc; > + IExtractIconW *ei; > + WCHAR icon_file[MAX_PATH]; > + INT source_index; > + UINT gil_in_flags = 0, gil_out_flags; > + INT iconsize; > + > + iconsize = min(size.cx, size.cy); > + if (iconsize <= 0 || iconsize > 0x7fff) > + iconsize = 0x7fff; > + > + hr = CreateBindCtx(0, &pbc); > + if (FAILED(hr)) goto done; > + > + hr = IShellItem2_BindToHandler(&This->IShellItem2_iface, pbc, &BHID_SFUIObject, > + &IID_IExtractIconW, (void **)&ei); > + IBindCtx_Release(pbc); > + if (FAILED(hr)) goto done; > + > + hr = IExtractIconW_GetIconLocation(ei, gil_in_flags, icon_file, MAX_PATH, &source_index, &gil_out_flags); > + if (FAILED(hr)) goto free_ei; You probably can get rid of gil_in_flags and pbc. > + > + if (!(gil_out_flags & GIL_NOTFILENAME)) > + { > + UINT ei_res; > + > + if (source_index == -1) > + source_index = 0; /* special case for some control panel applications */ > + > + FIXME("%s %d\n", debugstr_w(icon_file), source_index); > + ei_res = ExtractIconExW(icon_file, source_index, big_icon, small_icon, 1); > + if (!ei_res || ei_res == (UINT)-1) > + { > + WARN("Failed to extract icon.\n"); > + hr = E_FAIL; > + } Instead of this you can probably do SHGetFileInfo(SHGFI_SYSICONINDEX), and then pick appropriate imagelist. > + } > + else > + { > + hr = IExtractIconW_Extract(ei, icon_file, source_index, big_icon, small_icon, MAKELONG(iconsize, iconsize)); > + } > + > +free_ei: > + IExtractIconW_Release(ei); > +done: > + return hr; > +} > + > +static HICON choose_best_icon(HICON *icons, UINT count, SIZE size_limit, SIZE *out_size) > +{ > + HICON best_icon = NULL; > + SIZE best_size = {0, 0}; > + UINT i; > + > + for (i = 0; i < count; i++) > + { > + ICONINFO iinfo; > + BITMAP bm; > + SIZE size; > + BOOL is_color, ret; > + > + if (!icons[i] || !GetIconInfo(icons[i], &iinfo)) continue; > + > + is_color = iinfo.hbmColor != NULL; > + ret = GetObjectW(is_color ? iinfo.hbmColor : iinfo.hbmMask, sizeof(bm), &bm); > + DeleteObject(iinfo.hbmColor); > + DeleteObject(iinfo.hbmMask); > + if (!ret) continue; > + > + size.cx = bm.bmWidth; > + size.cy = is_color ? abs(bm.bmHeight) : abs(bm.bmHeight) / 2; > + > + if (!best_icon || (best_size.cx < size.cx && size.cx <= size_limit.cx && > + best_size.cy < size.cy && size.cy <= size_limit.cy)) > + { > + best_icon = icons[i]; > + best_size = size; > + } > + } > + > + *out_size = best_size; > + return best_icon; > +} This looks too complicated. System imaglists come in few sizes. Desired size is know on GetImage(), I think it make sense to look for exact match first, and load that. For non-file based case Extract() has size argument on its own, should that do something to pick "the best" size? > + > +static HRESULT ShellItem_get_icon_bitmap(ShellItem *This, IWICImagingFactory *imgfactory, > + SIZE size, SIIGBF flags, IWICBitmap **bitmap) > +{ > + HRESULT hr; > + HICON icons[2] = { NULL, NULL }, best_icon; > + SIZE best_icon_size; > + UINT i; > + > + *bitmap = NULL; > + > + hr = ShellItem_get_icons(This, size, &icons[0], &icons[1]); > + if (FAILED(hr)) return hr; > + > + best_icon = choose_best_icon(icons, ARRAY_SIZE(icons), size, &best_icon_size); > + for (i = 0; i < ARRAY_SIZE(icons); i++) > + if (icons[i] && icons[i] != best_icon) DeleteObject(icons[i]); > + > + if (!best_icon) return E_FAIL; > + > + hr = IWICImagingFactory_CreateBitmapFromHICON(imgfactory, best_icon, bitmap); > + DeleteObject(best_icon); > + return hr; > +} > + > +static HRESULT convert_wicbitmapsource_to_gdi(IWICImagingFactory *imgfactory, > + IWICBitmapSource *bitmapsource, HBITMAP *gdibitmap) > +{ > + BITMAPINFOHEADER bmi; > + HRESULT hr; > + UINT width, height; > + IWICBitmapSource *newsrc; > + HDC dc; > + HBITMAP bm; > + void *bits; > + > + *gdibitmap = NULL; > + > + hr = WICConvertBitmapSource(&GUID_WICPixelFormat32bppBGRA, bitmapsource, &newsrc); > + if (FAILED(hr)) goto done; > + > + hr = IWICBitmapSource_GetSize(newsrc, &width, &height); > + if (FAILED(hr)) goto free_newsrc; > + > + dc = CreateCompatibleDC(NULL); > + if (!dc) > + { > + hr = E_FAIL; > + goto free_newsrc; > + } > + > + memset(&bmi, 0, sizeof(bmi)); > + bmi.biSize = sizeof(bmi); > + bmi.biWidth = width; > + bmi.biHeight = -height; > + bmi.biPlanes = 1; > + bmi.biBitCount = 32; > + bmi.biCompression = BI_RGB; > + > + bm = CreateDIBSection(dc, (const BITMAPINFO *)&bmi, DIB_RGB_COLORS, &bits, NULL, 0); > + DeleteDC(dc); I don't think you need a device context for this. > + if (!bm) > + { > + WARN("Cannot create bitmap.\n"); > + hr = E_FAIL; > + goto free_newsrc; > + } > + > + hr = IWICBitmapSource_CopyPixels(newsrc, NULL, width * 4, width * height * 4, bits); > + if (FAILED(hr)) > + { > + DeleteObject(bm); > + goto free_newsrc; > + } > + > + hr = S_OK; > + *gdibitmap = bm; > + > +free_newsrc: > + IWICBitmapSource_Release(newsrc); > +done: > + return hr; > +} > + > static HRESULT WINAPI ShellItem_IShellItemImageFactory_GetImage(IShellItemImageFactory *iface, > SIZE size, SIIGBF flags, HBITMAP *phbm) > { > ShellItem *This = impl_from_IShellItemImageFactory(iface); > + HRESULT hr; > + IWICImagingFactory *imgfactory; > + IWICBitmap *bitmap = NULL; > static int once; > > if (!once++) > - FIXME("%p ({%lu, %lu} %d %p): stub\n", This, size.cx, size.cy, flags, phbm); > + FIXME("%p ({%lu, %lu} %d %p): partial stub\n", This, size.cx, size.cy, flags, phbm); > > *phbm = NULL; > - return E_NOTIMPL; > + > + if (flags != SIIGBF_BIGGERSIZEOK) return E_NOTIMPL; > + > + hr = WICCreateImagingFactory_Proxy(WINCODEC_SDK_VERSION, &imgfactory); > + if (SUCCEEDED(hr)) > + { > + hr = ShellItem_get_icon_bitmap(This, imgfactory, size, flags, &bitmap); > + if (SUCCEEDED(hr)) > + { > + hr = convert_wicbitmapsource_to_gdi(imgfactory, (IWICBitmapSource *)bitmap, phbm); > + IWICBitmap_Release(bitmap); > + } > + IWICImagingFactory_Release(imgfactory); > + } > + > + return hr; > }

April 29, 2022

Re: [PATCH 6/6] wineoss: Move DRVM_INIT and DRVM_EXIT to the unixlib.

by Andrew Eikum

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Fri, Apr 29, 2022 at 08:29:58AM +0100, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > dlls/wineoss.drv/midi.c | 63 -------------------------------------- > dlls/wineoss.drv/oss.c | 1 - > dlls/wineoss.drv/ossmidi.c | 41 +++++++++++++++++++------ > dlls/wineoss.drv/unixlib.h | 7 ----- > 4 files changed, 31 insertions(+), 81 deletions(-) > > diff --git a/dlls/wineoss.drv/midi.c b/dlls/wineoss.drv/midi.c > index 84a4fac4b74..dda5dabf522 100644 > --- a/dlls/wineoss.drv/midi.c > +++ b/dlls/wineoss.drv/midi.c > @@ -34,19 +34,7 @@ > * timers (like select on fd) > */ > > -#include "config.h" > - > -#include <stdlib.h> > -#include <string.h> > #include <stdarg.h> > -#include <stdio.h> > -#include <sys/types.h> > -#include <unistd.h> > -#include <fcntl.h> > -#include <errno.h> > -#include <sys/ioctl.h> > -#include <poll.h> > -#include <sys/soundcard.h> > > #include "windef.h" > #include "winbase.h" > @@ -67,44 +55,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(midi); > * Low level MIDI implementation * > *======================================================================*/ > > -static int MIDI_loadcount; > -/************************************************************************** > - * OSS_MidiInit [internal] > - * > - * Initializes the MIDI devices information variables > - */ > -static LRESULT OSS_MidiInit(void) > -{ > - struct midi_init_params params; > - UINT err; > - > - TRACE("(%i)\n", MIDI_loadcount); > - if (MIDI_loadcount++) > - return 1; > - > - TRACE("Initializing the MIDI variables.\n"); > - > - params.err = &err; > - OSS_CALL(midi_init, &params); > - > - return err; > -} > - > -/************************************************************************** > - * OSS_MidiExit [internal] > - * > - * Release the MIDI devices information variables > - */ > -static LRESULT OSS_MidiExit(void) > -{ > - TRACE("(%i)\n", MIDI_loadcount); > - > - if (--MIDI_loadcount) > - return 1; > - > - return 0; > -} > - > static void notify_client(struct notify_context *notify) > { > TRACE("dev_id = %d msg = %d param1 = %04lX param2 = %04lX\n", > @@ -130,12 +80,6 @@ DWORD WINAPI OSS_midMessage(UINT wDevID, UINT wMsg, DWORD_PTR dwUser, > > TRACE("(%04X, %04X, %08lX, %08lX, %08lX);\n", > wDevID, wMsg, dwUser, dwParam1, dwParam2); > - switch (wMsg) { > - case DRVM_INIT: > - return OSS_MidiInit(); > - case DRVM_EXIT: > - return OSS_MidiExit(); > - } > > params.dev_id = wDevID; > params.msg = wMsg; > @@ -167,13 +111,6 @@ DWORD WINAPI OSS_modMessage(UINT wDevID, UINT wMsg, DWORD_PTR dwUser, > TRACE("(%04X, %04X, %08lX, %08lX, %08lX);\n", > wDevID, wMsg, dwUser, dwParam1, dwParam2); > > - switch (wMsg) { > - case DRVM_INIT: > - return OSS_MidiInit(); > - case DRVM_EXIT: > - return OSS_MidiExit(); > - } > - > params.dev_id = wDevID; > params.msg = wMsg; > params.user = dwUser; > diff --git a/dlls/wineoss.drv/oss.c b/dlls/wineoss.drv/oss.c > index b0a411ecd9b..a5aea9ee724 100644 > --- a/dlls/wineoss.drv/oss.c > +++ b/dlls/wineoss.drv/oss.c > @@ -1405,7 +1405,6 @@ unixlib_entry_t __wine_unix_call_funcs[] = > set_volumes, > set_event_handle, > is_started, > - midi_init, > midi_release, > midi_out_message, > midi_in_message, > diff --git a/dlls/wineoss.drv/ossmidi.c b/dlls/wineoss.drv/ossmidi.c > index 072a9815c35..6677609a5a6 100644 > --- a/dlls/wineoss.drv/ossmidi.c > +++ b/dlls/wineoss.drv/ossmidi.c > @@ -83,6 +83,7 @@ static pthread_mutex_t in_buffer_mutex = PTHREAD_MUTEX_INITIALIZER; > static unsigned int num_dests, num_srcs, num_synths, seq_refs; > static struct midi_dest dests[MAX_MIDIOUTDRV]; > static struct midi_src srcs[MAX_MIDIINDRV]; > +static int load_count; > > static unsigned int num_midi_in_started; > static int rec_cancel_pipe[2]; > @@ -301,22 +302,23 @@ static int seq_close(int fd) > return 0; > } > > -NTSTATUS midi_init(void *args) > +static UINT midi_init(void) > { > - struct midi_init_params *params = args; > int i, status, synth_devs = 255, midi_devs = 255, fd, len; > struct synth_info sinfo; > struct midi_info minfo; > struct midi_dest *dest; > struct midi_src *src; > > + TRACE("(%i)\n", load_count); > + > + if (load_count++) > + return 1; > + > /* try to open device */ > fd = seq_open(); > if (fd == -1) > - { > - *params->err = -1; > - return STATUS_SUCCESS; > - } > + return -1; > > /* find how many Synth devices are there in the system */ > status = ioctl(fd, SNDCTL_SEQ_NRSYNTHS, &synth_devs); > @@ -324,8 +326,7 @@ NTSTATUS midi_init(void *args) > { > ERR("ioctl for nr synth failed.\n"); > seq_close(fd); > - *params->err = -1; > - return STATUS_SUCCESS; > + return -1; > } > > if (synth_devs > MAX_MIDIOUTDRV) > @@ -506,9 +507,17 @@ wrapup: > /* close file and exit */ > seq_close(fd); > > - *params->err = 0; > + return 0; > +} > > - return STATUS_SUCCESS; > +static UINT midi_exit(void) > +{ > + TRACE("(%i)\n", load_count); > + > + if (--load_count) > + return 1; > + > + return 0; > } > > NTSTATUS midi_release(void *args) > @@ -1634,6 +1643,12 @@ NTSTATUS midi_out_message(void *args) > > switch (params->msg) > { > + case DRVM_INIT: > + *params->err = midi_init(); > + break; > + case DRVM_EXIT: > + *params->err = midi_exit(); > + break; > case DRVM_ENABLE: > case DRVM_DISABLE: > /* FIXME: Pretend this is supported */ > @@ -1688,6 +1703,12 @@ NTSTATUS midi_in_message(void *args) > > switch (params->msg) > { > + case DRVM_INIT: > + *params->err = midi_init(); > + break; > + case DRVM_EXIT: > + *params->err = midi_exit(); > + break; > case DRVM_ENABLE: > case DRVM_DISABLE: > /* FIXME: Pretend this is supported */ > diff --git a/dlls/wineoss.drv/unixlib.h b/dlls/wineoss.drv/unixlib.h > index d3dda7c76f2..6a7dc9288d9 100644 > --- a/dlls/wineoss.drv/unixlib.h > +++ b/dlls/wineoss.drv/unixlib.h > @@ -209,11 +209,6 @@ struct is_started_params > HRESULT result; > }; > > -struct midi_init_params > -{ > - UINT *err; > -}; > - > struct notify_context > { > BOOL send_notify; > @@ -280,14 +275,12 @@ enum oss_funcs > oss_set_volumes, > oss_set_event_handle, > oss_is_started, > - oss_midi_init, > oss_midi_release, > oss_midi_out_message, > oss_midi_in_message, > oss_midi_notify_wait, > }; > > -NTSTATUS midi_init(void *args) DECLSPEC_HIDDEN; > NTSTATUS midi_release(void *args) DECLSPEC_HIDDEN; > NTSTATUS midi_out_message(void *args) DECLSPEC_HIDDEN; > NTSTATUS midi_in_message(void *args) DECLSPEC_HIDDEN; > -- > 2.25.1 > >

April 29, 2022

Re: [PATCH 5/6] wineoss: Move MIDM_OPEN and MIDM_CLOSE to the unixlib.

by Andrew Eikum

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Fri, Apr 29, 2022 at 08:29:57AM +0100, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > dlls/wineoss.drv/Makefile.in | 2 +- > dlls/wineoss.drv/midi.c | 243 ----------------------------------- > dlls/wineoss.drv/oss.c | 3 - > dlls/wineoss.drv/ossmidi.c | 193 +++++++++++++++++++++++++--- > dlls/wineoss.drv/unixlib.h | 35 ----- > 5 files changed, 175 insertions(+), 301 deletions(-) > > diff --git a/dlls/wineoss.drv/Makefile.in b/dlls/wineoss.drv/Makefile.in > index 04b438da71e..13fb18b6004 100644 > --- a/dlls/wineoss.drv/Makefile.in > +++ b/dlls/wineoss.drv/Makefile.in > @@ -3,7 +3,7 @@ MODULE = wineoss.drv > UNIXLIB = wineoss.so > IMPORTS = uuid ole32 user32 advapi32 > DELAYIMPORTS = winmm > -EXTRALIBS = $(OSS4_LIBS) > +EXTRALIBS = $(OSS4_LIBS) $(PTHREAD_LIBS) > EXTRAINCL = $(OSS4_CFLAGS) > > EXTRADLLFLAGS = -mcygwin > diff --git a/dlls/wineoss.drv/midi.c b/dlls/wineoss.drv/midi.c > index c83dd55fd6b..84a4fac4b74 100644 > --- a/dlls/wineoss.drv/midi.c > +++ b/dlls/wineoss.drv/midi.c > @@ -63,23 +63,10 @@ > > WINE_DEFAULT_DEBUG_CHANNEL(midi); > > -static WINE_MIDIIN *MidiInDev; > - > -/* this is the total number of MIDI out devices found */ > -static int MIDM_NumDevs = 0; > - > -static int numStartedMidiIn = 0; > - > -static int rec_cancel_pipe[2]; > -static HANDLE hThread; > - > /*======================================================================* > * Low level MIDI implementation * > *======================================================================*/ > > -static int midiOpenSeq(void); > -static int midiCloseSeq(int); > - > static int MIDI_loadcount; > /************************************************************************** > * OSS_MidiInit [internal] > @@ -100,11 +87,6 @@ static LRESULT OSS_MidiInit(void) > params.err = &err; > OSS_CALL(midi_init, &params); > > - if (!err) > - { > - MidiInDev = params.srcs; > - MIDM_NumDevs = params.num_srcs; > - } > return err; > } > > @@ -120,9 +102,6 @@ static LRESULT OSS_MidiExit(void) > if (--MIDI_loadcount) > return 1; > > - MidiInDev = NULL; > - MIDM_NumDevs = 0; > - > return 0; > } > > @@ -135,224 +114,6 @@ static void notify_client(struct notify_context *notify) > notify->instance, notify->param_1, notify->param_2); > } > > -/************************************************************************** > - * MIDI_NotifyClient [internal] > - */ > -static void MIDI_NotifyClient(UINT wDevID, WORD wMsg, > - DWORD_PTR dwParam1, DWORD_PTR dwParam2) > -{ > - DWORD_PTR dwCallBack; > - UINT uFlags; > - HANDLE hDev; > - DWORD_PTR dwInstance; > - > - TRACE("wDevID = %04X wMsg = %d dwParm1 = %04lX dwParam2 = %04lX\n", > - wDevID, wMsg, dwParam1, dwParam2); > - > - switch (wMsg) { > - case MIM_OPEN: > - case MIM_CLOSE: > - case MIM_DATA: > - case MIM_LONGDATA: > - case MIM_ERROR: > - case MIM_LONGERROR: > - case MIM_MOREDATA: > - if (wDevID > MIDM_NumDevs) return; > - > - dwCallBack = MidiInDev[wDevID].midiDesc.dwCallback; > - uFlags = MidiInDev[wDevID].wFlags; > - hDev = MidiInDev[wDevID].midiDesc.hMidi; > - dwInstance = MidiInDev[wDevID].midiDesc.dwInstance; > - break; > - default: > - ERR("Unsupported MSW-MIDI message %u\n", wMsg); > - return; > - } > - > - DriverCallback(dwCallBack, uFlags, hDev, wMsg, dwInstance, dwParam1, dwParam2); > -} > - > -/************************************************************************** > - * midiOpenSeq [internal] > - */ > -static int midiOpenSeq(void) > -{ > - struct midi_seq_open_params params; > - > - params.close = 0; > - params.fd = -1; > - OSS_CALL(midi_seq_open, &params); > - > - return params.fd; > -} > - > -/************************************************************************** > - * midiCloseSeq [internal] > - */ > -static int midiCloseSeq(int fd) > -{ > - struct midi_seq_open_params params; > - > - params.close = 1; > - params.fd = fd; > - OSS_CALL(midi_seq_open, &params); > - > - return 0; > -} > - > -static void handle_midi_data(unsigned char *buffer, unsigned int len) > -{ > - struct midi_handle_data_params params; > - > - params.buffer = buffer; > - params.len = len; > - OSS_CALL(midi_handle_data, &params); > -} > - > -static DWORD WINAPI midRecThread(void *arg) > -{ > - int fd = (int)(INT_PTR)arg; > - unsigned char buffer[256]; > - int len; > - struct pollfd pollfd[2]; > - > - pollfd[0].fd = rec_cancel_pipe[0]; > - pollfd[0].events = POLLIN; > - pollfd[1].fd = fd; > - pollfd[1].events = POLLIN; > - > - while (1) > - { > - /* Check if an event is present */ > - if (poll(pollfd, ARRAY_SIZE(pollfd), -1) <= 0) > - continue; > - > - if (pollfd[0].revents & POLLIN) /* cancelled */ > - break; > - > - len = read(fd, buffer, sizeof(buffer)); > - > - if (len > 0 && len % 4 == 0) > - handle_midi_data(buffer, len); > - } > - return 0; > -} > - > -/************************************************************************** > - * midOpen [internal] > - */ > -static DWORD midOpen(WORD wDevID, LPMIDIOPENDESC lpDesc, DWORD dwFlags) > -{ > - int fd; > - > - TRACE("(%04X, %p, %08X);\n", wDevID, lpDesc, dwFlags); > - > - if (lpDesc == NULL) { > - WARN("Invalid Parameter !\n"); > - return MMSYSERR_INVALPARAM; > - } > - > - /* FIXME : > - * how to check that content of lpDesc is correct ? > - */ > - if (wDevID >= MIDM_NumDevs) { > - WARN("wDevID too large (%u) !\n", wDevID); > - return MMSYSERR_BADDEVICEID; > - } > - if (MidiInDev[wDevID].state == -1) { > - WARN("device disabled\n"); > - return MIDIERR_NODEVICE; > - } > - if (MidiInDev[wDevID].midiDesc.hMidi != 0) { > - WARN("device already open !\n"); > - return MMSYSERR_ALLOCATED; > - } > - if ((dwFlags & MIDI_IO_STATUS) != 0) { > - WARN("No support for MIDI_IO_STATUS in dwFlags yet, ignoring it\n"); > - dwFlags &= ~MIDI_IO_STATUS; > - } > - if ((dwFlags & ~CALLBACK_TYPEMASK) != 0) { > - FIXME("Bad dwFlags\n"); > - return MMSYSERR_INVALFLAG; > - } > - > - fd = midiOpenSeq(); > - if (fd < 0) { > - return MMSYSERR_ERROR; > - } > - > - if (numStartedMidiIn++ == 0) { > - pipe(rec_cancel_pipe); > - hThread = CreateThread(NULL, 0, midRecThread, (void *)(INT_PTR)fd, 0, NULL); > - if (!hThread) { > - close(rec_cancel_pipe[0]); > - close(rec_cancel_pipe[1]); > - numStartedMidiIn = 0; > - WARN("Couldn't create thread for midi-in\n"); > - midiCloseSeq(fd); > - return MMSYSERR_ERROR; > - } > - SetThreadPriority(hThread, THREAD_PRIORITY_TIME_CRITICAL); > - TRACE("Created thread for midi-in\n"); > - } > - > - MidiInDev[wDevID].wFlags = HIWORD(dwFlags & CALLBACK_TYPEMASK); > - > - MidiInDev[wDevID].lpQueueHdr = NULL; > - MidiInDev[wDevID].midiDesc = *lpDesc; > - MidiInDev[wDevID].state = 0; > - MidiInDev[wDevID].incLen = 0; > - MidiInDev[wDevID].startTime = 0; > - MidiInDev[wDevID].fd = fd; > - > - MIDI_NotifyClient(wDevID, MIM_OPEN, 0L, 0L); > - return MMSYSERR_NOERROR; > -} > - > -/************************************************************************** > - * midClose [internal] > - */ > -static DWORD midClose(WORD wDevID) > -{ > - int ret = MMSYSERR_NOERROR; > - > - TRACE("(%04X);\n", wDevID); > - > - if (wDevID >= MIDM_NumDevs) { > - WARN("wDevID too big (%u) !\n", wDevID); > - return MMSYSERR_BADDEVICEID; > - } > - if (MidiInDev[wDevID].midiDesc.hMidi == 0) { > - WARN("device not opened !\n"); > - return MMSYSERR_ERROR; > - } > - if (MidiInDev[wDevID].lpQueueHdr != 0) { > - return MIDIERR_STILLPLAYING; > - } > - > - if (MidiInDev[wDevID].fd == -1) { > - WARN("ooops !\n"); > - return MMSYSERR_ERROR; > - } > - if (--numStartedMidiIn == 0) { > - TRACE("Stopping thread for midi-in\n"); > - write(rec_cancel_pipe[1], "x", 1); > - if (WaitForSingleObject(hThread, 5000) != WAIT_OBJECT_0) { > - WARN("Thread end not signaled, force termination\n"); > - TerminateThread(hThread, 0); > - } > - close(rec_cancel_pipe[0]); > - close(rec_cancel_pipe[1]); > - TRACE("Stopped thread for midi-in\n"); > - } > - midiCloseSeq(MidiInDev[wDevID].fd); > - MidiInDev[wDevID].fd = -1; > - > - MIDI_NotifyClient(wDevID, MIM_CLOSE, 0L, 0L); > - MidiInDev[wDevID].midiDesc.hMidi = 0; > - return ret; > -} > - > /*======================================================================* > * MIDI entry points * > *======================================================================*/ > @@ -374,10 +135,6 @@ DWORD WINAPI OSS_midMessage(UINT wDevID, UINT wMsg, DWORD_PTR dwUser, > return OSS_MidiInit(); > case DRVM_EXIT: > return OSS_MidiExit(); > - case MIDM_OPEN: > - return midOpen(wDevID, (LPMIDIOPENDESC)dwParam1, dwParam2); > - case MIDM_CLOSE: > - return midClose(wDevID); > } > > params.dev_id = wDevID; > diff --git a/dlls/wineoss.drv/oss.c b/dlls/wineoss.drv/oss.c > index c5b422a60c9..b0a411ecd9b 100644 > --- a/dlls/wineoss.drv/oss.c > +++ b/dlls/wineoss.drv/oss.c > @@ -1410,7 +1410,4 @@ unixlib_entry_t __wine_unix_call_funcs[] = > midi_out_message, > midi_in_message, > midi_notify_wait, > - > - midi_seq_open, > - midi_handle_data, > }; > diff --git a/dlls/wineoss.drv/ossmidi.c b/dlls/wineoss.drv/ossmidi.c > index 9c8ca8a8f39..072a9815c35 100644 > --- a/dlls/wineoss.drv/ossmidi.c > +++ b/dlls/wineoss.drv/ossmidi.c > @@ -33,6 +33,7 @@ > #include <stdint.h> > #include <time.h> > #include <unistd.h> > +#include <poll.h> > #include <errno.h> > #include <sys/types.h> > #include <sys/stat.h> > @@ -45,6 +46,7 @@ > #define WIN32_NO_STATUS > #include "winternl.h" > #include "audioclient.h" > +#include "mmddk.h" > > #include "wine/debug.h" > #include "wine/unixlib.h" > @@ -62,12 +64,30 @@ struct midi_dest > int fd; > }; > > +struct midi_src > +{ > + int state; /* -1 disabled, 0 is no recording started, 1 in recording, bit 2 set if in sys exclusive recording */ > + MIDIOPENDESC midiDesc; > + WORD wFlags; > + MIDIHDR *lpQueueHdr; > + unsigned char incoming[3]; > + unsigned char incPrev; > + char incLen; > + UINT startTime; > + MIDIINCAPSW caps; > + int fd; > +}; > + > static pthread_mutex_t in_buffer_mutex = PTHREAD_MUTEX_INITIALIZER; > > static unsigned int num_dests, num_srcs, num_synths, seq_refs; > static struct midi_dest dests[MAX_MIDIOUTDRV]; > static struct midi_src srcs[MAX_MIDIINDRV]; > > +static unsigned int num_midi_in_started; > +static int rec_cancel_pipe[2]; > +static pthread_t rec_thread_id; > + > static pthread_mutex_t notify_mutex = PTHREAD_MUTEX_INITIALIZER; > static pthread_cond_t notify_read_cond = PTHREAD_COND_INITIALIZER; > static pthread_cond_t notify_write_cond = PTHREAD_COND_INITIALIZER; > @@ -281,18 +301,6 @@ static int seq_close(int fd) > return 0; > } > > -NTSTATUS midi_seq_open(void *args) > -{ > - struct midi_seq_open_params *params = args; > - > - if (!params->close) > - params->fd = seq_open(); > - else > - seq_close(params->fd); > - > - return STATUS_SUCCESS; > -} > - > NTSTATUS midi_init(void *args) > { > struct midi_init_params *params = args; > @@ -499,8 +507,6 @@ wrapup: > seq_close(fd); > > *params->err = 0; > - params->num_srcs = num_srcs; > - params->srcs = srcs; > > return STATUS_SUCCESS; > } > @@ -1313,11 +1319,8 @@ static void handle_regular_data(struct midi_src *src, unsigned char value, UINT > } > } > > -NTSTATUS midi_handle_data(void *args) > +static void handle_midi_data(unsigned char *buffer, unsigned int len) > { > - struct midi_handle_data_params *params = args; > - unsigned char *buffer = params->buffer; > - unsigned int len = params->len; > unsigned int time = get_time_msec(), i; > struct midi_src *src; > unsigned char value; > @@ -1339,7 +1342,153 @@ NTSTATUS midi_handle_data(void *args) > else > handle_regular_data(src, value, time - src->startTime); > } > - return STATUS_SUCCESS; > +} > + > +static void *rec_thread_proc(void *arg) > +{ > + int fd = PtrToLong(arg); > + unsigned char buffer[256]; > + int len; > + struct pollfd pollfd[2]; > + > + pollfd[0].fd = rec_cancel_pipe[0]; > + pollfd[0].events = POLLIN; > + pollfd[1].fd = fd; > + pollfd[1].events = POLLIN; > + > + while (1) > + { > + /* Check if an event is present */ > + if (poll(pollfd, ARRAY_SIZE(pollfd), -1) <= 0) > + continue; > + > + if (pollfd[0].revents & POLLIN) /* cancelled */ > + break; > + > + len = read(fd, buffer, sizeof(buffer)); > + > + if (len > 0 && len % 4 == 0) > + handle_midi_data(buffer, len); > + } > + return NULL; > +} > + > +static UINT midi_in_open(WORD dev_id, MIDIOPENDESC *desc, UINT flags, struct notify_context *notify) > +{ > + struct midi_src *src; > + int fd; > + > + TRACE("(%04X, %p, %08X);\n", dev_id, desc, flags); > + > + if (desc == NULL) > + { > + WARN("Invalid Parameter !\n"); > + return MMSYSERR_INVALPARAM; > + } > + > + /* FIXME : > + * how to check that content of lpDesc is correct ? > + */ > + if (dev_id >= num_srcs) > + { > + WARN("wDevID too large (%u) !\n", dev_id); > + return MMSYSERR_BADDEVICEID; > + } > + src = srcs + dev_id; > + if (src->state == -1) > + { > + WARN("device disabled\n"); > + return MIDIERR_NODEVICE; > + } > + if (src->midiDesc.hMidi != 0) > + { > + WARN("device already open !\n"); > + return MMSYSERR_ALLOCATED; > + } > + if ((flags & MIDI_IO_STATUS) != 0) > + { > + WARN("No support for MIDI_IO_STATUS in dwFlags yet, ignoring it\n"); > + flags &= ~MIDI_IO_STATUS; > + } > + if ((flags & ~CALLBACK_TYPEMASK) != 0) > + { > + FIXME("Bad flags\n"); > + return MMSYSERR_INVALFLAG; > + } > + > + fd = seq_open(); > + if (fd < 0) > + return MMSYSERR_ERROR; > + > + if (num_midi_in_started++ == 0) > + { > + pipe(rec_cancel_pipe); > + if (pthread_create(&rec_thread_id, NULL, rec_thread_proc, LongToPtr(fd))) > + { > + close(rec_cancel_pipe[0]); > + close(rec_cancel_pipe[1]); > + num_midi_in_started = 0; > + WARN("Couldn't create thread for midi-in\n"); > + seq_close(fd); > + return MMSYSERR_ERROR; > + } > + TRACE("Created thread for midi-in\n"); > + } > + > + src->wFlags = HIWORD(flags & CALLBACK_TYPEMASK); > + > + src->lpQueueHdr = NULL; > + src->midiDesc = *desc; > + src->state = 0; > + src->incLen = 0; > + src->startTime = 0; > + src->fd = fd; > + > + set_in_notify(notify, src, dev_id, MIM_OPEN, 0, 0); > + return MMSYSERR_NOERROR; > +} > + > +static UINT midi_in_close(WORD dev_id, struct notify_context *notify) > +{ > + struct midi_src *src; > + > + TRACE("(%04X);\n", dev_id); > + > + if (dev_id >= num_srcs) > + { > + WARN("dev_id too big (%u) !\n", dev_id); > + return MMSYSERR_BADDEVICEID; > + } > + src = srcs + dev_id; > + if (src->midiDesc.hMidi == 0) > + { > + WARN("device not opened !\n"); > + return MMSYSERR_ERROR; > + } > + if (src->lpQueueHdr != 0) > + return MIDIERR_STILLPLAYING; > + > + if (src->fd == -1) > + { > + WARN("ooops !\n"); > + return MMSYSERR_ERROR; > + } > + if (--num_midi_in_started == 0) > + { > + TRACE("Stopping thread for midi-in\n"); > + write(rec_cancel_pipe[1], "x", 1); > + pthread_join(rec_thread_id, NULL); > + close(rec_cancel_pipe[0]); > + close(rec_cancel_pipe[1]); > + TRACE("Stopped thread for midi-in\n"); > + } > + seq_close(src->fd); > + src->fd = -1; > + > + set_in_notify(notify, src, dev_id, MIM_CLOSE, 0, 0); > + src->midiDesc.hMidi = 0; > + > + return MMSYSERR_NOERROR; > } > > static UINT midi_in_add_buffer(WORD dev_id, MIDIHDR *hdr, UINT hdr_size) > @@ -1544,6 +1693,12 @@ NTSTATUS midi_in_message(void *args) > /* FIXME: Pretend this is supported */ > *params->err = MMSYSERR_NOERROR; > break; > + case MIDM_OPEN: > + *params->err = midi_in_open(params->dev_id, (MIDIOPENDESC *)params->param_1, params->param_2, params->notify); > + break; > + case MIDM_CLOSE: > + *params->err = midi_in_close(params->dev_id, params->notify); > + break; > case MIDM_ADDBUFFER: > *params->err = midi_in_add_buffer(params->dev_id, (MIDIHDR *)params->param_1, params->param_2); > break; > diff --git a/dlls/wineoss.drv/unixlib.h b/dlls/wineoss.drv/unixlib.h > index 90d0c47421c..d3dda7c76f2 100644 > --- a/dlls/wineoss.drv/unixlib.h > +++ b/dlls/wineoss.drv/unixlib.h > @@ -209,27 +209,9 @@ struct is_started_params > HRESULT result; > }; > > -#include <mmddk.h> /* temporary */ > - > -typedef struct midi_src > -{ > - int state; /* -1 disabled, 0 is no recording started, 1 in recording, bit 2 set if in sys exclusive recording */ > - MIDIOPENDESC midiDesc; > - WORD wFlags; > - MIDIHDR *lpQueueHdr; > - unsigned char incoming[3]; > - unsigned char incPrev; > - char incLen; > - UINT startTime; > - MIDIINCAPSW caps; > - int fd; > -} WINE_MIDIIN; > - > struct midi_init_params > { > UINT *err; > - unsigned int num_srcs; > - struct midi_src *srcs; > }; > > struct notify_context > @@ -273,18 +255,6 @@ struct midi_notify_wait_params > struct notify_context *notify; > }; > > -struct midi_seq_open_params > -{ > - int close; > - int fd; > -}; > - > -struct midi_handle_data_params > -{ > - unsigned char *buffer; > - unsigned int len; > -}; > - > enum oss_funcs > { > oss_test_connect, > @@ -315,9 +285,6 @@ enum oss_funcs > oss_midi_out_message, > oss_midi_in_message, > oss_midi_notify_wait, > - > - oss_midi_seq_open, /* temporary */ > - oss_midi_handle_data, > }; > > NTSTATUS midi_init(void *args) DECLSPEC_HIDDEN; > @@ -325,8 +292,6 @@ NTSTATUS midi_release(void *args) DECLSPEC_HIDDEN; > NTSTATUS midi_out_message(void *args) DECLSPEC_HIDDEN; > NTSTATUS midi_in_message(void *args) DECLSPEC_HIDDEN; > NTSTATUS midi_notify_wait(void *args) DECLSPEC_HIDDEN; > -NTSTATUS midi_seq_open(void *args) DECLSPEC_HIDDEN; > -NTSTATUS midi_handle_data(void *args) DECLSPEC_HIDDEN; > > extern unixlib_handle_t oss_handle; > > -- > 2.25.1 > >

April 29, 2022

Re: [PATCH 4/6] wineoss: Use a pipe to signal the end of the record thread.

by Andrew Eikum

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Fri, Apr 29, 2022 at 08:29:56AM +0100, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > dlls/wineoss.drv/midi.c | 42 ++++++++++++++++++++--------------------- > 1 file changed, 21 insertions(+), 21 deletions(-) > > diff --git a/dlls/wineoss.drv/midi.c b/dlls/wineoss.drv/midi.c > index 0afd9985c03..c83dd55fd6b 100644 > --- a/dlls/wineoss.drv/midi.c > +++ b/dlls/wineoss.drv/midi.c > @@ -70,7 +70,7 @@ static int MIDM_NumDevs = 0; > > static int numStartedMidiIn = 0; > > -static int end_thread; > +static int rec_cancel_pipe[2]; > static HANDLE hThread; > > /*======================================================================* > @@ -214,30 +214,26 @@ static DWORD WINAPI midRecThread(void *arg) > int fd = (int)(INT_PTR)arg; > unsigned char buffer[256]; > int len; > - struct pollfd pfd; > + struct pollfd pollfd[2]; > > - TRACE("Thread startup\n"); > - > - pfd.fd = fd; > - pfd.events = POLLIN; > - > - while(!end_thread) { > - TRACE("Thread loop\n"); > + pollfd[0].fd = rec_cancel_pipe[0]; > + pollfd[0].events = POLLIN; > + pollfd[1].fd = fd; > + pollfd[1].events = POLLIN; > > + while (1) > + { > /* Check if an event is present */ > - if (poll(&pfd, 1, 250) <= 0) > + if (poll(pollfd, ARRAY_SIZE(pollfd), -1) <= 0) > continue; > - > - len = read(fd, buffer, sizeof(buffer)); > - TRACE("Received %d bytes\n", len); > > - if (len < 0) continue; > - if ((len % 4) != 0) { > - WARN("Bad length %d, errno %d (%s)\n", len, errno, strerror(errno)); > - continue; > - } > + if (pollfd[0].revents & POLLIN) /* cancelled */ > + break; > + > + len = read(fd, buffer, sizeof(buffer)); > > - handle_midi_data(buffer, len); > + if (len > 0 && len % 4 == 0) > + handle_midi_data(buffer, len); > } > return 0; > } > @@ -286,9 +282,11 @@ static DWORD midOpen(WORD wDevID, LPMIDIOPENDESC lpDesc, DWORD dwFlags) > } > > if (numStartedMidiIn++ == 0) { > - end_thread = 0; > + pipe(rec_cancel_pipe); > hThread = CreateThread(NULL, 0, midRecThread, (void *)(INT_PTR)fd, 0, NULL); > if (!hThread) { > + close(rec_cancel_pipe[0]); > + close(rec_cancel_pipe[1]); > numStartedMidiIn = 0; > WARN("Couldn't create thread for midi-in\n"); > midiCloseSeq(fd); > @@ -338,11 +336,13 @@ static DWORD midClose(WORD wDevID) > } > if (--numStartedMidiIn == 0) { > TRACE("Stopping thread for midi-in\n"); > - end_thread = 1; > + write(rec_cancel_pipe[1], "x", 1); > if (WaitForSingleObject(hThread, 5000) != WAIT_OBJECT_0) { > WARN("Thread end not signaled, force termination\n"); > TerminateThread(hThread, 0); > } > + close(rec_cancel_pipe[0]); > + close(rec_cancel_pipe[1]); > TRACE("Stopped thread for midi-in\n"); > } > midiCloseSeq(MidiInDev[wDevID].fd); > -- > 2.25.1 > >

April 29, 2022