From owner-freebsd-multimedia Tue Aug 19 21:47:56 1997 Return-Path: Received: (from root@localhost) by hub.freebsd.org (8.8.5/8.8.5) id VAA28543 for multimedia-outgoing; Tue, 19 Aug 1997 21:47:56 -0700 (PDT) Received: from labinfo.iet.unipi.it (labinfo.iet.unipi.it [131.114.9.5]) by hub.freebsd.org (8.8.5/8.8.5) with SMTP id VAA28533 for ; Tue, 19 Aug 1997 21:47:49 -0700 (PDT) Received: from localhost (luigi@localhost) by labinfo.iet.unipi.it (8.6.5/8.6.5) id FAA12548; Wed, 20 Aug 1997 05:40:20 +0200 From: Luigi Rizzo Message-Id: <199708200340.FAA12548@labinfo.iet.unipi.it> Subject: [bt848] comments and diffs To: multimedia@freebsd.org Date: Wed, 20 Aug 1997 05:40:19 +0200 (MET DST) Cc: luigi@labinfo.iet.unipi.it (Luigi Rizzo) X-Mailer: ELM [version 2.4 PL23] Content-Type: text Sender: owner-freebsd-multimedia@freebsd.org X-Loop: FreeBSD.org Precedence: bulk At the end of this message you can find the diffs between Amancio's latest bt848 driver, and my version of the code. I have tried to merge into my version all the new code, including bsdi support. The remaining differences can be classified as follows: 1. minor code cleanups: * added some #define for BT848 bitfields; * removed the definition of METPRI, now is BKTRPRI; * removed dead code related to the use of bt_enable_cnt; * added several comments in the code; * moved code common to FreeBSD and bsdi out of conditional blocks (e.g. BKTRPRI 2. minor improvements with no impact on the rest of the code: * added two ioctl() calls (BT848_SLNOTCH,BT848_GLNOTCH) to set/get the value of the luma notch register (ioctl_bt848.h, brooktree848.c); * in the definition of SYNC_LEVEL, I do not use BT848_ADC_CRUSH, and I suggest to remove it. This bit enables a mechanism which reduces video gain in case of overflows, but does not increase it when the signal is low. With this mechanism and weak signals (e.g. some of those coming from the antenna), when the Bt848 loses sync the chip might erroneously think that the gain is too hign (because it samples the signal in some wrong place instead of next to the sync pulse) and reduces the gain, falling into an avalanche effect which I and someone else noticed which makes the image completely disappear. Without this bit set, the device behaves well with weak and well-behaved signals, and at most saturates with out-of-spec (too strong) video signals. * initialization of the frame buffer is done using the real size rather than 640*480*4 ; * unless there is some backward compatibility issue, write() to the device should return EINVAL or ENXIO, not 0; 3. the use of a couple of macros (I2 and I5) to make the DMA program more readable. This way each line corresponds to one DMA instruction, and the code is more compact, easier to read and less error-prone. 4. a major rewrite of the DMA code: * all capture routines have been rewritten following the specification given below; the only exception is the clip code which is still unchanged (hence not working for me). * I have some (untested) code to support read() from the device; 5. some local code used for testing purposes: * two formats have been defined, SECAM625 and TTXT625, used by some experimental code to do software decoding of teletext pages (this code is available from my home page); code for setting these two formats has been added; * my code defaults to western europe tv channels. Items 1,2 and 3 (this requires some work on Amancio's size...) in my opinion deserve inclusion in the -current driver, and they have no impact on the rest of the code. Item #5 obviously should not be included. About item #4: while I suggest _not_ to adopt my code in the -current driver (because it still does not have adequate support for clipping), I would urge Amancio and other interested to review the DMA code in the future. One (the biggest) problem I had with the previous driver is that there is no spec on how the driver is expected to behave. E.g.: 1.* when capturing full frames, should we get the odd field first, followed by the even one (I do believe this is the way to do, reading some CCIR documents the odd field is always mentioned as the first one in a frame), the even field first, or whatever we find first (the latter is _very_ tricky to program since the dma program waits for a specific field); 2.* when capturing single fields, do we return only one field per frame (i.e. have a max of 25/30 fields per second) or, if the user asks for more, we return both even and odd fields ? The latter is also tricky since we need a different DMA program than the one which only returns all even (or all odd) fields; it is also a problem for applications because the fields have a slight vertical offset, and any application trying to do compression on subsequent frames will be negatively affected by this offset. Of course one could say that if the user asks for something the hardware can deliver, why not support it... 3.* related to the above, what is the max value we can use in the "fps" call when operating in field (as opposed to full frame) mode ? Do we limit to 25/30 or go up to 50/60 ? And how does the hardware support this (do you remember, the data sheets were a bit unclear on this...). With Amancio's driver (at least april's version, but I don't think it has changed much in this respect) I could not find an answer to the above questions which could be made consistent with the code's behaviour. Especially, the problem was with field capture modes. In writing my code I made the following assumptions: 1. odd field followed by the even field; 2. only one field per frame is returned; 3. as a consequence, fps argument can be at most 25/30 and the code is mostly consistent with the assumptions. The are only two problems with my new dma code: * the fps stuff is not working well since I did not find the time to experiment all the possible interpretations of the manpage; but it will just take a few days to fix this; * I have not rewritten the program which does clipping, hence it does not work for me. There are several reasons for that, the main ones being that a) I have no use for them, and b) I am not too satisfied with the way clip regions are passed to the kernel. But since Amancio has some working code and I don't even have a detailed proposal, I'll shut up on this issue... This said, both Amancio's and my version of the driver work satisfactorily in most cases (i.e. for tv/fxtv and vic, is anyone using the driver for other applications ?). But I am strongly convinced that we should aim at making the driver behave consistently with some specification which a) can be applied to other video drivers as well, and b) can be used by application writers as a reference for what the driver should do, as opposed to writing the application by trial and error (a procedure which produces unreliable and hard to maintain code). The following things ought to be done (and Amancio is the candidate for these changes): * there is some code depending on BROOKTREE_IRQ being defined. Someone some time ago told me that similar code (in the meteor driver) might not even work. I suggest it to remove it from both drivers. * Probably the initial comment from Amancio can be moved to the revision history. * Would it be possible to have just a single copy of the copyright messages, including proper credits for Amancio, Mark and Jim ? The diffs between Amancio's (---) and my driver (+++) follow: Cheers Luigi begin 664 bt848diffs.gz M'XL("!YP^C,"`V$`[#MK=QK'DI]'OZ+BG#@@D#0/GL)V@B4LLT;``K*=N^3, M&9@1FF-@R,R@1R+GMV]5=<^#IV3G?KA[SF)+H.FNZNIZ5W5CN]?7<+0N-PZGYBBL%"K'-W"R?_S@Z.CH"13*.]^%2^L!=`/4 M\JE>.2T60*M6RP>Y7.XI_,HGQQ;`%80Y5=53K2R`?_T5CC1#S9<@1V\5^/77 M`_C1=J[=N:,,/@ZNVHV>V6\,ZN_.%+/9^93Y^?[G/!2-/+CS,*LH)X<0+OTY MX#AX\Q,/&7)X$F.`",-%C*$G,112&/Y8.O[#.HJ#7$3&VP%NPNRWVIW!V?LT M%<44CL`)8;J<63#WPO$-88C@0S+ROX2^XYB^,XFT9N-QK"P;(TI_B9)83@"EJQ9/ MB\9IP5C3D4T84@V"T56",?!_(5$-73-(->A-6]$-R=1F>V"^NT2V7BA*1GOU M2LT>,%>0K8IR:N@UYJVF5O+XJYK75`M_1LQ=>KT=?.KTSB$S6;B>:<\L$W4V M6Q,@8P*Q<;J#/]=;!'I^63?/!BVSV]*-0;>`!*BO7I6R!+V86G/+UR#TW'T7O%9Z$H&A(P)C%P)#WXM`B[=?^([M:_'N"]^Q M>RW9?.%[-J\E>R]\Q]X_Q%O7!;0U_D+6^)R=?X@WOA6V\@1LM.VMP/MW_2'9 M]%9HL6?8#MUK]L_,1EM0KF5W37O7?->)IT5V^7;0ZK0O(./J8].VPK3!:2H: MG*;E-4W''X,(V.J@/(^]!\65<=JGI)^ON:CTT*:/,LI;?=0*T*J3*JJGNI&* M7^42QR]\J[*/VO-"Q^3X_G(1@AO,?P[!=J;NK>,[=AZLN4TNW.'?Y*8/0%$4 MA#FK=\U^LWW1:@3HWZT0KCT?.N?G9J?=^@TG.U,[.*:`I1UKE6--48HG>N6D M6B;8Z=*=N+^Z3GB\G+L+]]@-#W("Z\R:/\#XQII/G("B!(D5%KXW\:U9`)GQ MU+'FRT4`3CC.'N0.1$R$']UKE#:8YCODS=O^N6D*%E383VN5B`4_RCP`7B`W MCV]>X!-GCO+$H)8&EI$6(UT.`UH3-X-,H!@V=^[#:#'?PB<^C+WY]3)`QAP? M'\/4KY;5BJX2W`F3!3\(-;0S*?Q9>'R$3/OMAT$/WH`*+U]"NWO6I,]9L1U) MYBL6O(5[/[YY(T)/`?608D^A@@&`-W4`X$XJ!-;TSGH(0*V! M9PD@`++"](TPJ./+?W3ASZ/10Q]PYW+GA#?0[K9-&IR7$ M[\Z#T%\B%F\>'`LD@FA%65FY1MM!H1VAT&8.YB8/2,C4&ULA:2]J)SJ!6)=8 MM$_..H;W:`;H>;X<'\"*]R&'TNUU+LQZJ]4Y0W^"CA&Z]8L&&L6_&B1&%E>Y MD-=T%!>]&T('6>-2_JG7Z:`B-!HQIK4G)F'MKV-/G"A.W2K;W%:[@%CB+&S, M=LP08K_R/R"T\O?:)H%F^^HR,PO]K)+)$"PQ*X?SK>=29!G,VF ME7`GI?%25^WF('./J6<&?\-+4._5ZY0KOVRV,4N])P+NXTB#[]Q M1XI]:_+*JQNAQ\%RM#J2(A^&@M2JRJ3R&_OQ0QC-=_6KUL`\>]]N84$#V61S:T.* M?#?;];=G_4&B@+OF?6I<]3K=1J+Q;)RTI8*JT9;H3=-6\N=WG=YE?8`:WZM? M]LWVH']6U(N@4O#Z"_!C'G0]#X4*1F6H;@8 M2Z_1;_0^-L[A41D>'&U;@J(%#:ZOSP@'ZP244^M+K:/-E"H%VDRI4I9&A.AD M`9K)<-`\S*H4Y+ZB_N]*"7*)(DNT9;5(:#'F1SQ:FE-O/E&4Z^6\MB,S.5J- M6O`:XR1"LDM]G7:IY!E^9RP*TI.A/^'-:^&AL_"7H$!7\UH124"!ZJJ4$^=T M+PGAT9O%S4-@CJS`@:Q`A93A,N@:34I=T$];=H8=)&8@YEGG\K+>/C?[@_K@ MJH_2N4"HG."Q@^X&`^4=1>PIB53L(>`A;JF`%5!@GUGH;-W0);?#HE#BU>Y\ M-W3V+9<'HN\177N:?4E$;/;^FXOG:]^;\<*$UITL,1O#]``SM:G#HE(H>5-$ MGP)3C:0A09Z3TA;(V![2CBR]\_PO@/DK9@'P"P,1R:A-INO_L9M56.0W>KVK M[D!R:?76=>D)!_LD^1_M`:WYP"H1'ILRUX]!,W M87ZRA_,7>=(-TJ(\9*+M4?R\OL[BDX@(^017B2PA4GR.CBO4,ZL1J^@T80PB MB23%!-)A3TE=?&\9DL;$RC"S%I1GBAW&62=202GG898?Y>'EW`E-=V8%7VC/ MNT,T&T)5HUR_7*U("V>;@=$2HQU:&)#!X/9'?SJ^AR9OV;:/^0F-Y]=3,&:P M0I"OX3;TR((R^!=9`@^P75U/K4F`$T1;"R74'-1;F)NA)XN>U:\&G0,> MHYH)7W+LO/%136;V+MYJI5J"G)H^E(*B,[%LE.1K>%?'F%KCG59+Y&UR58P3 M:/G2XE$`#_/QC>_-O24JN+5`W^;`#!7]6(P3YYDE:?)?0D92<-9I#YKMA"+R MKO5NEK@6U4%D29A1-$[YK_'2]YUYB&G]G4@W(%@N%IX?@K&R?'`*GFW+A,3! M!$5\9!PXD.-'K#)3:XQ;S=!LKA[QMQ^$V6.HH^-9^E@5S-ANYUX:0&!R7"[& M&%FG1XBS:.5+1()LO'41K*1"IJARQH090!86[IAH#&#A^(PB<-#PL.`=+4/A M%0C:=_Y8NK@3=AN1%J,]A=YD,G58W7&+.(.LS9HY@ABT1`C=F2.J9V_NB$8I M-0X<8EJZQ&"*",];;DHS@CJY4:0FP-4IGJ*^ M!5A)C6_6Y"W%2S6Z^:[9:)WWS]41.`TNL=8_3_5 MVP.:*S144\L<%S6UJLOFN<(!XNC-[<@UJ:UC4JT0A[ZU,=N9RB$*EL)B96TK M?J1,'%>);Z"D]@$']Q#8C+3YS@,"FTHL",.W:O M/1/]FN<'Z8@M;'7;<]:'P)2V8(M!YHZNZLP=75?C;"0N7&-3"T(7`Q+JV@B# MW(U#G3"9P:/R'T35&T8)U,!;DWHVMWD0A3(L70\.\9>H>ER/Q(6IO5@=5ZW2 MZD:XT6"\=`9%.CA;`G2VFS@RU&1F%D8J"S^8Z1ZDL*;.6)K&D MUJ4/KY'IO)>Y)^R3_#W:8*KBSPCRKBW,'-!IK&^Q<]FXI+4%ZXPJ%2S(.RPX M=%TJ=MJ^7L/?L4>M=]FT$I_*1L+6)C(QI*M/Q5OL+SEQX7HN4IN,"(214^9V M&\TW2($J"UCCU@92F&)M.QUI/%HW60ETMY0)";U`G`%OC>MB;K[B'@1 M+LFNPF#J.(LD;L=LP>H[#R_$_E_D0(1FY/!*ULC^V6/FP)@2];W.RMY#>W5OG7Q'ST05_(#'/KO?!T"$2IBD+]='-> M]%I-!]?"[=\1(B1$!MK'W:BBEP1">FM[UI5*3GMFQR.5D9<2&W\N]&-L(`*8 M5GY[?O&^^30"3$^GU@,%M_OR]=/31ZGI>NU)INX59]1!V8=&2#GW##%3Y^3_ MI?R?)V79T7J&D,FNUZ4:E6"G:2?Q#4+DE'>MG)/)0+&DBF2@I):B:P"*LB,L MB%.S9P4&:H)Q/OD-88\#0^Y)P-TA192&`OKER^@3%LZ-7J,_J/<&V:A"H'@C M#D40XHZ*&IM.#.FB2)QZB%2+#VH(0N:&$#:CO8YU`QVT.]T*V??6B<#_7B)[/Q$6G42Y^H,G0<&!K&;U@MN!$H:]$M*5F<\;-(]$OB>,XKCKUH5I6BWHE?6?)"<=P!`;5DZXD)7+5:<[;E(IBJ2$G`E3"()$:J&5L_#J%12! M%57N/EZ5/;EZ[Z@KP][^X03Z,>;!#OB-"9&+7-G>16I[DVW;2^\)MTBWPB"# M?G-C-T#$9NE(KIA=6Y'+'*$&:ET M$WU/O2QI`1#3EK0Q][C6BHL#'4 MB8;.UH:ZGS]&0^K:$!UPRR%M;:B?#.FK1[TKQ*-(%QATVTY_9")?P@J*2GMP9N:T]106`:R1,_M MY",!3DF5;`+,0P%&#Z$3;`*E.1#!AA,+:FM/1OCD:PJXR,!8#^03[P7;W>N'=YP2B:O;X%;.XAS:I.P,"!_ MF'NV(_VG8519$PMT=56+KX7)HU!YR$#5-KVB:S81><%BZH;;"+OUICAOZH`X M_83#>)LI:@$3I15\^.<_12C/.N4L;^&(8T`Q:>'>.U.3IW)223-3B$4S^!!" MC"Y.:(K&H(`<>].`#XQRWPYUD/N+G$=$$Z6(>?ZMDX`HR\X(Z@_3!&*RP#>* M9'3LMIH#B(YX=:-H\!FO7M"K=)N(4R*BBQLKKZ7IU>(;?E^!&CC1J%I3J)?& M(X>9F)%9UL"89?`HYN/[^GTN,C(=GV\ANR:RA*>0[H"5=P^1)0G$#Z]C9YJE M='*3Y(S@+-KFX8H0N,@2NUQY#KG7^Y:7O4=QEH(9S32>3PUG^I?FZ5_Q:?PF M'#6;J7@2B^%?I!")$,N8L!1(B)3";`I1'!U$#_3H+.$K:NX>!N\1E^0\PK.S MW,(".`$]^VP1[L$@.+)'B-N53TH2YZQ)C#!N"G$W!2#.(+Z?3_C^[^&3_A_/ M*'F)M"BN)1:J)=+*6!GCU/X6JZ;7H-56GWOIYZ2:ZYU['#,XL!/#C?CJB!O` MA&];S#'_6VG^BZR7;(JFY3S6I\?":90X*:27L3ZIBH/F"+@ M=7?'"\1Q=/(_OG<78#@Q=+KTL*$<"SH4K@$;#.Z3.4[;I.L#<,KY'5,E MZ@H^7II9H=C_:O8B4U>R+9F)X:>H1)&B?P*]G4*?H]PMC33)]A*T>7%P((DG MI'EY'CZWW3%?.$K70UNH5FM\@Z1]U6H!?\V&:T[IL]EE$V>)SHPKIKO(S`PQ M]23F=1:?YG)"QF0>@LN;(A(RK6AYO4(R+29WM,3A/*YADML0F+@^UKA5*3<' MM[Y4O1V\?Z0*2V32D1CX4U2[U39!MS'@:.L*_W5UV552*VQ!EGBF^*+,BCT) MMYD6["K%HB9($9R/3H8B("(B#91_>DT0+4V1U`H39=;J:ZSUGF"MML98-:&S M\X\9^V]B9LS`31*WLO(?L,_X9^Q+W,,WL&\'\U@AQA+_*JX]?/1LV]S@Y196 M;CB>=69NH2*?N/8]BV97>I;*U^A;`D6ME&2&3S!]JRM^ M%J\W5&NO`UY_K3O0;_&@3[O08J$JOO52HJ]BR4/?KU&LWLZB_1S:KI?L+Y^C MEMNU=BI@XR+1_?+95/R,6?HVO(]%-I/6;:@_+VRC"RLN% MT9VE/M]7HLL\T45`FC_R4"VH:?@2#2$/JW?WHJ\DI+Z"=$`K\"4P6C4CVAD+ M_`EE:X`+:2*GXN[8*=464I'R?!^YDSE\S"A7J,)'` M=H\:J\#R9O=:Y9]D8YPC"UIE2WX%/D)PF.2:@OQ*@8X/D?Y",=;MZ*30FWH^ MY(A2W,F+NHBC`]ZA`_[B@:,US;UT0:NANID<(JD#:>@32W MEZ`8"4\_U+-[BI#O*4%*JI'75*I!_K>Z*^MMXP;"SYM?P:)H8%M:9P_)NI"' M5+8;%TG=VLY#$12"CE5BP)84'4Y<(/^]G!F>>U*RDJ`!DMC+8SD\AIR/,]^> MG$3UEO0P+JS&UC?"[9+:5&2Y\-G$'RH]<$2"]':R278T`U+'E,R&Z7:T&ZD= M#WYSWO(R^['41:G'J6[++197E6,U-NI!SWH96U"/`,XEKL01YS!C"[J6/JX+ MS*$O MC_"4@D8OM8);O"%#/TB^BI1O.<\FL-O,4;EX-U7XGDLQ>1JD,XB?8\SYRICS MW8TYRS;:IQ&WFZWA:QO`EP)%*8'F[@+M[?#_9&EJ6FFH*9=2%[6J26!C$=(^ M>TC9D)&54S8@)R)!Z'Z]H M^1OJ[$,R`S34#!M2_8_;H#3,X9A5F5;A;56Y7 MF*EXN^+]><#4Z8?&>C;_C$XL$$IC."DX#;=>S#7/:2$P)EV=;Y!HY\9=,'Q[>LJ^#/D,YU[SB5GL7JSK?;%Z'VG2N5>9^7-39AO]]X'_O M-@]U(*CA&W61U9WS>L/JKI6DULEE`RP0`^025N9]LDZX.;ZX_3*]7P^P=V9# M;NTMIL)=[;E(6H-I]UYH?'J&C!HY1VLYK-_@9%T0_L/2\`*T3OYYR4@8(VT[ MP]P5?_`<005BPLI%#B1%V!,AAV(P1?E5HOL\>+,'(H0O@HC_3A-Y7<)ZJV&! M-LE@-;Z;SQ?:X?YR<-U_<\FU4?_5;WWDE8)(W?LA&WX8BRAZ(UP+KEG+:Z#; M.WU7NQH/[Y+!QUO('WP9!\0OA^Z)DOIM\5ML!AO``3].2_8TOI2O>[-M4@?SZL#$-J@Z=1*G+2JN499[5@.4D M,&*QTE)PY2LBC4QB!^BVR>T*.XD0M_%\N4PH3G&9W,\?AG?62.4#=/DPE%^" M6N$&;R`^Z,>7!_J4(D=,0D@CF>J+.18AA;"!O+R`W1[,WA_9 M61\@ZV:1:2QE7EJ9>78Z;M@6B)`)2U`Z1N*"0D!Z%/(Y]OAQI7""%8)J>X/42%!P M4Y0'@S0`A(:.E-U5I6^CTW?"Z>Q66J7%Q.+GJ]L\(%7[#BP`0Q3F\_(Q,\+-&"MF/])CYANB>XN*ADT:7P8HFC,\`]7H,//41\@,( MB/\.5_[H]$(,R/0\(EXGO73R0#X,8%1*IQSG$V0^43M$DN>H'2G:,^U%:H?9 MFU'FO(W2[TIP,0);#6`CIQ@K^"G00AP0;G&G@-F-`D8431P.SZ1`C+ES%DN(@8>NXMW>7KZ_:6+0Y*NT93DKM6C M5B83$3-BS\\NGD5T^GDJK- M,2JI!@2UZXGL>KZ:HU3(_:69'9@;YY@8O":%2W<@4BZPL/7U)!G;@3?3!02[ M3!>"<&\`-ZF'>$:SBY`D1P=V3GZ`Y140C7@\-08!\Q/Q_^74A_'+.<(5LD<0]F7Q9)..UO`]4P;B;@0ANA9NF MU6>^->`0'C#]&.[KS-S/:E7948_F77/![5;V9DO>:1W3-WMZ2'R,=X/BPHR_ M9ZH9#^,@CDC()$\];SP6A]H,)L">/5A5;<^D%WA'7X5VQM<2LF1HJXQ8]@84<*%P:!Q^)V)_#X$'L7Y^=7 M9W_1O:_-1W%Y?@ZWC(("I0LWXG&R6\T7R_A_>A="XDT[=0_:-`U[E<2?@ MU4!]YZ_Z-Y=7AW4O@&M?KNQ%ID8K/(Z:Z4R41N]+I?%I[U8:FIDI*_LK;-4I M+A^SANTX6XV1WBFMZL2JJM5\0E5-,VL[*JA)IA\W2ZJ*S:J:\7%KYZI8\SMIHEPM87E5H5=73,#YFZ:;\*F@OG_^W`QG"5` M!+/D9Y`QV*[]C\/9C&O0KOC>01A@I#3_H:$S02EX7/X,-K@UXN;:\GL MC+LF/(0&#?IG0)E]=MJE#QZH[QW"*"B+DI\)$H.5]E*G2T6?+BH@Y*;V!&5=3CV@>^K+!9](\V7J$D?R.#&E*1AL\7_ B\XV4/C>%^LRY8U[?/KE?JJIP[)9:+;];_@/)IJ`*@G$``$D? ` end