On Windows, libmpv sometimes creates a detached window with wid option

[olhovpav]

Our team is developing a custom Qt application which uses a window embedding feature. We've followed this example, but sometimes we get unexpected behavior. Sometimes wid is negative. We noticed this behavior on long-running machines (some machines have 49+ days of uptime). Here are a few examples of such wids from the same machine at the same time: 357500724, -797637628, -1665923796, 5572158.

The expected behavior is that mpv creates an embedded window, but actual behavior is that a window is created as a detached window.

I've briefly looked at mpv source code, and it seems that mpv relies on the fact that the wid value is always positive. Can it be assumed as a bug of mpv and should wid be handled in another way? Or could it be a bug in Qt/Windows and wid really must be positive?

• mpv version: v0.34.1
• Windows Version: Windows 10 20H2
• Source of the mpv binary: built from sources with MXE on Ubuntu 21.10
  Additional flags

  --enable-libmpv-shared
  --disable-debug-build
  --disable-egl-angle
  --disable-tv
  --disable-x11
• It's unknown which version has introduced this bug, but the issue also appears on v0.29.1

[avih]

You give very little info, and no code. No one can help you like this.

Sometimes wid is negative. We noticed this behavior on long-running machines (some machines have 49+ days of uptime). Here are a few examples of such wids from the same machine at the same time: 357500724, -797637628, -1665923796, 5572158.

What do you mean? where do you get this negative wid from?

Did you read the docs on how you should provide the wid to libmpv? Did you follow it?

On win32, the ID is interpreted as HWND. Pass it as value cast to intptr_t. mpv will create its own window, and set the wid window as parent

I've briefly looked at mpv source code, and it seems that mpv relies on the fact that the wid value is always positive.

As far as I can tell, on Windows, it only checks whether the wid is 0 (don't embed) or not-0 (do try to embed). Where at the code do you see that mpv assumes it's positive, or rely on it in some way?

[sfan5]

IIRC this was briefly discussed on IRC one day, but nobody bothered to fix it.

video/out/w32_common.c
1520:    if (w32->opts->WinID >= 0)
1521:        w32->parent = (HWND)(intptr_t)(w32->opts->WinID);

[avih]

if (w32->opts->WinID >= 0)

Right, so it's not taken from the option (and therefore embedding is disabled) if it's negative as 64 bits integer.

Additionally, the win32 code in general considers 0 as "don't embed".

So the real question is whether or not an HWND as int64_t can be negative.

If it can be negative then it's an issue at the API and/or mpv.

If it can't be negative then it doesn't matter anyway, and negative values would be invalid anyway.

So again, where did the negative wid values come from? and were the docs followed by casting a HWND to intptr_t ?

[olhovpav]

So again, where did the negative wid values come from?

Sometimes just calls to QWidget::winId return negative values, and I don't know if it's a bug or not. In my code it looks like this:

void MyWidget::someMethod() {
    int64_t wid = this->winId();
    mpv_set_option(mpv, "wid", MPV_FORMAT_INT64, &wid);
}

The actual code is much larger, but I think the one above can be considered MRE.

I assumed that high uptime leads to this behavior, because we create quite a lot of windows during OS lifetime.

If it is unknown if negative values are really valid, I will try to patch mpv myself and prepare a commit if it really helps. But if someone can tell the exact answer, that would help.

[avih]

357500724, -797637628, -1665923796, 5572158

FWIW, all these values fit in signed 32 bit int.

int64_t wid = this->winId();

So you didn't follow the docs, which state to use intptr_t. Nevertheless, I doubt that's the issue.

Currently my guess is that someplace (inside Qt code) it goes through a 32 bit variable, which ends up negative.

Just to confirm, after you get the value, how do you conclude it's negative? Do you use printf? something else? show us the line which prints the negative value please.

Maybe you could use some external tool (spy++ maybe?) to check the HWND of your window, and compare it to your negative wid?

[olhovpav]

So you didn't follow the docs, which state to use intptr_t. Nevertheless, I doubt that's the issue.

Shouldn't intptr_t be the same as int64_t? Thanks I will check it out.

Just to confirm, after you get the value, how do you conclude it's negative?

QString::number((int64_t)this->winId()).toStdString(); // #1
QString::number(this->winId()).toStdString();          // #2

In the first line, the result is string representing a negative value. If I use the second, I get something like 18446744072911913988.

Just in case, the std::stringstream approach produces the same results:

std::stringstream ss; ss << (int64_t)this->winId();
std::stringstream ss; ss << this->winId();

And I will try spy++ a bit later.

[avih]

I get something like 18446744072911913988

Interesting, in hex it's FFFFFFFFD0750404, so the upper 32 bits are all 1. It doesn't look random, but I don't really know how HWND are generated.

FWIW, on a Windows 7 system which has been running for months without reboot, with a new (scite editor) window, spy++ shows a HWND of 004C0024.

On a win10 system, spy++ (v14, 2016) also shows values such as 000C0BE4.

In both systems the handle display seem limited to 32 bits.

This also suggests HWND is limited to 32 bits, including an example with all high 32 bits as 1 - https://stackoverflow.com/questions/43012454/is-it-normal-for-an-hwnd-to-have-its-high-bit-set

So it might actually be related to not using intptr_t.

From https://docs.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication?redirectedfrom=MSDN :

64-bit versions of Windows use 32-bit handles for interoperability. When sharing a handle between 32-bit and 64-bit applications, only the lower 32 bits are significant, so it is safe to truncate the handle (when passing it from 64-bit to 32-bit) or sign-extend the handle (when passing it from 32-bit to 64-bit).

[avih]

So far the issue looks like this:

• HWND and all handles are 32 bits.
• Windows acceps sign-extended values of any 32 bit values as valid (which are negative as 64 bit signed if it was negative as 32 bit signed).
• mpv ignores negative values.
• mpv documents to first cast the HWND to intptr_t
  • We don't yet know whether this would end up as never-negative as 64 bits.

Regardless, a client can also first cast the value to unsigned 32, and then pass it as int64_t - would would be positive and it should work.

[olhovpav]

So it might actually be related to not using intptr_t.

Do I understand correctly that I should try the following?

std::intptr_t wid = this->winId();
mpv_set_option(mpv, "wid", MPV_FORMAT_INT64, &wid);

Or should it be changed on the Qt side, which uses quintptr as a result of the winId method? Or maybe it would be right to try to zero out all high bits?

Unfortunately, I can't reproduce this bug on my own PC, and experiments take a lot of time on machines where this issue appears. Also on my PC I tried the following example but without success. I couldn't get negative values.

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    while(true) {
        MainWindow w;
        w.show();
        std::cout << (std::int64_t)w.winId() << std::endl;
        if (wid < 0) std::this_thread::sleep_for(100h);
    }
    return a.exec();
}

