{"action":"created","issue":{"url":"https://api.github.com/repos/darktable-org/rawspeed/issues/26","repository_url":"https://api.github.com/repos/darktable-org/rawspeed","labels_url":"https://api.github.com/repos/darktable-org/rawspeed/issues/26/labels{/name}","comments_url":"https://api.github.com/repos/darktable-org/rawspeed/issues/26/comments","events_url":"https://api.github.com/repos/darktable-org/rawspeed/issues/26/events","html_url":"https://github.com/darktable-org/rawspeed/issues/26","id":199959031,"number":26,"title":"Improve readability and usability of the CFA data tag in the xml file","user":{"login":"axxel","id":1038346,"avatar_url":"https://avatars.githubusercontent.com/u/1038346?v=3","gravatar_id":"","url":"https://api.github.com/users/axxel","html_url":"https://github.com/axxel","followers_url":"https://api.github.com/users/axxel/followers","following_url":"https://api.github.com/users/axxel/following{/other_user}","gists_url":"https://api.github.com/users/axxel/gists{/gist_id}","starred_url":"https://api.github.com/users/axxel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/axxel/subscriptions","organizations_url":"https://api.github.com/users/axxel/orgs","repos_url":"https://api.github.com/users/axxel/repos","events_url":"https://api.github.com/users/axxel/events{/privacy}","received_events_url":"https://api.github.com/users/axxel/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":13,"created_at":"2017-01-10T23:05:31Z","updated_at":"2017-01-13T21:33:52Z","closed_at":null,"body":"The fact that `CFA2` was introduced (probably because someone noticed that this verbose `x, y, color` interface is inappropriate to specify a 6x6 array) shows that the CFA tag has an issue. But the solution to come up with a CFA2 like it is, is not a good solution to this problem.\r\n\r\nMy issues with this are:\r\n1. the interface is too generic, i.e. it allows more stuff than is useful and this extra freedom allows plenty of errors, e.g. specifying two colors for position `0x0`, or specifying a 2x3 pattern.\r\n1. having two options for one problem is at least confusing\r\n1. it is way too much text to look at just convey the info \"RGGB\" (almost half of the whole file content could be removed)\r\n\r\nProposal by example:\r\n```\r\n                <CFA width=\"2\" height=\"2\">\r\n                        <Color x=\"0\" y=\"0\">RED</Color>\r\n                        <Color x=\"1\" y=\"0\">GREEN</Color>\r\n                        <Color x=\"0\" y=\"1\">GREEN</Color>\r\n                        <Color x=\"1\" y=\"1\">BLUE</Color>\r\n                </CFA>\r\n\r\n                <CFA2 width=\"6\" height=\"6\">\r\n                        <ColorRow y=\"0\">RBGBRG</ColorRow>\r\n                        <ColorRow y=\"1\">GGRGGB</ColorRow>\r\n                        <ColorRow y=\"2\">GGBGGR</ColorRow>\r\n                        <ColorRow y=\"3\">BRGRBG</ColorRow>\r\n                        <ColorRow y=\"4\">GGBGGR</ColorRow>\r\n                        <ColorRow y=\"5\">GGRGGB</ColorRow>\r\n                </CFA2>\r\n\r\n```\r\nbecomes\r\n```\r\n                <CFA>RGGB</CFA>\r\n\r\n                <CFA>RBGBRG GGRGGB GGBGGR BRGRBG GGBGGR GGRGGB</CFA>\r\n```\r\nNote: one could insert line breaks inside the string if desired to align the different lines below each other\r\n\r\nAdvantages:\r\n1. way less unnecessary text to look at and edit\r\n1. no way to specify one entry twice or miss one\r\n1. only one way to do it and only one code path to parse it\r\n\r\nThe implementation would simply remove all whitespace from the string, then the number of characters has to be a square (well, actually only 4 and 36 are interesting right now). Then the string is parsed character by character and the internal array is filled. done."},"comment":{"url":"https://api.github.com/repos/darktable-org/rawspeed/issues/comments/272553706","html_url":"https://github.com/darktable-org/rawspeed/issues/26#issuecomment-272553706","issue_url":"https://api.github.com/repos/darktable-org/rawspeed/issues/26","id":272553706,"user":{"login":"axxel","id":1038346,"avatar_url":"https://avatars.githubusercontent.com/u/1038346?v=3","gravatar_id":"","url":"https://api.github.com/users/axxel","html_url":"https://github.com/axxel","followers_url":"https://api.github.com/users/axxel/followers","following_url":"https://api.github.com/users/axxel/following{/other_user}","gists_url":"https://api.github.com/users/axxel/gists{/gist_id}","starred_url":"https://api.github.com/users/axxel/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/axxel/subscriptions","organizations_url":"https://api.github.com/users/axxel/orgs","repos_url":"https://api.github.com/users/axxel/repos","events_url":"https://api.github.com/users/axxel/events{/privacy}","received_events_url":"https://api.github.com/users/axxel/received_events","type":"User","site_admin":false},"created_at":"2017-01-13T21:33:52Z","updated_at":"2017-01-13T21:33:52Z","body":"> We are going in circles. I have already answered that question a day ago, #26 (comment)\r\n\r\nAlright. I was just trying to summarize the made decisions we agreed on to prevent misunderstandings and explicitly asked you to provide constructive feedback if there is something in the summary you do not agree with. I listed 3 decisions/suggestions and all I got as feedback was `So I don't agree.`. \r\n\r\nRegarding my earlier claim that all EOS use the same RGGB pattern: that was founded on my own experience with a couple of EOS models and this statement: http://lclevy.free.fr/cr2/#interpol. It turns out that **my claim was false**. While most of the EOS models use RGGB, the following don't according to the xml: `50D, 60D, 550D, 600D, 5DM2, 1200D, 1300D, 1D, 1Ds, 1DsM2`. And I verified it for two of them. And since the CR2 code also mentions the `GREEN2` as the default, we can not remove the CFA info from the other EOS models **and** keep the forward compatibility of old code.\r\n\r\n> I had seen minimum one camera where the cfa pattern was wrong in dng and I it could have been wrong in raw (if it was included).\r\n\r\nMaybe it was this one?\r\n```\r\nRAW_NIKON_E5400.NEF   0x828e SubImage2    CFAPattern                  Byte        4  2 1 1 0\r\nRAW_NIKON_E5400.NEF   0xa302 Photo        CFAPattern                  Undefined   8  0 2 0 2 5 3 1 4\r\n```\r\nSo you can see, it has two contradicting CFAPattern infos. Maybe the script picked the wrong one? FYI: I checked all the samples we have and only NEF files have two CFAPattern tags and only this single one has contradicting information.\r\n\r\n> Once dt-2.4 gets released, i will start branching rawspeed; but as for actual releases i'm not quite sure.\r\n\r\nWhat is your reasoning here? Something like \"stable\" vs. \"development\" kind of branching?"}}
{"actor":{"display_login":"axxel"}}