On Fri Sep 13 17:38:30 2024 +0000, Rémi Bernon wrote:
> Actually looks like it's some leftover of an older version of the code,
> it's not doing that anymore even in later changes.
Woohoo, I helped :sweat_smile:
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6480#note_82152
On Fri Sep 13 17:35:55 2024 +0000, Rémi Bernon wrote:
> The stream context needs to be allocated on the PE side while the
> demuxer is a unix-side pointer. We could create a PE side object
> wrapping both, but that didn't feel very useful.
I was thinking about allocating each on their respective side adding the ctx alongside the handle in `wine/winedmo.h`. Might work, but it doesn't seem too pretty.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6480#note_82151
On Fri Sep 13 17:35:55 2024 +0000, Rémi Bernon wrote:
> That was to return the actual detected media size from the demuxer, in
> case it can figure it out better than the caller (which may also be
> unable to provide it).
Actually looks like it's some leftover of an older version of the code, it's not doing that anymore even in later changes.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6480#note_82148
On Fri Sep 13 16:51:13 2024 +0000, Emil Velikov wrote:
> Out of curiosity: why do we pass stream_size as pointer to an int,
> instead of an int directly?
> Sorry for the belated questions :bow:
That was to return the actual detected media size from the demuxer, in case it can figure it out better than the caller (which may also be unable to provide it).
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6480#note_82147
On Fri Sep 13 16:43:58 2024 +0000, Emil Velikov wrote:
> Any chance we get an inline comment saying how `0x10000` was chosen?
Not sure what to say other than it's pretty arbitrary. Larger than common read sizes, to reduce the chances of having to call the callbacks multiple times, but otherwise nothing very interesting about it. Maybe it should even be a bit shorter to avoid wasting two 64k page instead of one though.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6480#note_82146
On Fri Sep 13 16:42:03 2024 +0000, Emil Velikov wrote:
> Out of curiosity: is the `stream_context` expected to have different
> life-time than `winedmo_demuxer`?
> Alternatively it seems cleaner to me to have the `stream_context`
> pointer within the `winedmo_demuxer` itself removing the rather uncommon
> read-back pointer from the destructor.
The stream context needs to be allocated on the PE side while the demuxer is a unix-side pointer. We could create a PE side object wrapping both, but that didn't feel very useful.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6480#note_82145
I kept the first patch (`Remember if timer was blocked.`) to avoid SetTimer on timers that weren't killed, mostly because it would apply on every task (which is likely to execute way faster than the ms granularity), not sure if it would skew the timer in such case.
The other patches are split, with tests now showing why they're needed. Some places where wine-gecko callbacks into us and in which we call into external code need to be treated the same as a recursive task since it can end up in a message loop.
I haven't supplied all of them from the previous MR, mostly because I didn't find a way to test those (e.g. show context menu, and the gecko async navigation).
--
v6: mshtml: Don't process tasks recursively from Gecko events.
mshtml: Don't process tasks recursively from script runners.
mshtml: Don't process tasks recursively.
mshtml: Remember if timer was blocked.
mshtml: Don't cast to int to bound the timer.
https://gitlab.winehq.org/wine/wine/-/merge_requests/6375
Emil Velikov (@xexaxo) commented about dlls/winedmo/main.c:
> {
> struct stream_context *context;
>
> - if (!(context = malloc( sizeof(*context) ))) return NULL;
> + if (!(context = malloc( 0x10000 ))) return NULL;
Any chance we get an inline comment saying how `0x10000` was chosen?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6480#note_82137