VC1 Intensity Compensation is always used once encountered

[mcarter]

I noticed that VC1 decoding performance dropped after upgrading to the latest git code. I profiled it and found that a lot (double-digit %) of time was spent in libavcodec/vc1dec.c:vc1_mc_1mv under "if (use_ic) ...".

I created the attached patch, intcomp.patch (just output from git diff), to instrument libavcodec and I ran:

% ffmpeg -t 60 -i BigBuckBunny_1080p24.wmv -an -f null -
...
g_identity_lut_count: 41704276, g_non_identity_lut_count: 154612
g_vc1_mc_1mv_calls: 10474337, g_use_ic_times: 10464722

The file is from ​http://bit.ly/bbbwmv which is from ​http://msdn.microsoft.com/en-us/expression/hh553539.aspx.

The result was that for 60 sec of decoding this VC1 Advanced Profile video, vc1_mc_1mv was called 10,474,337 times and of that, it did intensity compensation 10,464,722 times (99.9% of the time). It seems highly improbable that intensity compensation is needed on every single call. Indeed, the instrumentation shows the number of times the intensity compensation lookup tables (luty, lutuv) don't do anything (because they are the identity mapping) and it is the majority of the time (41704276/(41704276+154612)=99.63%). Thus, this suggests a bug in the code.

I looked at the git history and this might be the commit that introduced this issue, but I'm not sure:

​http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=782ebd611824c95201123aeccc85809ce1554e6e

I looked at the source code and it seems that once v->next_use_ic and/or v->last_use_ic are set to 1, they might never go back to 0, perhaps causing the issue (once intensity compensation is encountered in the file, it is used forever). But I'm not very familiar with this code, so I'm not sure. Maybe I'm totally misunderstanding everything and intensity compensation is required for the entire file, even though most of the times it doesn't do anything?

[mcarter]

vc1dec.c instrumentation patch

[Carl Eugen Hoyos]

782ebd61 is the cause for a >5% speed regression in vc1 decoding.

[mcarter]

Here is a (probably suboptimal) naive patch that might solve the issue:

In vc1.c, in rotate_luts() after v->curr_use_ic = 0, apply this patch:

ROTATE(int tmp,             v->last_use_ic, v->next_use_ic, v->curr_use_ic, v->aux_use_ic);
ROTATE(uint8_t tmp[2][256], v->last_luty,   v->next_luty,   v->curr_luty,   v->aux_luty);
ROTATE(uint8_t tmp[2][256], v->last_lutuv,  v->next_lutuv,  v->curr_lutuv,  v->aux_lutuv);

INIT_LUT(32, 0, v->curr_luty[0], v->curr_lutuv[0], 0);
INIT_LUT(32, 0, v->curr_luty[1], v->curr_lutuv[1], 0);
v->curr_use_ic = 0;

+if (v->curr_luty == v->next_luty) {
+    v->next_use_ic = 0;
+}

Here's why this makes sense: the previous lines call INIT_LUT(32, 0, v->curr_luty... which sets v->curr_lut{y,uv}[0,1] to the identity mapping. But note that due to the ROTATE() calls, v->curr_luty points to either v->next_luty or v->aux_luty. If v->curr_luty points to v->next_luty, then the INIT_LUT() calls just set v->next_luty to the identity mapping. If v->next_luty is set to the identity mapping, then v->next_use_ic should be set to 0, just as v->curr_use_ic has already been set to 0. (We don't need to set v->aux_use_ic since it is never read by any code.)

An alternative patch would be to use the same test that ROTATE() uses: if (!(v->s.pict_type == AV_PICTURE_TYPE_BI || v->s.pict_type == AV_PICTURE_TYPE_B)) { v->next_use_ic = 0; }

I'm not 100% sure if either of these are a good fix, but I hope it gives some insight into the problem.

[Carl Eugen Hoyos]

Please send all patches to the ffmpeg-devel mailing list where they can be discussed and will be reviewed.

[Michael Niedermayer]

Patch applied and simplified
sorry for the delay, i have totally missed the patch in the ticket.
patches should be sent to ffmpeg-devel ML
also dont hesitate to mail / ping me per private mail if some change from me caused a regression