Thanks for reviewing Hans. My answer below - On 15/05/15 3:28 pm, "Hans Verkuil" <hverkuil@xxxxxxxxx> wrote: >Hi Prashant, > >Sorry for the very late review, I finally have time today to go through >my pending patches. > >I have one question, see below: > > >> @@ -539,9 +555,11 @@ bool v4l2_detect_gtf(unsigned frame_height, >> >> /* Vertical */ >> v_fp = GTF_V_FP; >> - >> v_bp = (GTF_MIN_VSYNC_BP * hfreq + 500000) / 1000000 - vsync; >> - image_height = (frame_height - v_fp - vsync - v_bp + 1) & ~0x1; >> + if (interlaced) >> + image_height = (frame_height - 2 * v_fp - 2 * vsync - 2 * v_bp) & >>~0x1; >> + else >> + image_height = (frame_height - v_fp - vsync - v_bp + 1) & ~0x1; >> >> if (image_height < 0) >> return false; >> @@ -586,11 +604,20 @@ bool v4l2_detect_gtf(unsigned frame_height, >> fmt->bt.hsync = hsync; >> fmt->bt.vsync = vsync; >> fmt->bt.hbackporch = frame_width - image_width - h_fp - hsync; >> - fmt->bt.vbackporch = frame_height - image_height - v_fp - vsync; >> + fmt->bt.vbackporch = v_bp; > >Is this change correct? My main concern comes from the earlier >image_height calculation >in the chunk above. The image_height value is rounded to an even value, >but if the value >is actually changed due to rounding, then one of the v_fp, vsync or v_bp >values must >change by one as well. After all, the frame_height is fixed and >image_height must be >even. And frame_height can be even or odd, both are possible. So that one >line of rounding >difference must go somewhere in the blanking timings. Thanks for spotting this. Yes, I agree that it does not look correct. If the image_height gets rounded to next even value, above calculation will result in one extra line. For interlaced case, I am wondering where to absorb the rounding difference ? I suppose, we cannot absorb this difference in vsync. Should it be v_bp of even field or odd field ? Not sure, but probably, the bottom (even) field ? Regards, Prashant > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
