resnbl / pyTardis

demo project using PySimpleGUI, PIL and VLC
GNU General Public License v3.0
1 stars 0 forks source link

Have you created GIFs on the fly? #1

Closed PySimpleGUI closed 2 years ago

PySimpleGUI commented 2 years ago

I've been looking at ways to capture screenshots recently and part of that investigation includes seeing if Pillow could be used to create an animated GIF. From reading your readme I get the impression you've been immersed in this kind of image processing.

How would you go about capturing a video of a window being used?

I use ShareX to capture my screenshots. I hope to replace ShareX with psgscreeenshot in the future.

resnbl commented 2 years ago

Actually, all the images here are created using an image editor and layered together. A few exceptions, like the "TARDIS Takeoff" and "It goes ding" effects were made with utility scripts with heavy use of Pillow.

Pillow does have an ImageGrab class, but it is broken when used on Retina-screen Macs (that's all I have). I looked into the code, and it simply makes a subprocess call to the Mac "screencapture" command, then tries to clip the image down to the requested size (but fails because the image is pixel-doubled at 144 dpi and its pixel co-ordinates are off).

I ran the Demo_Graph_Drawing_And_Dragging_Figures.py demo program, which was touted to perform screen captures, but it threw an exception on my Mac (OSError: cannot write mode RGBA as JPEG).

Actual screen capture (pulling pixels from the screen buffer) is a very platform-dependent process. If ShareX is working for you, I would stick with it for the time being.

PySimpleGUI commented 2 years ago

Pillow does have an ImageGrab class, but it is broken when used on Retina-screen Macs (that's all I have).

Ohhhhhh,..... this is really good to know.

If ShareX is working for you, I would stick with it for the time being.

I'm working on building this into PySimpleGUI itself... Since tkinter can't do it, I'm going to launch a PySimpleGUI-based application and I was planning on using Pillow to do a basic screen capture. I assumed that there was going to be platform-specific code involved, I just didn't think that ImageGrab would be so limiting right away. I recall seeing some problems with multi-monitor support, but that's not a huge thing to limit use cases around.

(OSError: cannot write mode RGBA as JPEG).

Oh dear... the Mac.... image

Of course it works OK on Windows. test.jpg is written to my drive if I run the demo - Demo_Graph_Drawing_And_Dragging_Figures_2_Windows.py

test

This is great information... really appreciate your help.

resnbl commented 2 years ago

Just to close this out... I'm not sure that invoking standard OS screen capture utilities to repeatedly output images will be fast enough to make a "screen recording". I suspect one would need code that runs at a low level to quickly grab images and buffer them for output, probably compressing them into a video format on the fly (as opposed to a series of individual uncompressed frame files).

PySimpleGUI commented 2 years ago

I've been surprised by the performance of Python a number of times. I guess in this case it's going to depend on how quickly a launch happens. I'm not familiar enough with Mac hardware to know if the Retina displays are a rarity or the norm. We're already on the fringes supporting the Mac. Could be that I'm only able to capture "video" on Windows or Linux. I'll run some performance tests once I get into the development of this a bit more. Sorry that I've been on the fringes, but Tanay's certainly actively working on it.

resnbl commented 2 years ago

I'm not familiar enough with Mac hardware to know if the Retina displays are a rarity or the norm.

Ooooh.. not something said by Mac enthusiasts... FYI: Apple no longer has any non-Retina Macs, desktop or laptop, in its current lineup. (There may be some back-level and re-conditioned models still for sale, but they are hard to find on Apple's site.)

I wouldn't be surprised if some of the higher-end models can keep up, but I wouldn't count on just any model being able to. OTOH, maybe you don't really need a 30 fps recording: other than cursor moves, how often does a screen change? I know in some of my testing with animated PNGs, I had to drop the window.read(n) value below 10 msecs to get over 30 fps on my machine with only a single animation running, but I doubt animations are common for PSG.

resnbl commented 2 years ago

In a vaguely related note (or not...), I have been getting seg-faults in PSG on my Mac. I was able to trace it down to the following conditions:

I found some ominous ramblings about how the Mac version of Tcl does several font-related steps before actually creating a root window, and there are memory-pool related concerns there, which maybe tkinter is not heeding. My SWAG on the checkbox tie-in is that Tcl is relying on Apple's symbol font to draw the actual check-mark on the screen, and is tripping over something not handled well when listing the fonts.

I doubt this is an issue for PSG, but thought you might like to know about it. If I ever bother to learn enough low-level tkinter to create a simple app, I may submit a bug report to them...

PySimpleGUI commented 2 years ago

Thanks for the info on the Mac! It's definitely a learning process for me.

Animation / video playback was the reason PySimpleGUI was created. It was used to build a media player. You'll find a number of Demo Programs that use OpenCV to playback video. The YOLO Repo is a good example of playing back video in realtime while also performing object identification.

I typically use a timeout=0 when playing back video, especially when I know the rate will be throttled by another part of the system. For OpenCV, the combination of the input frame arrival time and the overhead of doing the image processing pretty much guarantees that there's little risk in using a value of 0.

One of the uses for the screenshot application and why I want to capture video is for submitting bugs. It's a dual-purpose utility. One is for submitting screenshots to "The PySimpleGUI Catalog", the other is capturing interaction with windows as part of logging an issue. It's a whole lot easier debugging when a video fo the problem is submitted, but not everyone knows how to capture and make a GIF.


The fonts installed crash is odd because if no window has been created prior to the call, that function creates a temporary window and destroys it after getting back the information from tkinter.

I'll try it out on Windows to see if it's a generic PySimpleGUI problem. Do events need to be enabled on the Checkbox to have the problem?

Thanks for all the help! It's really appreciated.

PySimpleGUI commented 2 years ago

pycharm64_gbq4DQYlRF

I wasn't able to replicate the problem with the Checkbox. Maybe I'm not doing something right.

resnbl commented 2 years ago

I did a screen record, but it generated a > 100MB file, so a screenshot will have to do:

checkbox-crash

I'm pretty sure this is a tkinter/tcl problem. I found this in _tkmacosx-README.md on their GitHub:

One additional minor caveat for developers is that there are several steps of the Tk initialization which precede the call to TkpInit. Notably, the font package is initialized first. Since there is no NSAutoreleasePool in scope prior to calling TkpInit, the functions called in these preliminary stages need to create and drain their own NSAutoreleasePools whenever they call methods of Appkit objects (e.g. NSFont).

I suspect this is where the issue lies... (NSAutoreleasePool is the Mac's AppKit memory management class.)

PySimpleGUI commented 2 years ago

WOW!

A segmentation fault flat out with no recovery? Is that a normal way Python programs "crash" on the mac? No traceback printed in the console?

Yea, 100 MB is a bit much. It's all I-Frames so they're going to add up. I wouldn't think they would go that fast though. I don't need 30 FPS video saved, maybe 10 FPS at most would capture mouse motion. Could be asking for too much using just PySimpleGUI and PIL at this point. I've not looked to see what other packages are out there that could help.

Thanks for all your help Bob... really appreciated it.

resnbl commented 2 years ago

Actually, that wasn't a Python error, it was a Tcl error (which I believe is written in C), and you can't generally recover from attempting to write to a random location in memory... Python "uncaught exceptions" on the Mac probably produce the same stack trace you see on Windows.

In other news...I found a simple work-around: create a virtually empty Window, fetch the font list, then close() the window. Then go ahead and create the "real" window using a font from the list. Nuts: this still crashes when a Checkbox is in the main window...

BTW: My goal behind this was to implement font selection ala HTML/CSS: specify a list of desired fonts, then use the first one found on the list of fonts tkinter supports. I've virtually always specified a default font for a sg.Window because the sg.DEFAULT_FONT is too small for my eyesight. Note that a 10-pt font on a (virtual) 144 DPI screen is much smaller than on a 96 DPI screen, so I always boost font sizes by 20% or more on anything developed primarily for Windows - especially the Mac versions of Word and Excel.

PySimpleGUI commented 2 years ago

For the crash, maybe make a Window first that's not empty, but has the Alpha set to 0 so that it's not visible, then call the font function. It's such a strange bug... very specific. I've no clue how you managed to find it. You're quite a tenacious developer... willing to go down into the lower layers to see what's REALLY happening. I'm impressed.

I don't know much about the DPI issues, but Jason does. Last year I added a dpi_awareness setting in the set_options and scaling in the Window creation. The dpi setting is for Windows only at the moment. I've never used these settings other than testing them. It could be that we can do something similar to what was done on Windows to help with your Mac.

PySimpleGUI commented 2 years ago

There's a discussion that just started on Python Announcements email list that's about fonts and DPI scaling. There is mention of the same scaling that I provide access to using the scaling parameter to Window.

I should add a method to get the current value. You can get the value using:

print(window.TKroot.tk.call('tk', 'scaling'))

On my system it is: 1.3333333333333333

This was the initial question:

On the same topic of HiDPI screens, the Treeview seems to calculate wrongly the font height (see attach image) Do you know any way to correct that?

Vasilis

And this is one reply:

Hi,

On Thu, 17 Mar 2022 09:47:16 +0000 Vasilis Vlachoudis [Vasilis.Vlachoudis@cern.ch](mailto:Vasilis.Vlachoudis@cern.ch) wrote:

Hi Michael,

no I am using the default fonts. What is different is the I have set a high DPI for the X-server to match the screen resolution.

I saw in the page https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww. tcl.tk%2Fman%2Ftcl%2FTkCmd%2Fttk_treeview.html&data=04%7C01%7C%7Ce c779b471aa7465afe9808da080078f4%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1 %7C0%7C637831095367116460%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=mognVr rh2B45Kw7WdOTIEWs6ql%2BOhz4seSzeA1veNig%3D&reserved=0

under "Styling Options' there is parameter to set the -rowheight that needs to be defined as

ttk::style configure Treeview \ -rowheight [expr {[font metrics font -linespace] + 2}]

however I could not find how to call this command, and when If I do the following

style = ttk.Style() style.configure("Treeview", rowheight="30")

it works, but it is not dynamic with the font size/dpi

you could do something like

f=font.Font(name='TkDefaultFont', exists=1) rowheight=f.metrics('linespace')+2 rowheight 17

This value is in pixels; I am not sure, if you did something to the dpi setting, maybe you still need to increase this value somehow according to the scaling factor in use, which you should be able to query with:

root.tk.call('tk', 'scaling') 1.2450980392156863

I am just doing guesswork here, though.

Regards

Michael

.-.. .. ...- . .-.. --- -. --. .- -. -.. .--. .-. --- ... .--. . .-.

It would be illogical to kill without reason. -- Spock, "Journey to Babel", stardate 3842.4 ___ Tkinter-discuss mailing list Tkinter-discuss@python.org https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.python.org%2Fmailman%2Flistinfo%2Ftkinter-discuss&data=04%7C01%7C%7Cec779b471aa7465afe9808da080078f4%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637831095367116460%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=FU0p9N4MTNyme%2BYdxHQUFQg3x1ca2MEXnrIJfP1B1eM%3D&reserved=0

resnbl commented 2 years ago

This is what happens when you program for over 40 years, then retire and dislike housework...

Anyway, it looks like this issue is cropping up elsewhere, like in Window.get_screen_size(), Text.char_width_in_pixels(), etc. Basically, anywhere there is PSG code that performs:

root = tk.Tk()
...
root.destroy()

seems to cause problems later for a Checkbox.

PySimpleGUI commented 2 years ago

That's the code that I use to temporarily create a window. Is there another technique that should be used instead? I thought that's the right way to create and destroy windows in tkinter. Perhaps an update is needed between the two so that part of the event loop will be run. I'll make a temporary change and check it into GitHub for you to try if you would like.

I just tried a test where I created a temp window and then called popup:

import PySimpleGUI as sg

root = sg.tkinter.Tk()
root.destroy()
sg.popup('test')

and got a strange error:

can't invoke "event" command: application has been destroyed
    while executing
"event generate $w <<ThemeChanged>>"
    (procedure "ttk::ThemeChanged" line 6)
    invoked from within
**"ttk::ThemeChanged"

The popup is shown without problems, I just get this unusual error message.

I'm not aware of any previously reported problems with Macs crashing due to previously calling one of these functions where I create a temp window. I'm wondering if this is another 8.6.12 problem.

Can you try adding an update between the create and the destroy?

I can add it on GitHub if you would like. It's odd I don't see this error if I call the Text.fonts_installed_list method, even if I only do the create and destroy. Very strange. I only see the error if I put the popup inside the function in PySimpleGUI.py. The call to root.update() seems to fix it. If you are able to get around the problem by adding the update, then I'll roll it into the PySimpleGUI code and fix all the other places that use this technique.

resnbl commented 2 years ago

Yes, I've seen the "can't invoke event" message in some of my testing. Please ensure there is a Checkbox/Checkbutton in your window to display: this seems to be a necessary condition for my failures (haven't tried any of the other Button derivatives). Here is what fails for me:

import tkinter as tk
from tkinter import font

def main():
    root = tk.Tk()
    fonts = list(font.families())
    root.update()
    root.destroy()

    msg = f'{len(fonts)} fonts found'
    print(msg)

    root = tk.Tk()
    chk_var = tk.IntVar()
    chk_btn = tk.Checkbutton(root, text=msg, variable=chk_var, height=3, width=20)
    chk_btn.pack()
    root.mainloop()

if __name__ == '__main__':
    main()
resnbl commented 2 years ago

Nerts - I had a long reply on scaling, but it got lost somehow... Your scaling results are what I expect: 96 virtual DPI / 72 logical DPI = 1.333333333. On my Mac, I would expect 144 virtual DPI / 72 logical DPI = 2.0. Instead I get 1.0006771314250538 ??? Tried adding scaling=2.0 to window creation, but it had no effect on the display, other than returning 2.0013542628501075 for the tkinter value (2x the default response).

Mac life is hard...

Before I forget: Happy St. Pat's!

PySimpleGUI commented 2 years ago

That's fantastic that you've narrowed down the problem using tkinter-only code!

Have you seen this on multiple versions of tkinter?

I tried it on both 8.6.6 and 8.6.12 and of course, there was no problem on Windows.


I just tried the scaling parameter on a program I was updating on Stack Overflow. It appears that the scaling did the right thing...

image

resnbl commented 2 years ago

I'm starting to think that creating another root = tk.Tk() is a bad idea. From the official docs at Python docs, we have the following:

While tkinter allows you to create more than one instance of a Tk object (with its own interpreter), all interpreters that are part of the same thread share a common event queue, which gets ugly fast. In practice, don’t create more than one instance of Tk at a time. Otherwise, it’s best to create them in separate threads and ensure you’re running a thread-aware Tcl/Tk build.

Creating (and destroying?) a Tcl interpreter seems like unnecessary overhead. And I suspect there are "thread local storage" items left behind from the first Tcl that cause issues for subsequent instances.

In the README file for the package at macosx, in section 5.2 Autorelease pools, is the following:

One additional minor caveat for developers is that there are several steps of the Tk initialization which precede the call to TkpInit. Notably, the font package is initialized first. Since there is no NSAutoreleasePool in scope prior to calling TkpInit, the functions called in these preliminary stages need to create and drain their own NSAutoreleasePools whenever they call methods of Appkit objects (e.g. NSFont).

I suspect this is the issue I am seeing.

PySimpleGUI commented 2 years ago

I'm not creating another tk.Tk. I'm creating one, destroying it, and then creating another later. We should be able to do that all day long. For over a year I was running creating and destroying Tk objects. It was only after I ran into some race conditions that I started using the hidden master root that I didn't destroy. It used to come and go with every window/popup/progress meter/debug window. This problem is the first time I've ever seen any side effect of creating and destroying a tkinter window where resources are corrupted.

Have you tested this problem on multiple versions of tkinter?

Seems like it's a bug in tkinter worth reporting to them.

If there is items that are not getting properly deleted and garbage collected, then they need to fix this memory/resource leak. We should be able to make as many of these windows as we want in a single program. I've never seen any of the many books I have on Tkinter and tutorials give any cautionary advice about creating multiple Tk objects in a single session/program. Very suspect that a single creation of one of these objects is enough of a leak to cause a major problem.

Have you seen any suggestions about how to get the screen dimensions, font families, etc, using another technique? I believe I picked up this from another site... I don't think I made it up, but I have created a number of twisted ways I use tkinter, so maybe I did. I'll ask @israel-dryer for advice as well as he's a real tkinter 🧙‍♂️... I'm a mere mortal.

I've tried using a TopLevel instead to make the temporary window but that really goes badly.

PySimpleGUI commented 2 years ago

Can you try this one with a different update call?

import tkinter as tk
from tkinter import font

def main():
    root = tk.Tk()
    fonts = list(font.families())
    root.update_idletasks()
    root.destroy()

    msg = f'{len(fonts)} fonts found'
    print(msg)

    root = tk.Tk()
    chk_var = tk.IntVar()
    chk_btn = tk.Checkbutton(root, text=msg, variable=chk_var, height=3, width=20)
    chk_btn.pack()
    root.mainloop()

if __name__ == '__main__':
    main()
resnbl commented 2 years ago

Sorry - same problem...

I see from the crash report that the deepest levels of Tk/Tcl active at the time are:

45  libtk8.6.dylib   TkpDisplayButton + 997
46  libtcl8.6.dylib  TclServiceIdle + 75
47  libtcl8.6.dylib  Tcl_DoOneEvent + 341
48  libtk8.6.dylib   -[TKContentView(TKWindowEvent) generateExposeEvents:] + 237
49  libtk8.6.dylib   -[TKContentView(TKWindowEvent) drawRect:] + 152

If I can find the source for TkpDisplayButton, maybe I can get a clue as to what is failing...

PySimpleGUI commented 2 years ago

Oh, one more thing... since cleanup is perhaps needed, then an explicit delete may help?

import tkinter as tk
from tkinter import font

def main():
    root = tk.Tk()
    fonts = list(font.families())
    root.update_idletasks()
    root.destroy()
    del root

    msg = f'{len(fonts)} fonts found'
    print(msg)

    root = tk.Tk()
    chk_var = tk.IntVar()
    chk_btn = tk.Checkbutton(root, text=msg, variable=chk_var, height=3, width=20)
    chk_btn.pack()
    root.mainloop()

if __name__ == '__main__':
    main()
resnbl commented 2 years ago

Didn't help. I even tried a deepcopy of the list returned from font.families() in case some of that data was actually memory allocated in the low-level NSFont code that was being freed.

PySimpleGUI commented 2 years ago

OK, I'll rework the PySimpleGUI code so that these temp windows don't created and destroyed and instead use them as the hidden-master-root. It's the cleanest way of going about things and will get rid of this particular problem. Doesn't matter if it should work, I can find a way around it that is in reality cleaner.

resnbl commented 2 years ago

That sounds like a good plan. I'll be happy to test when you're ready.

BTW: it is not just tk.Checkbutton that fails, but tk.Button as well. I suspecttk.Radiobutton fails, too, as they are just "types" of the basic button code.

PySimpleGUI commented 2 years ago

I would suspect many many failures... maybe all tk.widgets.

I've made the change to 4.57.0.15. Wanting to make sure I get a lot of runtime on this change just in case I messed it up or there is a problem with creating the hidden root window at the places where I did it. It's a fundamental underpinning of PySimpleGUI on all platforms.

Give it a go. This should fix the crash. I think it would still be worthwhile reporting this problem to the tkinter team. They shouldn't have this kind of leak in their code, anywhere.

resnbl commented 2 years ago

Looking good! I haven't looked at what you changed, but preliminary testing seems to work:

Screen Shot 2022-03-19 at 2 36 39 PM
resnbl commented 2 years ago

Completely unrelated nit: lines 17225-17227 are:

    if pysimplegui_settings_filename is not None or pysimplegui_settings_filename is not None:
        _pysimplegui_user_settings = UserSettings(filename=DEFAULT_USER_SETTINGS_PYSIMPLEGUI_FILENAME,
                                                  path=DEFAULT_USER_SETTINGS_PYSIMPLEGUI_PATH)

1) Its ORing the same condition twice: should one _filename be _path? 2) Is _pysimplegui_user_settings actually used anywhere?

resnbl commented 2 years ago

FYI: latest Pillow release (9.1.0) contains fix for ImageGrab on the Mac...