[videoinfo] get video aspect ration from CDataCacheCore

[BigNoid]

…ect by dividing videowidth by videoheight

I noticed that in recent builds VideoPlayer.VideoAspect returns empty. While I couldn't find the root cause of this, I did find we return empty in case the stream details don't return the videoaspect ratio. This adds a fallback in that case by calculating the aspect ratio from the width and height.

EDIT:
Instead of adding a fallback I changed to PR to pull the aspect from CDataCacheCore instead of m_videoInfo as that is deprecated.

[BigNoid]

@Rechi I addressed your comments. There shouldn't be division by zero possible now.

[BigNoid]

ping @FernetMenta as this touches videoplayer

[FernetMenta]

VP does expose DAR in CDataCacheCore:
https://github.com/xbmc/xbmc/search?utf8=%E2%9C%93&q=GetVideoDAR&type=

you should query this

[BigNoid]

@FernetMenta I am not sure what to do here. Whenever I debug none of those lines hit and I am sure this area is far beyond my expertise.
Doesn't adding a fallback make sense in any case?

[FernetMenta]

nope, a fallback without having added the normal path does not make sense. the gui has the pull ar from the source I pointed to

[BigNoid]

the gui has the pull ar from the source I pointed to

Can you take a look at why it doesn't do that in current master?

[FernetMenta]

m_videoInfo is deprecated. guiInfoManager needs to pull the info from CDataCacheCore

[ksooo]

But only for the currently playing video. This only holds for VIDEOPLAYER_VIDEO_ASPECT, not LISTITEM_VIDEO_ASPECT.

[FernetMenta]

m_videoInfo is only for playing video

[ksooo]

m_videoInfo is only for playing video

Ha! Then, LISTITEM_VIDEO_ASPECT never was implemented correctly, it seems. Implementation does the same as VIDEOPLAYER_VIDEO_ASPECT, which seems wrong to me.

Good news is that for Estuary, LISTITEM_VIDEO_ASPECT is not used. ;-)

[ksooo]

@BigNoid:

diff --git a/xbmc/GUIInfoManager.cpp b/xbmc/GUIInfoManager.cpp
index b3e62cf7be..81815b4878 100644
--- a/xbmc/GUIInfoManager.cpp
+++ b/xbmc/GUIInfoManager.cpp
@@ -6207,7 +6207,7 @@ std::string CGUIInfoManager::GetLabel(int info, int contextWindow, std::string *
   case VIDEOPLAYER_VIDEO_ASPECT:
     if (g_application.m_pPlayer->IsPlaying())
     {
-      strLabel = CStreamDetails::VideoAspectToAspectDescription(m_videoInfo.videoAspectRatio);
+      strLabel = StringUtils::Format("%.2f", CServiceBroker::GetDataCacheCore().GetVideoDAR());
     }
     break;
   case VIDEOPLAYER_AUDIO_CHANNELS:

[BigNoid]

thx, will try that!

[ksooo]

It works. I have tested that. :-)

[BigNoid]

I changed it to @ksooo suggestion, but I had to change it to still run through the description routine to get the values skins expect.

[ksooo]

@BigNoid what's the description routine? If the value is different from what it was formerly, we should fix this in the core. So, what exactly is the problem (if any)?

[BigNoid]

what's the description routine? If the value is different from what it was formerly, we should fix this in the core. So, what exactly is the problem (if any)?

I mean https://github.com/xbmc/xbmc/blob/master/xbmc/utils/StreamDetails.cpp#L585
Skins are expecting any of the values defined there, many have images for these values to display. In these cases the images are not shown for example for 2.39 DAR.

See https://github.com/BigNoid/Aeon-Nox/tree/master/media/flags/aspectratio for example

[FernetMenta]

looks ok to me

[ksooo]

@BigNoid: Yeah, looks good (was a little bit blind, tbh, to dumb to spot the obvious diff between my diff and your code).

[ksooo]

jenkins build this please

[ksooo]

jenkins errors are unrelated.