| Message ID | 20240716171155.31838-12-anton@khirnov.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/39] tests/fate/vcodec: add vsynth tests for FFV1 version 2 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Tue, Jul 16, 2024 at 07:11:27PM +0200, Anton Khirnov wrote: > In all cases except decoding version 1 it's either not used, or contains > a copy of a table from quant_tables, which we can just as well use > directly. > > When decoding version 1, we can just as well decode into > quant_tables[0], which would otherwise be unused. > --- > libavcodec/ffv1.h | 1 - > libavcodec/ffv1dec.c | 10 +++++++--- > libavcodec/ffv1enc.c | 6 ++---- > 3 files changed, 9 insertions(+), 8 deletions(-) the code is 1 line longer and using the first entry as a special case doesnt seem simpler thx [...]
On 7/17/2024 7:37 PM, Michael Niedermayer wrote: > On Tue, Jul 16, 2024 at 07:11:27PM +0200, Anton Khirnov wrote: >> In all cases except decoding version 1 it's either not used, or contains >> a copy of a table from quant_tables, which we can just as well use >> directly. >> >> When decoding version 1, we can just as well decode into >> quant_tables[0], which would otherwise be unused. >> --- >> libavcodec/ffv1.h | 1 - >> libavcodec/ffv1dec.c | 10 +++++++--- >> libavcodec/ffv1enc.c | 6 ++---- >> 3 files changed, 9 insertions(+), 8 deletions(-) > > the code is 1 line longer and using the first entry as a special Two lines are comments... > case doesnt seem simpler > > thx > > [...] > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Quoting Michael Niedermayer (2024-07-18 00:37:25) > On Tue, Jul 16, 2024 at 07:11:27PM +0200, Anton Khirnov wrote: > > In all cases except decoding version 1 it's either not used, or contains > > a copy of a table from quant_tables, which we can just as well use > > directly. > > > > When decoding version 1, we can just as well decode into > > quant_tables[0], which would otherwise be unused. > > --- > > libavcodec/ffv1.h | 1 - > > libavcodec/ffv1dec.c | 10 +++++++--- > > libavcodec/ffv1enc.c | 6 ++---- > > 3 files changed, 9 insertions(+), 8 deletions(-) > > the code is 1 line longer and using the first entry as a special > case doesnt seem simpler The simplicity is in the fact it's actually not special at all. quant_tables[quant_table_idx] is always the location of the quant table for any bitstream version, so the rest of the code does not need to care.
diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h index dce6e177eb..4d57172d5b 100644 --- a/libavcodec/ffv1.h +++ b/libavcodec/ffv1.h @@ -106,7 +106,6 @@ typedef struct FFV1Context { int ac; ///< 1=range coder <-> 0=golomb rice int ac_byte_count; ///< number of bytes used for AC coding PlaneContext plane[MAX_PLANES]; - int16_t quant_table[MAX_CONTEXT_INPUTS][256]; int16_t quant_tables[MAX_QUANT_TABLES][MAX_CONTEXT_INPUTS][256]; int context_count[MAX_QUANT_TABLES]; uint8_t state_transition[256]; diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c index a1f7206871..66d9f63c1a 100644 --- a/libavcodec/ffv1dec.c +++ b/libavcodec/ffv1dec.c @@ -764,7 +764,7 @@ static int read_header(FFV1Context *f) ff_dlog(f->avctx, "%d %d %d\n", 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); + context_count = read_quant_tables(c, f->quant_tables[0]); if (context_count < 0) { av_log(f->avctx, AV_LOG_ERROR, "read_quant_table error\n"); return AVERROR_INVALIDDATA; @@ -834,7 +834,7 @@ static int read_header(FFV1Context *f) sizeof(p->quant_table)); context_count = f->context_count[idx]; } else { - memcpy(p->quant_table, f->quant_table, sizeof(p->quant_table)); + memcpy(p->quant_table, f->quant_tables[0], sizeof(p->quant_table)); } if (f->version <= 2) { @@ -1067,7 +1067,11 @@ static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src) fdst->use32bit = fsrc->use32bit; memcpy(fdst->state_transition, fsrc->state_transition, sizeof(fdst->state_transition)); - memcpy(fdst->quant_table, fsrc->quant_table, sizeof(fsrc->quant_table)); + + // in version 1 there is a single per-keyframe quant table, so + // we need to propagate it between threads + if (fsrc->version < 2) + memcpy(fdst->quant_tables[0], fsrc->quant_tables[0], sizeof(fsrc->quant_tables[0])); for (int i = 0; i < fdst->num_h_slices * fdst->num_v_slices; i++) { FFV1Context *fssrc = fsrc->slice_context[i]; diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c index 5692bfa1fc..1d00a46cdd 100644 --- a/libavcodec/ffv1enc.c +++ b/libavcodec/ffv1enc.c @@ -367,7 +367,7 @@ static void write_header(FFV1Context *f) put_symbol(c, state, f->chroma_v_shift, 0); put_rac(c, state, f->transparency); - write_quant_tables(c, f->quant_table); + write_quant_tables(c, f->quant_tables[f->context_model]); } else if (f->version < 3) { put_symbol(c, state, f->slice_count, 0); for (i = 0; i < f->slice_count; i++) { @@ -735,15 +735,13 @@ static av_cold int encode_init(AVCodecContext *avctx) } s->context_count[0] = (11 * 11 * 11 + 1) / 2; s->context_count[1] = (11 * 11 * 5 * 5 * 5 + 1) / 2; - memcpy(s->quant_table, s->quant_tables[s->context_model], - sizeof(s->quant_table)); for (i = 0; i < s->plane_count; i++) { PlaneContext *const p = &s->plane[i]; - memcpy(p->quant_table, s->quant_table, sizeof(p->quant_table)); p->quant_table_index = s->context_model; p->context_count = s->context_count[p->quant_table_index]; + memcpy(p->quant_table, s->quant_tables[p->quant_table_index], sizeof(p->quant_table)); } if ((ret = ff_ffv1_allocate_initial_states(s)) < 0)