| Message ID | 20181227192806.8152-1-alex@mogurenko.com |
|---|---|
| State | Accepted |
| Commit | e4788ae31b2e9af45d11f4bf4498c075dcc25a6c |
| Headers | show |
On 27/12/2018 19:28, Alex Mogurenko wrote: > --- > libavcodec/proresenc_kostya.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Can you give a little more detail about what's changed and why, in the commit message? It looks like custom_chroma_q is zero initialized and never set to anything? - Derek
problem occurs in slice quant estimation and slice encoding.
if slice quant >= MAX_STORED_Q we dont use pre-calculated quant matrices
but generate new:
qmat = ctx->custom_q;
qmat_chroma = ctx->custom_q;
for (i = 0; i < 64; i++) {
qmat[i] = ctx->quant_mat[i] * quant;
qmat_chroma[i] = ctx->quant_chroma_mat[i] * quant;
}
as you see both qmat and qmat_chroma both point to same ctx -> custom_q
as result they will contain chroma qunatizers as
qmat_chroma[i] = ctx->quant_chroma_mat[i] * quant;
last in loop, after all we pass qmat/qmat_chroma to function where we
estimate encoded slice size or encode slice:
estimate_slice_plane / encode_slice_plane
I will make new patch with detailed description
пт, 28 дек. 2018 г. в 18:43, Derek Buitenhuis <derek.buitenhuis@gmail.com>:
> On 27/12/2018 19:28, Alex Mogurenko wrote:
> > ---
> > libavcodec/proresenc_kostya.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Can you give a little more detail about what's changed and why,
> in the commit message?
>
> It looks like custom_chroma_q is zero initialized and never set to
> anything?
>
> - Derek
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
On 28/12/2018 17:04, Alex Mogurenko wrote: > problem occurs in slice quant estimation and slice encoding. > > if slice quant >= MAX_STORED_Q we dont use pre-calculated quant matrices > but generate new: > > qmat = ctx->custom_q; > > qmat_chroma = ctx->custom_q; > > for (i = 0; i < 64; i++) { > > qmat[i] = ctx->quant_mat[i] * quant; > > qmat_chroma[i] = ctx->quant_chroma_mat[i] * quant; > > } > > > as you see both qmat and qmat_chroma both point to same ctx -> custom_q > > as result they will contain chroma qunatizers as > > > qmat_chroma[i] = ctx->quant_chroma_mat[i] * quant; > > > last in loop, after all we pass qmat/qmat_chroma to function where we > estimate encoded slice size or encode slice: > > estimate_slice_plane / encode_slice_plane > > > I will make new patch with detailed description > Thanks, this makes sense, I missed that the fact it was setting it in the loop. - Derek
diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c index 9a77d24fb6..e045a972f1 100644 --- a/libavcodec/proresenc_kostya.c +++ b/libavcodec/proresenc_kostya.c @@ -222,6 +222,7 @@ typedef struct ProresThreadData { DECLARE_ALIGNED(16, int16_t, blocks)[MAX_PLANES][64 * 4 * MAX_MBS_PER_SLICE]; DECLARE_ALIGNED(16, uint16_t, emu_buf)[16 * 16]; int16_t custom_q[64]; + int16_t custom_chroma_q[64]; struct TrellisNode *nodes; } ProresThreadData; @@ -232,6 +233,7 @@ typedef struct ProresContext { int16_t quants[MAX_STORED_Q][64]; int16_t quants_chroma[MAX_STORED_Q][64]; int16_t custom_q[64]; + int16_t custom_chroma_q[64]; const uint8_t *quant_mat; const uint8_t *quant_chroma_mat; const uint8_t *scantable; @@ -574,7 +576,7 @@ static int encode_slice(AVCodecContext *avctx, const AVFrame *pic, qmat_chroma = ctx->quants_chroma[quant]; } else { qmat = ctx->custom_q; - qmat_chroma = ctx->custom_q; + qmat_chroma = ctx->custom_chroma_q; for (i = 0; i < 64; i++) { qmat[i] = ctx->quant_mat[i] * quant; qmat_chroma[i] = ctx->quant_chroma_mat[i] * quant; @@ -902,7 +904,7 @@ static int find_slice_quant(AVCodecContext *avctx, qmat_chroma = ctx->quants_chroma[q]; } else { qmat = td->custom_q; - qmat_chroma = td->custom_q; + qmat_chroma = td->custom_chroma_q; for (i = 0; i < 64; i++) { qmat[i] = ctx->quant_mat[i] * q; qmat_chroma[i] = ctx->quant_chroma_mat[i] * q;