Regardless, a client can also first cast the value to unsigned 32, and then pass it as int64_t - would would be positive and it should work.

I'll look into it, thank you!

[olhovpav]

Oh there's an update! With the previous code with a while(true) loop, I got a negative value (-2105014840) on my own PC after an hour. So I will try to cast it to uint32_t and then pass it as int64_t.

Might be worth show this in the example?

[olhovpav]

I have checked that casting to std::uint32_t really works on my PC with the above example and mpv.exe --wid=N. After the weekend I will check this workaround on our application on the target machines.

So, are we going to just update the example or to try somehow solving this on the mpv side?

[avih]

So, are we going to just update the example or to try somehow solving this on the mpv side?

Let's first confirm the solution.

After that, assuming it works - not sure yet. Let's leave this open for now.

[stax76]

It also appeared on the mpv.net tracker 10 days ago here:

https://github.com/stax76/mpv.net/issues/413

SetPropertyLong("wid", handle.ToInt64());

        public void SetPropertyLong(string name, long value, bool throwException = false)
        {
            mpv_error err = mpv_set_property(Handle, GetUtf8Bytes(name), mpv_format.MPV_FORMAT_INT64, ref value);

            if (err < 0)
                HandleError(err, throwException, $"error setting property: {name} = {value}");
        }

    [DllImport("mpv-2.dll", CallingConvention = CallingConvention.Cdecl)]
    public static extern mpv_error mpv_set_property(
        IntPtr mpvHandle, byte[] name, mpv_format format, ref long data);

[olhovpav]

I've created a repository with a simple Qt app that allows to reproduce the bug on my PC. After running the app, you will need to wait a few minutes to get a negative wid. In my case, it takes about 50-200K iterations. Then I forked mpv and just removed the check, built mpv and sequentially ran it with both negative int64_t id and uint32_t id. Both work correctly, in both cases mpv embeds my video in the same window. Yes, you can safely pass any of these values, the result is exactly the same.

We've updated a few production machines, and now I am waiting for the result from them. If everything goes well, then I will create a PR.

[EzeKees]

Same problem here. It only happens when the computers are on for many (more than 7) days. The mpv log reports nothing.

[olhovpav]

It took some time to get results as our support rebooted all machines, but now it looks like removal of the checks in the mpv source code doesn't work as expected. Indeed, the issue has begun to occur less often with production machines, but didn't disappear completely, and some windows have begun to randomly appear in a detached state and disappear. We're not completely sure yet if it's the same bug, but on several machines we've also tried to zero out all high bits on the application side and this has been working better.

Now I'm going to update my mpv fork and test a new version on our production.

[stax76]

Most users boot at least once per day, I guess.

[EzeKees]

@stax76 Not always. In my case, for example, we must use libmpv 24/7. Every day. We have a digital signage system.

[Neo-XP]

Hello,

I still have this issue with the latest version of mpv.

Is there a beta release somewhere to try?

Thank you

[olhovpav]

Is there a beta release somewhere to try?

You can try your luck with my fork, but in our case the bug is still not completely gone. But strangely, the same fix on the side of our app fixes this.

auto uwid = static_cast<std::uint32_t>(qtwidget->winId());
std::int64_t wid = uwid;
mpv_set_option(handle, "wid", MPV_FORMAT_INT64, &wid);

[tsl0922]

This also happens on libmpv application that do not use wid and enable force-window.

https://github.com/tsl0922/ImPlay/issues/2

upgrading to the latest mpv doesn't fix it cleanly.

[sfan5]

This doesn't have anything to do with your issue.

---

@olhovpav named the correct solution to fix this in the application code. What we should do is update the example and documentation, but otherwise this is solved.

[Neo-XP]

This issue is still not fixed for me, after a while it stills open two mpv windows instead of one.

The uptime: Days : 30 Hours : 17 Minutes : 32 Seconds : 36 Milliseconds : 803 Ticks : 26551568035967 TotalDays : 30,73098152311 TotalHours : 737,543556554639 TotalMinutes : 44252,6133932783 TotalSeconds : 2655156,8035967 TotalMilliseconds : 2655156803,5967

Should I create a new issue?