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-huffmangit 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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
