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

[Andreas Cadhalpun]

On 12.06.2015 20:40, Michael Niedermayer wrote:
> On Fri, Jun 12, 2015 at 06:19:47PM +0200, Andreas Cadhalpun wrote:
>> On 10.06.2015 21:50, Michael Niedermayer wrote:
>>> On Wed, Jun 10, 2015 at 09:10:31PM +0200, Andreas Cadhalpun wrote:
>>>> Could these be fixed to use a new H264Context field instead?
>>>
>>> i think it might be possible for them to use the AVFrame height
>>> but the code should be tested
>>
>> I tried a few things, but couldn't get it to work that way.
>> In the end I decided to work around the differing expectations of the h264
>> decoder and the API users by backing up and restoring the internally
>> used values. See attached patch.
>> I tested this extensively and it fixes the problem without introducing
>> another one as far as I can tell.
>> One can see the effect, when using -loglevel debug, e.g. for the
>> reinit-small_422_9-to-small_420_9.h264 sample:
>>  * Without the patch:
>> [...]
>> [h264 @ 0x196e320] Reinit context to 240x208, pix_fmt: yuv420p9le
>> Frame parameters mismatch context 240,196,80 != 240,196,70
>>     Last message repeated 1 times
>> Input stream #0:0 frame changed from size:240x196 fmt:yuv422p9le to size:240x196 fmt:yuv420p9le
>> [...]
>>
>>  * With the patch:
>> [...]
>> [h264 @ 0x2707320] Reinit context to 240x208, pix_fmt: yuv420p9le
>> Input stream #0:0 frame changed from size:240x196 fmt:yuv422p9le to size:240x196 fmt:yuv420p9le
>> [...]
>>
>> The messages about the parameter mismatch for the two frames disappear.
> 
> Maybe the code should be moved to avcodec_decode_video2()
> and the variables to AVCodecInternal
> and then maybe a FF_CODEC_CAP_... could be used to mark decoders which
> need this code. That way it could be used for other decoders too

As far as I know, no other decoder has this problem and I really hope
that newly added ones won't have it either.
So letting this workaround stay h264-specific seems better.

> but the patch as is is probably ok too as is, though it does feel a
> bit inelegant either way

I agree that it is not really elegant, but removing the assumption that
the avctx width/height/pix_fmt are from the last decoded frame instead
of the last returned frame from the h264 decoder is beyond me.
If someone more familiar with the h264 decoder could do that, it would
be great.
But until then, having this workaround is better than the current state.
So I've pushed this.

Best regards,
Andreas