October 2023 - Wine-gitlab - list.winehq.org

Re: [PATCH v13 0/7] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 20 Oct '23

20 Oct '23

On Mon Oct 16 23:45:52 2023 +0000, Zebediah Figura wrote: > We have amt_from_wg_format(), that should be usable here. Good point. It fills in a few fields in unexpected ways, but most are correct, so it's an improvement. And some of the incorrect fields look like omissions in that function, which are better fixed there. I tried running some tests locally, to check if something depends on the previous definition, but one failed because I compiled Wine without SChannel, another hit an assertion because I compiled without Vulkan, and several of them create windows all over which steal focus and make it hard to do anything else on the computer. I'll just leave that check for the CI. (And hope I don't miss it between the flaky tests already in master.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_49348

1 0

Re: [PATCH v13 0/7] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 20 Oct '23

20 Oct '23

On Mon Oct 16 23:45:45 2023 +0000, Zebediah Figura wrote: > This is correct, I think, but also orthogonal to the purpose of this > patch. Can you please split this out into a separate patch? I hope it doesn't mess with the decodebin chaining thing. But the mpegsplit and mpegvideo tests pass, so probably no big deal. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_49344

1 0

Re: [PATCH v13 0/7] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 20 Oct '23

20 Oct '23

On Mon Oct 16 23:45:46 2023 +0000, Zebediah Figura wrote: > Isn't this redundant with the same logic done in wg_parser_stream_seek()? Only if wg_parser_stream_seek has access to the duration. Which, uh, it does, so let's just force push that and pretend it didn't happen. (On a totally related note, GST_ERROR/etc shouldn't end with \n. I'll go clean that up too.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_49345

1 0

Re: [PATCH v13 0/7] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 20 Oct '23

20 Oct '23

On Mon Oct 16 23:45:56 2023 +0000, Zebediah Figura wrote: > Does the application actually use IAMStreamSelect::Info()? If so we > should have tests for it. Also, ideally this'd be a separate patch. > If not I'd just drop this hunk. Does "it calls the function, but doesn't really do anything with the result" count? https://github.com/krkrz/krkr2/blob/dec49af97e174d31059c3ccd7efc700ba3c6b78… It would be satisfied if IStreamSelect::Count was a semi-stub that just returns zero, but I suspect that's not the best idea. But tests, yes, absolutely. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_49346

1 0

Re: [PATCH v13 0/7] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 20 Oct '23

20 Oct '23

On Mon Oct 16 23:45:47 2023 +0000, Zebediah Figura wrote: > That doesn't seem right. What is this doing? It silences these warnings from the Wagamama demo: ``` (wine:1578450): GStreamer-CRITICAL **: 01:44:15.602: gst_event_new_qos: assertion 'diff >= 0 || -diff <= timestamp' failed 0:00:00.608062458 1578450 0x7f9190049ef0 ERROR WINE dlls/winegstreamer/wg_parser.c:486:wg_parser_stream_notify_qos: Failed to create QOS event. (wine:1578450): GStreamer-CRITICAL **: 01:44:15.602: gst_pad_push_event: assertion 'GST_IS_EVENT (event)' failed ``` But like the format change thing, it doesn't seem to do any actual harm. And I agree it is a weird solution. I guess it's safest to remove it and leave it for later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_49347

1 0

Re: [PATCH v13 0/7] MR3938: winegstreamer: Implement CLSID_CMpegVideoCodec
by Alfred Agrell (＠Alcaro) 20 Oct '23

20 Oct '23

On Mon Oct 16 23:45:45 2023 +0000, Zebediah Figura wrote: > This is overestimation because we can't do any better, right? In that > case it probably deserves a comment just to clarify that's what's happening. Yeah, that entire function is confusing. It's supposed to return the max size of a compressed frame, but I don't think there is any real upper bound for that, you can cram in as many optional extensions as you want. So yes, it's guessing decompressed size as an upper bound. It'll break for 1x1 videos (can't fit the MPEG headers in three bytes), and videos crammed full of junk data, but let's just not care about those. But comments, sure, can do. And the Cinepak branch's comment is suspicious at best - what does biSizeImage have to do with compressed size? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3938#note_49343

1 0

Re: [PATCH v2 0/1] MR4144: mfmediaengine: Mask some source flags returned from GetResourceCharacteristics(). - approved
by Alexandre Julliard (＠julliard) 19 Oct '23

19 Oct '23

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4144

1 0

Re: [PATCH v3 0/1] MR4137: commdlg: Set lCustData the same in GetSaveFileName as GetOpenFileName. - approved
by Alexandre Julliard (＠julliard) 19 Oct '23

19 Oct '23

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4137

1 0

Re: [PATCH v2 0/1] MR4124: wined3d: Use debugstr_fourcc instead of debugstr_an. - approved
by Alexandre Julliard (＠julliard) 19 Oct '23

19 Oct '23

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4124

1 0

Re: [PATCH v2 0/1] MR420: vkd3d-utils: Implement D3DStripShader(). - approved
by Alexandre Julliard (＠julliard) 19 Oct '23

19 Oct '23

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/420

1 0