diff mbox

[FFmpeg-devel] Implement optimal huffman encoding for (M)JPEG.

Message ID 20170203031826.4668518804B0@Jerrys-MacBook-Pro.local
State Accepted
Headers show

Commit Message

Jerry Jiang Feb. 2, 2017, 7:23 a.m. UTC
> seems to break
> make fate-vsynth1-mjpeg-444

Fixed.

---
 Changelog                                        |   1 +
 doc/encoders.texi                                |  21 ++
 libavcodec/Makefile                              |   8 +-
 libavcodec/mjpegenc.c                            | 243 +++++++++++++++++------
 libavcodec/mjpegenc.h                            |  68 ++++++-
 libavcodec/mjpegenc_common.c                     | 166 +++++++++++++++-
 libavcodec/mjpegenc_common.h                     |   2 +
 libavcodec/mjpegenc_huffman.c                    | 195 ++++++++++++++++++
 libavcodec/mjpegenc_huffman.h                    |  74 +++++++
 libavcodec/mpegvideo.h                           |   1 +
 libavcodec/mpegvideo_enc.c                       |  17 +-
 libavcodec/tests/.gitignore                      |   1 +
 libavcodec/tests/mjpegenc_huffman.c              | 163 +++++++++++++++
 tests/fate/libavcodec.mak                        |   6 +
 tests/fate/vcodec.mak                            |  12 +-
 tests/ref/vsynth/vsynth1-mjpeg-huffman           |   4 +
 tests/ref/vsynth/vsynth1-mjpeg-trell-huffman     |   4 +
 tests/ref/vsynth/vsynth2-mjpeg-huffman           |   4 +
 tests/ref/vsynth/vsynth2-mjpeg-trell-huffman     |   4 +
 tests/ref/vsynth/vsynth3-mjpeg-huffman           |   4 +
 tests/ref/vsynth/vsynth3-mjpeg-trell-huffman     |   4 +
 tests/ref/vsynth/vsynth_lena-mjpeg-huffman       |   4 +
 tests/ref/vsynth/vsynth_lena-mjpeg-trell-huffman |   4 +
 23 files changed, 914 insertions(+), 96 deletions(-)
 create mode 100644 libavcodec/mjpegenc_huffman.c
 create mode 100644 libavcodec/mjpegenc_huffman.h
 create mode 100644 libavcodec/tests/mjpegenc_huffman.c
 create mode 100644 tests/ref/vsynth/vsynth1-mjpeg-huffman
 create mode 100644 tests/ref/vsynth/vsynth1-mjpeg-trell-huffman
 create mode 100644 tests/ref/vsynth/vsynth2-mjpeg-huffman
 create mode 100644 tests/ref/vsynth/vsynth2-mjpeg-trell-huffman
 create mode 100644 tests/ref/vsynth/vsynth3-mjpeg-huffman
 create mode 100644 tests/ref/vsynth/vsynth3-mjpeg-trell-huffman
 create mode 100644 tests/ref/vsynth/vsynth_lena-mjpeg-huffman
 create mode 100644 tests/ref/vsynth/vsynth_lena-mjpeg-trell-huffman

Comments

Jerry Jiang Feb. 8, 2017, 7:09 a.m. UTC | #1
Hey is there an update on the status of the patch? Any other requested
changes?
Rostislav Pehlivanov Feb. 8, 2017, 2:11 p.m. UTC | #2
On 8 February 2017 at 07:09, Jerry Jiang <jerryjiang1128@gmail.com> wrote:

> Hey is there an update on the status of the patch? Any other requested
> changes?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Hi, sorry I missed your last patch.
I've already okay'd it last month so pushed.

Thanks

Since it does so well it would be a good idea to enable it by default like
I said before.
Michael Niedermayer Feb. 9, 2017, 2:01 a.m. UTC | #3
On Wed, Feb 01, 2017 at 11:23:04PM -0800, Jerry Jiang wrote:
> > seems to break
> > make fate-vsynth1-mjpeg-444
> 
> Fixed.
> 
> ---
>  Changelog                                        |   1 +
>  doc/encoders.texi                                |  21 ++
>  libavcodec/Makefile                              |   8 +-
>  libavcodec/mjpegenc.c                            | 243 +++++++++++++++++------
>  libavcodec/mjpegenc.h                            |  68 ++++++-
>  libavcodec/mjpegenc_common.c                     | 166 +++++++++++++++-
>  libavcodec/mjpegenc_common.h                     |   2 +
>  libavcodec/mjpegenc_huffman.c                    | 195 ++++++++++++++++++
>  libavcodec/mjpegenc_huffman.h                    |  74 +++++++
>  libavcodec/mpegvideo.h                           |   1 +
>  libavcodec/mpegvideo_enc.c                       |  17 +-
>  libavcodec/tests/.gitignore                      |   1 +
>  libavcodec/tests/mjpegenc_huffman.c              | 163 +++++++++++++++
>  tests/fate/libavcodec.mak                        |   6 +
>  tests/fate/vcodec.mak                            |  12 +-
>  tests/ref/vsynth/vsynth1-mjpeg-huffman           |   4 +
>  tests/ref/vsynth/vsynth1-mjpeg-trell-huffman     |   4 +
>  tests/ref/vsynth/vsynth2-mjpeg-huffman           |   4 +
>  tests/ref/vsynth/vsynth2-mjpeg-trell-huffman     |   4 +
>  tests/ref/vsynth/vsynth3-mjpeg-huffman           |   4 +
>  tests/ref/vsynth/vsynth3-mjpeg-trell-huffman     |   4 +
>  tests/ref/vsynth/vsynth_lena-mjpeg-huffman       |   4 +
>  tests/ref/vsynth/vsynth_lena-mjpeg-trell-huffman |   4 +
>  23 files changed, 914 insertions(+), 96 deletions(-)
>  create mode 100644 libavcodec/mjpegenc_huffman.c
>  create mode 100644 libavcodec/mjpegenc_huffman.h
>  create mode 100644 libavcodec/tests/mjpegenc_huffman.c
>  create mode 100644 tests/ref/vsynth/vsynth1-mjpeg-huffman
>  create mode 100644 tests/ref/vsynth/vsynth1-mjpeg-trell-huffman
>  create mode 100644 tests/ref/vsynth/vsynth2-mjpeg-huffman
>  create mode 100644 tests/ref/vsynth/vsynth2-mjpeg-trell-huffman
>  create mode 100644 tests/ref/vsynth/vsynth3-mjpeg-huffman
>  create mode 100644 tests/ref/vsynth/vsynth3-mjpeg-trell-huffman
>  create mode 100644 tests/ref/vsynth/vsynth_lena-mjpeg-huffman
>  create mode 100644 tests/ref/vsynth/vsynth_lena-mjpeg-trell-huffman

i somehow must have missed this patch resubmission

this breaks *jpeg slice multithreading

./ffmpeg  -i lena.pnm -threads 4 -thread_type slice file.jpg

result is just a green image and errors

[...]
Michael Niedermayer Feb. 9, 2017, 2:44 a.m. UTC | #4
On Wed, Feb 01, 2017 at 11:23:04PM -0800, Jerry Jiang wrote:
> > seems to break
> > make fate-vsynth1-mjpeg-444
> 
> Fixed.
> 

Missing commit message


[...]
> -void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
> +// Possibly reallocate the huffman code buffer, assuming blocks_per_mb.
> +// Set s->mjpeg_ctx->error on ENOMEM.
> +static void realloc_huffman(MpegEncContext *s, int blocks_per_mb)
>  {
> -    int i;
> +    MJpegContext *m = s->mjpeg_ctx;
> +    size_t num_mbs, num_blocks, num_codes;
> +    MJpegHuffmanCode *new_buf;
> +    if (m->error) return;
> +    // Make sure we have enough space to hold this frame.
> +    num_mbs = s->mb_width * s->mb_height;
> +    num_blocks = num_mbs * blocks_per_mb;
> +    av_assert0(m->huff_ncode <=
> +               (s->mb_y * s->mb_width + s->mb_x) * blocks_per_mb * 64);
> +    num_codes = num_blocks * 64;
> +
> +    new_buf = av_fast_realloc(m->huff_buffer, &m->huff_capacity,
> +                              num_codes * sizeof(MJpegHuffmanCode));
> +    if (!new_buf) {
> +        m->error = AVERROR(ENOMEM);
> +    } else {
> +        m->huff_buffer = new_buf;
> +    }
> +}

why is the macroblock loop calling a "framebuffer" reallocation
function?
the frame size does not change from one maroblock to the next


> +
> +int ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
> +{
> +    int i, is_chroma_420;
> +
> +    // Number of bits used depends on future data.
> +    // So, nothing that relies on encoding many times and taking the
> +    // one with the fewest bits will work properly here.
> +    if (s->i_tex_bits != MJPEG_HUFFMAN_EST_BITS_PER_CODE *
> +        s->mjpeg_ctx->huff_ncode) {

> +        av_log(NULL, AV_LOG_ERROR, "Unsupported encoding method\n");

Missing context for av_log()



> +        return AVERROR(EINVAL);
> +    }
> +
>      if (s->chroma_format == CHROMA_444) {
> +        realloc_huffman(s, 12);
>          encode_block(s, block[0], 0);
>          encode_block(s, block[2], 2);
>          encode_block(s, block[4], 4);
> @@ -196,10 +302,12 @@ void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
>              encode_block(s, block[11], 11);
>          }
>      } else {
> +        is_chroma_420 = (s->chroma_format == CHROMA_420);
> +        realloc_huffman(s, 5 + (is_chroma_420 ? 1 : 3));
>          for(i=0;i<5;i++) {
>              encode_block(s, block[i], i);
>          }
> -        if (s->chroma_format == CHROMA_420) {
> +        if (is_chroma_420) {
>              encode_block(s, block[5], 5);
>          } else {
>              encode_block(s, block[6], 6);
> @@ -207,8 +315,11 @@ void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
>              encode_block(s, block[7], 7);
>          }
>      }
> +    if (s->mjpeg_ctx->error)
> +        return s->mjpeg_ctx->error;
>  
> -    s->i_tex_bits += get_bits_diff(s);
> +    s->i_tex_bits = MJPEG_HUFFMAN_EST_BITS_PER_CODE * s->mjpeg_ctx->huff_ncode;
> +    return 0;
>  }

this patch also breaks 2 pass rate control

./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -vcodec mjpeg -pass 1 -t 10 -an -b 5000k test1.avi
./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -vcodec mjpeg -pass 2 -t 10 -an -b 5000k test2.avi



[...]
> diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
> index 7a6fe746..8ec3df9 100644
> --- a/libavcodec/mjpegenc_common.c
> +++ b/libavcodec/mjpegenc_common.c
> @@ -33,8 +33,35 @@
>  #include "put_bits.h"
>  #include "mjpegenc.h"
>  #include "mjpegenc_common.h"
> +#include "mjpegenc_huffman.h"
>  #include "mjpeg.h"
>  

> +av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t *uni_ac_vlc_len)
> +{
> +    int i;
> +
> +    for (i = 0; i < 128; i++) {
> +        int level = i - 64;
> +        int run;
> +        if (!level)
> +            continue;
> +        for (run = 0; run < 64; run++) {
> +            int len, code, nbits;
> +            int alevel = FFABS(level);
> +
> +            len = (run >> 4) * huff_size_ac[0xf0];
> +
> +            nbits= av_log2_16bit(alevel) + 1;
> +            code = ((15&run) << 4) | nbits;
> +
> +            len += huff_size_ac[code] + nbits;
> +
> +            uni_ac_vlc_len[UNI_AC_ENC_INDEX(run, i)] = len;
> +            // We ignore EOB as its just a constant which does not change generally
> +        }
> +    }
> +}

This code is moved, it should have been in a seperate cosmetic patch



