On Wed, Aug 5, 2020 at 4:27 AM Takio Yamaoka <y.ta...@gmail.com> wrote: > > Thank you for the review! > It is my first time to send a patch. So I was relieved to hear that. > > Is it OK to wait to merge? > > Best Regards, > Takio >
Sorry, decided to take a look at the logic but didn't have the time to actually test it locally. I have now tested it locally and it works as expected with the following test cases: <unset> and chromaoffset 0 (still kept at default, which is zero with x264) chromaoffset -3 chromaoffset 5 Looking at the history, it was originally implemented correctly in 5764d38173661c29d954711dd5abfddf709e9ba4 but then broken in b340bd8a58c32453172404a8e4240e3317e341da . If you are OK with the following rewording of the commit message, I can push it tomorrow after work: avcodec/libx264: fix chroma quantizer offset usage The default for the chromaoffset field in AVCodecContext is zero, which until now always ended up overriding the AVOption-set value, thus leading to the AVOption not working. Additionally, the previous usage prevented the usage of negative values, while both the variable as well as x264's API would successfully handle such. Thus, the default value of the AVOption is changed to match the default of x264 (and what is currently the default for the AVCodecContext chromaoffset field), and the checks are changed to check for nonzero values. This way: 1. the library default is still utilized if the value is zero. 2. both negative and positive values are correctly passed to x264. For historical context, this was initially similarly implemented in 5764d38173661c29d954711dd5abfddf709e9ba4, and then b340bd8a58c32453172404a8e4240e3317e341da broke the value. Partially reverts commit b340bd8a58c32453172404a8e4240e3317e341da. Signed-off-by: Takio Yamaoka <y.ta...@gmail.com> Best regards, Jan P.S. Top-posting is frowned upon on this mailing list, so please reply in-line or underneath in the future, if possible. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
