Opened 14 years ago

Closed 14 years ago

#989 closed patch (fixed)

Aspect Override global setting shifts OSD sideways

Reported by: willu.mailingLists@… Owned by: danielk
Priority: minor Milestone: 0.19
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

==Symptoms==

If I leave the global Aspect Overide setting on "None", then the TOGGLEASPECT button works fine. If I use the global override however (e.g. I set mine to "FILL") then the video looks right, but the OSD is shifted to the right slightly - the right hand side is shifted off the screen. This doesn't correct itself if you use the TOGGLEASPECT button.

==Diagnosis==

The VideoOutput::letterbox variable gets initialized in VideoOutput::Init() from the global setting and then VideoOutput::VideoAspectRatioChanged?() is called to set up the display, etc. Most of the heavy lifting ends up in VideoOutput::MoveResize?().

When the TOGGLEASPECT button is pressed, a very similar call chain happens (NuppelVideoPlayer::ToggleLetterbox?() calls VideoOutput::ToggleLetterbox?() which sets VideoOutput::letterbox and then calls VideoOutput::VideoAspectRatioChanged?()). The only difference being that NuppelVideoPlayer::ToggleLetterbox?() also calls NuppelVideoPlayer::ReinitOSD(). ReinitOSD() only does something if you have an OSD, and the OSD isn't initted at the time VideoOutput::Init() is called.

I then switched on the debugging output at the bottom of VideoOutput::MoveResize?(). It shows no difference between the cases that work and the cases that don't. This doesn't look like a bug in MoveResize?().

I then tried moving the initialization of letterbox until after the initial OSD setup. This means leaving VideoOutput::letterbox set to the default off in VideoOutput::Init(), then setting it and calling aspectRatioChanged() later on. If I move that setup after the OSD has been initialized then the OSD is centred, but incorrectly sized - I need to re-initialize the OSD.

This leads to the following fix that works. It moves the setup of the letterbox mode after the initialization of the OSD. It uses the NuppelVideoPlayer? function to set the letterbox mode to the global setting. That call also re-inits the OSD for us.

Index: libs/libmythtv/NuppelVideoPlayer.cpp
===================================================================
--- libs/libmythtv/NuppelVideoPlayer.cpp	(revision 8516)
+++ libs/libmythtv/NuppelVideoPlayer.cpp	(working copy)
@@ -2229,6 +2229,9 @@
         }
     }
 
+    ToggleLetterbox(gContext->GetNumSettingOnHost(
+        "AspectOverride",     gContext->GetHostName(), kLetterbox_Off));
+
     playing = true;
 
     rewindtime = fftime = 0;
Index: libs/libmythtv/videooutbase.cpp
===================================================================
--- libs/libmythtv/videooutbase.cpp	(revision 8516)
+++ libs/libmythtv/videooutbase.cpp	(working copy)
@@ -260,10 +260,8 @@
         "PlaybackColour",     gContext->GetHostName(), 50);
     hue        = gContext->GetNumSettingOnHost(
         "PlaybackHue",        gContext->GetHostName(), 0);
-    letterbox  = gContext->GetNumSettingOnHost(
-        "AspectOverride",     gContext->GetHostName(), kLetterbox_Off);
 
-    VideoAspectRatioChanged(aspect); // apply aspect ratio and letterbox mode
+    VideoAspectRatioChanged(aspect);
 
     embedding = false;
 

If you move the new ToggleLetterbox?() call in NuppelVideoPlayer? up above the code block just before it (which inits the OSD) then the bug returns - it is definitely something to do with the OSD initialization. I don't know if the call to VideoAspectRatioChanged?() at the bottom of VideoOutput::Init() is still required. I left it in but removed the comment which now seems wrong.

This doesn't feel like a complete fix as I still don't understand exactly what in the OSD gets initialized incorrectly if the screen is set up for a letterbox mode when the OSD is first initted. The fix works and is relatively clean though. In the worst case, I've localized the underlying cause of the bug. (And this last reason is the reason this ticket is so verbose - if someone was looking for the true cause, then I think it'll help.)

Be well,

Will :-}

Change History (5)

comment:1 Changed 14 years ago by anonymous

I also experienced this problem. This doesn't seem to occur for with the latest svn 8617, although I don't know when it was actually fixed.

comment:2 Changed 14 years ago by anonymous

correction: does not occur in livetv anymore, but it is still present in watch recordings.

comment:3 Changed 14 years ago by anonymous

correction#2: with aspectoverride set to FILL, there is not OSD shift problem in livetv. if it is set to 16:9 zoom, there is a shift problem.

comment:4 Changed 14 years ago by danielk

Milestone: 0.19
Owner: changed from Isaac Richards to danielk
Type: defectpatch
Version: head

comment:5 Changed 14 years ago by danielk

Resolution: fixed
Status: newclosed

(In [8644]) Closes #989.

This gets a bit closer to the source of the problem than the patch attached to the ticket. OSD::LoadTheme?() doesn't like the letterboxing. I've set the bounds to standard ones while LoadTheme? runs. They are then reset to the correct ones at the bottom of the constructor (which we were doing anyway.)

Note: See TracTickets for help on using tickets.