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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
