Zebediah Figura (@zfigura) commented about dlls/wmvcore/tests/wmvcore.c:
> todo_wine
> ok(hr == E_UNEXPECTED, "Got hr %#lx.\n", hr);
>
> + if (winetest_platform_is_wine)
I don't think you need the if (winetest_platform_is_wine) here.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4449#note_54469
Zebediah Figura (@zfigura) commented about dlls/winegstreamer/wm_reader.c:
>
> - reader->stream_count = wg_parser_get_stream_count(reader->wg_parser);
> -
> - if (!(reader->streams = calloc(reader->stream_count, sizeof(*reader->streams))))
> + if (!reader->streams)
> {
> - hr = E_OUTOFMEMORY;
> - goto out_disconnect_parser;
> + reader->stream_count = wg_parser_get_stream_count(reader->wg_parser);
> + if (!(reader->streams = calloc(reader->stream_count, sizeof(*reader->streams))))
> + {
> + hr = E_OUTOFMEMORY;
> + goto out_disconnect_parser;
> + }
> + for (i = 0; i < reader->stream_count; ++i)
> + reader->streams[i].selection = WMT_ON;
I don't like this pattern; in cases like this we should instead just split up the init_stream() helper to the part that's done on initialization and the part that's done on recreation. That said I would imagine more of this should be done on initialization...?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4449#note_54466
Zebediah Figura (@zfigura) commented about dlls/winegstreamer/wm_reader.c:
> parser = reader->wg_parser;
> LeaveCriticalSection(&reader->init_cs);
>
> + if (!parser)
> + {
> + Sleep(10);
> + continue;
> + }
> +
This is ugly. Either we should explicitly block the read thread, or shut it down and restart it. The latter is probably simpler.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4449#note_54465
Zebediah Figura (@zfigura) commented about dlls/winegstreamer/quartz_parser.c:
> else
> format->u.video_wmv.format = WG_WMV_VIDEO_FORMAT_UNKNOWN;
>
> + format->u.video_wmv.codec_data_len = mt->cbFormat - sizeof(VIDEOINFOHEADER);
> + if (format->u.video_wmv.codec_data_len > sizeof(format->u.video_wmv.codec_data))
> + {
> + WARN("Too big codec_data value (%u).\n", (UINT)format->u.video_wmv.codec_data_len);
This cast should be unnecessary. I'd also make this an ERR or FIXME.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4449#note_54464
On Thu Nov 30 21:42:02 2023 +0000, Zebediah Figura wrote:
> The codec_data introduced in 1/6 isn't validated in the tests. Why is that?
Because WMV version 1 doesn't have codec data.
```
$ gst-launch-1.0 -vvv filesrc location=dlls/wmvcore/tests/test.wmv ! asfdemux ! avdec_wmv1 ! fakesink
[...]
/GstPipeline:pipeline0/avdec_wmv1:avdec_wmv1-0.GstPad:sink: caps = video/x-wmv, wmvversion=(int)1, width=(int)64, height=(int)48, pixel-aspect-ratio=(fraction)1/1, format=(string)WMV1
$ gst-launch-1.0 -vvv filesrc location=waga.wmv ! asfdemux ! avdec_wmv3 ! fakesink
[...]
/GstPipeline:pipeline0/avdec_wmv3:avdec_wmv3-0.GstPad:sink: caps = video/x-wmv, wmvversion=(int)3, format=(string)WMV3, width=(int)1280, height=(int)720, codec_data=(buffer)4ff1080100, pixel-aspect-ratio=(fraction)1/1, framerate=(fraction)10000000/333333
```
(Its absense is tested by the cbFormat check in compare_media_types().)
But yes, certainly suboptimal. I'd be happy to test it if someone can find a suitable test file.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/4449#note_54462
This comes from behavioral study of Windows, which doesn't seem to check if the
lock is actually exclusively held, and just simply decrement the value stored
in the lock.
This fixes a dead lock which prevents WeCom from starting up.
--
v27: ntdll: allow SRWLOCKs to be quickly re-acquired
ntdll: An implementation of SRWLOCK that closer matches Windows'.
https://gitlab.winehq.org/wine/wine/-/merge_requests/3504