[pieh]

Finally got some time to test it and I must say that for me it works great. As I'm lazy I added those lines to keyboard.xml so i dont have to go into video osd to move it:

Code:

<minus mod="shift">verticalshiftdown</minus>
      <plus mod="shift">verticalshiftup</plus>

PS. in Key.h there was already #define ACTION_PLAYER_PLAYPAUSE 227 so ACTION_VSHIFT_UP needed to be defined with different value, but this is just cosmetic

Also, when using keyboard as I wrote above, when moving on slider that show on top there is showing "delay by/ ahead by" before values - when i'll finish my stuff i'll take a look into this

If You are already here maybe we could discuss adding subtitle shifting (if You decide to add it I could help with it)

[Aardvark]

Code:

<minus mod="shift">verticalshiftdown</minus>
      <plus mod="shift">verticalshiftup</plus>

Good selection of keys, I might copy that. I was just using ctrl-Q and ctrl-W for testing, and mapped some remote buttons to them. Lot of stuff is only accessible with mouse by default, that's why I did not add keys. Think Zoom in/out.

PS. in Key.h there was already #define ACTION_PLAYER_PLAYPAUSE 227 so ACTION_VSHIFT_UP needed to be defined with different value, but this is just cosmetic

Also, when using keyboard as I wrote above, when moving on slider that show on top there is showing "delay by/ ahead by" before values - when i'll finish my stuff i'll take a look into this

Both side effects of re-used codes. I'll double check, but when applied to correct revision it should display relevant text at the 'delay/ahead by" position. Think some other patch was applied before this, they will fix it when submitting.

If You are already here maybe we could discuss adding subtitle shifting (if You decide to add it I could help with it)

You know it's already there? Only no shortkeys (just mousing), and that was indeed what I was going to add as well.

[pieh]

You know it's already there? Only no shortkeys (just mousing), and that was indeed what I was going to add as well.

Well I see it in Video Calibration and this is not really handy. BTW Maybe consider also adding Video Shift to Video Calibration.

[pieh]

ok, if You are about to add shifting subtitles - here's some begining:
GUIWindowFullScreen.cpp

Code:

case ACTION_SUBTITLES_VSHIFT_UP:
    {
      RESOLUTION_INFO& res_info =  g_settings.m_ResInfo[g_graphicsContext.GetVideoResolution()];
      res_info.iSubtitles --;
      if (res_info.iSubtitles < 0)
        res_info.iSubtitles = 0;
      ShowSlider(action.GetID(), 274, res_info.iSubtitles, 0, 1, res_info.iHeight);
      break;
    }
  case ACTION_SUBTITLES_VSHIFT_DOWN:
    {
    RESOLUTION_INFO& res_info =  g_settings.m_ResInfo[g_graphicsContext.GetVideoResolution()];
    res_info.iSubtitles ++;
    if (res_info.iSubtitles >= res_info.iHeight)
      res_info.iSubtitles = res_info.iHeight - 1;

    ShowSlider(action.GetID(), 274, res_info.iSubtitles, 0, -1, res_info.iHeight);
    break;
    }

[...]

CGUIWindowFullScreen::OnSliderChange:

  if (m_sliderAction == ACTION_ZOOM_OUT || m_sliderAction == ACTION_ZOOM_IN ||
      m_sliderAction == ACTION_INCREASE_PAR || m_sliderAction == ACTION_DECREASE_PAR ||
      m_sliderAction == ACTION_VSHIFT_UP || m_sliderAction == ACTION_VSHIFT_DOWN ||
      m_sliderAction == ACTION_SUBTITLES_VSHIFT_UP || m_sliderAction == ACTION_SUBTITLES_VSHIFT_DOWN)
  {

Key.h

Code:

#define ACTION_SUBTITLES_VSHIFT_UP              230 // shift up subtitles
#define ACTION_SUBTITLES_VSHIFT_DOWN            231 // shift down subtitles

ButtonTranslator.cpp

Code:

{"subtitlesshiftup"   , ACTION_SUBTITLES_VSHIFT_UP},
        {"subtitlesshiftdown" , ACTION_SUBTITLES_VSHIFT_DOWN},

and keyboard.xml

Code:

<q mod="ctrl">subtitlesshiftdown</q>
      <w mod="ctrl">subtitlesshiftup</w>

It's working - reused stuff from calibration code
Well, dont know if to add slider to Video OSD or not, it's quite cluttered anyway

As for Delayed by / Ahead by stuff:
it was caused because somehow CGUIWindowFullScreen::OnSliderChange wasn't modified - weird, it's ok now

[Aardvark]

At first sight your subtitle shift it looks all complete, I'll try to give it a spin tomorrow. Any particular reason you are not submitting the patch yourself?

About video calibration: IMHO the subtitle shift should not be there. The size calibration is only to compensate overscan issues and define monitor pixel ratio, it effects the complete GUI. It does not make sense to have it accessible outside the system menu and set up for any video, except that someone put the subtitle shift on it. But I do not think as a newbie I would ever dare to remove it :-) So I will definitely not add any functionality myself to that screen, not for sizing, panning, shifting, whatever.

