Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Steve Longerbeam writes: > One final note, it is incorrect to assign 'seq-tb' to a PAL signal according > to this new understanding. Because according to various sites (for example > [1]), both standard definition NTSC and PAL are bottom field dominant, > so 'seq-tb' is not correct for PAL. Actually this isn't the case: - real PAL (= analog) is (was) interlaced, so you could choose any "dominant field" and it would work fine (stuff originating as film movies being an exception). - the general idea at these times was that NTSC-style digital video was bottom-first while PAL-style was top-first. - for example, most (practically all?) commercial TV-style interlaced PAL DVDs were top-first (movies were usually progressive). - standard TV (DVB 576i) uses (used) top-first as well. - this seems to be confirmed by e.g. ITU-R REC-BR.469-7-2002 (annex 1). Hovewer, this suggests that field 1 is the top one and 2 is bottom (and not first and second in terms of time). - if field 1 = top and 2 = bottom indeed, then we're back at BT.656 and my original idea that the SAV/EAV codes show straigh the so called odd/even lines and not some magic, standard-dependent lines of first and second fields. This needs to be verified. I think the ADV7180 forces the SAV/EAV codes (one can't set them depending od PAL/NTSC etc), so we could assume it does it right. - a notable exception to PAL = top first rule was DV and similar stuff (the casette camcorder things, including Digital8, miniDV, and probably derivatives). DV (including PAL) used bottom-first universally. I think we should stick to default PAL=TB, NTSC=BT rule, though the user should be able to override it (if working with e.g. PAL DV camcorder connected with an analog cable - not very probable, I guess). -- Krzysztof Halasa Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Hi Philipp, I think now I understand your interpretation of the v4l2_field enums: SEQ_BT/TB is specifying both field order and field dominance. But the TB/BT in INTERLACED_TB/BT has a different interpretation, in this case it is specifying _only_ field dominance, and top field is first in memory for both cases. I wasn't interpreting it this way. My interpretation was that BT/TB for all field types referred only to field order and does not say anything about field dominance. So in summary: SEQ_BT = bottom field first in memory AND bottom field is dominant (B lines are older lines in time, e.g. recorded first, then T lines). SEQ_TB = top field first in memory AND top field is dominant. INTERLACED_BT = top field first in memory and bottom field is dominant. INTERLACED_TB = top field first in memory and top field is dominant. With that, your explanations below make sense... On 06/06/2018 02:05 AM, Philipp Zabel wrote: Hi Steve, On Tue, 2018-06-05 at 12:00 -0700, Steve Longerbeam wrote: I'm probably misunderstanding you, so at the risk of overexplaining: There is no way we can ever produce a correct interlaced-tb frame in memory from a seq-bt frame output by the CSI, as the interweaving step only has access to a single frame. I don't follow you, yes the interweaving step only has access to a single frame, but why would interweave need access to another frame to carry out seq-bt -> interlaced-tb ? See below... A seq-bt frame has a bottom field (first in memory) with an older timestamp than the top field (second in memory). Without access to a second input frame we can only ever produce an interlaced frame where the bottom lines are older than the top lines, which is interlaced-bt. interlaced-tb requires the top lines to have the older timestamp, and the bottom lines to be newer. Right, in seq-bt the CSI can skip bottom field and capture top field, then bottom field of next frame, which results in flipping field dominance. So seq-bt becomes seq-tb and indeed the top field in seq-tb is now the older (dominant) field, and the bottom field is now newer. Which means we need to span two frames to accomplish seq-bt -> seq-tb in order to flip field dominance. A seq-tb PAL frame has the older top field in lines 0-287 and the newer bottom field in lines 288-576. From that interlaced-tb can be written via 0-287 -> 0,2,4,...,286 and 288-575 -> 1,3,5,...,287 [1]. This is what interweaving does if the interlace offset is set to positive line stride. Right, that was my understanding as well. And how interweave actually works in the IDMAC to achieve the above is : By turning on SO bit in cpmem, the IDMAC will write the first one-half lines of the frame received by the IDMAC channel to memory, starting at the EBA address, with a line stride equal to cpmem SLY. When it completes writing out the first half lines of the frame, the IDMAC begins to write the lines from the second half of frame to memory, but starts again at EBA address, and with an offset equal to cpmem ILO. So by setting SO=1, SLY=2*linestride, and ILO=linestride, we achieve interweave where: lines from first half of frame are written to lines 0,2,4,...,HEIGHT-2 in memory, and lines from the second half of the frame are written to lines 1,3,...,HEIGHT-1 in memory. So this setting achieves seq-bt -> interlaced-bt This is incorrect. Since the bottom field comes first in memory, with this setting the bottom lines are written to where the top lines should be and the top lines end up where the bottom lines should be. Since odd and even lines are switched, this will not produce a correct frame. Yes, if 'interlaced-bt' is interpreted as: bottom field is dominant and top field must be first in memory. or seq-tb -> interlaced-tb, This is correct. Yes, a simple interweave accomplishes this, and since odd/even lines are not switched, interlaced-tb would look correct if displayed. And top is still dominant. e.g. interweave *does not change field order*, bottom lines 1,3,,, are written to 0,2,4,,, in memory if IDMAC receives seq-bt, or top lines 0,2,4,,, are written to 0,2,4,,, in memory if IDMAC receives seq-tb. Of course we can't change field order, we are arguing the same point here. I think our misunderstanding comes from the definition of interlaced-bt [1]: "Images contain both fields, interleaved line by line, top field first. The bottom field is transmitted first." [1] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/field-order.html?highlight=field#enum-v4l2-field The BT or TB part of the v4l2_field enums refer to the order in time, not to the order in memory. Yes BT/TB refers to field dominance for both 'seq-*' and 'interlaced-*'. But TB/BT only refers to field order for the 'seq-*' definitions, in 'interlaced-*' top field is fixed as first field in memory. But I'm still not so sure because of the sentence "The bottom/top field is transmitted first" in interlaced-bt/tb. Is that referring to field dom
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Hi Steve, On Tue, 2018-06-05 at 12:00 -0700, Steve Longerbeam wrote: > > I'm probably misunderstanding you, so at the risk of overexplaining: > > There is no way we can ever produce a correct interlaced-tb frame in > > memory from a seq-bt frame output by the CSI, as the interweaving step > > only has access to a single frame. > > I don't follow you, yes the interweaving step only has access to > a single frame, but why would interweave need access to another > frame to carry out seq-bt -> interlaced-tb ? See below... A seq-bt frame has a bottom field (first in memory) with an older timestamp than the top field (second in memory). Without access to a second input frame we can only ever produce an interlaced frame where the bottom lines are older than the top lines, which is interlaced-bt. interlaced-tb requires the top lines to have the older timestamp, and the bottom lines to be newer. > > A seq-tb PAL frame has the older top field in lines 0-287 and the newer > > bottom field in lines 288-576. From that interlaced-tb can be written > > via 0-287 -> 0,2,4,...,286 and 288-575 -> 1,3,5,...,287 [1]. This is > > what interweaving does if the interlace offset is set to positive line > > stride. > > Right, that was my understanding as well. And how interweave > actually works in the IDMAC to achieve the above is : > > By turning on SO bit in cpmem, the IDMAC will write the first one-half > lines of the frame received by the IDMAC channel to memory, starting > at the EBA address, with a line stride equal to cpmem SLY. When it > completes writing out the first half lines of the frame, the IDMAC begins > to write the lines from the second half of frame to memory, but starts > again at EBA address, and with an offset equal to cpmem ILO. > > So by setting SO=1, SLY=2*linestride, and ILO=linestride, we achieve > interweave where: > > lines from first half of frame are written to lines 0,2,4,...,HEIGHT-2 > in memory, and > lines from the second half of the frame are written to lines > 1,3,...,HEIGHT-1 in memory. > > So this setting achieves seq-bt -> interlaced-bt This is incorrect. Since the bottom field comes first in memory, with this setting the bottom lines are written to where the top lines should be and the top lines end up where the bottom lines should be. Since odd and even lines are switched, this will not produce a correct frame. > or seq-tb -> interlaced-tb, This is correct. > e.g. > interweave *does not change field order*, bottom lines 1,3,,, are written to > 0,2,4,,, in memory if IDMAC receives seq-bt, or top lines 0,2,4,,, are > written to > 0,2,4,,, in memory if IDMAC receives seq-tb. Of course we can't change field order, we are arguing the same point here. I think our misunderstanding comes from the definition of interlaced-bt [1]: "Images contain both fields, interleaved line by line, top field first. The bottom field is transmitted first." [1] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/field-order.html?highlight=field#enum-v4l2-field The BT or TB part of the v4l2_field enums refer to the order in time, not to the order in memory. > And by setting SO=1, SLY=2*linestride, ILO=-linestride, and adding one > linestride to EBA: > > lines from first half of the frame are written to lines 1,3,...,HEIGHT-1 > in memory, and > lines from the second half of the frame are written to lines > 0,2,...,HEIGHT-2 in memory. > > So this achieves seq-bt -> interlaced-tb No this is seq-bt -> interlaced-bt > or seq-tb -> interlaced-bt, and this again produces an incorrect frame. > e.g. field order has been swapped in memory. > > > > A seq-bt NTSC frame has the older bottom field in lines 0-239 and the > > newer top field in lines 240-439. We can create an interlaced-bt frame > > from that by writing 0-239 -> 1,3,5,...,239 and 240-439 -> 0,2,4,...,238 > > [2]. This can be achieved by offsetting EBA by +stride and setting ILO > > to -stride. > > Agreed except that this is seq-bt -> interlaced-tb, not > seq-bt -> interlaced-bt: No, see above. > Bottom lines = 1,3,5,...,H-1 > Top lines = 0,2,4,...,H-2 Yes, but the bottom lines are older, so this is interlaced-bt. > So seq-bt is written as interlaced-tb to memory: > > Bottom lines = 1,3,5,...,H-1 go to memory lines 1,3,5,...H-1 > Top lines = 0,2,4,...,H-2 go to memory lines 0,2,4,...,H-2. > > So this is TB order in memory. All V4L2 interlaced variants are stored in TB order, which allows to just display them as a progressive frame. The V4L2_FIELD_INTERLACED description is not clear on that, but the V4L2_FIELD_INTERLACED_TB/BT descriptions explicitly state TB order in memory. So that is seq-bt or alternate on the CSI sink pad, seq-tb on the CSI > Well sure, we could use the negative ILO idea to swap field order > (for a second time) at the CSI src pad to capture interface, e.g. > seq-bt at CSI sink -> seq-tb at CSI src -> interlaced-bt at capture > interface. Yes, we swap field order (in memory), as that's ho
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Steve Longerbeam writes: > Yes, I had already implemented this idea yesterday, I've added it > to branch fix-csi-interlaced.3. The CSI will swap field capture > (field 1 first, then field 2, by inverting F bit in CCIR registers) if > the field order input to the CSI is different from the requested > output field order. It seems the fix-csi-interlaced.2 was a bit better. Now I do: media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x576 field:alternate]" media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x576]" media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x576 field:interlaced]" and get: "adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:alternate] "ipu2_csi1_mux":1 [fmt:UYVY2X8/720x576 field:alternate] "ipu2_csi1_mux":2 [fmt:UYVY2X8/720x576 field:alternate] "ipu2_csi1":0 [fmt:UYVY2X8/720x576 field:alternate] "ipu2_csi1":2 [fmt:AYUV32/720x576 field:seq-tb] Needless to say, the output isn't an interlaced frame. -- Krzysztof Halasa Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Steve Longerbeam writes: > I don't follow you, yes the interweaving step only has access to > a single frame, but why would interweave need access to another > frame to carry out seq-bt -> interlaced-tb ? See below... You can't to that. You can delay the input stream (skip one field) so the bottom-first becomes top-first (or top-first - bottom-first), probably with some loss of chroma quality, but you can't reorder odd and even lines. To convert (anything)-bt -> (anything)-tb you need two consecutive fields, the top one and then the bottom one. If the input is *-bt, this means two "frames" (if the word "frame" is applicable at this point). CCIR_CODE_* registers are fine, though. They don't change the geometry, the just skip a single field (sort of, actually they sync to the required field). -- Krzysztof Halasa Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Hi Philipp, On 06/05/2018 01:07 AM, Philipp Zabel wrote: Hi Steve, On Mon, 2018-06-04 at 17:56 -0700, Steve Longerbeam wrote: On 06/04/2018 01:27 AM, Philipp Zabel wrote: On Mon, 2018-06-04 at 07:35 +0200, Krzysztof Hałasa wrote: Philipp Zabel writes: This is ok in this patch, but we can't use this check in the following TRY_FMT patch as there is no way to interweave SEQ_TB -> INTERLACED_BT (because in SEQ_TB the B field is newer than T, but in INTERLACED_BT it has to be older) or SEQ_BT -> INTERLACED_TB (the other way around). Actually we can do SEQ_TB -> INTERLACED_BT and SEQ_BT -> INTERLACED_TB rather easily. We only need to skip a single field at start :-) That's what CCIR_CODE_* registers do. To be honest, SEQ_TB and SEQ_BT are precisely the same thing (i.e., SEQUENTIAL). It's up to the user to say which field is the first. There is the progressive sensor exception, though, and the TB/BT could be a hint for downstream elements (i.e., setting the default field order). But I think we should be able to request INTERLACED_TB or INTERLACED_BT (with any analog signal on input) and the CCIR_CODE registers should be set accordingly. This should all magically work fine. The CSI subdevice itself can't interweave at all, this is done in the IDMAC. In my opinion the CSI subdev should allow the following src -> sink field transformations for BT.656: none -> none seq-tb -> seq-tb seq-tb -> seq-bt seq-bt -> seq-bt seq-bt -> seq-tb alternate -> seq-tb alternate -> seq-bt interlaced -> interlaced interlaced-tb -> interlaced-tb interlaced-bt -> interlaced-bt The capture video device should then additionally allow selecting the field order that can be produced by IDMAC interweaving: INTERLACED_TB if the pad is seq-tb and INTERLACED_BT if the pad is seq- bt, as that is what the IDMAC can convert. Good idea. This is also in-line with how planar YUV is selected at the capture interface instead of at the CSI/PRPENCVF source pad. Philipp, Krzysztof, please see branch fix-csi-interlaced.3 in my github mediatree fork. I've implemented the above and it works great for both NTSC and PAL sources to the ADV7180. Thanks! I'll have a look. seq-tb -> seq-tb and seq-bt -> seq-bt should always capture field 0 first, as we currently do for PAL. seq->tb -> seq-bt and seq-bt -> seq-tb should always capture field 1 first, as we currently do for NTSC. alternate -> seq-tb and alternate -> seq-bt should match seq-tb -> * for PAL and seq-bt -> * for NTSC. Yes, I had already implemented this idea yesterday, I've added it to branch fix-csi-interlaced.3. The CSI will swap field capture (field 1 first, then field 2, by inverting F bit in CCIR registers) if the field order input to the CSI is different from the requested output field order. Philipp, a word about the idea of using negative ILO line stride and an extra line added to EBA start address, for interweaving. I believe the result of this is to also invert field order when interweaving 'seq-bt/tb', which would produce 'interlaced-tb/bt' in memory. I'm probably misunderstanding you, so at the risk of overexplaining: There is no way we can ever produce a correct interlaced-tb frame in memory from a seq-bt frame output by the CSI, as the interweaving step only has access to a single frame. I don't follow you, yes the interweaving step only has access to a single frame, but why would interweave need access to another frame to carry out seq-bt -> interlaced-tb ? See below... A seq-tb PAL frame has the older top field in lines 0-287 and the newer bottom field in lines 288-576. From that interlaced-tb can be written via 0-287 -> 0,2,4,...,286 and 288-575 -> 1,3,5,...,287 [1]. This is what interweaving does if the interlace offset is set to positive line stride. Right, that was my understanding as well. And how interweave actually works in the IDMAC to achieve the above is : By turning on SO bit in cpmem, the IDMAC will write the first one-half lines of the frame received by the IDMAC channel to memory, starting at the EBA address, with a line stride equal to cpmem SLY. When it completes writing out the first half lines of the frame, the IDMAC begins to write the lines from the second half of frame to memory, but starts again at EBA address, and with an offset equal to cpmem ILO. So by setting SO=1, SLY=2*linestride, and ILO=linestride, we achieve interweave where: lines from first half of frame are written to lines 0,2,4,...,HEIGHT-2 in memory, and lines from the second half of the frame are written to lines 1,3,...,HEIGHT-1 in memory. So this setting achieves seq-bt -> interlaced-bt or seq-tb -> interlaced-tb, e.g. interweave *does not change field order*, bottom lines 1,3,,, are written to 0,2,4,,, in memory if IDMAC receives seq-bt, or top lines 0,2,4,,, are written to 0,2,4,,, in memory if IDMAC receives seq-tb. And by setting SO=1, SLY=2*linestride, ILO=-linestride, and adding one linestride to EBA: lines from first half of the frame are written to l
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Philipp Zabel writes: > I'm probably misunderstanding you, so at the risk of overexplaining: > There is no way we can ever produce a correct interlaced-tb frame in > memory from a seq-bt frame output by the CSI, as the interweaving step > only has access to a single frame. Anyway we can use CCIR_CODE registers to get the fields in required order. Actually I think it's the preferred way, even if there are others. -- Krzysztof Halasa Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Hi Steve, On Mon, 2018-06-04 at 17:56 -0700, Steve Longerbeam wrote: > > On 06/04/2018 01:27 AM, Philipp Zabel wrote: > > On Mon, 2018-06-04 at 07:35 +0200, Krzysztof Hałasa wrote: > > > Philipp Zabel writes: > > > > > > > This is ok in this patch, but we can't use this check in the following > > > > TRY_FMT patch as there is no way to interweave > > > > SEQ_TB -> INTERLACED_BT (because in SEQ_TB the B field is newer than T, > > > > but in INTERLACED_BT it has to be older) or SEQ_BT -> INTERLACED_TB (the > > > > other way around). > > > > > > Actually we can do SEQ_TB -> INTERLACED_BT and SEQ_BT -> INTERLACED_TB > > > rather easily. We only need to skip a single field at start :-) > > > That's what CCIR_CODE_* registers do. > > > > > > To be honest, SEQ_TB and SEQ_BT are precisely the same thing > > > (i.e., SEQUENTIAL). It's up to the user to say which field is the first. > > > There is the progressive sensor exception, though, and the TB/BT could > > > be a hint for downstream elements (i.e., setting the default field > > > order). > > > > > > But I think we should be able to request INTERLACED_TB or INTERLACED_BT > > > (with any analog signal on input) and the CCIR_CODE registers should be > > > set accordingly. This should all magically work fine. > > > > The CSI subdevice itself can't interweave at all, this is done in the > > IDMAC. > > In my opinion the CSI subdev should allow the following src -> sink > > field transformations for BT.656: > > > > none -> none > > seq-tb -> seq-tb > > seq-tb -> seq-bt > > seq-bt -> seq-bt > > seq-bt -> seq-tb > > alternate -> seq-tb > > alternate -> seq-bt > > interlaced -> interlaced > > interlaced-tb -> interlaced-tb > > interlaced-bt -> interlaced-bt > > > > The capture video device should then additionally allow selecting > > the field order that can be produced by IDMAC interweaving: > > INTERLACED_TB if the pad is seq-tb and INTERLACED_BT if the pad is seq- > > bt, as that is what the IDMAC can convert. > > Good idea. This is also in-line with how planar YUV is selected > at the capture interface instead of at the CSI/PRPENCVF source > pad. > > Philipp, Krzysztof, please see branch fix-csi-interlaced.3 in my github > mediatree fork. I've implemented the above and it works great for > both NTSC and PAL sources to the ADV7180. Thanks! I'll have a look. > > seq-tb -> seq-tb and seq-bt -> seq-bt should always capture field 0 > > first, as we currently do for PAL. > > seq->tb -> seq-bt and seq-bt -> seq-tb should always capture field 1 > > first, as we currently do for NTSC. > > alternate -> seq-tb and alternate -> seq-bt should match seq-tb -> * for > > PAL and seq-bt -> * for NTSC. > > Yes, I had already implemented this idea yesterday, I've added it > to branch fix-csi-interlaced.3. The CSI will swap field capture > (field 1 first, then field 2, by inverting F bit in CCIR registers) if > the field order input to the CSI is different from the requested > output field order. > > Philipp, a word about the idea of using negative ILO line stride and > an extra line added to EBA start address, for interweaving. I believe > the result of this is to also invert field order when interweaving > 'seq-bt/tb', which would produce 'interlaced-tb/bt' in memory. I'm probably misunderstanding you, so at the risk of overexplaining: There is no way we can ever produce a correct interlaced-tb frame in memory from a seq-bt frame output by the CSI, as the interweaving step only has access to a single frame. A seq-tb PAL frame has the older top field in lines 0-287 and the newer bottom field in lines 288-576. From that interlaced-tb can be written via 0-287 -> 0,2,4,...,286 and 288-575 -> 1,3,5,...,287 [1]. This is what interweaving does if the interlace offset is set to positive line stride. A seq-bt NTSC frame has the older bottom field in lines 0-239 and the newer top field in lines 240-439. We can create an interlaced-bt frame from that by writing 0-239 -> 1,3,5,...,239 and 240-439 -> 0,2,4,...,238 [2]. This can be achieved by offsetting EBA by +stride and setting ILO to -stride. [1] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/field-order.html?highlight=field#field-order-top-field-first-transmitted [2] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/field-order.html?highlight=field#field-order-bottom-field-first-transmitted Writing a seq-tb frame with negative ILO or a seq-bt frame with positive ILO causes misaligned interlaced frames with even and odd lines switched. > I don't think this is necessary now, because field order swapping > can already be done earlier at the CSI sink->src using the CCIR registers. > For example here is a pipeline for an NTSC adv7180 source that swapped > NTSC 'seq-bt' (well assumed NTSC 'seq-bt' since adv7180 is 'alternate') to > 'seq-tb' at the CSI source pad: > > 'adv7180 3-0021':0 > [fmt:UYVY8_2X8/720x480 field:alternate colorspace:smpte170m] > 'ipu1_csi0_mux':1 > [fmt:UYVY8_2X8/720x480
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
On 06/04/2018 01:27 AM, Philipp Zabel wrote: On Mon, 2018-06-04 at 07:35 +0200, Krzysztof Hałasa wrote: Philipp Zabel writes: This is ok in this patch, but we can't use this check in the following TRY_FMT patch as there is no way to interweave SEQ_TB -> INTERLACED_BT (because in SEQ_TB the B field is newer than T, but in INTERLACED_BT it has to be older) or SEQ_BT -> INTERLACED_TB (the other way around). Actually we can do SEQ_TB -> INTERLACED_BT and SEQ_BT -> INTERLACED_TB rather easily. We only need to skip a single field at start :-) That's what CCIR_CODE_* registers do. To be honest, SEQ_TB and SEQ_BT are precisely the same thing (i.e., SEQUENTIAL). It's up to the user to say which field is the first. There is the progressive sensor exception, though, and the TB/BT could be a hint for downstream elements (i.e., setting the default field order). But I think we should be able to request INTERLACED_TB or INTERLACED_BT (with any analog signal on input) and the CCIR_CODE registers should be set accordingly. This should all magically work fine. The CSI subdevice itself can't interweave at all, this is done in the IDMAC. In my opinion the CSI subdev should allow the following src -> sink field transformations for BT.656: none -> none seq-tb -> seq-tb seq-tb -> seq-bt seq-bt -> seq-bt seq-bt -> seq-tb alternate -> seq-tb alternate -> seq-bt interlaced -> interlaced interlaced-tb -> interlaced-tb interlaced-bt -> interlaced-bt The capture video device should then additionally allow selecting the field order that can be produced by IDMAC interweaving: INTERLACED_TB if the pad is seq-tb and INTERLACED_BT if the pad is seq- bt, as that is what the IDMAC can convert. Good idea. This is also in-line with how planar YUV is selected at the capture interface instead of at the CSI/PRPENCVF source pad. Philipp, Krzysztof, please see branch fix-csi-interlaced.3 in my github mediatree fork. I've implemented the above and it works great for both NTSC and PAL sources to the ADV7180. seq-tb -> seq-tb and seq-bt -> seq-bt should always capture field 0 first, as we currently do for PAL. seq->tb -> seq-bt and seq-bt -> seq-tb should always capture field 1 first, as we currently do for NTSC. alternate -> seq-tb and alternate -> seq-bt should match seq-tb -> * for PAL and seq-bt -> * for NTSC. Yes, I had already implemented this idea yesterday, I've added it to branch fix-csi-interlaced.3. The CSI will swap field capture (field 1 first, then field 2, by inverting F bit in CCIR registers) if the field order input to the CSI is different from the requested output field order. Philipp, a word about the idea of using negative ILO line stride and an extra line added to EBA start address, for interweaving. I believe the result of this is to also invert field order when interweaving 'seq-bt/tb', which would produce 'interlaced-tb/bt' in memory. I don't think this is necessary now, because field order swapping can already be done earlier at the CSI sink->src using the CCIR registers. For example here is a pipeline for an NTSC adv7180 source that swapped NTSC 'seq-bt' (well assumed NTSC 'seq-bt' since adv7180 is 'alternate') to 'seq-tb' at the CSI source pad: 'adv7180 3-0021':0 [fmt:UYVY8_2X8/720x480 field:alternate colorspace:smpte170m] 'ipu1_csi0_mux':1 [fmt:UYVY8_2X8/720x480 field:alternate colorspace:smpte170m] 'ipu1_csi0_mux':2 [fmt:UYVY8_2X8/720x480 field:alternate colorspace:smpte170m] 'ipu1_csi0':0 [fmt:UYVY8_2X8/720x480@1/30 field:alternate ...] crop.bounds:(0,0)/720x480 crop:(0,2)/720x480 compose.bounds:(0,0)/720x480 compose:(0,0)/720x480] 'ipu1_csi0':2 [fmt:AYUV8_1X32/720x480@1/30 field:seq-tb ...] And at the capture interface: # v4l2-ctl -d4 -V Format Video Capture: Width/Height : 720/480 Pixel Format : 'YV12' Field : Interlaced Top-Bottom Bytes per Line : 1440 Size Image : 691200 Colorspace : SMPTE 170M Transfer Function : Rec. 709 YCbCr/HSV Encoding: ITU-R 601 Quantization : Limited Range Flags : So we've accomplished 'seq-bt' -> 'interlaced-tb' without needing to swap field order using the modified interweave idea. I've run tests for both PAL and NTSC inputs to the adv7180 on SabreAuto, and the results are consistent: NTSC seq-bt -> interlaced-tb produces good interweave images as expected NTSC seq-bt -> interlaced-bt produces interweave images with a "mauve" artifact as expected PAL seq-tb -> interlaced-tb produces good interweave images as expected PAL seq-tb -> interlaced-bt produces interweave images with a "mauve" artifact as expected Steve
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
On Mon, 2018-06-04 at 07:35 +0200, Krzysztof Hałasa wrote: > Philipp Zabel writes: > > > This is ok in this patch, but we can't use this check in the following > > TRY_FMT patch as there is no way to interweave > > SEQ_TB -> INTERLACED_BT (because in SEQ_TB the B field is newer than T, > > but in INTERLACED_BT it has to be older) or SEQ_BT -> INTERLACED_TB (the > > other way around). > > Actually we can do SEQ_TB -> INTERLACED_BT and SEQ_BT -> INTERLACED_TB > rather easily. We only need to skip a single field at start :-) > That's what CCIR_CODE_* registers do. > > To be honest, SEQ_TB and SEQ_BT are precisely the same thing > (i.e., SEQUENTIAL). It's up to the user to say which field is the first. > There is the progressive sensor exception, though, and the TB/BT could > be a hint for downstream elements (i.e., setting the default field > order). > > But I think we should be able to request INTERLACED_TB or INTERLACED_BT > (with any analog signal on input) and the CCIR_CODE registers should be > set accordingly. This should all magically work fine. The CSI subdevice itself can't interweave at all, this is done in the IDMAC. In my opinion the CSI subdev should allow the following src -> sink field transformations for BT.656: none -> none seq-tb -> seq-tb seq-tb -> seq-bt seq-bt -> seq-bt seq-bt -> seq-tb alternate -> seq-tb alternate -> seq-bt interlaced -> interlaced interlaced-tb -> interlaced-tb interlaced-bt -> interlaced-bt The capture video device should then additionally allow selecting the field order that can be produced by IDMAC interweaving: INTERLACED_TB if the pad is seq-tb and INTERLACED_BT if the pad is seq- bt, as that is what the IDMAC can convert. seq-tb -> seq-tb and seq-bt -> seq-bt should always capture field 0 first, as we currently do for PAL. seq->tb -> seq-bt and seq-bt -> seq-tb should always capture field 1 first, as we currently do for NTSC. alternate -> seq-tb and alternate -> seq-bt should match seq-tb -> * for PAL and seq-bt -> * for NTSC. The interlaced* -> interlaced* would be handled as progressive. regards Philipp
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Philipp Zabel writes: > This is ok in this patch, but we can't use this check in the following > TRY_FMT patch as there is no way to interweave > SEQ_TB -> INTERLACED_BT (because in SEQ_TB the B field is newer than T, > but in INTERLACED_BT it has to be older) or SEQ_BT -> INTERLACED_TB (the > other way around). Actually we can do SEQ_TB -> INTERLACED_BT and SEQ_BT -> INTERLACED_TB rather easily. We only need to skip a single field at start :-) That's what CCIR_CODE_* registers do. To be honest, SEQ_TB and SEQ_BT are precisely the same thing (i.e., SEQUENTIAL). It's up to the user to say which field is the first. There is the progressive sensor exception, though, and the TB/BT could be a hint for downstream elements (i.e., setting the default field order). But I think we should be able to request INTERLACED_TB or INTERLACED_BT (with any analog signal on input) and the CCIR_CODE registers should be set accordingly. This should all magically work fine. -- Krzysztof Halasa Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
On 06/01/2018 06:33 AM, Philipp Zabel wrote:
Hi Steve,
On Thu, 2018-05-31 at 17:30 -0700, Steve Longerbeam wrote:
IDMAC interlaced scan, a.k.a. interweave, should be enabled at the
IDMAC output pads only if the input field type is 'seq-bt' or 'seq-tb',
and the IDMAC output pad field type is 'interlaced*'. Move this
determination to a new macro idmac_interweave().
V4L2_FIELD_HAS_BOTH() macro should not be used on the input to determine
enabling interlaced/interweave scan. That macro includes the 'interlaced'
field types, and in those cases the data is already interweaved with
top/bottom field lines.
The CSI will capture whole frames when the source specifies alternate
field mode. So the CSI also enables interweave at the IDMAC output pad
for alternate input field type.
Signed-off-by: Steve Longerbeam
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 22 ++
drivers/staging/media/imx/imx-media-csi.c | 22 ++
2 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index ae453fd..894db21 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -132,6 +132,18 @@ static inline struct prp_priv *sd_to_priv(struct
v4l2_subdev *sd)
return ic_priv->task_priv;
}
+/*
+ * If the field type at IDMAC output pad is interlaced, and
+ * the input is sequential fields, the IDMAC output channel
+ * can accommodate by interweaving.
+ */
+static inline bool idmac_interweave(enum v4l2_field outfield,
+ enum v4l2_field infield)
+{
+ return V4L2_FIELD_IS_INTERLACED(outfield) &&
+ V4L2_FIELD_IS_SEQUENTIAL(infield);
+}
This is ok in this patch, but we can't use this check in the following
TRY_FMT patch as there is no way to interweave
SEQ_TB -> INTERLACED_BT (because in SEQ_TB the B field is newer than T,
but in INTERLACED_BT it has to be older) or SEQ_BT -> INTERLACED_TB (the
other way around).
Agreed, let's enforce userspace field order to same order from
the originating source.
Steve
+
static void prp_put_ipu_resources(struct prp_priv *priv)
{
if (priv->ic)
@@ -353,6 +365,7 @@ static int prp_setup_channel(struct prp_priv *priv,
struct v4l2_mbus_framefmt *infmt;
unsigned int burst_size;
struct ipu_image image;
+ bool interweave;
int ret;
infmt = &priv->format_mbus[PRPENCVF_SINK_PAD];
@@ -365,6 +378,9 @@ static int prp_setup_channel(struct prp_priv *priv,
image.rect.width = image.pix.width;
image.rect.height = image.pix.height;
+ interweave = (idmac_interweave(image.pix.field, infmt->field) &&
+ channel == priv->out_ch);
+
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
swap(image.rect.width, image.rect.height);
@@ -405,9 +421,7 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
- if (image.pix.field == V4L2_FIELD_NONE &&
- V4L2_FIELD_HAS_BOTH(infmt->field) &&
- channel == priv->out_ch)
+ if (interweave)
ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
This only works for SEQ_TB -> INTERLACED_TB interweave.
For SEQ_BT -> INTERLACED_BT we have to start at +1 line offset and set
ipu_cpmem_interlaced_scan to -image.pix.bytesperline.
regards
Philipp
Re: [PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
Hi Steve,
On Thu, 2018-05-31 at 17:30 -0700, Steve Longerbeam wrote:
> IDMAC interlaced scan, a.k.a. interweave, should be enabled at the
> IDMAC output pads only if the input field type is 'seq-bt' or 'seq-tb',
> and the IDMAC output pad field type is 'interlaced*'. Move this
> determination to a new macro idmac_interweave().
>
> V4L2_FIELD_HAS_BOTH() macro should not be used on the input to determine
> enabling interlaced/interweave scan. That macro includes the 'interlaced'
> field types, and in those cases the data is already interweaved with
> top/bottom field lines.
>
> The CSI will capture whole frames when the source specifies alternate
> field mode. So the CSI also enables interweave at the IDMAC output pad
> for alternate input field type.
>
> Signed-off-by: Steve Longerbeam
> ---
> drivers/staging/media/imx/imx-ic-prpencvf.c | 22 ++
> drivers/staging/media/imx/imx-media-csi.c | 22 ++
> 2 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
> b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index ae453fd..894db21 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -132,6 +132,18 @@ static inline struct prp_priv *sd_to_priv(struct
> v4l2_subdev *sd)
> return ic_priv->task_priv;
> }
>
> +/*
> + * If the field type at IDMAC output pad is interlaced, and
> + * the input is sequential fields, the IDMAC output channel
> + * can accommodate by interweaving.
> + */
> +static inline bool idmac_interweave(enum v4l2_field outfield,
> + enum v4l2_field infield)
> +{
> + return V4L2_FIELD_IS_INTERLACED(outfield) &&
> + V4L2_FIELD_IS_SEQUENTIAL(infield);
> +}
This is ok in this patch, but we can't use this check in the following
TRY_FMT patch as there is no way to interweave
SEQ_TB -> INTERLACED_BT (because in SEQ_TB the B field is newer than T,
but in INTERLACED_BT it has to be older) or SEQ_BT -> INTERLACED_TB (the
other way around).
> +
> static void prp_put_ipu_resources(struct prp_priv *priv)
> {
> if (priv->ic)
> @@ -353,6 +365,7 @@ static int prp_setup_channel(struct prp_priv *priv,
> struct v4l2_mbus_framefmt *infmt;
> unsigned int burst_size;
> struct ipu_image image;
> + bool interweave;
> int ret;
>
> infmt = &priv->format_mbus[PRPENCVF_SINK_PAD];
> @@ -365,6 +378,9 @@ static int prp_setup_channel(struct prp_priv *priv,
> image.rect.width = image.pix.width;
> image.rect.height = image.pix.height;
>
> + interweave = (idmac_interweave(image.pix.field, infmt->field) &&
> + channel == priv->out_ch);
> +
> if (rot_swap_width_height) {
> swap(image.pix.width, image.pix.height);
> swap(image.rect.width, image.rect.height);
> @@ -405,9 +421,7 @@ static int prp_setup_channel(struct prp_priv *priv,
> if (rot_mode)
> ipu_cpmem_set_rotation(channel, rot_mode);
>
> - if (image.pix.field == V4L2_FIELD_NONE &&
> - V4L2_FIELD_HAS_BOTH(infmt->field) &&
> - channel == priv->out_ch)
> + if (interweave)
> ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
This only works for SEQ_TB -> INTERLACED_TB interweave.
For SEQ_BT -> INTERLACED_BT we have to start at +1 line offset and set
ipu_cpmem_interlaced_scan to -image.pix.bytesperline.
regards
Philipp
[PATCH v2 04/10] media: imx: interweave only for sequential input/interlaced output fields
IDMAC interlaced scan, a.k.a. interweave, should be enabled at the
IDMAC output pads only if the input field type is 'seq-bt' or 'seq-tb',
and the IDMAC output pad field type is 'interlaced*'. Move this
determination to a new macro idmac_interweave().
V4L2_FIELD_HAS_BOTH() macro should not be used on the input to determine
enabling interlaced/interweave scan. That macro includes the 'interlaced'
field types, and in those cases the data is already interweaved with
top/bottom field lines.
The CSI will capture whole frames when the source specifies alternate
field mode. So the CSI also enables interweave at the IDMAC output pad
for alternate input field type.
Signed-off-by: Steve Longerbeam
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 22 ++
drivers/staging/media/imx/imx-media-csi.c | 22 ++
2 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index ae453fd..894db21 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -132,6 +132,18 @@ static inline struct prp_priv *sd_to_priv(struct
v4l2_subdev *sd)
return ic_priv->task_priv;
}
+/*
+ * If the field type at IDMAC output pad is interlaced, and
+ * the input is sequential fields, the IDMAC output channel
+ * can accommodate by interweaving.
+ */
+static inline bool idmac_interweave(enum v4l2_field outfield,
+ enum v4l2_field infield)
+{
+ return V4L2_FIELD_IS_INTERLACED(outfield) &&
+ V4L2_FIELD_IS_SEQUENTIAL(infield);
+}
+
static void prp_put_ipu_resources(struct prp_priv *priv)
{
if (priv->ic)
@@ -353,6 +365,7 @@ static int prp_setup_channel(struct prp_priv *priv,
struct v4l2_mbus_framefmt *infmt;
unsigned int burst_size;
struct ipu_image image;
+ bool interweave;
int ret;
infmt = &priv->format_mbus[PRPENCVF_SINK_PAD];
@@ -365,6 +378,9 @@ static int prp_setup_channel(struct prp_priv *priv,
image.rect.width = image.pix.width;
image.rect.height = image.pix.height;
+ interweave = (idmac_interweave(image.pix.field, infmt->field) &&
+ channel == priv->out_ch);
+
if (rot_swap_width_height) {
swap(image.pix.width, image.pix.height);
swap(image.rect.width, image.rect.height);
@@ -405,9 +421,7 @@ static int prp_setup_channel(struct prp_priv *priv,
if (rot_mode)
ipu_cpmem_set_rotation(channel, rot_mode);
- if (image.pix.field == V4L2_FIELD_NONE &&
- V4L2_FIELD_HAS_BOTH(infmt->field) &&
- channel == priv->out_ch)
+ if (interweave)
ipu_cpmem_interlaced_scan(channel, image.pix.bytesperline);
ret = ipu_ic_task_idma_init(priv->ic, channel,
@@ -833,7 +847,7 @@ static void prp_try_fmt(struct prp_priv *priv,
infmt = __prp_get_fmt(priv, cfg, PRPENCVF_SINK_PAD, sdformat->which);
if (sdformat->pad == PRPENCVF_SRC_PAD) {
- if (sdformat->format.field != V4L2_FIELD_NONE)
+ if (!V4L2_FIELD_IS_INTERLACED(sdformat->format.field))
sdformat->format.field = infmt->field;
prp_bound_align_output(&sdformat->format, infmt,
diff --git a/drivers/staging/media/imx/imx-media-csi.c
b/drivers/staging/media/imx/imx-media-csi.c
index 9bc555c..2c77ef9 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -128,6 +128,19 @@ static inline bool is_parallel_16bit_bus(struct
v4l2_fwnode_endpoint *ep)
}
/*
+ * If the field type at IDMAC output pad is interlaced, and
+ * the input is sequential or alternating fields, the IDMAC
+ * output channel can accommodate by interweaving.
+ */
+static inline bool idmac_interweave(enum v4l2_field outfield,
+ enum v4l2_field infield)
+{
+ return V4L2_FIELD_IS_INTERLACED(outfield) &&
+ (V4L2_FIELD_IS_SEQUENTIAL(infield) ||
+infield == V4L2_FIELD_ALTERNATE);
+}
+
+/*
* Parses the fwnode endpoint from the source pad of the entity
* connected to this CSI. This will either be the entity directly
* upstream from the CSI-2 receiver, or directly upstream from the
@@ -368,10 +381,10 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
{
struct imx_media_video_dev *vdev = priv->vdev;
struct v4l2_mbus_framefmt *infmt;
+ bool passthrough, interweave;
struct ipu_image image;
u32 passthrough_bits;
dma_addr_t phys[2];
- bool passthrough;
u32 burst_size;
int ret;
@@ -389,6 +402,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
image.phys0 = phys[0];
image.phys1 = phys[1];
+ interweave = idmac_interweave(image.pix.field, infmt->field);
+
/*
* Chec
