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

[Michael Niedermayer]

On Sun, Jan 29, 2017 at 07:54:19PM -0800, Jerry Jiang wrote:
> Hey everyone,
> 
> Sorry for the long wait, here's a new patch addressing the feedback. In
> addition, we discovered that this current implementation of optimal huffman
> encoding is not compatible with QP RD, so we chose to disable QP RD for
> mjpeg or amv encoding. (Wondering if that's acceptable, since it appears
> like QP RD is an underused feature.)
> 
> > You forgot to readjust the range on the huffman option, it should be 0, 1,
> > not 1, 2.
> 
> Fixed.
> 
> > Couldn't the linked list be replaced with an array allocated during init (it
> > should be okay, I don't think resolution is allowed to change during encoding)?
> 
> Done.
> 
> > missing \n before opening brace in functions
> 
> Done.
> 
> > use of ++var instead of var++. this is not a c++ project.
> 
> No more ++s anymore.
> 
> > use of sizeof(x)/sizeof(*x) instead of FF_ARRAY_ELEMS
> 
> No more of that either.
> 
> > a few "MJpegBuffer* x" instead of "MJpegBuffer *x"
> 
> Done.
> 
> > missing vertical align with the "(" in various places such prototypes or
> > your fprintf calls
> 
> Done.
> 
> > add a NB_HUFFMAN_TABLE_OPTION or something at the end, which you can use
> > in the options to define the range from 0 to NB-1
> > btw, wrong indent.
> 
> Done.
> 
> > Wouldn't AV_QSORT() work just the same if you did return a-b?
> 
> Done.
> 
> > no camel case in variable names please
> 
> Done.
> 
> > not a fan of that html; would you mind just keeping the url? "@see http://..."
> 
> Done.
> 
> > static const HuffTable?
> 
> Done.
> 
> > i < FF_ARRAY_ELEMS(distincts)
> 
> Done.
> 
> > static const int
> 
> Done.
> 
> > ditto static const.
> 
> Done.
> 
> > wrong indent.
> 
> Done.
> ---
>  Changelog                                        |   1 +
>  doc/encoders.texi                                |  21 ++
>  libavcodec/Makefile                              |   8 +-
>  libavcodec/mjpegenc.c                            | 241 +++++++++++++++++------
>  libavcodec/mjpegenc.h                            |  68 ++++++-
>  libavcodec/mjpegenc_common.c                     | 166 ++++++++++++++--
>  libavcodec/mjpegenc_common.h                     |   2 +
>  libavcodec/mjpegenc_huffman.c                    | 195 ++++++++++++++++++
>  libavcodec/mjpegenc_huffman.h                    |  73 +++++++
>  libavcodec/mpegvideo.h                           |   1 +
>  libavcodec/mpegvideo_enc.c                       |  17 +-
>  libavcodec/tests/.gitignore                      |   1 +
>  libavcodec/tests/mjpegenc_huffman.c              | 162 +++++++++++++++
>  tests/fate/libavcodec.mak                        |   6 +
>  tests/fate/vcodec.mak                            |  14 +-
>  tests/ref/vsynth/vsynth1-mjpeg-444               |   8 +-
>  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 +
>  24 files changed, 916 insertions(+), 100 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

git refuses to apply this locally here

Applying: Implement optimal huffman encoding for (M)JPEG.
Using index info to reconstruct a base tree...
M       libavcodec/Makefile
error: tests/fate/libavcodec.mak: wrong type
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Patch failed at 0001 Implement optimal huffman encoding for (M)JPEG.
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please make sure you submit a git format-patch or use git send-email

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170130/d12d7048/attachment.sig>