[pieh]

Aardvark:

Well It's not complete - if You shift subtitles when no subtitle is visible than You can't see current position of subtitles - I will take a look if I can force player to display "Line 1\nLine 2\nLine 3" (or something like that) subtitle when shifting position. I think those "test" subtitles could be also displayed when shifting video

I will post complete patch (including Your stuff) for current revision when i'll think it's ready. I have a lot of custom stuff in my code so editing .patch file to include only relevant code isn't something I would like to do every 20 minutes.

The subtitles shifting is already in video calibration so nothing I can do about it, but I agree that it shouldn't be there.

[Aardvark]

Well It's not complete - if You shift subtitles when no subtitle is visible than You can't see current position of subtitles

I'd prefer it this way myself. I only move subtitles when I see them, much easier that way. The number of lines is depending on the movie, so you wouldn't know anyway if you'd have to show one, two or three (yes, I have them) lines.

What you might do is re-use the blue line from the calibration screen, and show it a short time after you shifted. That way both interfaces for shifting will look the same.

I will post complete patch (including Your stuff)

Don't include my stuff, keep it a clean subtitle shift patch. Chances of accepting are probably very low if mixing patches (rightly so - it will save them time).

[CrystalP]

Yes keep things separate please.
How does this work for videos of different aspect ratios? A new option to have the subtitles automatically snap to the bottom/top of the video could be nice.

[Aardvark]

How does this work for videos of different aspect ratios?

Currently I allow 100% up/down shifting where the percentage is related to the video size. Which means you can shift some image off-screen, even with a 1:2.35 image in a 4:3 screen. There is not enough shift theoretically to move a 1:2.35 image to the edges of a vertically mounted 16:9 screen but I am not sure that would target a big audience... ;-)

The reason I allow shifting over the edge is that it gives the user the ability to sacrifice some top image lines in order to have the subtitles all on black below. (A perfect player might allow user controlled independant cropping on all sides but I do not feel like implementing that - bit too much for XBMC I think. Let's leave that to Zoomplayer ;-)).

A new option to have the subtitles automatically snap to the bottom/top of the video could be nice.

Might be another nice feature. Some sort of "manual/auto-top/auto-bottom" toggle. And the distance between title and edge of image based on the fontsize. And with alignment at the bottom always aligning the lowest line. This would be a big improvement for 3-line subs on my 16:9 screen with 16:9 images, now for some movies I have to align them so the rarely used third line will show, but this means that the 90% single-line subs show up very prominently high in the movie.

[bleze]

How about scanning through the subtitles and count how many of each line count there are and base calculation on that, so that most the time it looks ok.

The positioning of video and sub should be saved to database if not already done.

[Aardvark]

How about scanning through the subtitles and count how many of each line count there are and base calculation on that, so that most the time it looks ok.

Not feasible for anything except external subs. Would have to read all file before being able to position.

The positioning of video and sub should be saved to database if not already done.

My patch saves its Vshift position in the database. The subposition I do not know, but makes very much sense it should be saved as well.

[mikper]

+100 points from me for an option to make subtitle snap to the bottom of the video (make the position relative to the video size rather than to the display dimension).

I have already asked about this, and was considering diving into the XBMC code to try to "fix" this, but it seems related work is already ongoing. This was my initial question:
http://forum.xbmc.org/showthread.php?tid=80170

@grajen3: Have you created a thread for your issue (or ticket) ? Also, if possible, could I have a look at your changes?
I have been looking a the XBMC code a bit today to figure out where to implement this kind of feature (i.e. option to make subtitle position relative to screen or video), and if you could point me in the right direction in the code I'd appreciate it.

Thanks

[pieh]

mikper: im now busy doing some other stuff, but basicly all things needed for basic subtitles shifting (just absolute position of subtitles) is posted on the previous page - the first part of GUIWindowFullScreen.cpp code change is placed in cGUIWindowFullScreen::onAction method - it's all You need to get this going

as I said I have LOT of custom code so i will create patch when i'll do some devlopment on "snap to video bottom" part (this might take some time)

[mikper]

OK, thanks. I'll have a look at that and see where it takes me.

[rocamoya]

Aardvark:

thanks for ur help. but im very noob with this and maybe u can send me or upload to media fire or rapidshare ur xbmc compiled with this patch. only the xbmc.exe maybe works
thanks for ur time for yes or not.