On 27 December 2016 at 07:37, Jerry Jiang <jerryjiang1...@gmail.com> wrote:
> Hey everyone, > > This is my first patch submitted to FFmpeg, so I'm sure that I missed > something. Please bear with me. :P This patch implements the solution > outlined here: https://guru.multimedia.cx/small-tasks-for-ffmpeg/ > > Hi, thanks for your patch It's good to see someone else work on encoders too! > Activated by passing the flag "-huffman optimal" to the mjpeg encoder, > otherwise the default huffman tables are used. > What's the performance impact like when you enable it? Any reason why it shouldn't be made the default? Encoders have a lot more lenience with respect to performance regressions than decoders (given they affect quality) so if the encoding time isn't doubled or so and the compression gains are decent I don't see a problem having it on by default. Would be nice to see SSIM or such comparisons as well. You can make such using "ffmpeg -i <source> -i <image1> -filter_complex ssim -f null -" which will work on compressed images. Be sure to give the compressed size (in bytes) too. > - encode_block(s, block[0], 0); > - encode_block(s, block[2], 2); > - encode_block(s, block[4], 4); > - encode_block(s, block[8], 8); > - encode_block(s, block[5], 5); > - encode_block(s, block[9], 9); > + if (!ret) ret = encode_block(s, block[0], 0); > + if (!ret) ret = encode_block(s, block[2], 2); > + if (!ret) ret = encode_block(s, block[4], 4); > + if (!ret) ret = encode_block(s, block[8], 8); > + if (!ret) ret = encode_block(s, block[5], 5); > + if (!ret) ret = encode_block(s, block[9], 9); > > Is all the error checking required? Couldn't the encode_block() function do error checking which just returns instantly if there was a failure? On first look this patch looks nice, I'll be able to do a more thorough review and some testing once I get some sleep now. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
