[FFmpeg-devel] [PATCH] lavc/ffv1: Properly check that the 4th and 5th quant tables are zeroes
Jerome Martinez
jerome at mediaarea.net
Thu Jan 2 18:17:31 EET 2020
On 02/01/2020 16:22, Derek Buitenhuis wrote:
> Currently, the decoder checks the 128th value of the 4th quant table during
> while deriving the context on each sample, in order to speed itself up. This
> is due to relying on the behavior of FFmpeg's FFV1 encoder, in which if that
> value is zero, the entire 4th and 5th quant tables are assumed to be entirely
> zero.
>
> This does not match the FFV1 spec, which has no such restriction, and after
> some discussion, it was decided to fix FFmpeg to abide by the spec, rather
> than change the spec.
Thanks for having taken care of this issue.
> [...]
> @@ -735,6 +737,7 @@ static int read_header(FFV1Context *f)
> f->chroma_h_shift, f->chroma_v_shift, f->avctx->pix_fmt);
> if (f->version < 2) {
> context_count = read_quant_tables(c, f->quant_table);
> + f->three_quant_table = is_three_quant_tables(f->quant_table);
> if (context_count < 0) {
> av_log(f->avctx, AV_LOG_ERROR, "read_quant_table error\n");
> return AVERROR_INVALIDDATA;
> @@ -797,9 +800,11 @@ static int read_header(FFV1Context *f)
> p->quant_table_index = idx;
> memcpy(p->quant_table, f->quant_tables[idx],
> sizeof(p->quant_table));
> + p->three_quant_tables = f->three_quant_tables[idx];
> context_count = f->context_count[idx];
> } else {
> memcpy(p->quant_table, f->quant_table, sizeof(p->quant_table));
> + p->three_quant_tables = f->three_quant_table;
> }
Couldn't three_quant_table be a local value instead of being stored in
the context?
> diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
I may have missed something, but I don't get what the additional code in
the encoding part does, value seems not used.
And I am reluctant to change anything in the encoder in this patch as we
consider that the issue is "only" a too aggressive optimization in the
decoder.
Jérôme
More information about the ffmpeg-devel
mailing list