Hi, On Sun, Sep 11, 2022 at 06:48:50AM +0200, Mateusz Kwiatkowski wrote: > >> Those extra vbp lines will be treated as a black bar at the top of the frame, > >> and extra vfp lines will be at the bottom of the frame. > >> > >> However if someone specifies e.g. 720x604, there's nothing more you could > >> remove from vfp, so your only option is to reduce vbp compared to the standard > >> mode, so you'll end up with (vfp==4, vsync==6, vbp==11). The image will not be > >> centered, the topmost lines will get cropped out, but that's the best we can do > >> and if someone is requesting such resolution, they most likely want to actually > >> access the VBI to e.g. emit teletext. > >> > >> Your current code always starts at (vfp==5 or 6, vsync=6, vbp==6) and then > >> increases both vfp and vbp proportionately. This puts vsync dead center in the > >> VBI, which is not how it's supposed to be - and that in turn causes the image > >> to be significantly shifted upwards. > >> > >> I hope this makes more sense to you now. > > > > I'm really struggling with this, so thanks for explaining this further > > (and patiently ;)) > > > > If I get this right, what you'd like to change is this part of the > > calculus (simplified a bit, and using PAL, 576i): > > > > vfp_min = params->vfp_lines.even + params->vfp_lines.odd; // 5 > > vbp_min = params->vbp_lines.even + params->vbp_lines.odd; // 6 > > vslen = params->vslen_lines.even + params->vslen_lines.odd; // 6 > > > > porches = params->num_lines - vactive - vslen; // 43 > > porches_rem = porches - vfp_min - vbp_min; // 32 > > > > vfp = vfp_min + (porches_rem / 2); // 21 > > vbp = porches - vfp; // 22 > > > > Which is indeed having sync centered. > > > > I initially changed it to: > > > > vfp = vfp_min; // 6 > > vbp = num_lines - vactive - vslen - vfp; // 38 > > > > Which is close enough for 576i, but at 480i/50Hz would end up with 134, > > so still fairly far off. > > > > I guess your suggestion would be along the line of: > > > > vfp_min = params->vfp_lines.even + params->vfp_lines.odd; // 5 > > vbp_min = params->vbp_lines.even + params->vbp_lines.odd; // 38 > > vslen = params->vslen_lines.even + params->vslen_lines.odd; // 6 > > > > porches = params->num_lines - vactive - vslen; // 0 > > porches_rem = porches - vfp_min - vbp_min; // 0 > > > > vfp = vfp_min + (porches_rem / 2); // 5 > > vbp = porches - vfp; // 38 > > > > Which is still close enough for 576i, but for 480i would end up with: > > > > porches = params->num_lines - vactive - vslen; // 139 > > porches_rem = porches - vfp_min - vbp_min; // 96 > > > > vfp = vfp_min + (porches_rem / 2); // 53 > > vbp = porches - vfp; // 86 > > > > Right? > > Yes. And if that's supposed to mean 480i in 50 Hz "PAL" mode, that's also > "close enough" to the values I suggested above. > > If you substitute values for true 60 Hz "NTSC" 480i, you should also get values > that are "close enough" to the official spec. > > The only thing I'd conceptually change is that the 38 lines is not really > "vbp_min". It's more like "vbp_typ". As I mentioned above, we may want to lower > this value if someone wants more active lines than the official 486/576. porches_rem is an int, so if vactive > (num_lines + vslen + vfp_min + vbp_min), porches_rem is going to be negative and we'll remove equally between vfp and vbp to match what's been asked So I believe this should work fine? Maxime
Attachment:
signature.asc
Description: PGP signature