Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not Lock file #603

Closed
daslicht opened this issue Jan 23, 2020 · 40 comments
Closed

Do not Lock file #603

daslicht opened this issue Jan 23, 2020 · 40 comments

Comments

@daslicht
Copy link

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
Copy link
Member

xupefei commented Feb 2, 2020

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
Copy link
Author

daslicht commented Feb 6, 2020

Thank you for the reply~!

@daslicht
Copy link
Author

daslicht commented Feb 6, 2020

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
Copy link
Member

xupefei commented Feb 6, 2020

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
Copy link
Author

daslicht commented Feb 6, 2020

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
Copy link

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
Copy link

WilliamDrt commented Feb 29, 2020

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
Copy link
Member

xupefei commented Mar 1, 2020

@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
Copy link

WilliamDrt commented Mar 1, 2020 via email

@WilliamDrt
Copy link

WilliamDrt commented Mar 5, 2020

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:

  • Temp graph
  • Load LavSplitterSource
  • Run the HasVideo logic
  • Unload LavSplitterSource
  • Released temp graph

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
Copy link
Author

daslicht commented Mar 5, 2020

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 :
9333656

@rabelux
Copy link
Member

rabelux commented Mar 5, 2020

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
Copy link

WilliamDrt commented Mar 5, 2020

@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
Copy link
Member

rabelux commented Mar 5, 2020

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
Copy link
Member

rabelux commented Mar 5, 2020

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
Copy link

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

@daslicht
Copy link
Author

daslicht commented Mar 25, 2020

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
Copy link

WilliamDrt commented Mar 25, 2020

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
Copy link
Author

daslicht commented Mar 25, 2020

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
Copy link
Author

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

@WilliamDrt
Copy link

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
Copy link
Author

daslicht commented Mar 25, 2020

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
Copy link

@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
Copy link
Author

daslicht commented Mar 26, 2020

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
Copy link

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
Copy link
Member

xupefei commented Mar 28, 2020

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
Copy link

WilliamDrt commented Mar 28, 2020

@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
Copy link
Author

@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
Copy link

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
Copy link
Author

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
Copy link

WilliamDrt commented Mar 29, 2020

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
Copy link
Author

AWESOME ! It works !

@rabelux
Copy link
Member

rabelux commented Mar 30, 2020

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
Copy link
Member

xupefei commented Apr 4, 2020

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

@Venipa
Copy link

Venipa commented Apr 7, 2020

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

👌

@xupefei
Copy link
Member

xupefei commented Apr 8, 2020

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
Copy link
Author

daslicht commented Apr 8, 2020

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

@WilliamDrt
Copy link

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
Copy link
Author

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
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants