PVR: Show full screen video information even if stream isn't working

[mikrohard]

I'm moving this pull request from opdekamps repo here to continue the discussion on what the best solution for this problem is.

Old pull request for description and discussion history: opdenkamp#623

This may not be the cleanest way to do it but it fixes the described problem without breaking pvr radio visualizations. Any other suggestions?

[jmarshallnz]

Wouldn't it be better is IsPlayingVideo returned true, or there was some better flag to check at the app level?

[opdenkamp]

-> opdenkamp#623 (comment)

[opdenkamp]

ping @mikrohard @elupus @FernetMenta
any updates on this? see the discussion on opdenkamp#623

[mikrohard]

I didn't research this any further. The code in current pull request doesn't break visualizations and fixes the described problem.

I agree that it would be cleaner to fix this in applications IsPlayingVideo or players HasVideo. But we need a non-locking way to check for radio if we don't want to break pvr radio visualizations.

[FernetMenta]

Maybe we are thinking much too complicated here. We are struggling to determine whether a video is playing or not, dealing with selected streams.
How about asking renderer: CXBMCRenderManager::IsConfigured() ?
As long as renderer is configured, we bring to display what renderer has to offer.

bool CDVDPlayer::HasVideo() const
{
  return g_renderManager.IsConfigured()
}

That would work during a channel switch and visualization for audio should work as well.

[opdenkamp]

sounds good to me

[opdenkamp]

i've tested changing CApplication::IsPlayingVideo() to check g_renderManager.IsConfigured() and it worked fine.

ping @FernetMenta @cptspiff what are your ideas about this? would this be safe to change for frodo? other option would be

--- a/xbmc/Application.cpp
+++ b/xbmc/Application.cpp
@@ -4333,6 +4333,8 @@ bool CApplication::IsPlayingVideo() const
 {
   if (!m_pPlayer)
     return false;
+  if (PVR::CPVRManager::Get().IsPlayingTV())
+    return true;
   if (!m_pPlayer->IsPlaying())
     return false;
   if (m_pPlayer->HasVideo())

[FernetMenta]

I have been testing this for a while: FernetMenta@3d95c5c

From a logical point of view it makes most sense to fix CDVDPlayer::HasVideo(). It is dvdplayer which calls renderManager.init/uninit. Hence between those two calls it clearly has video.

[elupus]

Check my upnp pull. I'm changing that check to is playing instead. No major
problems with that. Should solve this too right?

[elupus]

Okey.. i admit there are some issues with it. confluence seem to display both music title and video title in some places.

[opdenkamp]

@FernetMenta but then this would need to be fixed for other players too. would be nice if we could fix the check that applies to all players

@elupus does IsPlaying() return true when nothing is actually playing, e.g. when switching channels?

[FernetMenta]

I think this has to be fixed in several places. Even if a change in application may fix this particular problem, player::HasVideo is not correct. It might be called by a skin, right?. Yes, I would fix this method in all players.

[elupus]

@opdenkamp IsPlaying() does return true while switching players if the switch happens inside dvdplayer and not via a new call to OpenFile

[elupus]

I've updated my pull at #1597 to fix the display issues I had with my solution. Check the first commits of that pull request.

[opdenkamp]

that looks like it could fix it indeed after @elupus PR. i'll test it as soons as i got time.

[elupus]

I've noticed a bug with music tags that need to be resolved. But after
that, all those thing can go in separate of the upnp stuff.

[elupus]

Soo.. my upnp pull is not going to go in for Frodo. Maybe some parts can go in thou.

[elupus]

So what is the status of this now. The upnp stuff has been merged so it's time to take a look to see what on this pull is required.

[opdenkamp]

haven't had time to do anything with this yet.

[opdenkamp]

ping @FernetMenta FernetMenta@3d95c5c the right solution for this? if so, could you push it to master and close this PR