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

Michael Niedermayer michaelni at gmx.at
Mon Jan 14 16:55:32 CET 2013


Hi

On Sun, Jan 13, 2013 at 08:57:46PM -0800, Ronald S. Bultje wrote:
> Hi,
[...]
> 
> >> +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.

all or unused fields could be zeroed.


> I was actually considering putting
> this under #if DEBUG, I don't have non-corrupt/-fuzzed samples where
> this triggers. Is that even legal?

No, you are correct this is not legal, they have to match between
slices. A check would only make sense for detecting errors (possibly
usefull for error concealment), inconsistencies or spec violations.


> 
> >>          }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.

If they would exist per field and that one place where they are stored
per field is lost the MMCO list would be lost which would significantly
mess up the reference pictures and decoding of future fields. So i
guess the JVT preferred redundancy for this important information

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130114/c03bddaf/attachment.asc>


More information about the ffmpeg-devel mailing list