On 2/20/2016 9:24 PM, Michael Niedermayer wrote: >> This is a silent API break. You changed behavior of a function in such a way >> that functioning code no longer works. > > yes, i posted a patch that would have maintained API more but people > did not like it
Yes, I must have missed it. Perhaps it got drowned in a sea of Mats self-replies. Sorry about that. > peer review said: > "> I'd totally expect each line _and_ the start of the palette to be > > aligned to the requested slignment. > > It's what I would expect as well." I would argue, also, that I would not expect a "GRAY8" plane to: 1. Have a palette. 2. Require alignment. Grey is grey. Not colored with a palette. But I digress, that matter is beyond the scope here, I suppose. > so i did that and i added the check above to catch the case where > this results in unaligned AVFrame. > dependig on how the AVFrame is used that can be a problem or can also > be no problem. [...] > should i remove this check ? (this would be undefined behavior if > someone accesses the palette with int*, i belive there is some code in > our codebase which does this ...) Aside: An alignment of 4 is not going to help if someone accesses it as int* on a platform with 64 bit ints. > should i replace it by a warning ? > > should i revert the whole patchset (that will result in generated raw > rgb files to be invalid for nut, avi and mov) [...] > should i revert this and apply the other patchset that maintains API > more but that was rejected by people ? I didn't see this one either; I'll try and go back and look. > do you suggest something else ? > also iam very happy to leave this to others, if someone else wants to > take over, its rather difficult to implement this in a way that makes > everyone happy. I'm truly not sure. In my view, the technically correct thing to do is to introduce a new API function (according to Semantic Versioning). This introduces yet another some_function2, which is more API churn... I am not sure how many people outside of FFMS2 this has/will bite, or what is the "least bad" fix. I only noticed since FFMS2 apparently ceased to function entirely after that commit; I doubt anyone else hardcodes GRAY8. I am hoping the two people named in the commit (Stefano and wm4) can offer some insight. - Derek _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
