Skip to content

Implemented GUI feedback for playback errors#1828

Merged
CastagnaIT merged 4 commits intoxbmc:Piersfrom
CastagnaIT:gui_feedback
Apr 22, 2025
Merged

Implemented GUI feedback for playback errors#1828
CastagnaIT merged 4 commits intoxbmc:Piersfrom
CastagnaIT:gui_feedback

Conversation

@CastagnaIT
Copy link
Copy Markdown
Collaborator

@CastagnaIT CastagnaIT commented Apr 16, 2025

Description

Finally users will be happy to know what goes wrong with the playback
without necessarily having to read the debug log

this implementation will not make use of the localizations (language translations) of error details
because it would be too extensive a task and subject to frequent change depending on code change,
the only localization is done in the "generic" error message (Unable to play the stream)

GUI messages cover the most common problems that can occur during playback initialization, such as this example

immagine
immagine

Sidenote:
i have removed LogF from cdm library, i noticed only now that was conflicting with LogF defines in the utils/log.h

Motivation and context

When a playback fails for X reasons Kodi most of the time returns to the video list without letting the user know what happened... (sometimes there is a generic message "one or more items failed to play" but is not shown always)
so users are forced to read the debug log to know reasons for playback problems, or randomly try to blame something

a media player usually lets the user know what happened in a minimal way
but this currently is not supported by using InputStream API interface

hint from comment #1803 (comment)

and partially fix #497
about this last one, they also talk about revoked CDM widevine versions therefore no longer working, in this case we do not have the possibility to verify the situation in advance, the only possible way is to intercept a possible HTTP 400 error from the license request, which however not always this error code can have this specific meaning. theoretically, i think there should be a way to make a call to the ISHelper add-on to initiate a check/download of a different version of the CDM, but this is out of scope of this PR

How has this been tested?

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • I have read the Contributing document
  • My code follows the Code Guidelines of this project
  • My change requires a change to the Wiki documentation
  • I have updated the documentation accordingly

@CastagnaIT CastagnaIT added Type: Improvement non-breaking change which improves existing functionality v22 Piers labels Apr 16, 2025
@CastagnaIT CastagnaIT force-pushed the gui_feedback branch 2 times, most recently from e521a77 to 6c72e6b Compare April 17, 2025 07:30
@ksooo
Copy link
Copy Markdown
Member

ksooo commented Apr 17, 2025

@CastagnaIT from architectural point of view I'm asking myself whether an input stream add-on should really have GUI dependencies. I consider those add-ons purely backends, not frontends.

@CastagnaIT
Copy link
Copy Markdown
Collaborator Author

CastagnaIT commented Apr 17, 2025

@ksooo mainly the InputStream API interface dont allow a GUI feedback when the CInputStreamAdaptive::Open fails for some reason (and other callbacks such as OpenStream)

then users are unable to understand what is not working, and so whether it is something they can fix themselves, or whether they should report it as a possible bug

currently when playback fails for X reason, mainly two things happen:

  • Kodi do nothing, when you play a video that fails, kodi return to the video list and nothing more happens, user dont know the reason why kodi is returned to the video list without playing the video
  • Kodi (apparently at random) sometimes show a generic error message that dont explain nothing

At the moment i see this as the only possible solution
see screenshots for some examples

PS. anyway was already used GUI also for the manual stream selection (Ask quality)

@ksooo
Copy link
Copy Markdown
Member

ksooo commented Apr 17, 2025

Firstly, I don't want to block this PR, I'm only sharing some thoughts. If you feel to merge, I will not be in the way.

the InputStream API interface dont allow a GUI feedback

So that is the actual problem that should be fixed first. Not an easy task, but imo the right approach.

anyway was already used GUI also for the manual stream selection

That's not an excuse to make it even worse by putting more GUI stuff into the add-on.

If we ever want to have headless Kodi, where ia add-on can be used to serve streams, then things like "stream selection dialog" will just not work. And to make it work at least by means of not failing the add-on then would need to know about that headless mode, just because it mixed backend end frontend functionality together. You see my point?

@CastagnaIT
Copy link
Copy Markdown
Collaborator Author

CastagnaIT commented Apr 17, 2025

You see my point?

yes i had already understood all this since before i did this PR
modifying the IS API is quite chaotic work and i have no clear idea in how this change should be done, since also can affect several callbacks

i don't think it's the right time at today to make big changes on API for the Piers branch, since i guess the first releases aren't long in coming

Have a method to know from the addon whether the headless mode is enabled, yeah can help to handle particular use cases,
but, however i would also expect that in case i make a GUI call (example kodi::gui::dialogs::OK::ShowAndGetInput)
kodi should ignore the call when in headless mode, otherwise suggest that there is a underlying problem in kodi core if WindowManager loads GUI in headless mode simply by calling a method...

where ia add-on can be used to serve streams, then things like "stream selection dialog" will just not work

in case you don't know, the "Ask quality" feature (that make use stream selection dialog) has to be set manually through GUI addon settings or specific properties, whoever sets it "should" know what they are doing, generally if you don't have a GUI you don't go and set up a feature that requires it

@ksooo
Copy link
Copy Markdown
Member

ksooo commented Apr 17, 2025

however i would also expect that in case i make a GUI call (example kodi::gui::dialogs::OK::ShowAndGetInput)
kodi should ignore the call when in headless mode

Problem with that is that kodi can’t know how to properly ignore the call. Does ignore mean ‘cancel’, is it automatically return an empty string, the first entry of a list, … ?

generally if you don't have a GUI you don't go and set up a feature that requires it

works by luck for this settings, may not for others.

@CastagnaIT

This comment was marked as off-topic.

@ksooo

This comment was marked as off-topic.

@CastagnaIT

This comment was marked as off-topic.

@ksooo

This comment was marked as off-topic.

@CastagnaIT

This comment was marked as off-topic.

@CastagnaIT CastagnaIT marked this pull request as ready for review April 22, 2025 09:19
@CastagnaIT CastagnaIT merged commit c0f98bd into xbmc:Piers Apr 22, 2025
10 checks passed
@CastagnaIT CastagnaIT deleted the gui_feedback branch April 22, 2025 12:28
@CastagnaIT
Copy link
Copy Markdown
Collaborator Author

add future todos to roadmap InputStream Adaptive - Roadmap

@flubshi
Copy link
Copy Markdown

flubshi commented May 1, 2025

@CastagnaIT wow, nice addition!! I think it helps a lot of users to understand what is wrong.

One of the most frequently issue users of pvr.waipu are reporting to me is "it is not working", which is usually caused by missing/wrong Widevine installation. If users now see the error messages in UI, they can hopefully help themselves.
Great! Thank you!

@ksooo
Copy link
Copy Markdown
Member

ksooo commented May 1, 2025

wow, nice addition!! I think it helps a lot of users to understand what is wrong.

There is light and there is dark, with this. Now, the user gets presented two error message boxes (at least this happened to me yesterday). First box is raised by the addon, second raised by Kodi (the usual "some blabla failed to play. See log for blabla...")

And, for me the addon error message was like "failed to parse manifest". Not sure a normal user understands that very technical stuff. For me, those messages are rather something for a debug/error entry in the log file.

@CastagnaIT
Copy link
Copy Markdown
Collaborator Author

CastagnaIT commented May 1, 2025

There is light and there is dark, with this. Now, the user gets presented two error message boxes (at least this happened to me yesterday). First box is raised by the addon, second raised by Kodi (the usual "some blabla failed to play. See log for blabla...")

this is quite random, not always kodi core shown a kind of "error" message when playback fails
sometimes return to video list without any message
and sometimes show meaningless message such as "one or more items failed to play"

Edit: also one case is, when you start playback but ISA for x reasons do not provide packets to the demuxer, and so VP stops without any message, VP thinks the playback is finished but behind there is another problem if ISA dont provide packets

despite sometimes there is a chance that two messages will be shown, it is always better now than before

if for you this is very annoying
i can add a new addon setting that allow to disable GUI messages

And, for me the addon error message was like "failed to parse manifest". Not sure a normal user understands that very technical stuff. For me, those messages are rather something for a debug/error entry in the log file.

yes some errors are more technical, but for some of them i could use a more general message that suggest to read the log for more details about the problem

@ksooo
Copy link
Copy Markdown
Member

ksooo commented May 1, 2025

if for you this is very annoying
i can add a new addon setting that allow to disable GUI messages

Thanks, no need for that. Gladly, I encounter isa errors really very seldom.

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

Labels

Type: Improvement non-breaking change which improves existing functionality v22 Piers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IA does not expose playback errors using onPlayBackError()

3 participants