[FFmpeg-devel] [PATCH] h264: don't clobber mmco opcode tables for non-first slice headers.

[Ronald S. Bultje]

Hi,

On Sun, Jan 13, 2013 at 7:36 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> On Sun, Jan 13, 2013 at 04:40:24PM -0800, Ronald S. Bultje wrote:
>> -    if (h->nal_ref_idc && ff_h264_decode_ref_pic_marking(h0, &s->gb) < 0 &&
>> +    if (h->nal_ref_idc &&
>> +        ff_h264_decode_ref_pic_marking(h0, &s->gb,
>> +                                       first_mb_in_slice == 0) < 0 &&
>
> This depends on the first slice not being lost, if it is lost this
> will fail. It also will fail with ASO

I'm not too worried about ER at the moment, that can be fixed later. What's ASO?

>> +static int check_opcodes(MMCO *mmco1, MMCO *mmco2, int n_mmcos)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < n_mmcos; i++) {
>> +        if (mmco1[i].opcode != mmco2[i].opcode)
>> +            return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>
> this only checks the opcodes, the other fields should be checked
> too, also i wonder why you dont use a memcmp ?

Not all fields are filled in, so the memcmp may "fail" even though all
filled-in fields are identical. I was actually considering putting
this under #if DEBUG, I don't have non-corrupt/-fuzzed samples where
this triggers. Is that even legal?

>>          }else{
>> +            if (first_slice)
>>              ff_generate_sliding_window_mmcos(h);
>> +            mmco_index = -1;
>
> you skip checking the sliding window mmcos, they can mismatch too

Yeah I took that out while debugging and forgot to put it back in. I
wonder though, can that ever (practically) change between slices? I
guess if the first slice had explicit mmcos and the second slice used
the sliding window codes, the table contents could change. But as per
above, is that legal? I mean, if we can have multiple mmcos between
slices, firstly that doesn't work already in our decoder, because we
ever only call ff_h264_execute_ref_pic_marking() with the output of
decode_ref_pic_markings() once per field at most. I guess I don't
understand why mmcos exist per-slice instead of per-field.

Ronald