[FFmpeg-devel] [libav-devel] [PATCH] h264: update avctx width/height when flushing
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:
(const uint8_t **)prev->f->data,
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.
More information about the ffmpeg-devel