Qt UI becomes unresponsive on stream of errors

added Component::Core: Input Difficulty::hard labels

changed milestone to %Bugs paradize

This is not really new, and it is hard to avoid. You just get an infinite loop of trying to open a file that does not exist.

In fact, you can still close the player. It takes a while because Qt4 is slow at continuously updating the error dialog.

@courmish I don't think this is hard to avoid, but I didn't look at the code (maybe could you link me it?)

I think the playlist in memory is mapped as a linked list (maybe with back and foward pointers).

So when a file isn't available anymore just delete the node and check the next one. When the list is empty just stop.

This should be a theoretically way to implement this, but I say I didn't look at the code, maybe your implementation is so different that a change will be impossible to do, but if you link me the c source of this module I'll be so glad to help ;)

thanks

There is no generic way to check that an input item does not exist anymore. It might work if the item is a local file, but even then it is highly questionable that absent items should be deleted. The file could be created/recreated later. Besides, it would be quite hard to propagate the error back to the playlist model.

In my opinion, the current behaviour of using whole CPU is correct: the user requests an infinite loop, (s)he gets it. The responsiveness problem is all on Qt4 UI side.

added Component::Interface: Qt + 1 deleted label and removed Component::Core: Input label

You pointed out a really good question and I think you can be right.

Anyway just two hints, if you want to fix the problem you can close the file, and try to open it again, if the file has been deleted the open will return NULL.

The second is that there is a memory leak, when VLC goes into the infinite loop the memory consumption slowly increases until all the ram is taken (and this slow increase is really annoying because the kernel takes something like 5 hours to kill it and the system is unusable because the cpu is 100%)

So I think you may look at the leak and IMAO the problem can be fixed in two ways:

put a delay (1 second maybe) when a file isn't found, this will let the user close vlc without invoking a kill -9 from bash
when a file isn't found anymore ask for the user with "I cannot find the file, would you like to remove it from the playlist?" (this is the behavior of programs like vmp)

Patch welcome.

changed title from Vlc becomes unresponsive to Vlc becomes unresponsive when looping over an absent item

3 years. Can you at least add an option to autodelete missing items from playlist? It's really annoying when reorganizing your audio collection.

5 years! :-)

This doesn't occur often but it is still annoying and it's still a bug.

There are two separate issues:

The VLC playlist can go into an infinite. As I already wrote in comment:5, this is, in my opinion, a user error, not a VLC bug.
The Qt GUI becomes very unresponsive and almost unusable when the playlist generate errors at a high frequently. This is a VLC Qt GUI bug; there are no such issues with other interfaces.

the user requests an infinite loop, (s)he gets it.

The user does not request the infinite loop that makes their player unusable. It is trivial to reach this state not only by trying to play a dead file but also by pasting an incorrect web URL or similar.

My suggestion would be that if VLC tries to play the same file twice in a row (with no user interaction inbetween) without success, it stops auto-advancing the playlist. This is pretty much how other media players do it as well. If a file failed to be played twice in a row, it's unlikely that it will succeed a millisecond later. Don't proceed to the next (or rather, same) playlist entry anymore in that case.

I experienced this today after cleaning up my music library, reordering/renaming/re-tagging, etc. A lot of file names and paths changed.

I opened up a playlist I created before the reordering and none of the files could be found. Loop was on, so VLC kept throwing errors and eventually froze, as is described above.

the user requests an infinite loop, (s)he gets it.

... except I didn't. I opened a playlist, got some errors (great), and experienced a program crash (not great). So VLC can not handle the case where files move. I was never given the opportunity to repair my playlist. I haven't tried it, but I feel like this would be much more obnoxious on a mobile (Android) platform.

Windows Media Player handles this by marking missing files in the playback list and skips them. If I manually attempt to play that file again, it will attempt a read, but will throw another error again if it doesn't exist. No infinite loop. Throws errors that I can resolve.

If a "valid" tag can't be added to the play queue list, maybe limiting the rate at which the player moves to the next file after an error would help? If there are lots of errors in succession, VLC would increment a delay before attempting to load the next file.

- Load file
- Fail load
- wait(0)    
- Load file  <--
- Fail load     |
- wait(+ N)  ---

