QL-Win / QuickLook

Bring macOS “Quick Look” feature to Windows
http://pooi.moe/QuickLook/
GNU General Public License v3.0
17.18k stars 1.08k forks source link

Do not Lock file #603

Closed daslicht closed 4 years ago

daslicht commented 4 years ago

Hi, it would be useful if quicklook would NOT lock the file. (as under macOS) This is usefull when for example browsing wav samples and you lieke to rename or move them. At the momnet we have to selecte another file first to be able to move or rename teh previous.

xupefei commented 4 years ago

It depends on file types. The locking happens usually with multimedia files.

macOS (and *nix OSs) can lock a file by the inode (which is the underlying unique file ID in the filesystem) so you can do whatever you want to the file (since the inode will never change), but I don't know how to do the same for Windows/NTFS.

I will be happy if anyone can give a hint :)

daslicht commented 4 years ago

Thank you for the reply~!

daslicht commented 4 years ago

Is there maybe a way to destroy the loaded instance somehow after playback ends ? Or what happens when you select another file to play that back and unlock the previously previewed ?

xupefei commented 4 years ago

Is there maybe a way to destroy the loaded instance somehow after playback ends?

Currently the instance will be destroyed when you close the window or switch away to another file. We don't destroy it when playback ends, since the user may want to play it again.

what happens when you select another file to play that back and unlock the previously previewed

The player destroys itself. So the file will be unlocked.

daslicht commented 4 years ago

maybe keep it when loop is enabled and destroy after play if not or would be rebuilding if someone wants to listen again be to much performance related ?

puffpppp commented 4 years ago

even after closing the preview window the file stays locked for a second or two, this is pretty annoying when I have to delete the file after previewing as instead of just disappearing I see a confirmation dialog that doesnt allow to delete

I dont have any hints to solve this problem but for what it's worth: this issue doesnt appear with windows previewer (the one that appears when you press ALT+P), in there you can even delete a file while it's playing, but I have no clue as to how this voodoo works

Thanks for such an awesome software though :)

WilliamDrt commented 4 years ago

The question is who (or what) locks the file. At first I thought it was MediaUriElement at ViewPanelXaml.cs line 128. So I cloned WPF Media Kit and run their "Test Application".

Sure enough in their app file is NOT locked. File keeps playing even if user deletes it or renames it. I checked with an .mp3 file in both NTFS & FAT drives.

So... back to QuickLook to see what's different. It turns out the offender is ViewPanelXaml.cs lines 62,63 mediaElement.MediaUriPlayer.LAVFilterDirectory = IntPtr.Size == 8 ? "LAVFilters-0.72-x64\\" : "LAVFilters-0.72-x86\\"; I commented out these lines, build it and run it. File is not locked and preview keeps playing, even if I rename or delete the actual file on disk.

*Notice: I did my test using my own file manager. For explorer it is pretty much the same, with the exception of renaming, because QuickLook picks up the key stroke on renaming and starts playing the file from the start(but the renaming operation goes through )

Now the question is what LAVFilterDirectory is for ? I did not had time to familiarize myself with QuickLook code base, but I did a search on the entire WPF Media Kit solution and did not find a single hit. Maybe WPF Media Kit is a newer version? I don't know

That's my 2 cents on the subject. Hope it helps

P.S. I just realize that "WPF Media Kit" is under QL-Win, I thought it was a completely other project :-) Nevertheless I don't know where mediaElement.MediaUriPlayer.LAVFilterDirectory is used. If someone knows, please point to the code where this is used, maybe we can find an alternative, or eliminate these lines all together.

xupefei commented 4 years ago

@WilliamDrt I believe that the original WPFMediaKit prioritise system decoders. In my fork, I modified the logic to use bundled LAVFilter first.

See the AddFilterToGraph method in DirectShowUtil.cs: https://github.com/QL-Win/WPF-MediaKit/blob/master/Source/DirectShow/MediaPlayers/DirectShowUtil.cs#L15

