Login at Kodi Home

Aardvark · 2010-09-07, 12:14

Hi All,

Before I go through the effort of creating an official patch I would like to test the waters if a patch for vertical shift of the video screen would possibly be accepted.

The reason for this feature is twofold:
- when shifting non-fullscreen images up there is more room for subtitles below in the black bar
- when using a projector you can shift down the image so the bottom edge touches the black screenborder, this way the perceived contrast is improved and often it's easier on the neck (depending on the position of the screen obviously). Also vertical masking can now be used from the top only, which is a lot easier for DIY solutions.

I have it implemented similar like zoomin/zoomout, with both action ID's and a slider on the GUIDialogVideoSettings.cpp.

I realize for a complete implementation a textstring and SQL setting must be added which is a no-go for the current release, so what would make the most sense:

1) submit a patch using action ID's only for Dharma, and later a additional patch implementing GUI and SQL support

2) wait until later and submit it all in once

3) forget about it as it is not deemed interesting enough to apply anyway.

So what should I do: 1 or 2 or 3?

**spiff** · 2010-09-07, 12:24

2). i'd consider this a useful feature, but it has no chance of going into dharma, not even bits of it. dharma is frozen.

Aardvark · 2010-09-07, 12:28

spiff Wrote:2). i'd consider this a useful feature, but it has no chance of going into dharma, not even bits of it. dharma is frozen.

Makes sense. In which case: when should such a patch be submitted against which tree?

**spiff** · 2010-09-07, 12:29

against trunk. whenever you're done.

**spiff** · 2010-09-07, 12:37

that being said, do we really need a slider? is that level of granularity really needed? wouldn't a spinner with top, center, bottom suffice?

Aardvark · 2010-09-07, 12:37

OK, I'll start reading how to prepare my patch then :-)

One thing I could not find yet in the coding guidelines: how should I handle adding new string ID's, action ID's and such.

I noticed the ranges mostly leave no gaps at the moment, but codingwise the new ID's should be inserted where they make sense and not added at the numerical end of the range.

What is the easiest way for you guys to process a patch? Adding a beyond-the-used-range number but locating the code itself in the logical place? And in that case will you reshuffle all ID's when a major release comes near?

**spiff** · 2010-09-07, 12:40

for the most case, IDs are at at first-grab basis. the exception is a few ranges which are 'reserved' through comments in the xml file.

if your patch happens to use an id that has been taken in trunk in the mean time, we deal with it when we apply.

we do not reshuffle string id's on/after releases.

Aardvark · 2010-09-07, 12:49

spiff Wrote:that being said, do we really need a slider? is that level of granularity really needed? wouldn't a spinner with top, center, bottom suffice?

Hmm. Good point. The only reason for not aligning the image top or bottom with the screen top or bottom (or alternatively the default centering) would be when for instance you zoom out a 4:3 image on a 16:9 screen, but want to shift it up a little to create black bars below for subtitles (some people prefer subs on black even though they would loose part of the image).

But in that case you would probably want to drop some image lines from both top & bottom to keep the image centered, in which case simply shifting would not be adequate anyway.

So I think you are right. With three options even a spinner going both up and down would be overkill, but I think that is normal for the GUI even with only three options.

I'll think some more on it but will probably implement it like that.

Aardvark · 2010-09-07, 15:51

spiff Wrote:do we really need a slider? is that level of granularity really needed? wouldn't a spinner with top, center, bottom suffice?

Did some more thinking & testing: on a projector I would always just shift down to match the image bottom to the screenbottom.

But on a TV I use the shift for putting the subs fully in the black. And in those cases I shift the image just a handful of pixels up, usually only if a movie contains many double-line subs. And I just tested: shifting a 1:2.35 movie all the way up to the top on a 16:9 screen with the subs below is _ugly_ !

So for now I'll just stick with the fully variable V-shift (currently -1.0 to +1.0 screenheight).

Aardvark · 2010-09-09, 13:58

Patch submitted: http://trac.xbmc.org/ticket/10113

Aardvark · 2010-09-13, 09:38

Hi All,

Four days no reaction to my patch, unclear to me if this is normal 'cause everybody is incredibly busy, or did nobody notice it and should I have created a new thread?

**spiff** · 2010-09-13, 09:41

we're in the middle of a release. that's the cause. don't worry, it's not forgotten.

pieh · 2010-09-13, 10:53

Aardvark, I find your idea really useful I will test patch when I get back home and give some feedback here

rocamoya · 2010-09-15, 05:26

Hello Guys!
The thread is about this?...
http://forum.xbmc.org/showthread.php?p=6...post603617

How i can use this patch?

Thans.

Aardvark · 2010-09-15, 09:02

rocamoya Wrote:The thread is about this?...
http://forum.xbmc.org/showthread.php?p=6...post603617

This is indeed a patch for that feature.

Quote:How i can use this patch?

I'll contact you outside this thread for some pointers - let's not pollute the developer forum ;-)