Thread: Re: [libdc1394-devel] IIDC Proposal v1.2
Capture and control API for IIDC compliant cameras
Brought to you by:
ddouxchamps,
gordp
From: Johann S. <j.s...@ir...> - 2007-05-29 21:33:24
|
Damien Douxchamps wrote: > > The version 1.2 of the document can be found here: > > http://damien.douxchamps.net/ieee1394/libdc1394/IIDC_propositions_1.2.odt > http://damien.douxchamps.net/ieee1394/libdc1394/IIDC_propositions_1.2.pdf Oops! Some more little fixes: 1. The heading of section 5.2 "Defind a MaxPacketPerFrame (Format_7)" should read "Define a MinBytePerPacket (Format_7)". 2. In section 5.2, the fourth paragraph, "BUTE_PER_PACKET" should be spelled "BYTE_PER_PACKET". 3. In section 5.2, remove the quote marks around the fifth paragraph. (However, I think we are about to remove that whole paragraph anyway, as you suggested.) You are doing a great job here Damien! Thanks, Johann This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. |
From: Johann S. <j.s...@ir...> - 2007-05-30 00:41:55
|
Damien Douxchamps wrote: > > On Mon, 2007-05-28 at 17:46 +1200, Johann Schoonees wrote: > > > 1. That the existing PACKET_PARA_INQ register (offset 040h) be > > renamed PACKET_UNIT_MAX_BYTES_INQ. The register will retain its > > current meaning and operation. > > I think that renaming an offset may be confusing and risky. It would be > better to name the new offset "PACKET_PARA_2_INQ" or something similar. I completely agree, and if others are of the same mind, I suggest the following editing to your section 5.2 to avoid the register renaming: 1. Replace "PACKET_UNIT_MAX_BYTES" with "PACKET_PARA_INQ" throughout. 2. Replace "PACKET_MIN_BYTES_INQ" with "PACKET_PARA_2_INQ" throughout. 3. Remove the fifth paragraph beginning with "It then becomes...". 4. In the next paragraph, under "Existing registers affected:" remove the first entry (CSR, Offset, Name) for PACKET_PARA_INQ. 5. Under the "Specification changes" heading, remove points 3 and 7, and renumber all points. That makes the proposal clearer too. Johann This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. |
From: Damien D. <da...@do...> - 2007-05-30 06:28:55
|
On Wed, 2007-05-30 at 12:41 +1200, Johann Schoonees wrote: > Damien Douxchamps wrote: > > > > On Mon, 2007-05-28 at 17:46 +1200, Johann Schoonees wrote: > > > > > 1. That the existing PACKET_PARA_INQ register (offset 040h) be > > > renamed PACKET_UNIT_MAX_BYTES_INQ. The register will retain its > > > current meaning and operation. > > > > I think that renaming an offset may be confusing and risky. It would be > > better to name the new offset "PACKET_PARA_2_INQ" or something similar. > > I completely agree, and if others are of the same mind, I suggest the > following editing to your section 5.2 to avoid the register renaming: > > 1. Replace "PACKET_UNIT_MAX_BYTES" with "PACKET_PARA_INQ" throughout. > > 2. Replace "PACKET_MIN_BYTES_INQ" with "PACKET_PARA_2_INQ" throughout. > > 3. Remove the fifth paragraph beginning with "It then becomes...". > > 4. In the next paragraph, under "Existing registers affected:" > remove the first entry (CSR, Offset, Name) for PACKET_PARA_INQ. > > 5. Under the "Specification changes" heading, remove points 3 and 7, > and renumber all points. > > That makes the proposal clearer too. OK, I updated the document. Damien -- _ Damien 高原 Douxchamps ('- Assistant Professor //\ Image Processing Laboratory, NAIST V_/_ http://damien.douxchamps.net/ |
From: Johann S. <j.s...@ir...> - 2007-05-30 02:18:50
|
Damien Douxchamps wrote: > > Although avoiding a division looks nice, I'm a bit afraid of having two > different expression for the same information. My experience with > different manufacturers is that it's going to be an interpretation mess > and we will end up with two values that will sometimes contradict each > other. > > I would really prefer if we had only one choice. At worst, we should > clarify what to do when two values are present. For instance, we could > add that either MaxPacketPerFrame or MinBytePerPacket can be > implemented, but not both. ... > No new register is nice ;) > > I would say that we can include that in the propositions as an > alternative. The text could be merged in the MinBytes proposal, to show > an alternative. The good thing is that we don't have to decide: we just > have to propose! ;) I was also worried about having two mutually dependent quantities that can conflict. You obviously have experience of the mess implementers can make if given the opportunity. I'll try to write a patch to the current 5.2 MinBytesPerPacket proposal. I'll try to make it either-or, not both. If it turns out too confusing, we'll have to think again about how to propose the two alternatives, or drop one of them. I'm not getting any work done around here, with all this proposal writing. :-) Johann This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. |
From: Johann S. <j.s...@ir...> - 2007-05-30 04:26:18
|
Ralf wrote: > > Johann Schoonees wrote: > >>However, I have since started thinking about it from the camera >>manufacturers' point of view. MinBytePerPacket will typically >>require a firmware calculation such as >> >> TotalBytes = (TOTAL_BYTES_HI_INQ << 32) + TOTAL_BYTES_LO_INQ; >> MinBytePerPacket = (TotalBytes + MaxPacketPerFrame - >>1)/MaxPacketPerFrame); >> >>which involves a presumably expensive division operation, where I > > as MaxBytePerPacket does, if the camera supports a speed increase > in dependance on the AOI height. > >>A manufacturer may find it easier to report the MaxPacketPerFrame >>value (hard-coded number, no computation) and leave it up to the host >>to work out the corresponding minimum number of bytes per packet. > > Even if it's somewhat expensive, a fixed MaxPacketPerFrame may lead > to other problems or may be confusing e.g.: > > - if MaxPacketPerFrame * 4 > TOTAL_BYTES_INQ > - if the camera requires a minimum sensor readout bandwidth and > insists on a minimum value for BytePerPacket > > So a camera may have to adjust MaxPacketPerFrame in dependance on other > current format 7 settings. > >>So, I propose that we keep the PACKET_MIN_BYTES_INQ as proposed in >>Damien's Proposition document, but that we add another proposal for a >>MaxPacketPerFrame in such a way that manufacturers can choose to >>implement one of them, or neither, or both. > > This could help of course, but seems to be somewhat redundant. Good points. I would be happy to drop MaxPacketPerFrame and keep just the proposed MinBytePerPacket. What do others think? >>Adding a definition of a MaxPacketPerFrame field can be done in bits >>[16:31] of the existing PACKET_PER_FRAME_INQ register as I suggested >>originally. There would be no need to define a new register. > > If the proposals of this list are to be included in an IIDC spec 1.x, > I suggest not to change existing CSRs (up to 1.31) to keep newer cameras > compatible to older host software... Yes, it's probably good to be conservative about existing registers. Johann This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. |
From: Damien D. <da...@do...> - 2007-05-30 06:34:18
|
On Wed, 2007-05-30 at 16:24 +1200, Johann Schoonees wrote: > Ralf wrote: > > > > Johann Schoonees wrote: > > > >>However, I have since started thinking about it from the camera > >>manufacturers' point of view. MinBytePerPacket will typically > >>require a firmware calculation such as > >> > >> TotalBytes = (TOTAL_BYTES_HI_INQ << 32) + TOTAL_BYTES_LO_INQ; > >> MinBytePerPacket = (TotalBytes + MaxPacketPerFrame - > >>1)/MaxPacketPerFrame); > >> > >>which involves a presumably expensive division operation, where I > > > > as MaxBytePerPacket does, if the camera supports a speed increase > > in dependance on the AOI height. > > > >>A manufacturer may find it easier to report the MaxPacketPerFrame > >>value (hard-coded number, no computation) and leave it up to the host > >>to work out the corresponding minimum number of bytes per packet. > > > > Even if it's somewhat expensive, a fixed MaxPacketPerFrame may lead > > to other problems or may be confusing e.g.: > > > > - if MaxPacketPerFrame * 4 > TOTAL_BYTES_INQ > > - if the camera requires a minimum sensor readout bandwidth and > > insists on a minimum value for BytePerPacket > > > > So a camera may have to adjust MaxPacketPerFrame in dependance on other > > current format 7 settings. > > > >>So, I propose that we keep the PACKET_MIN_BYTES_INQ as proposed in > >>Damien's Proposition document, but that we add another proposal for a > >>MaxPacketPerFrame in such a way that manufacturers can choose to > >>implement one of them, or neither, or both. > > > > This could help of course, but seems to be somewhat redundant. > > Good points. I would be happy to drop MaxPacketPerFrame and keep just > the proposed MinBytePerPacket. > > What do others think? I too would keep only MinBytePerPacket. It's nice and clear to have a min and a max for a single value. Damien -- _ Damien 高原 Douxchamps ('- Assistant Professor //\ Image Processing Laboratory, NAIST V_/_ http://damien.douxchamps.net/ |
From: Damien D. <da...@do...> - 2007-05-30 06:31:39
|
On Wed, 2007-05-30 at 09:32 +1200, Johann Schoonees wrote: > Damien Douxchamps wrote: > > > > The version 1.2 of the document can be found here: > > > > http://damien.douxchamps.net/ieee1394/libdc1394/IIDC_propositions_1.2.odt > > http://damien.douxchamps.net/ieee1394/libdc1394/IIDC_propositions_1.2.pdf > > Oops! Some more little fixes: > > 1. The heading of section 5.2 "Defind a MaxPacketPerFrame > (Format_7)" should read "Define a MinBytePerPacket (Format_7)". > > 2. In section 5.2, the fourth paragraph, "BUTE_PER_PACKET" should be > spelled "BYTE_PER_PACKET". > > 3. In section 5.2, remove the quote marks around the fifth > paragraph. (However, I think we are about to remove that whole > paragraph anyway, as you suggested.) Done. > You are doing a great job here Damien! Thanks ;) Damien -- _ Damien 高原 Douxchamps ('- Assistant Professor //\ Image Processing Laboratory, NAIST V_/_ http://damien.douxchamps.net/ |