[FFmpeg-devel] [libav-devel] [PATCH] h264: update avctx width/height when flushing

Andreas Cadhalpun andreas.cadhalpun at googlemail.com
Sun Jun 14 12:38:45 CEST 2015


On 14.06.2015 03:12, Luca Barbato wrote:
> I prefer fix bugs properly and I prefer to wait a second and listen to
> everybody before do something.

That's a commendable ideal, but if it results in problems not getting
fixed at all, it's counter-productive.

> Discuss with Hendrik, since he had been
> quite critical about it and I'd take in account his feedback.

I discussed it with Hendrik and asked him for a better way to fix the
problem. It seems he hasn't found one, yet.

> Now some explanations on how you see the problem and which codecs should
> be affected.
> 
> You have the dimensions and the pixel format in the avcodec context.
> 
> They might be updated or not by the codec, some already do not update
> those value nor consider them at all, so I would rather have the
> programs using badly the API get a patch

Did I just hear you volunteer to check all API users and send patches,
where you find it necessary?

> than perpetuate some mistakes
> that, again, hadn't happened here to begin with.

I'd like to invite you to take a closer look at the h264 decoder,
in particular the following code in libavcodec/h264_slice.c:
    av_image_copy(h->short_ref[0]->f->data,
                  h->short_ref[0]->f->linesize,
                  (const uint8_t **)prev->f->data,
                  prev->f->linesize,
                  h->avctx->pix_fmt,
                  h->mb_width  * 16,
                  h->mb_height * 16);

As you can see, it uses the pixel format from the codec context and
it doesn't check for width/height/format inconsistencies.
I'd say that's the opposite of "hadn't happened here". ;)

> Discounting that issue, let see how those fields could be updated.
> 
> The codec using them need to update them as they parse the data usually
> since they do use them while decoding.
> 
> If you have multiple threads involved or you have any delay between
> outputting a frame and parsing you can have this kind of mismatch
> between what is in the frame and what the codec sets there.
> 
> Any kind of frame reordering would get the user that kind of discrepancy.
> 
> So the codec affected needs threads, b-frames (or assimilable) and frame
> properties changes to get the poor misguided guy to have issues randomly.
> 
> You asked which other codec should have the issue, HEVC seems to me a
> good candidate. You parse a new set of sps while you are still
> outputting something using the previous set and you have the issue.

If that's the case, Michael's idea of moving this backup/restore into
avcodec_decode_video2 and using it for codecs declaring some FF_CODEC_CAP_*
(or maybe just unconditionally) looks like the way to go.

> No, that kind of usage is not working at all and should not be supported.

Unfortunately the API documentation hadn't mentioned this, thus I wouldn't
be surprised if that usage happens in the wild.
So removing that pitfall from the API is a worthwhile objective.

Best regards,
Andreas



More information about the ffmpeg-devel mailing list