I answer here to Carl, because his comments are about the display window/data window patch.
2016-04-08 13:00 GMT+02:00 Carl Eugen Hoyos <ceho...@ag.or.at>: > Martin Vignali <martin.vignali <at> gmail.com> writes: > > > - line = AV_RL32(src - 8); > > + line = (int32_t)AV_RL32(src - 8); > > This change is ugly and should be unneeded. > Before this patch datawindow/displaywindow, are interpret as positive value. For a correct management of display window/datawindow, we need to use int32_t. Without this line modification, i have had some trouble (compiler clang, OS 10.9), where "line" doesn't always get a negative value as it should be. What do you recommand instead ? > > > + channel_buffer[0] += channelLineSize * line_to_skip; > > + channel_buffer[1] += channelLineSize * line_to_skip; > > + channel_buffer[2] += channelLineSize * line_to_skip; > > + if (channel_buffer[3]) > > + channel_buffer[3] += channelLineSize * line_to_skip; > > > + channel_buffer[0] += channelLineSize; > > + channel_buffer[1] += channelLineSize; > > + channel_buffer[2] += channelLineSize; > > + if (channel_buffer[3]) > > + channel_buffer[3] += channelLineSize; > > for() loops? > I mostly follow here, the same way that already is in the EXR Decoder. But i can make a loop. The other solution, is to let for now, these lines. And i will change it in a future patch (for Luma, Luma/Alpha support, where i will need to change theses parts) > > > + if (s->pixel_type == EXR_HALF) > > + s->pixel_size = 2; > > + else > > + s->pixel_size = 4; > > + > > Please add braces. > Ok, i can add, it, i thought, in ffmpeg, you prefer to doesn't add brace, i there is only one line after if. Thanks for the comment Martin Jokyo Images _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
