October 2023 - wine-gitlab

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 31 Oct '23

31 Oct '23

On Tue Oct 31 18:36:04 2023 +0000, Zebediah Figura wrote: > We can delete this line. https://learn.microsoft.com/en-us/windows/win32/api/strmif/nf-strmif-iamstr… says that parameter can return non-NULL, so it felt wrong to potentially leak it. But that same page says this specific filter, and every Windows builtin, always returns NULL, so checking for something that's documented to never happen is also kinda wrong. Which means I have no particular opinion in either direction. If you do, let's go with that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50384

1 0

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 31 Oct '23

31 Oct '23

On Tue Oct 31 18:36:03 2023 +0000, Zebediah Figura wrote: > This can be static now. > Let's use enum wg_video_format instead of wg_video_format; the latter is > a typedef of UINT32 and therefore the compiler won't recognize it as an enum. > I don't particularly like the naming "get_bytes"; that sounds like it's > retrieving the actual data in some sense. How about > "wg_format_get_max_size_video_raw()", matching the naming scheme for wg_format_to_caps()? Yeah, I refactored that one about 38 times while creating this MR, some of which called it from other source files. But yeah, this version doesn't, so let's fix it. (Did anyone try compiling Wine with -Wmissing-declarations? That one throws a warning for nonstatic functions with no preceding prototype, and is perfect for catching things that should be static.) I have no particular opinion on the name, so sure, we can do that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50383

1 0

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 31 Oct '23

31 Oct '23

On Tue Oct 31 18:36:02 2023 +0000, Zebediah Figura wrote: > We need to leave the CS on these return paths. Ah, good point. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50382

1 0

[PATCH v2 0/6] MR4224: dmsynth: Various fixes
by Anton Baskanov (＠baskanov) 31 Oct '23

31 Oct '23

-- v2: dmsynth: Return void from add_mod_from_connection(). https://gitlab.winehq.org/wine/wine/-/merge_requests/4224

4 10

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Zebediah Figura (＠zfigura) 31 Oct '23

31 Oct '23

Thanks, this is looking almost right now, just a few more nitpicks. And again sorry about the late review :-/ -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50378

1 0

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Zebediah Figura (＠zfigura) 31 Oct '23

31 Oct '23

Zebediah Figura (@zfigura) commented about dlls/quartz/tests/mpegsplit.c: > + LPWSTR name; > + HRESULT hr; > + LCID lcid; > + > + IAMStreamSelect_QueryInterface(sel, &IID_IBaseFilter, (void **)&filter); > + IBaseFilter_FindPin(filter, L"Video", &pin_video); > + IBaseFilter_FindPin(filter, L"Audio", &pin_audio); > + IPin_QueryInterface(pin_video, &IID_IUnknown, (void **)&unk_video); > + IPin_QueryInterface(pin_audio, &IID_IUnknown, (void **)&unk_audio); > + > + hr = IAMStreamSelect_Count(sel, &stream_count); > + ok(hr == S_OK, "Got hr %#lx.\n", hr); > + ok(stream_count == 2, "Got %lu streams.\n", stream_count); > + > + hr = IAMStreamSelect_Info(sel, 0, &mt, &flags, &lcid, &group, &name, &object, &unk); > + ok(hr == S_OK, "Got hr %#lx.\n", hr); Since we've implemented "mt", can we check its contents? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50377

1 0

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Zebediah Figura (＠zfigura) 31 Oct '23

31 Oct '23

Zebediah Figura (@zfigura) commented about dlls/quartz/tests/mpegsplit.c: > + ok(hr == S_OK, "Got hr %#lx.\n", hr); > + todo_wine ok(flags == AMSTREAMSELECTINFO_ENABLED, "Got flags %lx.\n", flags); > + ok(lcid == 0, "Got LCID %lx.\n", lcid); > + ok(group == 0, "Got group %lx.\n", group); > + todo_wine ok(name && !wcscmp(name, L"Stream(E0)"), "Got name %ls.\n", name ? name : L"(null)"); > + todo_wine ok(object == unk_video, "Got object %p, expected %p.\n", object, unk_video); > + ok(unk == NULL, "Got unknown %p.\n", unk); > + > + if (name) > + CoTaskMemFree(name); > + if (mt) > + DeleteMediaType(mt); > + if (object) > + IUnknown_Release(object); > + if (unk) > + IUnknown_Release(unk); We can delete this line. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50376

1 0

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Zebediah Figura (＠zfigura) 31 Oct '23

31 Oct '23