I guess the system decoder does not lock the file, we can't use it since it supports only a few formats (5-10 types as I remembered).

WilliamDrt commented 4 years ago

Ok i will have a look tomorrow, thank you for your input 😊

WilliamDrt commented 4 years ago

Ok I made it work @xupefei I am new to this git thing, can you help me out? I cloned directly from QL-Win/WPF-MediaKit. Is this correct or should I have first fork it, and then cloned my own (forked) repo ?

Alternative I made a gist with the changes to 2 files QL-WPF-MediaKit\Source\DirectShow\MediaPlayers\DirectShowUtil.cs QL-WPF-MediaKit\Source\DirectShow\MediaPlayers\MediaUriPlayer.cs gist is here I'm sorry, but I am new to git

Now a little bit of explanation of what is going on: LavSplitterSource was locking the file, so I switch to the system AsyncFileSource, but LavSplitterSource could do something AsyncFileSource could not. Check if file has video.

So I ended up by creating a dedicated private bool DoesItHaveVideo(string filename) function. Inside this function I made:

File locks and then unlocks, user will not notice, this is even before we load the window Then proceed with the orininal logic but used AsyncFileSource instead of LavSplitterSource (plus the LavSplitter which was unused).

Debug from Load looks like this: WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Added filter: LAV Splitter Source to graph WPFMediaKit.DirectShow.MediaPlayers.MediaUriPlayer - Remove filter from graph: LAV Splitter Source 0 WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Added filter: System AsyncFileSource to graph WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Added filter: LAV Splitter to graph WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Added filter: LAV Video Decoder to graph WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Added filter: LAV Audio Decoder to graph

Debug from Unload looks like this: WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Remove filter from graph: Renderer: EnhancedVideoRenderer 0 WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Remove filter from graph: Default DirectSound Device 0 WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Remove filter from graph: LAV Audio Decoder 0 WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Remove filter from graph: LAV Video Decoder 0 WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Remove filter from graph: LAV Splitter 0 WPFMediaKit.DirectShow.MediaPlayers.DirectShowUtil - Remove filter from graph: System AsyncFileSource 0

daslicht commented 4 years ago

yes you clone it, make a change in the clone and then craete a pull request to merge your changes. That way others can easy see what has been changed. e.g: like here : https://github.com/QL-Win/QuickLook/pull/415/commits/933365699cf1e3bf883da6ec41218245d8b93c9c

rabelux commented 4 years ago

I'd say the usual workflow is to fork the project, apply your changes, and then create a pull request to merge your branch into the QL-Win/WPF-MediaKit master-branch:

image

WilliamDrt commented 4 years ago

@rabelux ok this is what I read in some articles. I only have a question. If I fork it first and then do all the other stuff when let's say a month passes and I want to do another fix how can I make the code to be current?

I mean in one month code base might change a lot. How can my forked repo be in sync with the original?

Maybe there's a method and this is a silly question, maybe my understanding about fork is complete wrong. I though forking is for when you want a project to go in a different road.

I'm a bit confused about this, and I would like to learn and start right, so I can see some other issues on QL

Ps this fix should also fix long path support for audio/video but I haven't tested yet

rabelux commented 4 years ago

I hope this answers your question: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/syncing-a-fork If you want to do this in a browser you can do it the same way you get your code into other repos: You open your fork and create a pull request, this time just in the opposite direction. As you are the owner of your fork noone else has to approve it. Github also informs you that there are changes in the master repo which looks like this:

image

And the pull request looks like this, showing you the diff and all commits:

image

Note that switching direction is a little big bothersome and my solution is the following, where I am open for improvements: switch base repo to yours, github switches interface, then click on "compare across forks". Now the appropriate source should be selected

rabelux commented 4 years ago

