| Message ID | 20190530140400.6205-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
On Thu, 30 May 2019, Michael Niedermayer wrote: > Fixes: Ticket7880 > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/qtrle.c | 42 +++++++++++++++++++++++++++++++++++---- > tests/ref/fate/qtrle-8bit | 1 + > 2 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/qtrle.c b/libavcodec/qtrle.c > index 2c29547e5a..4add1aded6 100644 > --- a/libavcodec/qtrle.c > +++ b/libavcodec/qtrle.c > @@ -45,6 +45,8 @@ typedef struct QtrleContext { > > GetByteContext g; > uint32_t pal[256]; > + int need_flush; > + AVPacket flush_pkt; > } QtrleContext; > > #define CHECK_PIXEL_PTR(n) \ > @@ -444,6 +446,12 @@ static av_cold int qtrle_decode_init(AVCodecContext *avctx) > return 0; > } > > +static void qtrle_flush(AVCodecContext *avctx){ > + QtrleContext *s = avctx->priv_data; > + > + s->need_flush = 0; > +} > + > static int qtrle_decode_frame(AVCodecContext *avctx, > void *data, int *got_frame, > AVPacket *avpkt) > @@ -454,11 +462,32 @@ static int qtrle_decode_frame(AVCodecContext *avctx, > int has_palette = 0; > int ret, size; > > + if (!avpkt->data) { > + if (s->need_flush) { > + s->need_flush = 0; > + if ((ret = ff_reget_buffer(avctx, s->frame)) < 0) > + return ret; > + s->frame->pkt_pos = s->flush_pkt.pos; > + s->frame->pkt_duration = s->flush_pkt.duration; > + s->frame->pkt_dts = s->flush_pkt.dts; > + s->frame->pkt_pts = > + s->frame->pts = s->flush_pkt.pts; This does not seem to work, because the magic in decode_simple_internal in decode.c will overwrite these fields. Alternative approach which seems to work is assigning the AVPacket fields instead: avpkt->pos = s->flush_pkt.pos; avpkt->duration = s->flush_pkt.duration; avpkt->dts = s->flush_pkt.dts; avpkt->pts = s->flush_pkt.pts; Looks a bit ugly, but as far as I see we are using an internal packet, and not the one provided by the user, so it might be OK. Regards, Marton
On Fri, May 31, 2019 at 10:28:15PM +0200, Marton Balint wrote: > > > On Thu, 30 May 2019, Michael Niedermayer wrote: > > >Fixes: Ticket7880 > > > >Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >--- > >libavcodec/qtrle.c | 42 +++++++++++++++++++++++++++++++++++---- > >tests/ref/fate/qtrle-8bit | 1 + > >2 files changed, 39 insertions(+), 4 deletions(-) > > > >diff --git a/libavcodec/qtrle.c b/libavcodec/qtrle.c > >index 2c29547e5a..4add1aded6 100644 > >--- a/libavcodec/qtrle.c > >+++ b/libavcodec/qtrle.c > >@@ -45,6 +45,8 @@ typedef struct QtrleContext { > > > > GetByteContext g; > > uint32_t pal[256]; > >+ int need_flush; > >+ AVPacket flush_pkt; > >} QtrleContext; > > > >#define CHECK_PIXEL_PTR(n) \ > >@@ -444,6 +446,12 @@ static av_cold int qtrle_decode_init(AVCodecContext *avctx) > > return 0; > >} > > > >+static void qtrle_flush(AVCodecContext *avctx){ > >+ QtrleContext *s = avctx->priv_data; > >+ > >+ s->need_flush = 0; > >+} > >+ > >static int qtrle_decode_frame(AVCodecContext *avctx, > > void *data, int *got_frame, > > AVPacket *avpkt) > >@@ -454,11 +462,32 @@ static int qtrle_decode_frame(AVCodecContext *avctx, > > int has_palette = 0; > > int ret, size; > > > >+ if (!avpkt->data) { > >+ if (s->need_flush) { > >+ s->need_flush = 0; > >+ if ((ret = ff_reget_buffer(avctx, s->frame)) < 0) > >+ return ret; > >+ s->frame->pkt_pos = s->flush_pkt.pos; > >+ s->frame->pkt_duration = s->flush_pkt.duration; > >+ s->frame->pkt_dts = s->flush_pkt.dts; > >+ s->frame->pkt_pts = > >+ s->frame->pts = s->flush_pkt.pts; > > This does not seem to work, because the magic in decode_simple_internal in > decode.c will overwrite these fields. Alternative approach which seems to > work is assigning the AVPacket fields instead: > > avpkt->pos = s->flush_pkt.pos; > avpkt->duration = s->flush_pkt.duration; > avpkt->dts = s->flush_pkt.dts; > avpkt->pts = s->flush_pkt.pts; > > Looks a bit ugly, but as far as I see we are using an internal packet, and > not the one provided by the user, so it might be OK. ill repost this with a different, less ugly variant. Please recheck if that works! [...]
On Mon, 12 Aug 2019, Michael Niedermayer wrote: > On Fri, May 31, 2019 at 10:28:15PM +0200, Marton Balint wrote: >> >> >> On Thu, 30 May 2019, Michael Niedermayer wrote: >> >>> Fixes: Ticket7880 >>> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavcodec/qtrle.c | 42 +++++++++++++++++++++++++++++++++++---- >>> tests/ref/fate/qtrle-8bit | 1 + >>> 2 files changed, 39 insertions(+), 4 deletions(-) >>> >>> diff --git a/libavcodec/qtrle.c b/libavcodec/qtrle.c >>> index 2c29547e5a..4add1aded6 100644 >>> --- a/libavcodec/qtrle.c >>> +++ b/libavcodec/qtrle.c >>> @@ -45,6 +45,8 @@ typedef struct QtrleContext { >>> >>> GetByteContext g; >>> uint32_t pal[256]; >>> + int need_flush; >>> + AVPacket flush_pkt; >>> } QtrleContext; >>> >>> #define CHECK_PIXEL_PTR(n) \ >>> @@ -444,6 +446,12 @@ static av_cold int qtrle_decode_init(AVCodecContext *avctx) >>> return 0; >>> } >>> >>> +static void qtrle_flush(AVCodecContext *avctx){ >>> + QtrleContext *s = avctx->priv_data; >>> + >>> + s->need_flush = 0; >>> +} >>> + >>> static int qtrle_decode_frame(AVCodecContext *avctx, >>> void *data, int *got_frame, >>> AVPacket *avpkt) >>> @@ -454,11 +462,32 @@ static int qtrle_decode_frame(AVCodecContext *avctx, >>> int has_palette = 0; >>> int ret, size; >>> >>> + if (!avpkt->data) { >>> + if (s->need_flush) { >>> + s->need_flush = 0; >>> + if ((ret = ff_reget_buffer(avctx, s->frame)) < 0) >>> + return ret; >>> + s->frame->pkt_pos = s->flush_pkt.pos; >>> + s->frame->pkt_duration = s->flush_pkt.duration; >>> + s->frame->pkt_dts = s->flush_pkt.dts; >>> + s->frame->pkt_pts = >>> + s->frame->pts = s->flush_pkt.pts; >> >> This does not seem to work, because the magic in decode_simple_internal in >> decode.c will overwrite these fields. Alternative approach which seems to >> work is assigning the AVPacket fields instead: >> >> avpkt->pos = s->flush_pkt.pos; >> avpkt->duration = s->flush_pkt.duration; >> avpkt->dts = s->flush_pkt.dts; >> avpkt->pts = s->flush_pkt.pts; >> >> Looks a bit ugly, but as far as I see we are using an internal packet, and >> not the one provided by the user, so it might be OK. > > ill repost this with a different, less ugly variant. > > Please recheck if that works! Seems fine now, thanks. Regards, Marton
diff --git a/libavcodec/qtrle.c b/libavcodec/qtrle.c index 2c29547e5a..4add1aded6 100644 --- a/libavcodec/qtrle.c +++ b/libavcodec/qtrle.c @@ -45,6 +45,8 @@ typedef struct QtrleContext { GetByteContext g; uint32_t pal[256]; + int need_flush; + AVPacket flush_pkt; } QtrleContext; #define CHECK_PIXEL_PTR(n) \ @@ -444,6 +446,12 @@ static av_cold int qtrle_decode_init(AVCodecContext *avctx) return 0; } +static void qtrle_flush(AVCodecContext *avctx){ + QtrleContext *s = avctx->priv_data; + + s->need_flush = 0; +} + static int qtrle_decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPacket *avpkt) @@ -454,11 +462,32 @@ static int qtrle_decode_frame(AVCodecContext *avctx, int has_palette = 0; int ret, size; + if (!avpkt->data) { + if (s->need_flush) { + s->need_flush = 0; + if ((ret = ff_reget_buffer(avctx, s->frame)) < 0) + return ret; + s->frame->pkt_pos = s->flush_pkt.pos; + s->frame->pkt_duration = s->flush_pkt.duration; + s->frame->pkt_dts = s->flush_pkt.dts; + s->frame->pkt_pts = + s->frame->pts = s->flush_pkt.pts; + if ((ret = av_frame_ref(data, s->frame)) < 0) + return ret; + *got_frame = 1; + } + return 0; + } + s->flush_pkt = *avpkt; + bytestream2_init(&s->g, avpkt->data, avpkt->size); /* check if this frame is even supposed to change */ - if (avpkt->size < 8) + if (avpkt->size < 8) { + s->need_flush = 1; return avpkt->size; + } + s->need_flush = 0; /* start after the chunk size */ size = bytestream2_get_be32(&s->g) & 0x3FFFFFFF; @@ -471,14 +500,18 @@ static int qtrle_decode_frame(AVCodecContext *avctx, /* if a header is present, fetch additional decoding parameters */ if (header & 0x0008) { - if (avpkt->size < 14) + if (avpkt->size < 14) { + s->need_flush = 1; return avpkt->size; + } start_line = bytestream2_get_be16(&s->g); bytestream2_skip(&s->g, 2); height = bytestream2_get_be16(&s->g); bytestream2_skip(&s->g, 2); - if (height > s->avctx->height - start_line) + if (height > s->avctx->height - start_line) { + s->need_flush = 1; return avpkt->size; + } } else { start_line = 0; height = s->avctx->height; @@ -572,5 +605,6 @@ AVCodec ff_qtrle_decoder = { .init = qtrle_decode_init, .close = qtrle_decode_end, .decode = qtrle_decode_frame, - .capabilities = AV_CODEC_CAP_DR1, + .flush = qtrle_flush, + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY, }; diff --git a/tests/ref/fate/qtrle-8bit b/tests/ref/fate/qtrle-8bit index 27bb8aad71..39a03b7b6c 100644 --- a/tests/ref/fate/qtrle-8bit +++ b/tests/ref/fate/qtrle-8bit @@ -61,3 +61,4 @@ 0, 160, 160, 1, 921600, 0xcfd6ad2b 0, 163, 163, 1, 921600, 0x3b372379 0, 165, 165, 1, 921600, 0x36f245f5 +0, 166, 166, 1, 921600, 0x36f245f5
Fixes: Ticket7880 Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/qtrle.c | 42 +++++++++++++++++++++++++++++++++++---- tests/ref/fate/qtrle-8bit | 1 + 2 files changed, 39 insertions(+), 4 deletions(-)