| Message ID | 20191017231847.6426-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 2019-10-18 02:18, James Almer wrote: > Actually reorder the values. > > Should effectively fix ticket #8300. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Now unconditionally propagating the field, since checking its value is > not correct usage of the field. James, do you still plan working on this patch? > libavcodec/libdav1d.c | 25 ++++++++++++++++++++++++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c > index 8aa248e6cd..1793c9e4f0 100644 > --- a/libavcodec/libdav1d.c > +++ b/libavcodec/libdav1d.c > @@ -164,6 +164,11 @@ static void libdav1d_data_free(const uint8_t *data, void *opaque) { > av_buffer_unref(&buf); > } > > +static void libdav1d_user_data_free(const uint8_t *data, void *opaque) { > + av_assert2(data == opaque); > + av_free(opaque); > +} > + > static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame) > { > Libdav1dContext *dav1d = c->priv_data; > @@ -179,6 +184,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame) > return res; > > if (pkt.size) { > + uint8_t *reordered_opaque; > + > res = dav1d_data_wrap(data, pkt.data, pkt.size, libdav1d_data_free, pkt.buf); > if (res < 0) { > av_packet_unref(&pkt); > @@ -191,6 +198,21 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame) > > pkt.buf = NULL; > av_packet_unref(&pkt); > + > + reordered_opaque = av_malloc(sizeof(c->reordered_opaque)); > + if (!reordered_opaque) { > + dav1d_data_unref(data); > + return AVERROR(ENOMEM); > + } > + > + memcpy(reordered_opaque, &c->reordered_opaque, sizeof(c->reordered_opaque)); > + res = dav1d_data_wrap_user_data(data, reordered_opaque, > + libdav1d_user_data_free, reordered_opaque); > + if (res < 0) { > + av_free(reordered_opaque); > + dav1d_data_unref(data); > + return res; > + } > } > } > > @@ -260,7 +282,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame) > else > frame->format = c->pix_fmt = pix_fmt[p->p.layout][p->seq_hdr->hbd]; > > - frame->reordered_opaque = c->reordered_opaque; > + av_assert0(p->m.user_data.data); > + memcpy(&frame->reordered_opaque, p->m.user_data.data, sizeof(frame->reordered_opaque)); > > // match timestamps and packet size > frame->pts = frame->best_effort_timestamp = p->m.timestamp; >
On 10/22/2019 11:01 AM, Andrey Semashev wrote: > On 2019-10-18 02:18, James Almer wrote: >> Actually reorder the values. >> >> Should effectively fix ticket #8300. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> Now unconditionally propagating the field, since checking its value is >> not correct usage of the field. > > James, do you still plan working on this patch? Yes, but i have no way to test it. Can you confirm the current implementation in the tree misbehaves, and that this approach corrects it?
On 2019-10-22 17:09, James Almer wrote: > On 10/22/2019 11:01 AM, Andrey Semashev wrote: >> On 2019-10-18 02:18, James Almer wrote: >>> Actually reorder the values. >>> >>> Should effectively fix ticket #8300. >>> >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> Now unconditionally propagating the field, since checking its value is >>> not correct usage of the field. >> >> James, do you still plan working on this patch? > > Yes, but i have no way to test it. Can you confirm the current > implementation in the tree misbehaves, and that this approach corrects it? No, I don't have a test video file that would cause frame reordering. At least I don't think any of my files cause it. I tried my v3 patch with the files I have, it worked as intended.
On 2019-10-22 17:14, Andrey Semashev wrote: > On 2019-10-22 17:09, James Almer wrote: >> On 10/22/2019 11:01 AM, Andrey Semashev wrote: >>> On 2019-10-18 02:18, James Almer wrote: >>>> Actually reorder the values. >>>> >>>> Should effectively fix ticket #8300. >>>> >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> Now unconditionally propagating the field, since checking its value is >>>> not correct usage of the field. >>> >>> James, do you still plan working on this patch? >> >> Yes, but i have no way to test it. Can you confirm the current >> implementation in the tree misbehaves, and that this approach corrects >> it? > > No, I don't have a test video file that would cause frame reordering. At > least I don't think any of my files cause it. > > I tried my v3 patch with the files I have, it worked as intended. Actually, no, one of the files causes a delay for one frame, so I can test it. Your v2 and my v3 patches work (i.e. the decoded reordered_opaque lags behind input pts for one frame).
On 10/22/2019 11:51 AM, Andrey Semashev wrote: > On 2019-10-22 17:14, Andrey Semashev wrote: >> On 2019-10-22 17:09, James Almer wrote: >>> On 10/22/2019 11:01 AM, Andrey Semashev wrote: >>>> On 2019-10-18 02:18, James Almer wrote: >>>>> Actually reorder the values. >>>>> >>>>> Should effectively fix ticket #8300. >>>>> >>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>> --- >>>>> Now unconditionally propagating the field, since checking its value is >>>>> not correct usage of the field. >>>> >>>> James, do you still plan working on this patch? >>> >>> Yes, but i have no way to test it. Can you confirm the current >>> implementation in the tree misbehaves, and that this approach >>> corrects it? >> >> No, I don't have a test video file that would cause frame reordering. >> At least I don't think any of my files cause it. >> >> I tried my v3 patch with the files I have, it worked as intended. > > Actually, no, one of the files causes a delay for one frame, so I can > test it. Your v2 and my v3 patches work (i.e. the decoded > reordered_opaque lags behind input pts for one frame). Ok, patch applied then. Thanks for testing.
On Tue, Oct 22, 2019 at 4:51 PM Andrey Semashev <andrey.semashev@gmail.com> wrote: > > On 2019-10-22 17:14, Andrey Semashev wrote: > > On 2019-10-22 17:09, James Almer wrote: > >> On 10/22/2019 11:01 AM, Andrey Semashev wrote: > >>> On 2019-10-18 02:18, James Almer wrote: > >>>> Actually reorder the values. > >>>> > >>>> Should effectively fix ticket #8300. > >>>> > >>>> Signed-off-by: James Almer <jamrial@gmail.com> > >>>> --- > >>>> Now unconditionally propagating the field, since checking its value is > >>>> not correct usage of the field. > >>> > >>> James, do you still plan working on this patch? > >> > >> Yes, but i have no way to test it. Can you confirm the current > >> implementation in the tree misbehaves, and that this approach corrects > >> it? > > > > No, I don't have a test video file that would cause frame reordering. At > > least I don't think any of my files cause it. > > > > I tried my v3 patch with the files I have, it worked as intended. > > Actually, no, one of the files causes a delay for one frame, so I can > test it. Delay automatically happens if you use frame-threaded decoding, its not something in the bitstream. - Hendrik
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c index 8aa248e6cd..1793c9e4f0 100644 --- a/libavcodec/libdav1d.c +++ b/libavcodec/libdav1d.c @@ -164,6 +164,11 @@ static void libdav1d_data_free(const uint8_t *data, void *opaque) { av_buffer_unref(&buf); } +static void libdav1d_user_data_free(const uint8_t *data, void *opaque) { + av_assert2(data == opaque); + av_free(opaque); +} + static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame) { Libdav1dContext *dav1d = c->priv_data; @@ -179,6 +184,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame) return res; if (pkt.size) { + uint8_t *reordered_opaque; + res = dav1d_data_wrap(data, pkt.data, pkt.size, libdav1d_data_free, pkt.buf); if (res < 0) { av_packet_unref(&pkt); @@ -191,6 +198,21 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame) pkt.buf = NULL; av_packet_unref(&pkt); + + reordered_opaque = av_malloc(sizeof(c->reordered_opaque)); + if (!reordered_opaque) { + dav1d_data_unref(data); + return AVERROR(ENOMEM); + } + + memcpy(reordered_opaque, &c->reordered_opaque, sizeof(c->reordered_opaque)); + res = dav1d_data_wrap_user_data(data, reordered_opaque, + libdav1d_user_data_free, reordered_opaque); + if (res < 0) { + av_free(reordered_opaque); + dav1d_data_unref(data); + return res; + } } } @@ -260,7 +282,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame) else frame->format = c->pix_fmt = pix_fmt[p->p.layout][p->seq_hdr->hbd]; - frame->reordered_opaque = c->reordered_opaque; + av_assert0(p->m.user_data.data); + memcpy(&frame->reordered_opaque, p->m.user_data.data, sizeof(frame->reordered_opaque)); // match timestamps and packet size frame->pts = frame->best_effort_timestamp = p->m.timestamp;
Actually reorder the values. Should effectively fix ticket #8300. Signed-off-by: James Almer <jamrial@gmail.com> --- Now unconditionally propagating the field, since checking its value is not correct usage of the field. libavcodec/libdav1d.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)