Replying to [comment:13 thumbcore]:

the user requests an infinite loop, (s)he gets it.

... except I didn't. I opened a playlist, got some errors (great), and experienced a program crash (not great). So VLC can not handle the case where files move. I was never given the opportunity to repair my playlist. I haven't tried it, but I feel like this would be much more obnoxious on a mobile (Android) platform.

So what's the difference between what you did, and a user actually asking for an infinite loop? And yes, this is a real use case, notably in unattended cases.

changed title from Vlc becomes unresponsive when looping over an absent item to Qt UI becomes unresponsive on stream of errors

Also #2284 (closed)

Regardless of whether an intinifte loop is initiated by user error, it is quite possible to warn the user that what he has requested is likely not intentional.

An infinite loop is indeed a valid use case, and thus should not trigger any kind of warning or error. Unless it means looping though items wich cannot be played, or looping though items which are each shorter than say 3 seconds. In those two examples, it is fair to assume the user does not intent infinite looping, as it will become hard to interact with VLC. If such occurances can be detected ahead of initationg playback, it should throw a warning and ask the user if this is truly his intent.

In practice, there might be need for another solution, as detecting these cases would mean parsing ahead of time which items can be played, and what the lenght of the playable items is (even the the playback speed setting becomes relevant, because playback at half speed might mean there is no issue to warn the user about, if short items are played slowly, the user will have plenty of time to interact with the user interface).

Instead of throwing a warning, another more elegant solution would be to revise the way the userinterface is updated, so even if short items are played in an infite loop, there is no difficulty interacting with the user interface. This solution would negate the need for parsing the items ahead of time, and negate the need for a warning dialog. Whether it is possible however is a much more technical discussion.

The above is quite obvious. The more interesting case in my eyes, is a combination of normal playable items, and a few unplayable items, in a looped playlist. There is a use case for retrying an item, however when playing from file, and the fle has not been altered since the last attempt, thowing an error again just gets annoying to the user and provides little to no new information. If it is possible, I would suggest to come up with a check whether or not an item has been attempted and failed playback before, and whether or not this item is a static file which has not been updated since the last attempt. If so, don't attempt again, and thow a warning instead of an error (warning: skipped item because it has been attempted before, and failed playback). Otherwise do attempt playback.

There is a difference between these cases. This ticket #5901 (closed) describes a looping though unplaiable items or very short items, causing the user interface to lock up. The issue I just described however, does not cause the user interface to lock up, as there are also plenty of normal lenght playable items populating the playlist, giving the user plenty of oppertunity to interact with VLC. Never the less, gettign errors for each re-attempt on a file which has already thrown an error before can also be regarded undesirable, and might also be fixed. The issues are closely related, also from a technical stance, but are not identical. I described the issue which does not tigger a lockup of the user interface in #18582 (closed). It has been marked a duplicate of this ticket (understandably so), but I would like to emphasise that difference, as it might get overlooked if we solely focus on the case of the user interface locking up.

There will always be ways for the UI to get a lot of errors. After the most obvious case of repeating a nonexistent file is addressed, there will be the problem of repeating a file that cannot be parsed, or a resource with an invalid URI, or an invalid sout chain, and so on.

IMO, the GUI really shall not become unresponsive, regardless of how many errors occur and how short played items are.

How about just implementing some kind of backoff for playing failed items?

As some previous commenters said, if the file fails to play x times in a row retrying instantly probably won't help and adding a few 100 ms won't be exactly noticable either, but would probably help keep the level of error messages to a sane amount.

It does not solve the problem. The UI is still unresponsive if there is a stream of errors - missing file in a loop is only one of the scenarii.

I really don't understand why this is so hard to fix, sure maybe some people require infinite looping, but the rest instead gets an unresponsive UI, it's like being back at the stone age of computing.