And to answer your other question: Forking is like branching, just with you as the owner of the branch. You can pick as many or as little changes as you want from the origin. Which means you could have a 100% copy or only keep a part of the code base updated

WilliamDrt commented 4 years ago

@rabelux thanks a lot, that cleared things in my head, thanks again

daslicht commented 4 years ago

Just tried version 3.6.6 it still locks the file ?! When I played a wav file and then rename it it still says :

WilliamDrt commented 4 years ago

Just tried version 3.6.6 Store Version on Win 10 It does not lock file I can rename/delete while QL is playing Did you restart QL after update? Task manager -> find QL-> End task -> Start menu -> Restart QL Screenshot (66)

Are you on Windows 7 ?

daslicht commented 4 years ago

I am on windows 10 x64 and I uninstalled the previous version and installed the msi from here. After that I restarted the computer. When I now play a file, and even if playback is finishes i cant rename teh file without selection another first.

I haven't tried the store version yet. Will do later. The msi version locks the file tho

daslicht commented 4 years ago

@WilliamDrt how did you get that macOS like column view in explorer ?

WilliamDrt commented 4 years ago

I just tested the msi version and is good. I see 2 possibilities here

  1. My system has something special and QL does not lock files
  2. Your system has something special and QL does lock files

Bottom line: we need more info from other users

@daslicht I have no idea what is going on here, so I am just fishing:

  1. Is this file on a special location? e.g. inside a long path, inside a device (phone?)?
  2. I see that the file in the screenshot has 00:00:00 Length. Can you try with other media files ?(audio & video)
  3. Do you have LavFilters installed on your system? (BSPlayer is an example of a software that would installed LavFilters on your system).

Again I am just fishing here, have no idea what's going on

@daslicht In my screenshot you're seeing Direttore File Manager, with DarkR theme. I am the author. If you want send me an email address, to send you a promo code. Having said that, Direttore right now has no keyboard support, an update should be up in a few days.

daslicht commented 4 years ago
1. Is this file on a special location? e.g. inside a long path, inside a device (phone?)?

nope, it also locks if i place a file at the root: c:\test.wav or c:\temp\test.wav

2\. I see that the file in the screenshot has 00:00:00 Length. Can you try with other media files ?(audio & video)

I get that with any file, here lets take this as reference:\ https://workupload.com/file/XzYTECgyQPK|

3\. Do you have LavFilters installed on your system? (BSPlayer is an example of a software that would installed LavFilters on your system).

nope i don't even know what this is.

Just tried the msstore version: Still locked, this is what i get when i try to delete a file after playback:

renaming images (png) or mp4 seams to work.

You tried it in MS Explorer ?

WilliamDrt commented 4 years ago

@daslicht You tried it in MS Explorer ? - Yes ok i think i got what is happening, if i hit spacebar -> del, it locks But if i hit spacebar -> click on explorer -> del, then it doesn't lock we are using QL in a different way. I'll look into this, it has something to do with QL not getting focus when loading, and the way QL intercepts explorer's keystrokes I 'll have to pinpoint the code where this is happening, if anyone can provide a hint, that would be nice :-) Maybe we should reopen this until I figure this out?

daslicht commented 4 years ago

It not only locks with del, but also with renaming the file, so no keystroke involved

Hit spacebar to audition a wav sample. and then try to rename the file.

WilliamDrt commented 4 years ago

let me phrase it like this: If QL receives focus (click on QL window) then everything (del/rename etc) works well (no file lock).

Unfortunately, QL does not get focused on its own when loaded, user must click on QL window for QL to get focus. If user then click back on explorer (now explorer gets focused) then no file lock happens.

@daslicht if you could confirm that for me, please.

  1. Hit spacebar to audition a wav sample.
  2. Click on QL window
  3. Click on Explorer window
  4. Rename or Delete

I know this is not optimal, I just need to know what works and what not, to try fix it. Thanks

xupefei commented 4 years ago