[...]
> diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
> index 6e51ca0..7d760f8 100644
> --- a/libavcodec/mjpegenc_common.h
> +++ b/libavcodec/mjpegenc_common.h
> @@ -40,4 +40,6 @@ void ff_mjpeg_init_hvsample(AVCodecContext *avctx, int hsample[4], int vsample[4
>  void ff_mjpeg_encode_dc(PutBitContext *pb, int val,
>                          uint8_t *huff_size, uint16_t *huff_code);
>  
> +av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t *uni_ac_vlc_len);

missing ff/av prefix


[...]
> +// Validate the computed lengths satisfy the JPEG restrictions and is optimal.
> +static int check_lengths(int L, int expected_length,
> +                         const int *probs, int nprobs)
> +{
> +    HuffTable lengths[256];
> +    PTable val_counts[256];
> +    int actual_length = 0, i, j, k, prob, length;
> +    int ret = 0;
> +    double cantor_measure = 0;

> +    assert(nprobs <= 256);

should be av_assert*()


[...]
> +static const int probs_zeroes[] = {6, 6, 0, 0, 0};
> +static const int probs_skewed[] = {2, 0, 0, 0, 0, 1, 0, 0, 20, 0, 2,
> +    0, 10, 5, 1, 1, 9, 1, 1, 6, 0, 5, 0, 1, 0, 7, 6, 1, 1, 5, 0, 0, 0, 0,
> +    11, 0, 0, 0, 51, 1, 0, 20, 0, 1, 0, 0, 0, 0, 6, 106, 1, 0, 1, 0, 2, 1,
> +    16, 0, 0, 5, 0, 0, 0, 4, 3, 15, 4, 4, 0, 0, 0, 3, 0, 0, 1, 0, 3, 0, 3,
> +    2, 2, 0, 0, 4, 3, 40, 1, 2, 0, 22, 0, 0, 0, 9, 0, 0, 0, 0, 1, 1, 0, 1,
> +    6, 11, 4, 10, 28, 6, 1, 0, 0, 9, 9, 4, 0, 0, 0, 0, 8, 33844, 2, 0, 2,
> +    1, 1, 5, 0, 0, 1, 9, 1, 0, 4, 14, 4, 0, 0, 3, 8, 0, 51, 9, 6, 1, 1, 2,
> +    2, 3, 1, 5, 5, 29, 0, 0, 0, 0, 14, 29, 6, 4, 13, 12, 2, 3, 1, 0, 5, 4,
> +    1, 1, 0, 0, 29, 1, 0, 0, 0, 0, 4, 0, 0, 1, 0, 1, 7, 0, 42, 0, 0, 0, 0,
> +    0, 2, 0, 3, 9, 0, 0, 0, 2, 1, 0, 0, 6, 5, 6, 1, 2, 3, 0, 0, 0, 3, 0, 0,
> +    28, 0, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 23, 0, 0, 0, 0,
> +    0, 21, 1, 0, 3, 24, 2, 0, 0, 7, 0, 0, 1, 5, 1, 2, 0, 5};
> +static const int probs_sat[] = {74, 8, 14, 7, 9345, 40, 0, 2014, 2, 1,
> +    115, 0, 2, 1, 194, 388, 20, 0, 0, 2, 1, 121, 1, 1583, 0, 16, 21, 2, 132,
> +    2, 15, 9, 13, 1, 0, 2293, 2, 8, 5, 2, 30, 0, 0, 4, 54, 783, 4, 1, 2, 4,
> +    0, 22, 93, 1, 143, 19, 0, 36, 32, 4, 6, 33, 3, 45, 0, 8, 1, 0, 18, 17, 1,
> +    0, 1, 0, 0, 1, 1004, 38, 3, 8, 90, 23, 0, 2819, 3, 0, 970, 158, 9, 6, 4,
> +    48, 4, 0, 1, 0, 0, 60, 3, 62, 0, 2, 2, 2, 279, 66, 16, 1, 20, 0, 7, 9,
> +    32, 1411, 6, 3, 27, 1, 5, 49, 0, 0, 0, 0, 0, 2, 10, 1, 1, 2, 3, 801, 3,
> +    25, 5, 1, 1, 0, 632, 0, 14, 18, 5, 8, 200, 4, 4, 22, 12, 0, 4, 1, 0, 2,
> +    4, 9, 3, 16, 7, 2, 2, 213, 0, 2, 620, 39303, 0, 1, 0, 2, 1, 183781, 1,
> +    0, 0, 0, 94, 7, 3, 4, 0, 4, 306, 43, 352, 76, 34, 13, 11, 0, 51, 1, 13,
> +    19, 0, 26, 0, 7276, 4, 207, 31, 1, 2, 4, 6, 19, 8, 17, 4, 6, 0, 1085, 0,
> +    0, 0, 3, 489, 36, 1, 0, 1, 9420, 294, 28, 0, 57, 5, 0, 9, 2, 0, 1, 2, 2,
> +    0, 0, 9, 2, 29, 2, 2, 7, 0, 5, 490, 0, 7, 5, 0, 1, 8, 0, 0, 23255, 0, 1};

vertical align



> +

> +// Test the example given on @see
> +// http://guru.multimedia.cx/small-tasks-for-ffmpeg/
> +int main(int argc, char **argv)
> +{
> +    int i, ret = 0;
> +    // Probabilities of symbols 0..4
> +    static PTable val_counts[] = {

static isnt needed here this is main()

i sadly dont have time to do a more complete review ATM (need sleep)
but this patch doesnt look like it was ready for being pushed

[...]
Rostislav Pehlivanov Feb. 9, 2017, 3:17 a.m. UTC | #5
On 9 February 2017 at 02:44, Michael Niedermayer <michael@niedermayer.cc>
wrote:

>
> > +
> > +int ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
> > +{
> > +    int i, is_chroma_420;
> > +
> > +    // Number of bits used depends on future data.
> > +    // So, nothing that relies on encoding many times and taking the
> > +    // one with the fewest bits will work properly here.
> > +    if (s->i_tex_bits != MJPEG_HUFFMAN_EST_BITS_PER_CODE *
> > +        s->mjpeg_ctx->huff_ncode) {
>
> > +        av_log(NULL, AV_LOG_ERROR, "Unsupported encoding method\n");
>
> Missing context for av_log()
>

Fixed


>
> [...]
> > diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
> > index 6e51ca0..7d760f8 100644
> > --- a/libavcodec/mjpegenc_common.h
> > +++ b/libavcodec/mjpegenc_common.h
> > @@ -40,4 +40,6 @@ void ff_mjpeg_init_hvsample(AVCodecContext *avctx,
> int hsample[4], int vsample[4
> >  void ff_mjpeg_encode_dc(PutBitContext *pb, int val,
> >                          uint8_t *huff_size, uint16_t *huff_code);
> >
> > +av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t
> *uni_ac_vlc_len);
>
> missing ff/av prefix
>

Fixed


> [...]
> > +// Validate the computed lengths satisfy the JPEG restrictions and is
> optimal.
> > +static int check_lengths(int L, int expected_length,
> > +                         const int *probs, int nprobs)
> > +{
> > +    HuffTable lengths[256];
> > +    PTable val_counts[256];
> > +    int actual_length = 0, i, j, k, prob, length;
> > +    int ret = 0;
> > +    double cantor_measure = 0;
>
> > +    assert(nprobs <= 256);
>
> should be av_assert*()
>
>
Fixed


>
> [...]
> > +static const int probs_zeroes[] = {6, 6, 0, 0, 0};
> > +static const int probs_skewed[] = {2, 0, 0, 0, 0, 1, 0, 0, 20, 0, 2,
> > +    0, 10, 5, 1, 1, 9, 1, 1, 6, 0, 5, 0, 1, 0, 7, 6, 1, 1, 5, 0, 0, 0,
> 0,
> > +    11, 0, 0, 0, 51, 1, 0, 20, 0, 1, 0, 0, 0, 0, 6, 106, 1, 0, 1, 0, 2,
> 1,
> > +    16, 0, 0, 5, 0, 0, 0, 4, 3, 15, 4, 4, 0, 0, 0, 3, 0, 0, 1, 0, 3, 0,
> 3,
> > +    2, 2, 0, 0, 4, 3, 40, 1, 2, 0, 22, 0, 0, 0, 9, 0, 0, 0, 0, 1, 1, 0,
> 1,
> > +    6, 11, 4, 10, 28, 6, 1, 0, 0, 9, 9, 4, 0, 0, 0, 0, 8, 33844, 2, 0,
> 2,
> > +    1, 1, 5, 0, 0, 1, 9, 1, 0, 4, 14, 4, 0, 0, 3, 8, 0, 51, 9, 6, 1, 1,
> 2,
> > +    2, 3, 1, 5, 5, 29, 0, 0, 0, 0, 14, 29, 6, 4, 13, 12, 2, 3, 1, 0, 5,
> 4,
> > +    1, 1, 0, 0, 29, 1, 0, 0, 0, 0, 4, 0, 0, 1, 0, 1, 7, 0, 42, 0, 0, 0,
> 0,
> > +    0, 2, 0, 3, 9, 0, 0, 0, 2, 1, 0, 0, 6, 5, 6, 1, 2, 3, 0, 0, 0, 3,
> 0, 0,
> > +    28, 0, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 23, 0, 0, 0,
> 0,
> > +    0, 21, 1, 0, 3, 24, 2, 0, 0, 7, 0, 0, 1, 5, 1, 2, 0, 5};
> > +static const int probs_sat[] = {74, 8, 14, 7, 9345, 40, 0, 2014, 2, 1,
> > +    115, 0, 2, 1, 194, 388, 20, 0, 0, 2, 1, 121, 1, 1583, 0, 16, 21, 2,
> 132,
> > +    2, 15, 9, 13, 1, 0, 2293, 2, 8, 5, 2, 30, 0, 0, 4, 54, 783, 4, 1,
> 2, 4,
> > +    0, 22, 93, 1, 143, 19, 0, 36, 32, 4, 6, 33, 3, 45, 0, 8, 1, 0, 18,
> 17, 1,
> > +    0, 1, 0, 0, 1, 1004, 38, 3, 8, 90, 23, 0, 2819, 3, 0, 970, 158, 9,
> 6, 4,
> > +    48, 4, 0, 1, 0, 0, 60, 3, 62, 0, 2, 2, 2, 279, 66, 16, 1, 20, 0, 7,
> 9,
> > +    32, 1411, 6, 3, 27, 1, 5, 49, 0, 0, 0, 0, 0, 2, 10, 1, 1, 2, 3,
> 801, 3,
> > +    25, 5, 1, 1, 0, 632, 0, 14, 18, 5, 8, 200, 4, 4, 22, 12, 0, 4, 1,
> 0, 2,
> > +    4, 9, 3, 16, 7, 2, 2, 213, 0, 2, 620, 39303, 0, 1, 0, 2, 1, 183781,
> 1,
> > +    0, 0, 0, 94, 7, 3, 4, 0, 4, 306, 43, 352, 76, 34, 13, 11, 0, 51, 1,
> 13,
> > +    19, 0, 26, 0, 7276, 4, 207, 31, 1, 2, 4, 6, 19, 8, 17, 4, 6, 0,
> 1085, 0,
> > +    0, 0, 3, 489, 36, 1, 0, 1, 9420, 294, 28, 0, 57, 5, 0, 9, 2, 0, 1,
> 2, 2,
> > +    0, 0, 9, 2, 29, 2, 2, 7, 0, 5, 490, 0, 7, 5, 0, 1, 8, 0, 0, 23255,
> 0, 1};
>
> vertical align
>
>
Fixed


>
> > +
>
> > +// Test the example given on @see
> > +// http://guru.multimedia.cx/small-tasks-for-ffmpeg/
> > +int main(int argc, char **argv)
> > +{
> > +    int i, ret = 0;
> > +    // Probabilities of symbols 0..4
> > +    static PTable val_counts[] = {
>
> static isnt needed here this is main()
>
>
Fixed


> i sadly dont have time to do a more complete review ATM (need sleep)
> but this patch doesnt look like it was ready for being pushed
>

Perhaps I was too hasty to review it

I'll take a look at fixing the 2 pass mode tomorrow if I have the time,
although if you have spare
time and know how to fix it go ahead.
Michael Niedermayer Feb. 9, 2017, 3:30 a.m. UTC | #6
On Thu, Feb 09, 2017 at 03:17:23AM +0000, Rostislav Pehlivanov wrote:
> On 9 February 2017 at 02:44, Michael Niedermayer <michael@niedermayer.cc>
[...]
> > i sadly dont have time to do a more complete review ATM (need sleep)
> > but this patch doesnt look like it was ready for being pushed
> >
> 
> Perhaps I was too hasty to review it
> 
> I'll take a look at fixing the 2 pass mode tomorrow if I have the time,
> although if you have spare
> time and know how to fix it go ahead.

its bad time for me, tomorrow is gsoc register deadline, and i also
need to do new releases, i doubt ill be able to do all
and that reminds me i should be sleeping

[...
Michael Niedermayer Feb. 9, 2017, 3:33 a.m. UTC | #7
On Thu, Feb 09, 2017 at 04:30:02AM +0100, Michael Niedermayer wrote:
> On Thu, Feb 09, 2017 at 03:17:23AM +0000, Rostislav Pehlivanov wrote:
> > On 9 February 2017 at 02:44, Michael Niedermayer <michael@niedermayer.cc>
> [...]
> > > i sadly dont have time to do a more complete review ATM (need sleep)
> > > but this patch doesnt look like it was ready for being pushed
> > >
> > 
> > Perhaps I was too hasty to review it
> > 
> > I'll take a look at fixing the 2 pass mode tomorrow if I have the time,
> > although if you have spare
> > time and know how to fix it go ahead.
> 
> its bad time for me, tomorrow is gsoc register deadline, and i also

i mean reynaldo already filled all out  (great thanks to him) so
we are good about that but i wanted to double check it all


> need to do new releases, i doubt ill be able to do all
> and that reminds me i should be sleeping
> 
> [...
> 
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Let us carefully observe those good qualities wherein our enemies excel us
> and endeavor to excel them, by avoiding what is faulty, and imitating what
> is excellent in them. -- Plutarch



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Michael Niedermayer Feb. 9, 2017, 12:20 p.m. UTC | #8
On Thu, Feb 09, 2017 at 03:44:59AM +0100, Michael Niedermayer wrote:
> On Wed, Feb 01, 2017 at 11:23:04PM -0800, Jerry Jiang wrote:
[...]
> i sadly dont have time to do a more complete review ATM (need sleep)
> but this patch doesnt look like it was ready for being pushed

more reviewing related to what is in git + fixes


[...]

>+static inline void ff_mjpeg_encode_code(MJpegContext *s, uint8_t table_id, int code)

static functions dont need ff_ prefix


>+{
>+    MJpegHuffmanCode *c = &s->huff_buffer[s->huff_ncode++];
>+    av_assert0(s->huff_ncode < s->huff_capacity);
>+    c->table_id = table_id;
>+    c->code = code;
>+}
>+
>+/**
>+ * Add the coefficient's data to the JPEG buffer.
>+ *
>+ * @param s The MJpegContext which contains the JPEG buffer.
>+ * @param table_id Which Huffman table the code belongs to.
>+ * @param val The coefficient.
>+ * @param run The run-bits.
>+ */

>+static void ff_mjpeg_encode_coef(MJpegContext *s, uint8_t table_id, int val, int run)

static functions dont need ff_ prefix

[...]

>-void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
>+// Possibly reallocate the huffman code buffer, assuming blocks_per_mb.
>+// Set s->mjpeg_ctx->error on ENOMEM.
>+static void realloc_huffman(MpegEncContext *s, int blocks_per_mb)
> {
>-    int i;
>+    MJpegContext *m = s->mjpeg_ctx;
>+    size_t num_mbs, num_blocks, num_codes;
>+    MJpegHuffmanCode *new_buf;
>+    if (m->error) return;
>+    // Make sure we have enough space to hold this frame.


>+    num_mbs = s->mb_width * s->mb_height;
>+    num_blocks = num_mbs * blocks_per_mb;
>+    av_assert0(m->huff_ncode <=
>+               (s->mb_y * s->mb_width + s->mb_x) * blocks_per_mb * 64);
>+    num_codes = num_blocks * 64;
>+
>+    new_buf = av_fast_realloc(m->huff_buffer, &m->huff_capacity,
>+                              num_codes * sizeof(MJpegHuffmanCode));

this will always reallocate the buffer to its maximum, in fact it
will allocate a much larger buffer than the maximum as
av_fast_realloc() overallocates

so this is always slower and uses more memory than if the maximum
buffer size is allocated during init.

I understand the intent might have been to allocate only what is
needed but the code does not do that


>+    if (!new_buf) {
>+        m->error = AVERROR(ENOMEM);
>+    } else {
>+        m->huff_buffer = new_buf;
>+    }
>+}
>+
>+int ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
>+{
>+    int i, is_chroma_420;
>+
>+    // Number of bits used depends on future data.
>+    // So, nothing that relies on encoding many times and taking the
>+    // one with the fewest bits will work properly here.
>+    if (s->i_tex_bits != MJPEG_HUFFMAN_EST_BITS_PER_CODE *
>+        s->mjpeg_ctx->huff_ncode) {
>+        av_log(s->avctx, AV_LOG_ERROR, "Unsupported encoding method\n");
>+        return AVERROR(EINVAL);
>+    }
>+
>     if (s->chroma_format == CHROMA_444) {
>+        realloc_huffman(s, 12);
>         encode_block(s, block[0], 0);
>         encode_block(s, block[2], 2);
>         encode_block(s, block[4], 4);
>@@ -196,10 +302,12 @@ void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
>             encode_block(s, block[11], 11);
>         }
>     } else {
>+        is_chroma_420 = (s->chroma_format == CHROMA_420);
>+        realloc_huffman(s, 5 + (is_chroma_420 ? 1 : 3));
>         for(i=0;i<5;i++) {
>             encode_block(s, block[i], i);
>         }
>-        if (s->chroma_format == CHROMA_420) {
>+        if (is_chroma_420) {
>             encode_block(s, block[5], 5);
>         } else {
>             encode_block(s, block[6], 6);
>@@ -207,8 +315,11 @@ void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
>             encode_block(s, block[7], 7);
>         }
>     }
>+    if (s->mjpeg_ctx->error)
>+        return s->mjpeg_ctx->error;
>

>-    s->i_tex_bits += get_bits_diff(s);
>+    s->i_tex_bits = MJPEG_HUFFMAN_EST_BITS_PER_CODE * s->mjpeg_ctx->huff_ncode;
>+    return 0;

i_tex_bits is for rate control, iam not sure what this use is trying to
do but the value its set to is not correct and i see nothing that
corrects it. This is likely what breaks 2pass

[...]


>@@ -372,14 +414,116 @@ void ff_mjpeg_escape_FF(PutBitContext *pb, int start)
>     }
> }
>
>+/**
>+ * Builds all 4 optimal Huffman tables.
>+ *
>+ * Uses the data stored in the JPEG buffer to compute the tables.
>+ * Stores the Huffman tables in the bits_* and val_* arrays in the MJpegContext.
>+ *
>+ * @param m MJpegContext containing the JPEG buffer.
>+ */
>+static void ff_mjpeg_build_optimal_huffman(MJpegContext *m)
>+{
>+    int i, ret, table_id, code;
>+
>+    MJpegEncHuffmanContext dc_luminance_ctx;
>+    MJpegEncHuffmanContext dc_chrominance_ctx;
>+    MJpegEncHuffmanContext ac_luminance_ctx;
>+    MJpegEncHuffmanContext ac_chrominance_ctx;
>+    MJpegEncHuffmanContext *ctx[4] = {&dc_luminance_ctx,
>+                                      &dc_chrominance_ctx,
>+                                      &ac_luminance_ctx,
>+                                      &ac_chrominance_ctx};
>+    for (i = 0; i < 4; i++) {
>+        ff_mjpeg_encode_huffman_init(ctx[i]);
>+    }
>+    for (i = 0; i < m->huff_ncode; i++) {
>+        table_id = m->huff_buffer[i].table_id;
>+        code = m->huff_buffer[i].code;
>+
>+        ff_mjpeg_encode_huffman_increment(ctx[table_id], code);
>+    }
>+

>+    ret = ff_mjpeg_encode_huffman_close(&dc_luminance_ctx,
>+                                        m->bits_dc_luminance,
>+                                        m->val_dc_luminance, 12);
>+    av_assert0(!ret);
>+    ret = ff_mjpeg_encode_huffman_close(&dc_chrominance_ctx,
>+                                        m->bits_dc_chrominance,
>+                                        m->val_dc_chrominance, 12);
>+    av_assert0(!ret);
>+    ret = ff_mjpeg_encode_huffman_close(&ac_luminance_ctx,
>+                                        m->bits_ac_luminance,
>+                                        m->val_ac_luminance, 256);
>+    av_assert0(!ret);
>+    ret = ff_mjpeg_encode_huffman_close(&ac_chrominance_ctx,
>+                                        m->bits_ac_chrominance,
>+                                        m->val_ac_chrominance, 256);
>+    av_assert0(!ret);

if the error cannot occur the assert belongs in the function,
theres no need to return a constant that is asserted on outside




>+
>+    ff_mjpeg_build_huffman_codes(m->huff_size_dc_luminance,
>+                                 m->huff_code_dc_luminance,
>+                                 m->bits_dc_luminance,
>+                                 m->val_dc_luminance);
>+    ff_mjpeg_build_huffman_codes(m->huff_size_dc_chrominance,
>+                                 m->huff_code_dc_chrominance,
>+                                 m->bits_dc_chrominance,
>+                                 m->val_dc_chrominance);
>+    ff_mjpeg_build_huffman_codes(m->huff_size_ac_luminance,
>+                                 m->huff_code_ac_luminance,
>+                                 m->bits_ac_luminance,
>+                                 m->val_ac_luminance);
>+    ff_mjpeg_build_huffman_codes(m->huff_size_ac_chrominance,
>+                                 m->huff_code_ac_chrominance,
>+                                 m->bits_ac_chrominance,
>+                                 m->val_ac_chrominance);
>+}
>+
>+/**
>+ * Writes the complete JPEG frame.
>+ *
>+ * Header + values + stuffing.
>+ *
>+ * @param s The MpegEncContext.
>+ * @return int Error code, 0 if successful.
>+ */
> int ff_mjpeg_encode_stuffing(MpegEncContext *s)
> {
>     int i;
>     PutBitContext *pbc = &s->pb;
>     int mb_y = s->mb_y - !s->mb_x;
>+    int ret;
>+    MJpegContext *m;
>+
>+    m = s->mjpeg_ctx;
>+
>+    if (m->error)
>+        return m->error;
>+
>+    if (s->huffman == HUFFMAN_TABLE_OPTIMAL) {
>+        ff_mjpeg_build_optimal_huffman(m);
>+
>+        // Replace the VLCs with the optimal ones.

>+        // The default ones may be used for trellis during quantization.
>+        ff_init_uni_ac_vlc(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
>+        ff_init_uni_ac_vlc(m->huff_size_ac_chrominance, m->uni_chroma_ac_vlc_len);

This is wrong and will break trellis with non default tables, trellis
needs to use the same VLCs as are used for encoding. which if non
default ones are used should be these non default ones

the code generating the default tables wrote them in shared
tables (uni*ac_vlc_len), now after the patch the default code is
shared, called from 2 places and the tables moved into the context yet
it still generates only the default tables. This looks unfinished
More so the default tables are unneccessary re-generated every frame
this is just broken unless iam missing some code


>+        s->intra_ac_vlc_length      =
>+        s->intra_ac_vlc_last_length = m->uni_ac_vlc_len;
>+        s->intra_chroma_ac_vlc_length      =
>+        s->intra_chroma_ac_vlc_last_length = m->uni_chroma_ac_vlc_len;
>+    }
>+
>+    ff_mjpeg_encode_picture_header(s->avctx, &s->pb, &s->intra_scantable,
>+                                   s->pred, s->intra_matrix, s->chroma_intra_matrix);
>+    ff_mjpeg_encode_picture_frame(s);

>+    if (m->error < 0) {
>+        ret = m->error;
>+        return ret;
>+    }

the whole m->error system is broken
there is no such thing as an error during encoding with optimal tables
there could be an error if allocation was done based on used codes,
iam not sure this would make sense but the code doesnt do that.

So either always allocate the maximum (which is less than currently
allocated), or reallocate depending on used codes
also either way no error check is neeed at encode_block() level.
A check at ff_mjpeg_encode_mb() is sufficient. The number of blocks
per mb is constant


>+
>+    ret = ff_mpv_reallocate_putbitbuffer(s, put_bits_count(&s->pb) / 8 + 100,
>+                                            put_bits_count(&s->pb) / 4 + 1000);
>
>-    int ret = ff_mpv_reallocate_putbitbuffer(s, put_bits_count(&s->pb) / 8 + 100,
>-                                                put_bits_count(&s->pb) / 4 + 1000);
>     if (ret < 0) {
>         av_log(s->avctx, AV_LOG_ERROR, "Buffer reallocation failed\n");
>         goto fail;

>diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
>index 6e51ca04f8..d9a565dfa9 100644
>--- a/libavcodec/mjpegenc_common.h
>+++ b/libavcodec/mjpegenc_common.h
>@@ -40,4 +40,6 @@ void ff_mjpeg_init_hvsample(AVCodecContext *avctx, int hsample[4], int vsample[4
> void ff_mjpeg_encode_dc(PutBitContext *pb, int val,
>                         uint8_t *huff_size, uint16_t *huff_code);
>
>+av_cold void ff_init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t *uni_ac_vlc_len);

This function is *jpeg specific, nothing in its name suggests that
also other codecs have their own init_uni_ac_vlc() so thats very
confusing naming wise

[...]

--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato
Michael Niedermayer Feb. 9, 2017, 10:34 p.m. UTC | #9
On Thu, Feb 09, 2017 at 01:20:09PM +0100, Michael Niedermayer wrote:
[...]
> > int ff_mjpeg_encode_stuffing(MpegEncContext *s)
> > {
> >     int i;
> >     PutBitContext *pbc = &s->pb;
> >     int mb_y = s->mb_y - !s->mb_x;
> >+    int ret;
> >+    MJpegContext *m;
> >+
> >+    m = s->mjpeg_ctx;
> >+
> >+    if (m->error)
> >+        return m->error;
> >+
> >+    if (s->huffman == HUFFMAN_TABLE_OPTIMAL) {
> >+        ff_mjpeg_build_optimal_huffman(m);
> >+
> >+        // Replace the VLCs with the optimal ones.
> 
> >+        // The default ones may be used for trellis during quantization.
> >+        ff_init_uni_ac_vlc(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
> >+        ff_init_uni_ac_vlc(m->huff_size_ac_chrominance, m->uni_chroma_ac_vlc_len);
> 
> This is wrong and will break trellis with non default tables, trellis
> needs to use the same VLCs as are used for encoding. which if non
> default ones are used should be these non default ones
> 
> the code generating the default tables wrote them in shared
> tables (uni*ac_vlc_len), now after the patch the default code is
> shared, called from 2 places and the tables moved into the context
> yet it still generates only the defau lt tables.

I did misread this part

[...]
diff mbox

Patch

diff --git a/Changelog b/Changelog
index c256121..4d5daca 100644
--- a/Changelog
+++ b/Changelog
@@ -18,6 +18,7 @@  version <next>:
 - readeia608 filter
 - Sample Dump eXchange demuxer
 - abitscope multimedia filter
+- Optimal Huffman tables for (M)JPEG encoding
 
 version 3.2:
 - libopenmpt demuxer
diff --git a/doc/encoders.texi b/doc/encoders.texi
index 8137465..4a23abf 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1200,6 +1200,27 @@  Same as @samp{3}, but with extra processing enabled.
 @end table
 @end table
 
+ at anchor{mjpegenc}
+ at section mjpeg
+
+Motion JPEG encoder.
+
+ at subsection Options
+
+ at table @option
+ at item huffman
+Set the huffman encoding strategy. Possible values:
+
+ at table @samp
+ at item default
+Use the default huffman tables. This is the default strategy.
+
+ at item optimal
+Compute and use optimal huffman tables.
+
+ at end table
+ at end table
+
 @anchor{wavpackenc}
 @section wavpack
 
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 43a6add..4765e9c 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -39,6 +39,7 @@  OBJS = allcodecs.o                                                      \
        mediacodec.o                                                     \
        mpeg12framerate.o                                                \
        options.o                                                        \
+       mjpegenc_huffman.o                                               \
        parser.o                                                         \
        profiles.o                                                       \
        qsv_api.o                                                        \
@@ -175,7 +176,8 @@  OBJS-$(CONFIG_AMRWB_DECODER)           += amrwbdec.o celp_filters.o   \
                                           celp_math.o acelp_filters.o \
                                           acelp_vectors.o             \
                                           acelp_pitch_delay.o
-OBJS-$(CONFIG_AMV_ENCODER)             += mjpegenc.o mjpegenc_common.o
+OBJS-$(CONFIG_AMV_ENCODER)             += mjpegenc.o mjpegenc_common.o \
+                                          mjpegenc_huffman.o
 OBJS-$(CONFIG_ANM_DECODER)             += anm.o
 OBJS-$(CONFIG_ANSI_DECODER)            += ansi.o cga_data.o
 OBJS-$(CONFIG_APE_DECODER)             += apedec.o
@@ -377,7 +379,8 @@  OBJS-$(CONFIG_METASOUND_DECODER)       += metasound.o metasound_data.o \
 OBJS-$(CONFIG_MICRODVD_DECODER)        += microdvddec.o ass.o
 OBJS-$(CONFIG_MIMIC_DECODER)           += mimic.o
 OBJS-$(CONFIG_MJPEG_DECODER)           += mjpegdec.o
-OBJS-$(CONFIG_MJPEG_ENCODER)           += mjpegenc.o mjpegenc_common.o
+OBJS-$(CONFIG_MJPEG_ENCODER)           += mjpegenc.o mjpegenc_common.o \
+                                          mjpegenc_huffman.o
 OBJS-$(CONFIG_MJPEGB_DECODER)          += mjpegbdec.o
 OBJS-$(CONFIG_MJPEG_VAAPI_ENCODER)     += vaapi_encode_mjpeg.o
 OBJS-$(CONFIG_MLP_DECODER)             += mlpdec.o mlpdsp.o
@@ -1014,6 +1017,7 @@  TESTPROGS = avpacket                                                    \
             jpeg2000dwt                                                 \
             mathops                                                    \
             options                                                     \
+            mjpegenc_huffman                                            \
             utils                                                       \
 
 TESTPROGS-$(CONFIG_CABAC)                 += cabac
diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
index 3d11377..a4899c5 100644
--- a/libavcodec/mjpegenc.c
+++ b/libavcodec/mjpegenc.c
@@ -39,39 +39,15 @@ 
 #include "mjpeg.h"
 #include "mjpegenc.h"
 
-static uint8_t uni_ac_vlc_len[64 * 64 * 2];
-static uint8_t uni_chroma_ac_vlc_len[64 * 64 * 2];
-
-static av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t *uni_ac_vlc_len)
-{
-    int i;
-
-    for (i = 0; i < 128; i++) {
-        int level = i - 64;
-        int run;
-        if (!level)
-            continue;
-        for (run = 0; run < 64; run++) {
-            int len, code, nbits;
-            int alevel = FFABS(level);
-
-            len = (run >> 4) * huff_size_ac[0xf0];
-
-            nbits= av_log2_16bit(alevel) + 1;
-            code = ((15&run) << 4) | nbits;
-
-            len += huff_size_ac[code] + nbits;
-
-            uni_ac_vlc_len[UNI_AC_ENC_INDEX(run, i)] = len;
-            // We ignore EOB as its just a constant which does not change generally
-        }
-    }
-}
+// Don't know, but let's guess 16 bits per code
+#define MJPEG_HUFFMAN_EST_BITS_PER_CODE 16
 
 av_cold int ff_mjpeg_encode_init(MpegEncContext *s)
 {
     MJpegContext *m;
 
+    av_assert0(s->slice_context_count == 1);
+
     if (s->width > 65500 || s->height > 65500) {
         av_log(s, AV_LOG_ERROR, "JPEG does not support resolutions above 65500x65500\n");
         return AVERROR(EINVAL);
@@ -84,7 +60,9 @@  av_cold int ff_mjpeg_encode_init(MpegEncContext *s)
     s->min_qcoeff=-1023;
     s->max_qcoeff= 1023;
 
-    /* build all the huffman tables */
+    // Build default Huffman tables.
+    // These may be overwritten later with more optimal Huffman tables, but
+    // they are needed at least right now for some processes like trellis.
     ff_mjpeg_build_huffman_codes(m->huff_size_dc_luminance,
                                  m->huff_code_dc_luminance,
                                  avpriv_mjpeg_bits_dc_luminance,
@@ -102,12 +80,18 @@  av_cold int ff_mjpeg_encode_init(MpegEncContext *s)
                                  avpriv_mjpeg_bits_ac_chrominance,
                                  avpriv_mjpeg_val_ac_chrominance);
 
-    init_uni_ac_vlc(m->huff_size_ac_luminance,   uni_ac_vlc_len);
-    init_uni_ac_vlc(m->huff_size_ac_chrominance, uni_chroma_ac_vlc_len);
+    init_uni_ac_vlc(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
+    init_uni_ac_vlc(m->huff_size_ac_chrominance, m->uni_chroma_ac_vlc_len);
     s->intra_ac_vlc_length      =
-    s->intra_ac_vlc_last_length = uni_ac_vlc_len;
+    s->intra_ac_vlc_last_length = m->uni_ac_vlc_len;
     s->intra_chroma_ac_vlc_length      =
-    s->intra_chroma_ac_vlc_last_length = uni_chroma_ac_vlc_len;
+    s->intra_chroma_ac_vlc_last_length = m->uni_chroma_ac_vlc_len;
+
+    // Buffers start out empty.
+    m->huff_buffer = NULL;
+    m->huff_ncode = 0;
+    m->huff_capacity = 0;
+    m->error = 0;
 
     s->mjpeg_ctx = m;
     return 0;
@@ -115,71 +99,193 @@  av_cold int ff_mjpeg_encode_init(MpegEncContext *s)
 
 av_cold void ff_mjpeg_encode_close(MpegEncContext *s)
 {
+    av_freep(&s->mjpeg_ctx->huff_buffer);
     av_freep(&s->mjpeg_ctx);
 }
 
+/**
+ * Encodes and outputs the entire frame in the JPEG format.
+ *
+ * @param s The MpegEncContext.
+ */
+void ff_mjpeg_encode_picture_frame(MpegEncContext *s)
+{
+    int i, nbits, code, table_id;
+    MJpegContext *m = s->mjpeg_ctx;
+    uint8_t *huff_size[4] = {m->huff_size_dc_luminance,
+                             m->huff_size_dc_chrominance,
+                             m->huff_size_ac_luminance,
+                             m->huff_size_ac_chrominance};
+    uint16_t *huff_code[4] = {m->huff_code_dc_luminance,
+                              m->huff_code_dc_chrominance,
+                              m->huff_code_ac_luminance,
+                              m->huff_code_ac_chrominance};
+    size_t total_bits = 0;
+    size_t bytes_needed;
+
+    // Estimate the total size first
+    for (i = 0; i < m->huff_ncode; i++) {
+        table_id = m->huff_buffer[i].table_id;
+        code = m->huff_buffer[i].code;
+        nbits = code & 0xf;
+
+        total_bits += huff_size[table_id][code] + nbits;
+    }
+
+    bytes_needed = (total_bits + 7) / 8;
+    ff_mpv_reallocate_putbitbuffer(s, bytes_needed, bytes_needed);
+
+    for (i = 0; i < m->huff_ncode; i++) {
+        table_id = m->huff_buffer[i].table_id;
+        code = m->huff_buffer[i].code;
+        nbits = code & 0xf;
+
+        put_bits(&s->pb, huff_size[table_id][code], huff_code[table_id][code]);
+        if (nbits != 0) {
+            put_sbits(&s->pb, nbits, m->huff_buffer[i].mant);
+        }
+    }
+
+    m->huff_ncode = 0;
+}
+
+/**
+ * Add code and table_id to the JPEG buffer.
+ *
+ * @param s The MJpegContext which contains the JPEG buffer.
+ * @param table_id Which Huffman table the code belongs to.
+ * @param code The encoded exponent of the coefficients and the run-bits.
+ */
+static inline void ff_mjpeg_encode_code(MJpegContext *s, uint8_t table_id, int code)
+{
+    MJpegHuffmanCode *c = &s->huff_buffer[s->huff_ncode++];
+    av_assert0(s->huff_ncode < s->huff_capacity);
+    c->table_id = table_id;
+    c->code = code;
+}
+
+/**
+ * Add the coefficient's data to the JPEG buffer.
+ *
+ * @param s The MJpegContext which contains the JPEG buffer.
+ * @param table_id Which Huffman table the code belongs to.
+ * @param val The coefficient.
+ * @param run The run-bits.
+ */
+static void ff_mjpeg_encode_coef(MJpegContext *s, uint8_t table_id, int val, int run)
+{
+    int mant, code;
+
+    if (val == 0) {
+        av_assert0(run == 0);
+        ff_mjpeg_encode_code(s, table_id, 0);
+    } else {
+        mant = val;
+        if (val < 0) {
+            val = -val;
+            mant--;
+        }
+
+        code = (run << 4) | (av_log2_16bit(val) + 1);
+
+        s->huff_buffer[s->huff_ncode].mant = mant;
+        ff_mjpeg_encode_code(s, table_id, code);
+    }
+}
+
+/**
+ * Add the block's data into the JPEG buffer.
+ *
+ * @param s The MJpegEncContext that contains the JPEG buffer.
+ * @param block The block.
+ * @param n The block's index or number.
+ */
 static void encode_block(MpegEncContext *s, int16_t *block, int n)
 {
-    int mant, nbits, code, i, j;
-    int component, dc, run, last_index, val;
+    int i, j, table_id;
+    int component, dc, last_index, val, run;
     MJpegContext *m = s->mjpeg_ctx;
-    uint8_t *huff_size_ac;
-    uint16_t *huff_code_ac;
+
+    if (m->error) return;
+
+    av_assert0(m->huff_capacity >= m->huff_ncode + 64);
 
     /* DC coef */
     component = (n <= 3 ? 0 : (n&1) + 1);
+    table_id = (n <= 3 ? 0 : 1);
     dc = block[0]; /* overflow is impossible */
     val = dc - s->last_dc[component];
-    if (n < 4) {
-        ff_mjpeg_encode_dc(&s->pb, val, m->huff_size_dc_luminance, m->huff_code_dc_luminance);
-        huff_size_ac = m->huff_size_ac_luminance;
-        huff_code_ac = m->huff_code_ac_luminance;
-    } else {
-        ff_mjpeg_encode_dc(&s->pb, val, m->huff_size_dc_chrominance, m->huff_code_dc_chrominance);
-        huff_size_ac = m->huff_size_ac_chrominance;
-        huff_code_ac = m->huff_code_ac_chrominance;
-    }
+
+    ff_mjpeg_encode_coef(m, table_id, val, 0);
+
     s->last_dc[component] = dc;
 
     /* AC coefs */
 
     run = 0;
     last_index = s->block_last_index[n];
+    table_id |= 2;
+
     for(i=1;i<=last_index;i++) {
         j = s->intra_scantable.permutated[i];
         val = block[j];
+
         if (val == 0) {
             run++;
         } else {
             while (run >= 16) {
-                put_bits(&s->pb, huff_size_ac[0xf0], huff_code_ac[0xf0]);
+                ff_mjpeg_encode_code(m, table_id, 0xf0);
                 run -= 16;
             }
-            mant = val;
-            if (val < 0) {
-                val = -val;
-                mant--;
-            }
-
-            nbits= av_log2_16bit(val) + 1;
-            code = (run << 4) | nbits;
-
-            put_bits(&s->pb, huff_size_ac[code], huff_code_ac[code]);
-
-            put_sbits(&s->pb, nbits, mant);
+            ff_mjpeg_encode_coef(m, table_id, val, run);
             run = 0;
         }
     }
 
     /* output EOB only if not already 64 values */
     if (last_index < 63 || run != 0)
-        put_bits(&s->pb, huff_size_ac[0], huff_code_ac[0]);
+        ff_mjpeg_encode_code(m, table_id, 0);
 }
 
-void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
+// Possibly reallocate the huffman code buffer, assuming blocks_per_mb.
+// Set s->mjpeg_ctx->error on ENOMEM.
+static void realloc_huffman(MpegEncContext *s, int blocks_per_mb)
 {
-    int i;
+    MJpegContext *m = s->mjpeg_ctx;
+    size_t num_mbs, num_blocks, num_codes;
+    MJpegHuffmanCode *new_buf;
+    if (m->error) return;
+    // Make sure we have enough space to hold this frame.
+    num_mbs = s->mb_width * s->mb_height;
+    num_blocks = num_mbs * blocks_per_mb;
+    av_assert0(m->huff_ncode <=
+               (s->mb_y * s->mb_width + s->mb_x) * blocks_per_mb * 64);
+    num_codes = num_blocks * 64;
+
+    new_buf = av_fast_realloc(m->huff_buffer, &m->huff_capacity,
+                              num_codes * sizeof(MJpegHuffmanCode));
+    if (!new_buf) {
+        m->error = AVERROR(ENOMEM);
+    } else {
+        m->huff_buffer = new_buf;
+    }
+}
+
+int ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
+{
+    int i, is_chroma_420;
+
+    // Number of bits used depends on future data.
+    // So, nothing that relies on encoding many times and taking the
+    // one with the fewest bits will work properly here.
+    if (s->i_tex_bits != MJPEG_HUFFMAN_EST_BITS_PER_CODE *
+        s->mjpeg_ctx->huff_ncode) {
+        av_log(NULL, AV_LOG_ERROR, "Unsupported encoding method\n");
+        return AVERROR(EINVAL);
+    }
+
     if (s->chroma_format == CHROMA_444) {
+        realloc_huffman(s, 12);
         encode_block(s, block[0], 0);
         encode_block(s, block[2], 2);
         encode_block(s, block[4], 4);
@@ -196,10 +302,12 @@  void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
             encode_block(s, block[11], 11);
         }
     } else {
+        is_chroma_420 = (s->chroma_format == CHROMA_420);
+        realloc_huffman(s, 5 + (is_chroma_420 ? 1 : 3));
         for(i=0;i<5;i++) {
             encode_block(s, block[i], i);
         }
-        if (s->chroma_format == CHROMA_420) {
+        if (is_chroma_420) {
             encode_block(s, block[5], 5);
         } else {
             encode_block(s, block[6], 6);
@@ -207,8 +315,11 @@  void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64])
             encode_block(s, block[7], 7);
         }
     }
+    if (s->mjpeg_ctx->error)
+        return s->mjpeg_ctx->error;
 
-    s->i_tex_bits += get_bits_diff(s);
+    s->i_tex_bits = MJPEG_HUFFMAN_EST_BITS_PER_CODE * s->mjpeg_ctx->huff_ncode;
+    return 0;
 }
 
 // maximum over s->mjpeg_vsample[i]
@@ -261,7 +372,9 @@  FF_MPV_COMMON_OPTS
     { "left",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 1 }, INT_MIN, INT_MAX, VE, "pred" },
     { "plane",  NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 2 }, INT_MIN, INT_MAX, VE, "pred" },
     { "median", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = 3 }, INT_MIN, INT_MAX, VE, "pred" },
-
+{ "huffman", "Huffman table strategy", OFFSET(huffman), AV_OPT_TYPE_INT, { .i64 = HUFFMAN_TABLE_DEFAULT }, 0, NB_HUFFMAN_TABLE_OPTION - 1, VE, "huffman" },
+    { "default", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = HUFFMAN_TABLE_DEFAULT }, INT_MIN, INT_MAX, VE, "huffman" },
+    { "optimal", NULL, 0, AV_OPT_TYPE_CONST, { .i64 = HUFFMAN_TABLE_OPTIMAL }, INT_MIN, INT_MAX, VE, "huffman" },
 { NULL},
 };
 
diff --git a/libavcodec/mjpegenc.h b/libavcodec/mjpegenc.h
index 60cd566..577fa1f 100644
--- a/libavcodec/mjpegenc.h
+++ b/libavcodec/mjpegenc.h
@@ -39,18 +39,67 @@ 
 #include "mpegvideo.h"
 #include "put_bits.h"
 
+/**
+ * Buffer of JPEG frame data.
+ *
+ * Optimal Huffman table generation requires the frame data to be loaded into
+ * a buffer so that the tables can be computed.
+ * There are at most mb_width*mb_height*12*64 of these per frame.
+ */
+typedef struct MJpegHuffmanCode {
+    // 0=DC lum, 1=DC chrom, 2=AC lum, 3=AC chrom
+    uint8_t table_id; ///< The Huffman table id associated with the data.
+    uint8_t code;     ///< The exponent.
+    uint16_t mant;    ///< The mantissa.
+} MJpegHuffmanCode;
+
+/**
+ * Holds JPEG frame data and Huffman table data.
+ */
 typedef struct MJpegContext {
-    uint8_t huff_size_dc_luminance[12]; //FIXME use array [3] instead of lumi / chroma, for easier addressing
-    uint16_t huff_code_dc_luminance[12];
-    uint8_t huff_size_dc_chrominance[12];
-    uint16_t huff_code_dc_chrominance[12];
+    //FIXME use array [3] instead of lumi / chroma, for easier addressing
+    uint8_t huff_size_dc_luminance[12];     ///< DC luminance Huffman table size.
+    uint16_t huff_code_dc_luminance[12];    ///< DC luminance Huffman table codes.
+    uint8_t huff_size_dc_chrominance[12];   ///< DC chrominance Huffman table size.
+    uint16_t huff_code_dc_chrominance[12];  ///< DC chrominance Huffman table codes.
+
+    uint8_t huff_size_ac_luminance[256];    ///< AC luminance Huffman table size.
+    uint16_t huff_code_ac_luminance[256];   ///< AC luminance Huffman table codes.
+    uint8_t huff_size_ac_chrominance[256];  ///< AC chrominance Huffman table size.
+    uint16_t huff_code_ac_chrominance[256]; ///< AC chrominance Huffman table codes.
 
-    uint8_t huff_size_ac_luminance[256];
-    uint16_t huff_code_ac_luminance[256];
-    uint8_t huff_size_ac_chrominance[256];
-    uint16_t huff_code_ac_chrominance[256];
+    /** Storage for AC luminance VLC (in MpegEncContext) */
+    uint8_t uni_ac_vlc_len[64 * 64 * 2];
+    /** Storage for AC chrominance VLC (in MpegEncContext) */
+    uint8_t uni_chroma_ac_vlc_len[64 * 64 * 2];
+
+    // Default DC tables have exactly 12 values
+    uint8_t bits_dc_luminance[17];   ///< DC luminance Huffman bits.
+    uint8_t val_dc_luminance[12];    ///< DC luminance Huffman values.
+    uint8_t bits_dc_chrominance[17]; ///< DC chrominance Huffman bits.
+    uint8_t val_dc_chrominance[12];  ///< DC chrominance Huffman values.
+
+    // 8-bit JPEG has max 256 values
+    uint8_t bits_ac_luminance[17];   ///< AC luminance Huffman bits.
+    uint8_t val_ac_luminance[256];   ///< AC luminance Huffman values.
+    uint8_t bits_ac_chrominance[17]; ///< AC chrominance Huffman bits.
+    uint8_t val_ac_chrominance[256]; ///< AC chrominance Huffman values.
+
+    unsigned int huff_capacity;      ///< Size of the buffer, in entries.
+    size_t huff_ncode;               ///< Number of current entries in the buffer.
+    MJpegHuffmanCode *huff_buffer;   ///< Buffer for Huffman code values.
+    int error;                       ///< Error code.
 } MJpegContext;
 
+/**
+ * Enum for the Huffman encoding strategy.
+ */
+enum HuffmanTableOption {
+    HUFFMAN_TABLE_DEFAULT = 0, ///< Use the default Huffman tables.
+    HUFFMAN_TABLE_OPTIMAL = 1, ///< Compute and use optimal Huffman tables.
+    NB_HUFFMAN_TABLE_OPTION = 2
+};
+
 static inline void put_marker(PutBitContext *p, enum JpegMarker code)
 {
     put_bits(p, 8, 0xff);
@@ -58,7 +107,8 @@  static inline void put_marker(PutBitContext *p, enum JpegMarker code)
 }
 
 int  ff_mjpeg_encode_init(MpegEncContext *s);
+void ff_mjpeg_encode_picture_frame(MpegEncContext *s);
 void ff_mjpeg_encode_close(MpegEncContext *s);
-void ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64]);
+int ff_mjpeg_encode_mb(MpegEncContext *s, int16_t block[12][64]);
 
 #endif /* AVCODEC_MJPEGENC_H */
diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
index 7a6fe746..8ec3df9 100644
--- a/libavcodec/mjpegenc_common.c
+++ b/libavcodec/mjpegenc_common.c
@@ -33,8 +33,35 @@ 
 #include "put_bits.h"
 #include "mjpegenc.h"
 #include "mjpegenc_common.h"
+#include "mjpegenc_huffman.h"
 #include "mjpeg.h"
 
+av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t *uni_ac_vlc_len)
+{
+    int i;
+
+    for (i = 0; i < 128; i++) {
+        int level = i - 64;
+        int run;
+        if (!level)
+            continue;
+        for (run = 0; run < 64; run++) {
+            int len, code, nbits;
+            int alevel = FFABS(level);
+
+            len = (run >> 4) * huff_size_ac[0xf0];
+
+            nbits= av_log2_16bit(alevel) + 1;
+            code = ((15&run) << 4) | nbits;
+
+            len += huff_size_ac[code] + nbits;
+
+            uni_ac_vlc_len[UNI_AC_ENC_INDEX(run, i)] = len;
+            // We ignore EOB as its just a constant which does not change generally
+        }
+    }
+}
+
 /* table_class: 0 = DC coef, 1 = AC coefs */
 static int put_huffman_table(PutBitContext *p, int table_class, int table_id,
                              const uint8_t *bits_table, const uint8_t *value_table)
@@ -104,15 +131,30 @@  static void jpeg_table_header(AVCodecContext *avctx, PutBitContext *p,
     ptr = put_bits_ptr(p);
     put_bits(p, 16, 0); /* patched later */
     size = 2;
-    size += put_huffman_table(p, 0, 0, avpriv_mjpeg_bits_dc_luminance,
-                              avpriv_mjpeg_val_dc);
-    size += put_huffman_table(p, 0, 1, avpriv_mjpeg_bits_dc_chrominance,
-                              avpriv_mjpeg_val_dc);
-
-    size += put_huffman_table(p, 1, 0, avpriv_mjpeg_bits_ac_luminance,
-                              avpriv_mjpeg_val_ac_luminance);
-    size += put_huffman_table(p, 1, 1, avpriv_mjpeg_bits_ac_chrominance,
-                              avpriv_mjpeg_val_ac_chrominance);
+
+    // Only MJPEG can have a variable Huffman variable. All other
+    // formats use the default Huffman table.
+    if (s->out_format == FMT_MJPEG && s->huffman == HUFFMAN_TABLE_OPTIMAL) {
+        size += put_huffman_table(p, 0, 0, s->mjpeg_ctx->bits_dc_luminance,
+                                  s->mjpeg_ctx->val_dc_luminance);
+        size += put_huffman_table(p, 0, 1, s->mjpeg_ctx->bits_dc_chrominance,
+                                  s->mjpeg_ctx->val_dc_chrominance);
+
+        size += put_huffman_table(p, 1, 0, s->mjpeg_ctx->bits_ac_luminance,
+                                  s->mjpeg_ctx->val_ac_luminance);
+        size += put_huffman_table(p, 1, 1, s->mjpeg_ctx->bits_ac_chrominance,
+                                  s->mjpeg_ctx->val_ac_chrominance);
+    } else {
+        size += put_huffman_table(p, 0, 0, avpriv_mjpeg_bits_dc_luminance,
+                                  avpriv_mjpeg_val_dc);
+        size += put_huffman_table(p, 0, 1, avpriv_mjpeg_bits_dc_chrominance,
+                                  avpriv_mjpeg_val_dc);
+
+        size += put_huffman_table(p, 1, 0, avpriv_mjpeg_bits_ac_luminance,
+                                  avpriv_mjpeg_val_ac_luminance);
+        size += put_huffman_table(p, 1, 1, avpriv_mjpeg_bits_ac_chrominance,
+                                  avpriv_mjpeg_val_ac_chrominance);
+    }
     AV_WB16(ptr, size);
 }
 
@@ -372,14 +414,116 @@  void ff_mjpeg_escape_FF(PutBitContext *pb, int start)
     }
 }
 
+/**
+ * Builds all 4 optimal Huffman tables.
+ *
+ * Uses the data stored in the JPEG buffer to compute the tables.
+ * Stores the Huffman tables in the bits_* and val_* arrays in the MJpegContext.
+ *
+ * @param m MJpegContext containing the JPEG buffer.
+ */
+static void ff_mjpeg_build_optimal_huffman(MJpegContext *m)
+{
+    int i, ret, table_id, code;
+
+    MJpegEncHuffmanContext dc_luminance_ctx;
+    MJpegEncHuffmanContext dc_chrominance_ctx;
+    MJpegEncHuffmanContext ac_luminance_ctx;
+    MJpegEncHuffmanContext ac_chrominance_ctx;
+    MJpegEncHuffmanContext *ctx[4] = {&dc_luminance_ctx,
+                                      &dc_chrominance_ctx,
+                                      &ac_luminance_ctx,
+                                      &ac_chrominance_ctx};
+    for (i = 0; i < 4; i++) {
+        ff_mjpeg_encode_huffman_init(ctx[i]);
+    }
+    for (i = 0; i < m->huff_ncode; i++) {
+        table_id = m->huff_buffer[i].table_id;
+        code = m->huff_buffer[i].code;
+
+        ff_mjpeg_encode_huffman_increment(ctx[table_id], code);
+    }
+
+    ret = ff_mjpeg_encode_huffman_close(&dc_luminance_ctx,
+                                        m->bits_dc_luminance,
+                                        m->val_dc_luminance, 12);
+    av_assert0(!ret);
+    ret = ff_mjpeg_encode_huffman_close(&dc_chrominance_ctx,
+                                        m->bits_dc_chrominance,
+                                        m->val_dc_chrominance, 12);
+    av_assert0(!ret);
+    ret = ff_mjpeg_encode_huffman_close(&ac_luminance_ctx,
+                                        m->bits_ac_luminance,
+                                        m->val_ac_luminance, 256);
+    av_assert0(!ret);
+    ret = ff_mjpeg_encode_huffman_close(&ac_chrominance_ctx,
+                                        m->bits_ac_chrominance,
+                                        m->val_ac_chrominance, 256);
+    av_assert0(!ret);
+
+    ff_mjpeg_build_huffman_codes(m->huff_size_dc_luminance,
+                                 m->huff_code_dc_luminance,
+                                 m->bits_dc_luminance,
+                                 m->val_dc_luminance);
+    ff_mjpeg_build_huffman_codes(m->huff_size_dc_chrominance,
+                                 m->huff_code_dc_chrominance,
+                                 m->bits_dc_chrominance,
+                                 m->val_dc_chrominance);
+    ff_mjpeg_build_huffman_codes(m->huff_size_ac_luminance,
+                                 m->huff_code_ac_luminance,
+                                 m->bits_ac_luminance,
+                                 m->val_ac_luminance);
+    ff_mjpeg_build_huffman_codes(m->huff_size_ac_chrominance,
+                                 m->huff_code_ac_chrominance,
+                                 m->bits_ac_chrominance,
+                                 m->val_ac_chrominance);
+}
+
+/**
+ * Writes the complete JPEG frame.
+ *
+ * Header + values + stuffing.
+ *
+ * @param s The MpegEncContext.
+ * @return int Error code, 0 if successful.
+ */
 int ff_mjpeg_encode_stuffing(MpegEncContext *s)
 {
     int i;
     PutBitContext *pbc = &s->pb;
     int mb_y = s->mb_y - !s->mb_x;
+    int ret;
+    MJpegContext *m;
+
+    m = s->mjpeg_ctx;
+
+    if (m->error)
+        return m->error;
+
+    if (s->huffman == HUFFMAN_TABLE_OPTIMAL) {
+        ff_mjpeg_build_optimal_huffman(m);
+
+        // Replace the VLCs with the optimal ones.
+        // The default ones may be used for trellis during quantization.
+        init_uni_ac_vlc(m->huff_size_ac_luminance,   m->uni_ac_vlc_len);
+        init_uni_ac_vlc(m->huff_size_ac_chrominance, m->uni_chroma_ac_vlc_len);
+        s->intra_ac_vlc_length      =
+        s->intra_ac_vlc_last_length = m->uni_ac_vlc_len;
+        s->intra_chroma_ac_vlc_length      =
+        s->intra_chroma_ac_vlc_last_length = m->uni_chroma_ac_vlc_len;
+    }
+
+    ff_mjpeg_encode_picture_header(s->avctx, &s->pb, &s->intra_scantable,
+                                   s->pred, s->intra_matrix, s->chroma_intra_matrix);
+    ff_mjpeg_encode_picture_frame(s);
+    if (m->error < 0) {
+        ret = m->error;
+        return ret;
+    }
+
+    ret = ff_mpv_reallocate_putbitbuffer(s, put_bits_count(&s->pb) / 8 + 100,
+                                            put_bits_count(&s->pb) / 4 + 1000);
 
-    int ret = ff_mpv_reallocate_putbitbuffer(s, put_bits_count(&s->pb) / 8 + 100,
-                                                put_bits_count(&s->pb) / 4 + 1000);
     if (ret < 0) {
         av_log(s->avctx, AV_LOG_ERROR, "Buffer reallocation failed\n");
         goto fail;
diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
index 6e51ca0..7d760f8 100644
--- a/libavcodec/mjpegenc_common.h
+++ b/libavcodec/mjpegenc_common.h
@@ -40,4 +40,6 @@  void ff_mjpeg_init_hvsample(AVCodecContext *avctx, int hsample[4], int vsample[4
 void ff_mjpeg_encode_dc(PutBitContext *pb, int val,
                         uint8_t *huff_size, uint16_t *huff_code);
 
+av_cold void init_uni_ac_vlc(const uint8_t huff_size_ac[256], uint8_t *uni_ac_vlc_len);
+
 #endif /* AVCODEC_MJPEGENC_COMMON_H */
diff --git a/libavcodec/mjpegenc_huffman.c b/libavcodec/mjpegenc_huffman.c
new file mode 100644
index 0000000..a5d7d53
--- /dev/null
+++ b/libavcodec/mjpegenc_huffman.c
@@ -0,0 +1,195 @@ 
+/*
+ * MJPEG encoder
+ * Copyright (c) 2016 William Ma, Ted Ying, Jerry Jiang
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <string.h>
+#include <stdint.h>
+#include <assert.h>
+#include <stdlib.h>
+#include "libavutil/common.h"
+#include "libavutil/error.h"
+#include "libavutil/qsort.h"
+#include "mjpegenc_huffman.h"
+
+/**
+ * Comparison function for two PTables by prob
+ *
+ * @param a First PTable to compare
+ * @param b Second PTable to compare
+ * @return < 0 for less than, 0 for equals, > 0 for greater than
+ */
+static int compare_by_prob(const void *a, const void *b)
+{
+    PTable a_val = *(PTable *) a;
+    PTable b_val = *(PTable *) b;
+    return a_val.prob - b_val.prob;
+}
+
+/**
+ * Comparison function for two HuffTables by length
+ *
+ * @param a First HuffTable to compare
+ * @param b Second HuffTable to compare
+ * @return < 0 for less than, 0 for equals, > 0 for greater than
+ */
+static int compare_by_length(const void *a, const void *b)
+{
+    HuffTable a_val = *(HuffTable *) a;
+    HuffTable b_val = *(HuffTable *) b;
+    return a_val.length - b_val.length;
+}
+
+/**
+ * Computes the length of the Huffman encoding for each distinct input value.
+ * Uses package merge algorithm as follows:
+ * 1. start with an empty list, lets call it list(0), set i = 0
+ * 2. add 1 entry to list(i) for each symbol we have and give each a score equal to the probability of the respective symbol
+ * 3. merge the 2 symbols of least score and put them in list(i+1), and remove them from list(i). The new score will be the sum of the 2 scores
+ * 4. if there is more than 1 symbol left in the current list(i), then goto 3
+ * 5. i++
+ * 6. if i < 16 goto 2
+ * 7. select the n-1 elements in the last list with the lowest score (n = the number of symbols)
+ * 8. the length of the huffman code for symbol s will be equal to the number of times the symbol occurs in the select elements
+ * Go to guru.multimedia.cx/small-tasks-for-ffmpeg/ for more details
+ *
+ * All probabilities should be positive integers. The output is sorted by code,
+ * not by length.
+ *
+ * @param prob_table input array of a PTable for each distinct input value
+ * @param distincts  output array of a HuffTable that will be populated by this function
+ * @param size       size of the prob_table array
+ * @param max_length max length of an encoding
+ */
+void ff_mjpegenc_huffman_compute_bits(PTable *prob_table, HuffTable *distincts, int size, int max_length)
+{
+    PackageMergerList list_a, list_b, *to = &list_a, *from = &list_b, *temp;
+
+    int times, i, j, k;
+
+    int nbits[257] = {0};
+
+    int min;
+
+    to->nitems = 0;
+    from->nitems = 0;
+    to->item_idx[0] = 0;
+    from->item_idx[0] = 0;
+    AV_QSORT(prob_table, size, PTable, compare_by_prob);
+
+    for (times = 0; times <= max_length; times++) {
+        to->nitems = 0;
+        to->item_idx[0] = 0;
+
+        j = 0;
+        k = 0;
+
+        if (times < max_length) {
+            i = 0;
+        }
+        while (i < size || j + 1 < from->nitems) {
+            to->nitems++;
+            to->item_idx[to->nitems] = to->item_idx[to->nitems - 1];
+            if (i < size &&
+                (j + 1 >= from->nitems ||
+                 prob_table[i].prob <
+                     from->probability[j] + from->probability[j + 1])) {
+                to->items[to->item_idx[to->nitems]++] = prob_table[i].value;
+                to->probability[to->nitems - 1] = prob_table[i].prob;
+                i++;
+            } else {
+                for (k = from->item_idx[j]; k < from->item_idx[j + 2]; k++) {
+                    to->items[to->item_idx[to->nitems]++] = from->items[k];
+                }
+                to->probability[to->nitems - 1] =
+                    from->probability[j] + from->probability[j + 1];
+                j += 2;
+            }
+        }
+        temp = to;
+        to = from;
+        from = temp;
+    }
+
+    min = (size - 1 < from->nitems) ? size - 1 : from->nitems;
+    for (i = 0; i < from->item_idx[min]; i++) {
+        nbits[from->items[i]]++;
+    }
+    // we don't want to return the 256 bit count (it was just in here to prevent
+    // all 1s encoding)
+    j = 0;
+    for (i = 0; i < 256; i++) {
+        if (nbits[i] > 0) {
+            distincts[j].code = i;
+            distincts[j].length = nbits[i];
+            j++;
+        }
+    }
+}
+
+void ff_mjpeg_encode_huffman_init(MJpegEncHuffmanContext *s)
+{
+    memset(s->val_count, 0, sizeof(s->val_count));
+}
+
+/**
+ * Produces a Huffman encoding with a given input
+ *
+ * @param s         input to encode
+ * @param bits      output array where the ith character represents how many input values have i length encoding
+ * @param val       output array of input values sorted by their encoded length
+ * @param max_nval  maximum number of distinct input values
+ * @return int      Return code, 0 if succeeded.
+ */
+int ff_mjpeg_encode_huffman_close(MJpegEncHuffmanContext *s, uint8_t bits[17],
+                                  uint8_t val[], int max_nval)
+{
+    int i, j;
+    int nval = 0;
+    PTable val_counts[257];
+    HuffTable distincts[256];
+
+    for (i = 0; i < 256; i++) {
+        if (s->val_count[i]) nval++;
+    }
+    if (nval > max_nval) {
+        return AVERROR(EINVAL);
+    }
+
+    j = 0;
+    for (i = 0; i < 256; i++) {
+        if (s->val_count[i]) {
+            val_counts[j].value = i;
+            val_counts[j].prob = s->val_count[i];
+            j++;
+        }
+    }
+    val_counts[j].value = 256;
+    val_counts[j].prob = 0;
+    ff_mjpegenc_huffman_compute_bits(val_counts, distincts, nval + 1, 16);
+    AV_QSORT(distincts, nval, HuffTable, compare_by_length);
+
+    memset(bits, 0, sizeof(bits[0]) * 17);
+    for (i = 0; i < nval; i++) {
+        val[i] = distincts[i].code;
+        bits[distincts[i].length]++;
+    }
+
+    return 0;
+}
diff --git a/libavcodec/mjpegenc_huffman.h b/libavcodec/mjpegenc_huffman.h
new file mode 100644
index 0000000..ec6251b
--- /dev/null
+++ b/libavcodec/mjpegenc_huffman.h
@@ -0,0 +1,74 @@ 
+/*
+ * MJPEG encoder
+ * Copyright (c) 2016 William Ma, Ted Ying, Jerry Jiang
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Huffman table generation for MJPEG encoder.
+ */
+
+#ifndef AVCODEC_MJPEGENC_HUFFMAN_H
+#define AVCODEC_MJPEGENC_HUFFMAN_H
+
+typedef struct MJpegEncHuffmanContext {
+    int val_count[256];
+} MJpegEncHuffmanContext;
+
+// Uses the package merge algorithm to compute the Huffman table.
+void ff_mjpeg_encode_huffman_init(MJpegEncHuffmanContext *s);
+static inline void ff_mjpeg_encode_huffman_increment(MJpegEncHuffmanContext *s,
+                                                     uint8_t val)
+{
+    s->val_count[val]++;
+}
+int ff_mjpeg_encode_huffman_close(MJpegEncHuffmanContext *s,
+                                  uint8_t bits[17], uint8_t val[],
+                                  int max_nval);
+
+
+/**
+ * Used to assign a occurrence count or "probability" to an input value
+ */
+typedef struct PTable {
+    int value;  ///< input value
+    int prob;   ///< number of occurences of this value in input
+} PTable;
+
+/**
+ * Used to store intermediate lists in the package merge algorithm
+ */
+typedef struct PackageMergerList {
+    int nitems;             ///< number of items in the list and probability      ex. 4
+    int item_idx[515];      ///< index range for each item in items                   0, 2, 5, 9, 13
+    int probability[514];   ///< probability of each item                             3, 8, 18, 46
+    int items[257 * 16];    ///< chain of all individual values that make up items    A, B, A, B, C, A, B, C, D, C, D, D, E
+} PackageMergerList;
+
+/**
+ * Used to store optimal huffman encoding results
+ */
+typedef struct HuffTable {
+    int code;       ///< code is the input value
+    int length;     ///< length of the encoding
+} HuffTable;
+
+void ff_mjpegenc_huffman_compute_bits(PTable *prob_table, HuffTable *distincts,
+                                      int size, int max_length);
+#endif /* AVCODEC_MJPEGENC_HUFFMAN_H */
diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
index c82fa3e..85df191 100644
--- a/libavcodec/mpegvideo.h
+++ b/libavcodec/mpegvideo.h
@@ -422,6 +422,7 @@  typedef struct MpegEncContext {
     struct MJpegContext *mjpeg_ctx;
     int esc_pos;
     int pred;
+    int huffman;
 
     /* MSMPEG4 specific */
     int mv_table_index;
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 10b4c5b..920613f 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -265,7 +265,8 @@  static void mpv_encode_defaults(MpegEncContext *s)
     s->picture_in_gop_number = 0;
 }
 
-av_cold int ff_dct_encode_init(MpegEncContext *s) {
+av_cold int ff_dct_encode_init(MpegEncContext *s)
+{
     if (ARCH_X86)
         ff_dct_encode_init_x86(s);
 
@@ -641,6 +642,15 @@  FF_ENABLE_DEPRECATION_WARNINGS
         return -1;
     }
 
+    if ((s->mpv_flags & FF_MPV_FLAG_QP_RD) &&
+            (s->codec_id == AV_CODEC_ID_AMV ||
+             s->codec_id == AV_CODEC_ID_MJPEG)) {
+        // Used to produce garbage with MJPEG.
+        av_log(avctx, AV_LOG_ERROR,
+               "QP RD is no longer compatible with MJPEG or AMV\n");
+        return -1;
+    }
+
 #if FF_API_PRIVATE_OPT
 FF_DISABLE_DEPRECATION_WARNINGS
     if (avctx->scenechange_threshold)
@@ -3897,9 +3907,8 @@  static int encode_picture(MpegEncContext *s, int picture_number)
     s->last_bits= put_bits_count(&s->pb);
     switch(s->out_format) {
     case FMT_MJPEG:
-        if (CONFIG_MJPEG_ENCODER)
-            ff_mjpeg_encode_picture_header(s->avctx, &s->pb, &s->intra_scantable,
-                                           s->pred, s->intra_matrix, s->chroma_intra_matrix);
+        /* The MJPEG headers are printed after the initial encoding so that the
+         * optimal huffman encoding can be found. */
         break;
     case FMT_H261:
         if (CONFIG_H261_ENCODER)
diff --git a/libavcodec/tests/.gitignore b/libavcodec/tests/.gitignore
index 0ab0296..f22ac82 100644
--- a/libavcodec/tests/.gitignore
+++ b/libavcodec/tests/.gitignore
@@ -10,6 +10,7 @@ 
 /imgconvert
 /jpeg2000dwt
 /mathops
+/mjpegenc_huffman
 /motion
 /options
 /rangecoder
diff --git a/libavcodec/tests/mjpegenc_huffman.c b/libavcodec/tests/mjpegenc_huffman.c
new file mode 100644
index 0000000..4559cbe
--- /dev/null
+++ b/libavcodec/tests/mjpegenc_huffman.c
@@ -0,0 +1,163 @@ 
+/*
+ * Copyright (c) 2016 William Ma, Sofia Kim, Dustin Woo
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Optimal Huffman Encoding tests.
+ */
+
+#include "libavcodec/avcodec.h"
+#include <stdlib.h>
+#include "libavcodec/mjpegenc.h"
+#include "libavcodec/mjpegenc_huffman.h"
+#include "libavcodec/mjpegenc_common.h"
+#include "libavcodec/mpegvideo.h"
+
+// Validate the computed lengths satisfy the JPEG restrictions and is optimal.
+static int check_lengths(int L, int expected_length,
+                         const int *probs, int nprobs)
+{
+    HuffTable lengths[256];
+    PTable val_counts[256];
+    int actual_length = 0, i, j, k, prob, length;
+    int ret = 0;
+    double cantor_measure = 0;
+    assert(nprobs <= 256);
+
+    for (i = 0; i < nprobs; i++) {
+        val_counts[i] = (PTable){.value = i, .prob = probs[i]};
+    }
+
+    ff_mjpegenc_huffman_compute_bits(val_counts, lengths, nprobs, L);
+
+    for (i = 0; i < nprobs; i++) {
+        // Find the value's prob and length
+        for (j = 0; j < nprobs; j++)
+            if (val_counts[j].value == i) break;
+        for (k = 0; k < nprobs; k++)
+            if (lengths[k].code == i) break;
+        if (!(j < nprobs && k < nprobs)) return 1;
+        prob = val_counts[j].prob;
+        length = lengths[k].length;
+
+        if (prob) {
+            actual_length += prob * length;
+            cantor_measure += 1. / (1 << length);
+        }
+
+        if (length > L || length < 1) return 1;
+    }
+    // Check that the codes can be prefix-free.
+    if (cantor_measure > 1) ret = 1;
+    // Check that the total length is optimal
+    if (actual_length != expected_length) ret = 1;
+
+    if (ret == 1) {
+      fprintf(stderr,
+              "Cantor measure: %f\n"
+              "Actual length: %d\n"
+              "Expected length: %d\n",
+              cantor_measure, actual_length, expected_length);
+    }
+
+    return ret;
+}
+
+static const int probs_zeroes[] = {6, 6, 0, 0, 0};
+static const int probs_skewed[] = {2, 0, 0, 0, 0, 1, 0, 0, 20, 0, 2,
+    0, 10, 5, 1, 1, 9, 1, 1, 6, 0, 5, 0, 1, 0, 7, 6, 1, 1, 5, 0, 0, 0, 0,
+    11, 0, 0, 0, 51, 1, 0, 20, 0, 1, 0, 0, 0, 0, 6, 106, 1, 0, 1, 0, 2, 1,
+    16, 0, 0, 5, 0, 0, 0, 4, 3, 15, 4, 4, 0, 0, 0, 3, 0, 0, 1, 0, 3, 0, 3,
+    2, 2, 0, 0, 4, 3, 40, 1, 2, 0, 22, 0, 0, 0, 9, 0, 0, 0, 0, 1, 1, 0, 1,
+    6, 11, 4, 10, 28, 6, 1, 0, 0, 9, 9, 4, 0, 0, 0, 0, 8, 33844, 2, 0, 2,
+    1, 1, 5, 0, 0, 1, 9, 1, 0, 4, 14, 4, 0, 0, 3, 8, 0, 51, 9, 6, 1, 1, 2,
+    2, 3, 1, 5, 5, 29, 0, 0, 0, 0, 14, 29, 6, 4, 13, 12, 2, 3, 1, 0, 5, 4,
+    1, 1, 0, 0, 29, 1, 0, 0, 0, 0, 4, 0, 0, 1, 0, 1, 7, 0, 42, 0, 0, 0, 0,
+    0, 2, 0, 3, 9, 0, 0, 0, 2, 1, 0, 0, 6, 5, 6, 1, 2, 3, 0, 0, 0, 3, 0, 0,
+    28, 0, 2, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, 23, 0, 0, 0, 0,
+    0, 21, 1, 0, 3, 24, 2, 0, 0, 7, 0, 0, 1, 5, 1, 2, 0, 5};
+static const int probs_sat[] = {74, 8, 14, 7, 9345, 40, 0, 2014, 2, 1,
+    115, 0, 2, 1, 194, 388, 20, 0, 0, 2, 1, 121, 1, 1583, 0, 16, 21, 2, 132,
+    2, 15, 9, 13, 1, 0, 2293, 2, 8, 5, 2, 30, 0, 0, 4, 54, 783, 4, 1, 2, 4,
+    0, 22, 93, 1, 143, 19, 0, 36, 32, 4, 6, 33, 3, 45, 0, 8, 1, 0, 18, 17, 1,
+    0, 1, 0, 0, 1, 1004, 38, 3, 8, 90, 23, 0, 2819, 3, 0, 970, 158, 9, 6, 4,
+    48, 4, 0, 1, 0, 0, 60, 3, 62, 0, 2, 2, 2, 279, 66, 16, 1, 20, 0, 7, 9,
+    32, 1411, 6, 3, 27, 1, 5, 49, 0, 0, 0, 0, 0, 2, 10, 1, 1, 2, 3, 801, 3,
+    25, 5, 1, 1, 0, 632, 0, 14, 18, 5, 8, 200, 4, 4, 22, 12, 0, 4, 1, 0, 2,
+    4, 9, 3, 16, 7, 2, 2, 213, 0, 2, 620, 39303, 0, 1, 0, 2, 1, 183781, 1,
+    0, 0, 0, 94, 7, 3, 4, 0, 4, 306, 43, 352, 76, 34, 13, 11, 0, 51, 1, 13,
+    19, 0, 26, 0, 7276, 4, 207, 31, 1, 2, 4, 6, 19, 8, 17, 4, 6, 0, 1085, 0,
+    0, 0, 3, 489, 36, 1, 0, 1, 9420, 294, 28, 0, 57, 5, 0, 9, 2, 0, 1, 2, 2,
+    0, 0, 9, 2, 29, 2, 2, 7, 0, 5, 490, 0, 7, 5, 0, 1, 8, 0, 0, 23255, 0, 1};
+
+// Test the example given on @see
+// http://guru.multimedia.cx/small-tasks-for-ffmpeg/
+int main(int argc, char **argv)
+{
+    int i, ret = 0;
+    // Probabilities of symbols 0..4
+    static PTable val_counts[] = {
+        {.value = 0, .prob = 1},
+        {.value = 1, .prob = 2},
+        {.value = 2, .prob = 5},
+        {.value = 3, .prob = 10},
+        {.value = 4, .prob = 21},
+    };
+    // Expected code lengths for each symbol
+    static const HuffTable expected[] = {
+        {.code = 0, .length = 3},
+        {.code = 1, .length = 3},
+        {.code = 2, .length = 3},
+        {.code = 3, .length = 3},
+        {.code = 4, .length = 1},
+    };
+    // Actual code lengths
+    HuffTable distincts[5];
+
+    // Build optimal huffman tree using an internal function, to allow for
+    // smaller-than-normal test cases. This mutates val_counts by sorting.
+    ff_mjpegenc_huffman_compute_bits(val_counts, distincts,
+                                     FF_ARRAY_ELEMS(distincts), 3);
+
+    for (i = 0; i < FF_ARRAY_ELEMS(distincts); i++) {
+        if (distincts[i].code != expected[i].code ||
+            distincts[i].length != expected[i].length) {
+            fprintf(stderr,
+                    "Built huffman does not equal expectations. "
+                    "Expected: code %d probability %d, "
+                    "Actual: code %d probability %d\n",
+                    expected[i].code, expected[i].length,
+                    distincts[i].code, distincts[i].length);
+            ret = 1;
+        }
+    }
+
+    // Check handling of zero probabilities
+    if (check_lengths(16, 18, probs_zeroes, FF_ARRAY_ELEMS(probs_zeroes)))
+        ret = 1;
+    // Check skewed distribution over 256 without saturated lengths
+    if (check_lengths(16, 41282, probs_skewed, FF_ARRAY_ELEMS(probs_skewed)))
+        ret = 1;
+    // Check skewed distribution over 256 with saturated lengths
+    if (check_lengths(16, 669904, probs_sat, FF_ARRAY_ELEMS(probs_sat)))
+        ret = 1;
+
+    return ret;
+}
diff --git a/tests/fate/libavcodec.mak b/tests/fate/libavcodec.mak
index 3bc74c1..32b0c1e 100644
--- a/tests/fate/libavcodec.mak
+++ b/tests/fate/libavcodec.mak
@@ -49,5 +49,11 @@  fate-libavcodec-utils: CMD = run libavcodec/tests/utils
 fate-libavcodec-utils: CMP = null
 fate-libavcodec-utils: REF = /dev/null
 
+FATE_LIBAVCODEC-yes += fate-libavcodec-huffman
+fate-libavcodec-huffman: libavcodec/tests/mjpegenc_huffman$(EXESUF)
+fate-libavcodec-huffman: CMD = run libavcodec/tests/mjpegenc_huffman
+fate-libavcodec-huffman: CMP = null
+fate-libavcodec-huffman: REF = /dev/null
+
 FATE-$(CONFIG_AVCODEC) += $(FATE_LIBAVCODEC-yes)
 fate-libavcodec: $(FATE_LIBAVCODEC-yes)
diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak
index a51f16c..8c24510 100644
--- a/tests/fate/vcodec.mak
+++ b/tests/fate/vcodec.mak
@@ -213,11 +213,13 @@  fate-vsynth%-jpeg2000-97:             DECINOPTS = -vcodec jpeg2000
 FATE_VCODEC-$(call ENCDEC, LJPEG MJPEG, AVI) += ljpeg
 fate-vsynth%-ljpeg:              ENCOPTS = -strict -1
 
-FATE_VCODEC-$(call ENCDEC, MJPEG, AVI)  += mjpeg mjpeg-422 mjpeg-444 mjpeg-trell
-fate-vsynth%-mjpeg:              ENCOPTS = -qscale 9 -pix_fmt yuvj420p
-fate-vsynth%-mjpeg-422:          ENCOPTS = -qscale 9 -pix_fmt yuvj422p
-fate-vsynth%-mjpeg-444:          ENCOPTS = -qscale 9 -pix_fmt yuvj444p
-fate-vsynth%-mjpeg-trell:        ENCOPTS = -qscale 9 -pix_fmt yuvj420p -trellis 1
+FATE_VCODEC-$(call ENCDEC, MJPEG, AVI)  += mjpeg mjpeg-422 mjpeg-444 mjpeg-trell mjpeg-huffman mjpeg-trell-huffman
+fate-vsynth%-mjpeg:                   ENCOPTS = -qscale 9 -pix_fmt yuvj420p
+fate-vsynth%-mjpeg-422:               ENCOPTS = -qscale 9 -pix_fmt yuvj422p
+fate-vsynth%-mjpeg-444:               ENCOPTS = -qscale 9 -pix_fmt yuvj444p
+fate-vsynth%-mjpeg-trell:             ENCOPTS = -qscale 9 -pix_fmt yuvj420p -trellis 1
+fate-vsynth%-mjpeg-huffman:           ENCOPTS = -qscale 9 -pix_fmt yuvj420p -huffman optimal
+fate-vsynth%-mjpeg-trell-huffman:     ENCOPTS = -qscale 9 -pix_fmt yuvj420p -trellis 1 -huffman optimal
 
 FATE_VCODEC-$(call ENCDEC, MPEG1VIDEO, MPEG1VIDEO MPEGVIDEO) += mpeg1 mpeg1b
 fate-vsynth%-mpeg1:              FMT     = mpeg1video
diff --git a/tests/ref/vsynth/vsynth1-mjpeg-huffman b/tests/ref/vsynth/vsynth1-mjpeg-huffman
new file mode 100644
index 0000000..e8c3f0f
--- /dev/null
+++ b/tests/ref/vsynth/vsynth1-mjpeg-huffman
@@ -0,0 +1,4 @@ 
+63ea9bd494e16bad8f3a0c8dbb3dc11e *tests/data/fate/vsynth1-mjpeg-huffman.avi
+1391380 tests/data/fate/vsynth1-mjpeg-huffman.avi
+9a3b8169c251d19044f7087a95458c55 *tests/data/fate/vsynth1-mjpeg-huffman.out.rawvideo
+stddev:    7.87 PSNR: 30.21 MAXDIFF:   63 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth1-mjpeg-trell-huffman b/tests/ref/vsynth/vsynth1-mjpeg-trell-huffman
new file mode 100644
index 0000000..20ce783
--- /dev/null
+++ b/tests/ref/vsynth/vsynth1-mjpeg-trell-huffman
@@ -0,0 +1,4 @@ 
+d9410fa80c07edbd2a2b44ceb06086ca *tests/data/fate/vsynth1-mjpeg-trell-huffman.avi
+1360456 tests/data/fate/vsynth1-mjpeg-trell-huffman.avi
+0266b223bdd7928426a951164bb4a366 *tests/data/fate/vsynth1-mjpeg-trell-huffman.out.rawvideo
+stddev:    7.68 PSNR: 30.42 MAXDIFF:   62 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-mjpeg-huffman b/tests/ref/vsynth/vsynth2-mjpeg-huffman
new file mode 100644
index 0000000..0cf998b
--- /dev/null
+++ b/tests/ref/vsynth/vsynth2-mjpeg-huffman
@@ -0,0 +1,4 @@ 
+9bf00cd3188b7395b798bb10df376243 *tests/data/fate/vsynth2-mjpeg-huffman.avi
+792742 tests/data/fate/vsynth2-mjpeg-huffman.avi
+2b8c59c59e33d6ca7c85d31c5eeab7be *tests/data/fate/vsynth2-mjpeg-huffman.out.rawvideo
+stddev:    4.87 PSNR: 34.37 MAXDIFF:   55 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-mjpeg-trell-huffman b/tests/ref/vsynth/vsynth2-mjpeg-trell-huffman
new file mode 100644
index 0000000..3686740
--- /dev/null
+++ b/tests/ref/vsynth/vsynth2-mjpeg-trell-huffman
@@ -0,0 +1,4 @@ 
+a59d99d31d24875161504820d4266e4d *tests/data/fate/vsynth2-mjpeg-trell-huffman.avi
+734728 tests/data/fate/vsynth2-mjpeg-trell-huffman.avi
+42376126213c73c86b408882e24ba015 *tests/data/fate/vsynth2-mjpeg-trell-huffman.out.rawvideo
+stddev:    5.03 PSNR: 34.09 MAXDIFF:   67 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth3-mjpeg-huffman b/tests/ref/vsynth/vsynth3-mjpeg-huffman
new file mode 100644
index 0000000..634a1a5
--- /dev/null
+++ b/tests/ref/vsynth/vsynth3-mjpeg-huffman
@@ -0,0 +1,4 @@ 
+eec435352485fec167179a63405505be *tests/data/fate/vsynth3-mjpeg-huffman.avi
+48156 tests/data/fate/vsynth3-mjpeg-huffman.avi
+c4fe7a2669afbd96c640748693fc4e30 *tests/data/fate/vsynth3-mjpeg-huffman.out.rawvideo
+stddev:    8.60 PSNR: 29.43 MAXDIFF:   58 bytes:    86700/    86700
diff --git a/tests/ref/vsynth/vsynth3-mjpeg-trell-huffman b/tests/ref/vsynth/vsynth3-mjpeg-trell-huffman
new file mode 100644
index 0000000..719029f
--- /dev/null
+++ b/tests/ref/vsynth/vsynth3-mjpeg-trell-huffman
@@ -0,0 +1,4 @@ 
+484fa337b71c06a0206243814c4894b0 *tests/data/fate/vsynth3-mjpeg-trell-huffman.avi
+47816 tests/data/fate/vsynth3-mjpeg-trell-huffman.avi
+f0ccfe4584d193fd6d690a85a70db188 *tests/data/fate/vsynth3-mjpeg-trell-huffman.out.rawvideo
+stddev:    8.27 PSNR: 29.78 MAXDIFF:   55 bytes:    86700/    86700
diff --git a/tests/ref/vsynth/vsynth_lena-mjpeg-huffman b/tests/ref/vsynth/vsynth_lena-mjpeg-huffman
new file mode 100644
index 0000000..6ac1b74
--- /dev/null
+++ b/tests/ref/vsynth/vsynth_lena-mjpeg-huffman
@@ -0,0 +1,4 @@ 
+007c989af621445dc7c9bd248b9df3b4 *tests/data/fate/vsynth_lena-mjpeg-huffman.avi
+635498 tests/data/fate/vsynth_lena-mjpeg-huffman.avi
+9d4bd90e9abfa18192383b4adc23c8d4 *tests/data/fate/vsynth_lena-mjpeg-huffman.out.rawvideo
+stddev:    4.32 PSNR: 35.40 MAXDIFF:   49 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth_lena-mjpeg-trell-huffman b/tests/ref/vsynth/vsynth_lena-mjpeg-trell-huffman
new file mode 100644
index 0000000..91fcd19
--- /dev/null
+++ b/tests/ref/vsynth/vsynth_lena-mjpeg-trell-huffman
@@ -0,0 +1,4 @@ 
+6eb36ab28a082f496f1f3bc165704a68 *tests/data/fate/vsynth_lena-mjpeg-trell-huffman.avi
+582534 tests/data/fate/vsynth_lena-mjpeg-trell-huffman.avi
+dcb183a6a5fa06e7234d46dd97ceb8ec *tests/data/fate/vsynth_lena-mjpeg-trell-huffman.out.rawvideo
+stddev:    4.51 PSNR: 35.05 MAXDIFF:   60 bytes:  7603200/  7603200