Zebediah Figura (@zfigura) commented about dlls/winegstreamer/quartz_parser.c: > > #define ALIGN(n, alignment) (((n) + (alignment) - 1) & ~((alignment) - 1)) > > +unsigned int wg_format_get_bytes_for_uncompressed(wg_video_format format, unsigned int width, unsigned int height) This can be static now. Let's use enum wg_video_format instead of wg_video_format; the latter is a typedef of UINT32 and therefore the compiler won't recognize it as an enum. I don't particularly like the naming "get_bytes"; that sounds like it's retrieving the actual data in some sense. How about "wg_format_get_max_size_video_raw()", matching the naming scheme for wg_format_to_caps()? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50375

1 0

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Zebediah Figura (＠zfigura) 31 Oct '23

31 Oct '23

Zebediah Figura (@zfigura) commented about dlls/winegstreamer/quartz_parser.c: > { > - FIXME("iface %p, index %ld, mt %p, flags %p, lcid %p, group %p, name %p, object %p, unknown %p, stub!\n", > - iface, index, mt, flags, lcid, group, name, object, unknown); > - return E_NOTIMPL; > + struct parser *filter = impl_from_IAMStreamSelect(iface); > + wg_parser_stream_t stream = wg_parser_get_stream(filter->wg_parser, index); > + HRESULT hr = S_OK; > + > + FIXME("filter %p, index %ld, mt %p, flags %p, lcid %p, group %p, name %p, object %p, unknown %p, semi-stub!\n", > + filter, index, mt, flags, lcid, group, name, object, unknown); > + EnterCriticalSection(&filter->filter.filter_cs); > + > + if (!filter->sink.pin.peer) > + return VFW_E_NOT_CONNECTED; > + if (index < 0 || index >= wg_parser_get_stream_count(filter->wg_parser)) > + return S_FALSE; We need to leave the CS on these return paths. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50374

1 0

Re: [PATCH v20 0/14] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Zebediah Figura (＠zfigura) 31 Oct '23

31 Oct '23

On Tue Oct 31 18:32:42 2023 +0000, Alfred Agrell wrote: > No, sorry, my bad, I cleaned up those logs too hard. > ``` > 01e4:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 1000, late -0.4180444, timestamp -0.2. > 01e4:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 1.0000000000000000e+00, diff 0, timestamp 0. > 0:00:00.153596244 5139 0x7fba10037860 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=0 stream_time=-1 diff=0 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 1000, late -0.4443777, timestamp -0.193. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 1.0000000000000000e+00, diff 0, timestamp 0. > 0:00:00.160425790 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=0 stream_time=-1 diff=0 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.063711, timestamp 0.221. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -637110, timestamp 2210000. > 0:00:00.574499800 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=221000000 stream_time=-1 diff=-63711000 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.0640444, timestamp 0.254. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -640444, timestamp 2540000. > 0:00:00.607456059 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=254000000 stream_time=27955556 diff=-64044400 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.0633777, timestamp 0.288. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -633777, timestamp 2880000. > 0:00:00.640834761 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=288000000 stream_time=61955556 diff=-63377700 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.062711, timestamp 0.322. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -627110, timestamp 3220000. > 0:00:00.674681804 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=322000000 stream_time=95955556 diff=-62711000 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.0640444, timestamp 0.354. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -640444, timestamp 3540000. > 0:00:00.707600604 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=354000000 stream_time=127955556 diff=-64044400 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.0633777, timestamp 0.388. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -633777, timestamp 3880000. > 0:00:00.741135357 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=388000000 stream_time=161955556 diff=-63377700 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.063711, timestamp 0.421. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -637110, timestamp 4210000. > 0:00:00.774577068 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=421000000 stream_time=194955556 diff=-63711000 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.0590443, timestamp 0.459. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -590443, timestamp 4590000. > 0:00:00.812276144 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=459000000 stream_time=232955556 diff=-59044300 > 01dc:trace:quartz:source_quality_control_Notify filter 00379A98, sender > 0AAB11EC, type 0, proportion 2000, late -0.0553776, timestamp 0.496. > 01dc:trace:quartz:wg_parser_stream_notify_qos stream 0x7fba100afe30, > underflow 0, proportion 5.0000000000000000e-01, diff -553776, timestamp 4960000. > 0:00:00.849423973 5139 0x7fba1004a2f0 ERROR WINE > dlls/winegstreamer/wg_parser.c:477:wg_parser_stream_notify_qos: > timestamp*100=496000000 stream_time=269955556 diff=-55377600 > ``` > The third wg_parser_stream_notify_qos call has timestamp\*100 = > 221000000, 0.221 seconds. But the stream_time is only 27955556, which is > 0.027 seconds; adding a -0.063 seconds diff to that goes out of bounds. > I have the offending mpeg file extracted; should I upload it somewhere, > or is that copyrightly improper? It's 518148 bytes. Copying offline discussion here; yes, this is coming from the application sink, not ours, and I think clamping is probably the right thing to do. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_50373

1 0

2025

2024

2023

2022

wine-gitlab October 2023