Is it possible to due to the HasVideo logic in which the LavSplitterSource was locking the file but did not release it in time?

WilliamDrt commented 4 years ago

@xupefei Yes, it releases the file as expected, ONLY if programs exchange focus. Funny thing is while debugging, VS would get focus on breakpoint, and during live testing I would always use my mouse to go from QL to explorer.

This happens because RCW class are cached for perfomance reasons, so even if we're telling the damn thing to Marshal.ReleaseComObject(sourceFilter); it doesn't do so, until QL loses focus. Two ways around this

  1. Force a GC collection after HasVideo logic
  2. Use Marshal.GetUniqueObjectForIUnknown which is not cached to create the LavSplitterSource so when released, it will actually go away (and unlock the file)

I have already tested 1 and it works, I will report back for 2

NOTICE: regardless of the final solution if user clicks spacebar->delete rapidly, depending of how many ms apart the clicks were, 1 of 3 things would happened:

  1. QL window will report "No file found" internal exception but no crash
  2. File would lock
  3. Work as expected. if clicks are normal apart (in a NOT "doubleclick speed") it's always number 3
daslicht commented 4 years ago

@daslicht if you could confirm that for me, please.

1. Hit spacebar to audition a wav sample.

2. Click on QL window

3. Click on Explorer window

4. Rename or Delete

yes that works

WilliamDrt commented 4 years ago

OK N 2 was a waste of time, before I make a new pull request it would be nice if someone else tested this

WPFMediaKit.zip

in msi version

  1. stop QL from Task Manager
  2. go to: C:\Users\UserTest\AppData\Local\Programs\QuickLook\QuickLook.Plugin\QuickLook.Plugin.VideoViewer and swap WPFMediaKit.dll with the one inside the zip
  3. restart QL
  4. test
  5. report back

Thanks and sorry everyone, I should have catch this the first time around, but it was a little bit sneaky

daslicht commented 4 years ago

I dont have that file at this location here but there:

C:\Program Files\WindowsApps\21090PaddyXu.QuickLook_3.6.6.0_neutral__egxr34yet59cg\Package\QuickLook.Plugin\QuickLook.Plugin.VideoViewer

But I cant delete it even after stopping Quicklook ?

WilliamDrt commented 4 years ago

you have the store version, you need the msi instalation, which I know is what you had originally, but switched to store version for testing, now you'll have to switch again to test, this is all my fault, sorry, but I think we've got it right this time

daslicht commented 4 years ago

AWESOME ! It works !

rabelux commented 4 years ago

Also confirming for Win 7, although there hasn't been an issue before. Renaming and deleting works with the released version and your edited .dll

xupefei commented 4 years ago

Thank you, guys! I have merged the PR from @WilliamDrt and will release an update soon.

Venipa commented 4 years ago

the app feels faster now, the locking seems to sustain if you switch fast between files

👌

xupefei commented 4 years ago

the app feels faster now, the locking seems to sustain if you switch fast between files

Yeah because we have no control over the GC. What we can do is to "suggest" the system to release the lock asap. This is the point that C++ (or Rust, or any "native" language) becomes superior to C#.

daslicht commented 4 years ago

here he shows how fast audio playback can get : https://resonic.at/

WilliamDrt commented 4 years ago

This can be a lot faster by avoiding the lock in the first place. I have a plan for this, but it 'll have to wait until next week, no time right now. I will post a a new dll for testing when ready. Cheers

daslicht commented 4 years ago

When you audition a wave file and then try to move the folder it resides, we get the lock message. To be able to move the file we have to either close the quicklook dialog or audition another file first. It woud; be awesome if it would just wor without locking, like teh files

KenBrand commented 2 years ago

I'm having a similar problem however I can delete the file that was just played but I can't delete the folder in which it resides or even resided after I deleted the file! I even tried the 4-step trick that seemed to work for many in the Store version but here in the v3.7.1 Portable version it doesn't work. Any ideas?