On 10.06.2015 21:50, Michael Niedermayer wrote:
> On Wed, Jun 10, 2015 at 09:10:31PM +0200, Andreas Cadhalpun wrote:
>> On 10.06.2015 12:01, Michael Niedermayer wrote:
>>>> also avctx->pix_fmt would then still potentially not match the last
>>>> output frame, also pix_fmt has the same problem as above
>>
>> Yes, pix_fmt should also be updated. Attached patch does this, but
>
>> this caused changes in some h264 reinit fate tests. Is that a problem?
>
> looking at the testfile with ffplay shows heavy artifacts after the
> patch and no artifacts before
Yes, that's bad...
>>>> ive fixed one use of width/height not to depend on the AVCodecContext
>>>> value but a 2nd remains
>>>
>>> actually theres more code that uses it
>>> ff_h264_draw_horiz_band() and its callers use h->avctx->height too
>>
>> 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.
> especially with a file that uses croping and some application that
> uses these codepathes
> iam not sure this codepath works currently with files using croping
> and using the AVFrame height and if needed an adjusted pointer
> offset might fix that or might break it depending on what applications
> expect
I don't know, whether or not this currently works, but it shouldn't
be affected by attached patch.
Best regards,
Andreas
>From 61f8aad99f05173a15ec28802ee8c92439bb2b0c Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Tue, 9 Jun 2015 23:38:26 +0200
Subject: [PATCH] h264: update avctx width/height/pix_fmt when returning frame
Inconsistencies between the dimensions/pixel format of avctx and the
frame can confuse API users.
For example this can crash the demuxing_decoding example.
Back up the previous values and restore them, when decoding the next
frame. This is necessary, because these can be different between the
returned frame and the last decoded frame.
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
libavcodec/h264.c | 24 ++++++++++++++++++++++++
libavcodec/h264.h | 8 ++++++++
libavcodec/h264_slice.c | 3 +++
3 files changed, 35 insertions(+)
diff --git a/libavcodec/h264.c b/libavcodec/h264.c
index 9a00214..9be317c 100644
--- a/libavcodec/h264.c
+++ b/libavcodec/h264.c
@@ -591,6 +591,9 @@ static int h264_init_context(AVCodecContext *avctx, H264Context *h)
int i;
h->avctx = avctx;
+ h->backup_width = -1;
+ h->backup_height = -1;
+ h->backup_pix_fmt = AV_PIX_FMT_NONE;
h->dequant_coeff_pps = -1;
h->current_sps_id = -1;
h->cur_chroma_format_idc = -1;
@@ -1675,6 +1678,14 @@ static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp)
av_dict_set(&dst->metadata, "stereo_mode", ff_h264_sei_stereo_mode(h), 0);
+ h->backup_width = h->avctx->width;
+ h->backup_height = h->avctx->height;
+ h->backup_pix_fmt = h->avctx->pix_fmt;
+
+ h->avctx->width = dst->width;
+ h->avctx->height = dst->height;
+ h->avctx->pix_fmt = dst->format;
+
if (srcp->sei_recovery_frame_cnt == 0)
dst->key_frame = 1;
if (!srcp->crop)
@@ -1726,6 +1737,19 @@ static int h264_decode_frame(AVCodecContext *avctx, void *data,
h->flags = avctx->flags;
+ if (h->backup_width != -1) {
+ avctx->width = h->backup_width;
+ h->backup_width = -1;
+ }
+ if (h->backup_height != -1) {
+ avctx->height = h->backup_height;
+ h->backup_height = -1;
+ }
+ if (h->backup_pix_fmt != AV_PIX_FMT_NONE) {
+ avctx->pix_fmt = h->backup_pix_fmt;
+ h->backup_pix_fmt = AV_PIX_FMT_NONE;
+ }
+
ff_h264_unref_picture(h, &h->last_pic_for_ec);
/* end of stream, output what is still in the buffers */
diff --git a/libavcodec/h264.h b/libavcodec/h264.h
index 95db912..548510d 100644
--- a/libavcodec/h264.h
+++ b/libavcodec/h264.h
@@ -519,6 +519,14 @@ typedef struct H264Context {
int width, height;
int chroma_x_shift, chroma_y_shift;
+ /**
+ * Backup frame properties: needed, because they can be different
+ * between returned frame and last decoded frame.
+ **/
+ int backup_width;
+ int backup_height;
+ enum AVPixelFormat backup_pix_fmt;
+
int droppable;
int coded_picture_number;
int low_delay;
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 9c4d613..0c0812f 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -497,6 +497,9 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
h->picture_structure = h1->picture_structure;
h->droppable = h1->droppable;
h->low_delay = h1->low_delay;
+ h->backup_width = h1->backup_width;
+ h->backup_height = h1->backup_height;
+ h->backup_pix_fmt = h1->backup_pix_fmt;
for (i = 0; i < H264_MAX_PICTURE_COUNT; i++) {
ff_h264_unref_picture(h, &h->DPB[i]);
--
2.1.4
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