For me running mac it's 100% unresponsive when the local file does not exist. Using over 300% CPU (and distnoted runs at ~100%), that's 400% of a total 800% (4 cores with hyper threading), as well as consuming 4MB of memory per second (haven't let it run and while it might slow down, I could imagine it consuming as much as it can). All this just because it cannot find the file on disk, it's absolutely ridiculous.

Just put an option in preferences to warn if such potentially detrimental infinite loops are detected or an option to wait for a second if an error occurs (no matter if it's a parsing error or network error or file error) to avoid locking up UI or something.

There are so many ways this can be solved without compromising this apparently cherished "feature" to loop and spit out errors as fast as possible without any disregard for anything else. It can't be that hard to implement something that works for both sides, I'd try myself, but I have no experience working with QT or VLC so I'd rather have someone who knows what they're doing do it.

Also #21564 (closed) which is NOT about a missing file. Proving that the problem is the Qt error dialog if it needed.

#21812 (closed)

Wow just disable the loop if less than one file played in the playlist. Until then I'll just continue uninstalling VLC everywhere I go.

somewhere in here https://github.com/videolan/vlc/search?q=playlist+loop

Added attachment:

0001-tight-playlist-loop.patch

It's perfectly possible, and quite common a "failure" case, to have a tight loop while one or more item played. If it were that simple, the bug would have been fixed years ago.

Replying to [comment:25 Rémi Denis-Courmont]:

It's perfectly possible, and quite common a "failure" case, to have a tight loop while one or more item played. If it were that simple, the bug would have been fixed years ago.

That is not correct; you are confusing an infinite loop with a tight loop. We don't care about infinite loops, only the sub classification of tight loop where no file plays, because even if one file plays for 1 ms there is a break in the loop for context switching to occur and permit the UI to be responsive. Even if the less tight loop of a 1 ms file is an issue it's still best to fix some of the issue instead of none (no software is bug free, but good software tries).

The simplicity of any bug is not relevant; if someone skilled cares it gets fixed.

Empirical evidence says you are wrong.

Replying to [comment:27 Rémi Denis-Courmont]:

Empirical evidence says you are wrong.

No it just says no one working on VLC cares about edge case bugs.

My bug #22118 (closed) was marked as a duplicate of this one. Just to be clear, I was not reporting this error from a missing file in a playlist, like this bug above describes. I had merely tried to open a corrupt file.

As a user I have no idea whether a file is playable until I try to open it. It's absurd to tell me that I "requested an infinite loop". Nobody EVER requests that VLC hang and have to be killed by OS intervention.

There appears to be a checkbox on the error report that is meant to suppress further errors. This is checked, but it will continue producing errors anyway. This is quite contrary to what this user expects that checkbox to do.

tim's suggestion above to add a special case for only 1 item in the playlist, obviously it doesn't fix the generic problem of any size playlist, but solving this problem for a single item probably would help A LOT of users. The 1 item case has come up for me many times since this bug has existed, the multi item case much more rarely.

Edit: maybe that's not quite what tim was suggesting. At any rate, I realize this is classified as a hard problem, but it feels severe to me. Is there really no alternative to allowing infinite errors to jam up the program? Maybe a developer would have a need for an infinite error log, but I think most users can't make use of any error past the first one here.

Could the error log be truncated after some high number like 100 errors? Maybe post an error that says "Error log truncated" and just stop adding to this infinite log that causes this serious issue for many users? Give an option to keep going for the kind of person that needs the infinite output, but that's certainly not what the average user needs from VLC.

Replying to [comment:29 rainwarrior]:

tim's suggestion above to add a special case for only 1 item in the playlist, obviously it doesn't fix the generic problem of any size playlist, but solving this problem for a single item probably would help A LOT of users. The 1 item case has come up for me many times since this bug has existed, the multi item case pretty much never.

I don't think tim meant that if you have only 1 item in the playlist in total but e.g. if you have 10 items in playlist, and repeating all is toggled and all of them error out and you reach the end of the playlist, then stop the loop. Again this doesn't have to be a forced feature (i.e. instead an option), so that those who likes their programs to hang can happily have them still do so.

That only leaves the case with when you repeat a single item instead of all which errors out infinitely.

The (current implementation of the) error dialog is the problem. Originally, it would crash completely when a lot of errors occurred, so the problem was hidden behind another bug.

Rate-limiting the playlist cannot fix the issue, only remove some of the most obvious ways to trigger it.

Like I wrote earlier, there are two separate problems.

The most immediate problem is that the VLC GUI becomes unresponsive (or mostly unresponsive) when a lot of errors occur in succession.

The other issue is that the playlist can be set up such that VLC consumes 100% CPU doing nothing useful. Note that if it were not for the unresponsive error dialog, you couldeasily press the stop button to resolve this manually (you can try with the HTTP interface already).

While they are often triggered together and confused, the two problems are independent. The error dialog can make the GUI unresponsive in other ways than looping the playlist. Conversely, VLC can be made to consume 100% uselessly without triggering any error. At this point, I really cannot take seriously anybody that still insists on confusing the two problems.

At this point, I really cannot take seriously anybody that still insists on confusing the two problems.

Well, could you explain to me how I, as an end user, can figure out which of the two problems I am having, so that I can report it and you can take me seriously? All that I have at my disposal is knowing what I did, and what I can observe the application doing.

Why is "removing some of the most obvious ways to trigger" a problem not a worthwhile thing to do?

I don't mean to sound like I'm demanding an answer, you know much more about the system than I ever will. I'm just saying that from my perspective as an end-user, the program effectively hanging is a BAD problem, especially because I can't forsee that it will happen before opening the file.

So if there is a partial solution that fixes some of the problem, or can remove the problem for a significant number of users, I just want to suggest that this partial fix would be valuable to many.

The new player will wait for 100, 200, 400, 800, 1600 and finally 3200ms before opening the next input in case of error.

So maybe this branch https://github.com/tguillem/vlc/commits/qml/39 is fixing the issue.

assigned to @tguillem

changed milestone to %4.0

Replying to [comment:33 rainwarrior]:

At this point, I really cannot take seriously anybody that still insists on confusing the two problems.

Well, could you explain to me how I, as an end user, can figure out which of the two problems I am having, so that I can report it and you can take me seriously? All that I have at my disposal is knowing what I did, and what I can observe the application doing.

Most users are hitting both problems at the same time. But then the problem whereby the error dialog makes the GUI (or the whole desktop) unresponsive is far worse, than the (easily recoverable/stoppable) busy loop.

Why is "removing some of the most obvious ways to trigger" a problem not a worthwhile thing to do?

I already answered that question: Doing so will not fix either of the issues. The GUI will still become unresponsive on errors, and the playlist will still enter accidental busy loops.

And among the many many "duplicates" of this bug, you can find quite a few reports that are not your typical playlist looping over deleted files.

Fixed since Qt switched to the new vlc_player_t API.

added Status::fixed label

closed

Still reproducible. It seems even worse now.

removed Status::fixed label

reopened

Could you tell us the step to reproduce this issue ?

I tried with the steps mentioned in the ticket summary and found out the bug was fixed.

As far as I can tell, and as I wrote quite a few times already in earlier comments, any situation leading to a storm of errors makes the Qt GUI unresponsive (and not say the HTTP or RC UIs).

Also as noted earlier, it's vain to try to fix that at the playlist level since a failing item is not the only way to trigger an error.

Originally, the Qt error dialog would quickly crash on error storm due to memory races.

The root cause of the problem is that, while I fixed the races a long time ago, pacing and/or pooling was never added to mitigate storms.

Replying to [comment:39 Rémi Denis-Courmont]:

Also as noted earlier, it's vain to try to fix that at the playlist level since a failing item is not the only way to trigger an error.

Yet, it's done now in the playlist level and will prevent most busy loop that are not necessarily Qt related.

I'm not sure if one module from only one input could trigger a dialog loop now. If it happen, we could fix these modules. I'm not sure it's a Qt issue now.

Another path to reproduce the error (Windows 7 SP1):

Add a shortcut from a mp3 file to a playlist
delete the file where the shortcut points to

To avoid this error, it should not be neccessary to delete the entry in the playlist, but mark it as void - for this session. Or - simply halt playlist processing (and thus looping) in case of an error - any error. And ask the user what to do. In other words - what is the sense in re-running a faulty playlist again and again, and not waiting for the user to react to the error?

Folks, there is a conceptual mistake in the playlist processing! Fascinating that this error is still alive after 7 years.

Qt UI becomes unresponsive on stream of errors

Child items ...

Activity