Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 2019-10-18 10:01, Andrey Semashev wrote:
On 2019-10-18 02:16, James Almer wrote:
On 10/17/2019 7:46 PM, Andrey Semashev wrote:
On 2019-10-18 01:28, James Almer wrote:
On 10/17/2019 7:13 PM, Andrey Semashev wrote:
On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+ if (c->reordered_opaque != AV_NOPTS_VALUE) {
I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.
AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as
set in
avcodec_alloc_context3(). This field is normally used for timestamps
(despite not being the only use), and 0 is a valid timestamp, so that
can't be considered a "not set" value.
Ok, I see. I was looking at AVFrame initialization, which sets it to 0
by default.
And I'd rather not make this code unconditional. It's an allocation per
frame in a zero copy optimized decoder, and i don't want that overhead
when reordered_opaque is rarely going to be used.
Fwiw, timestamps are properly reordered by libdav1d in this same
function and propagated in the output frame. reordered_opaque is not
really needed for them.
FWIW, I'm the reporter of #8300 and our main reason for using
reordered_opaque instead of pts is that we don't want to mess with
timestamp conversion between our time base and whatever time_base
libavcodec might select for a given codec. So, we use reordered_opaque
universally, and it just happened to break with libdav1d.
Testing for AV_NOPTS_VALUE is ok in our particular case, though I had
the impression that ffmpeg is not supposed to interpret
reordered_opaque, including not assume it contains a timestamp.
Unfortunately you're right, and the check is probably not proper use of
the field, even if valid for pretty much any normal use case for it.
Will remove it, and simply deal with the malloc overhead.
You could avoid malloc completely on 64-bit systems by passing
reordered_opaque inside the pointer to user data. It's not pretty, but
it would get the job done. 32-bit systems would still have to malloc,
unfortunately.
I've posted v3 patch with what I had in mind.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 2019-10-18 02:16, James Almer wrote:
On 10/17/2019 7:46 PM, Andrey Semashev wrote:
On 2019-10-18 01:28, James Almer wrote:
On 10/17/2019 7:13 PM, Andrey Semashev wrote:
On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+ if (c->reordered_opaque != AV_NOPTS_VALUE) {
I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.
AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as set in
avcodec_alloc_context3(). This field is normally used for timestamps
(despite not being the only use), and 0 is a valid timestamp, so that
can't be considered a "not set" value.
Ok, I see. I was looking at AVFrame initialization, which sets it to 0
by default.
And I'd rather not make this code unconditional. It's an allocation per
frame in a zero copy optimized decoder, and i don't want that overhead
when reordered_opaque is rarely going to be used.
Fwiw, timestamps are properly reordered by libdav1d in this same
function and propagated in the output frame. reordered_opaque is not
really needed for them.
FWIW, I'm the reporter of #8300 and our main reason for using
reordered_opaque instead of pts is that we don't want to mess with
timestamp conversion between our time base and whatever time_base
libavcodec might select for a given codec. So, we use reordered_opaque
universally, and it just happened to break with libdav1d.
Testing for AV_NOPTS_VALUE is ok in our particular case, though I had
the impression that ffmpeg is not supposed to interpret
reordered_opaque, including not assume it contains a timestamp.
Unfortunately you're right, and the check is probably not proper use of
the field, even if valid for pretty much any normal use case for it.
Will remove it, and simply deal with the malloc overhead.
You could avoid malloc completely on 64-bit systems by passing
reordered_opaque inside the pointer to user data. It's not pretty, but
it would get the job done. 32-bit systems would still have to malloc,
unfortunately.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On Fri, Oct 18, 2019 at 1:16 AM James Almer wrote:
>
> On 10/17/2019 7:46 PM, Andrey Semashev wrote:
> > On 2019-10-18 01:28, James Almer wrote:
> >> On 10/17/2019 7:13 PM, Andrey Semashev wrote:
> >>> On 2019-10-17 23:11, James Almer wrote:
> Actually reorder the values.
>
> Should effectively fix ticket #8300.
>
> Signed-off-by: James Almer
> ---
> libavcodec/libdav1d.c | 21 -
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index 8aa248e6cd..87abdb4569 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
> *c, AVFrame *frame)
> pkt.buf = NULL;
> av_packet_unref();
> +
> +if (c->reordered_opaque != AV_NOPTS_VALUE) {
> >>>
> >>> I'm not sure this comparison is valid. The default value of
> >>> reordered_opaque is 0, and there seems to be no convention whatsoever
> >>> about what this value represents (i.e. its semantics are completely
> >>> user-defined). I think, this check needs to be removed and the code
> >>> below should execute for any reordered_opaque values.
> >>
> >> AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as set in
> >> avcodec_alloc_context3(). This field is normally used for timestamps
> >> (despite not being the only use), and 0 is a valid timestamp, so that
> >> can't be considered a "not set" value.
> >
> > Ok, I see. I was looking at AVFrame initialization, which sets it to 0
> > by default.
> >
> >> And I'd rather not make this code unconditional. It's an allocation per
> >> frame in a zero copy optimized decoder, and i don't want that overhead
> >> when reordered_opaque is rarely going to be used.
> >>
> >> Fwiw, timestamps are properly reordered by libdav1d in this same
> >> function and propagated in the output frame. reordered_opaque is not
> >> really needed for them.
> >
> > FWIW, I'm the reporter of #8300 and our main reason for using
> > reordered_opaque instead of pts is that we don't want to mess with
> > timestamp conversion between our time base and whatever time_base
> > libavcodec might select for a given codec. So, we use reordered_opaque
> > universally, and it just happened to break with libdav1d.
> >
> > Testing for AV_NOPTS_VALUE is ok in our particular case, though I had
> > the impression that ffmpeg is not supposed to interpret
> > reordered_opaque, including not assume it contains a timestamp.
>
> Unfortunately you're right, and the check is probably not proper use of
> the field, even if valid for pretty much any normal use case for it.
>
> Will remove it, and simply deal with the malloc overhead.
>
The check is fine. The only thing that needs to happen is that the
same value is passed through, and implicitily assuming that its NOPTS
(the AVCodecContext default) when no user data was allocated will
ensure just that.
Make the code further down set it to NOPTS if no userdata is present,
and its absolutely correct.
- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 10/17/2019 7:46 PM, Andrey Semashev wrote:
> On 2019-10-18 01:28, James Almer wrote:
>> On 10/17/2019 7:13 PM, Andrey Semashev wrote:
>>> On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+ if (c->reordered_opaque != AV_NOPTS_VALUE) {
>>>
>>> I'm not sure this comparison is valid. The default value of
>>> reordered_opaque is 0, and there seems to be no convention whatsoever
>>> about what this value represents (i.e. its semantics are completely
>>> user-defined). I think, this check needs to be removed and the code
>>> below should execute for any reordered_opaque values.
>>
>> AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as set in
>> avcodec_alloc_context3(). This field is normally used for timestamps
>> (despite not being the only use), and 0 is a valid timestamp, so that
>> can't be considered a "not set" value.
>
> Ok, I see. I was looking at AVFrame initialization, which sets it to 0
> by default.
>
>> And I'd rather not make this code unconditional. It's an allocation per
>> frame in a zero copy optimized decoder, and i don't want that overhead
>> when reordered_opaque is rarely going to be used.
>>
>> Fwiw, timestamps are properly reordered by libdav1d in this same
>> function and propagated in the output frame. reordered_opaque is not
>> really needed for them.
>
> FWIW, I'm the reporter of #8300 and our main reason for using
> reordered_opaque instead of pts is that we don't want to mess with
> timestamp conversion between our time base and whatever time_base
> libavcodec might select for a given codec. So, we use reordered_opaque
> universally, and it just happened to break with libdav1d.
>
> Testing for AV_NOPTS_VALUE is ok in our particular case, though I had
> the impression that ffmpeg is not supposed to interpret
> reordered_opaque, including not assume it contains a timestamp.
Unfortunately you're right, and the check is probably not proper use of
the field, even if valid for pretty much any normal use case for it.
Will remove it, and simply deal with the malloc overhead.
>
> In any case, I've sent my version of the patch without the check before
> reading the replies. Feel free to ignore it, if you want to keep the check.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
Please, ignore this patch.
On 2019-10-18 01:44, James Almer wrote:
On 10/17/2019 7:34 PM, Andrey Semashev wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Not sure why you decided to send a modified patch by another dev, and
with the author name changed, but that's not ok.
---
libavcodec/libdav1d.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..774e741db8 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -25,6 +25,7 @@
#include "libavutil/mastering_display_metadata.h"
#include "libavutil/imgutils.h"
#include "libavutil/opt.h"
+#include "libavutil/mem.h"
#include "avcodec.h"
#include "decode.h"
@@ -164,6 +165,10 @@ static void libdav1d_data_free(const uint8_t *data, void
*opaque) {
av_buffer_unref();
}
+static void libdav1d_user_data_free(const uint8_t *data, void *opaque) {
+av_free(data);
This will generate a warning about dropped const qualifier.
+}
+
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) {
+int64_t *reordered_opaque;
+
res = dav1d_data_wrap(data, pkt.data, pkt.size,
libdav1d_data_free, pkt.buf);
if (res < 0) {
av_packet_unref();
@@ -191,6 +198,22 @@ static int libdav1d_receive_frame(AVCodecContext *c,
AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+reordered_opaque = av_malloc(sizeof(int64_t));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque = c->reordered_opaque;
+res = dav1d_data_wrap_user_data(data, (const uint8_t
*)reordered_opaque,
+libdav1d_user_data_free, NULL);
+if (res < 0) {
+av_free(reordered_opaque);
+dav1d_data_unref(data);
+return res;
+}
As i said, i don't want this done unconditionally. It's not going to be
used in 99% of cases.
}
}
@@ -260,7 +283,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;
+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
// match timestamps and packet size
frame->pts = frame->best_effort_timestamp = p->m.timestamp;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 2019-10-18 01:28, James Almer wrote:
On 10/17/2019 7:13 PM, Andrey Semashev wrote:
On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+ if (c->reordered_opaque != AV_NOPTS_VALUE) {
I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.
AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as set in
avcodec_alloc_context3(). This field is normally used for timestamps
(despite not being the only use), and 0 is a valid timestamp, so that
can't be considered a "not set" value.
Ok, I see. I was looking at AVFrame initialization, which sets it to 0
by default.
And I'd rather not make this code unconditional. It's an allocation per
frame in a zero copy optimized decoder, and i don't want that overhead
when reordered_opaque is rarely going to be used.
Fwiw, timestamps are properly reordered by libdav1d in this same
function and propagated in the output frame. reordered_opaque is not
really needed for them.
FWIW, I'm the reporter of #8300 and our main reason for using
reordered_opaque instead of pts is that we don't want to mess with
timestamp conversion between our time base and whatever time_base
libavcodec might select for a given codec. So, we use reordered_opaque
universally, and it just happened to break with libdav1d.
Testing for AV_NOPTS_VALUE is ok in our particular case, though I had
the impression that ffmpeg is not supposed to interpret
reordered_opaque, including not assume it contains a timestamp.
In any case, I've sent my version of the patch without the check before
reading the replies. Feel free to ignore it, if you want to keep the check.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 10/17/2019 7:34 PM, Andrey Semashev wrote:
> Actually reorder the values.
>
> Should effectively fix ticket #8300.
Not sure why you decided to send a modified patch by another dev, and
with the author name changed, but that's not ok.
> ---
> libavcodec/libdav1d.c | 26 +-
> 1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index 8aa248e6cd..774e741db8 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -25,6 +25,7 @@
> #include "libavutil/mastering_display_metadata.h"
> #include "libavutil/imgutils.h"
> #include "libavutil/opt.h"
> +#include "libavutil/mem.h"
>
> #include "avcodec.h"
> #include "decode.h"
> @@ -164,6 +165,10 @@ static void libdav1d_data_free(const uint8_t *data, void
> *opaque) {
> av_buffer_unref();
> }
>
> +static void libdav1d_user_data_free(const uint8_t *data, void *opaque) {
> +av_free(data);
This will generate a warning about dropped const qualifier.
> +}
> +
> 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) {
> +int64_t *reordered_opaque;
> +
> res = dav1d_data_wrap(data, pkt.data, pkt.size,
> libdav1d_data_free, pkt.buf);
> if (res < 0) {
> av_packet_unref();
> @@ -191,6 +198,22 @@ static int libdav1d_receive_frame(AVCodecContext *c,
> AVFrame *frame)
>
> pkt.buf = NULL;
> av_packet_unref();
> +
> +reordered_opaque = av_malloc(sizeof(int64_t));
> +
> +if (!reordered_opaque) {
> +dav1d_data_unref(data);
> +return AVERROR(ENOMEM);
> +}
> +
> +*reordered_opaque = c->reordered_opaque;
> +res = dav1d_data_wrap_user_data(data, (const uint8_t
> *)reordered_opaque,
> +libdav1d_user_data_free, NULL);
> +if (res < 0) {
> +av_free(reordered_opaque);
> +dav1d_data_unref(data);
> +return res;
> +}
As i said, i don't want this done unconditionally. It's not going to be
used in 99% of cases.
> }
> }
>
> @@ -260,7 +283,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;
> +if (p->m.user_data.data)
> +frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
>
> // match timestamps and packet size
> frame->pts = frame->best_effort_timestamp = p->m.timestamp;
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
Actually reorder the values.
Should effectively fix ticket #8300.
---
libavcodec/libdav1d.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..774e741db8 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -25,6 +25,7 @@
#include "libavutil/mastering_display_metadata.h"
#include "libavutil/imgutils.h"
#include "libavutil/opt.h"
+#include "libavutil/mem.h"
#include "avcodec.h"
#include "decode.h"
@@ -164,6 +165,10 @@ static void libdav1d_data_free(const uint8_t *data, void
*opaque) {
av_buffer_unref();
}
+static void libdav1d_user_data_free(const uint8_t *data, void *opaque) {
+av_free(data);
+}
+
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) {
+int64_t *reordered_opaque;
+
res = dav1d_data_wrap(data, pkt.data, pkt.size,
libdav1d_data_free, pkt.buf);
if (res < 0) {
av_packet_unref();
@@ -191,6 +198,22 @@ static int libdav1d_receive_frame(AVCodecContext *c,
AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+reordered_opaque = av_malloc(sizeof(int64_t));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque = c->reordered_opaque;
+res = dav1d_data_wrap_user_data(data, (const uint8_t
*)reordered_opaque,
+libdav1d_user_data_free, NULL);
+if (res < 0) {
+av_free(reordered_opaque);
+dav1d_data_unref(data);
+return res;
+}
}
}
@@ -260,7 +283,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;
+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
// match timestamps and packet size
frame->pts = frame->best_effort_timestamp = p->m.timestamp;
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 10/17/2019 7:27 PM, Hendrik Leppkes wrote:
> On Fri, Oct 18, 2019 at 12:13 AM Andrey Semashev
> wrote:
>>
>> On 2019-10-17 23:11, James Almer wrote:
>>> Actually reorder the values.
>>>
>>> Should effectively fix ticket #8300.
>>>
>>> Signed-off-by: James Almer
>>> ---
>>> libavcodec/libdav1d.c | 21 -
>>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>>> index 8aa248e6cd..87abdb4569 100644
>>> --- a/libavcodec/libdav1d.c
>>> +++ b/libavcodec/libdav1d.c
>>> @@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c,
>>> AVFrame *frame)
>>>
>>> pkt.buf = NULL;
>>> av_packet_unref();
>>> +
>>> +if (c->reordered_opaque != AV_NOPTS_VALUE) {
>>
>> I'm not sure this comparison is valid. The default value of
>> reordered_opaque is 0, and there seems to be no convention whatsoever
>> about what this value represents (i.e. its semantics are completely
>> user-defined). I think, this check needs to be removed and the code
>> below should execute for any reordered_opaque values.
>>
>
> AV_NOPTS_VALUE is the default value of that field in AVCodecContext
> (see init_context_defaults in avcodec\options.c), so we can easily
> avoid an allocation while conveying the same value, which will happen
> a lot since the field isn't used everywhere.
> I'm not sure if AVFrame also defaults to that value, but even if it
> doesn't, further down it could just set the field to that if no value
> is provided.
AVFrame sets it to 0 in get_frame_defaults in frame.h
I guess we could effectively set it to NOPTS in libdav1d.c if no value
is set in avctx->reordered_opaque.
>
> - Hendrik
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 17.10.2019 23:59, James Almer wrote:
On 10/17/2019 6:43 PM, Andrey Semashev wrote:
On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+ if (c->reordered_opaque != AV_NOPTS_VALUE) {
+ AVBufferRef *reordered_opaque =
av_buffer_alloc(sizeof(c->reordered_opaque));
+
+ if (!reordered_opaque) {
+ dav1d_data_unref(data);
+ return AVERROR(ENOMEM);
+ }
+
+ *reordered_opaque->data = c->reordered_opaque;
This slices int64_t to uint8_t. Should memcpy instead.
Good catch. Changed locally to
*(int64_t *)reordered_opaque->data = c->reordered_opaque;
Doesn't that violate strict aliasing rules?
I'd just use memcpy.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 2019-10-18 01:27, Hendrik Leppkes wrote:
On Fri, Oct 18, 2019 at 12:13 AM Andrey Semashev
wrote:
On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c,
AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+if (c->reordered_opaque != AV_NOPTS_VALUE) {
I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.
AV_NOPTS_VALUE is the default value of that field in AVCodecContext
(see init_context_defaults in avcodec\options.c), so we can easily
avoid an allocation while conveying the same value, which will happen
a lot since the field isn't used everywhere.
I'm not sure if AVFrame also defaults to that value, but even if it
doesn't, further down it could just set the field to that if no value
is provided.
AVFrame::reordered_opaque is zero by default, I looked at
get_frame_defaults.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On Fri, Oct 18, 2019 at 12:13 AM Andrey Semashev
wrote:
>
> On 2019-10-17 23:11, James Almer wrote:
> > Actually reorder the values.
> >
> > Should effectively fix ticket #8300.
> >
> > Signed-off-by: James Almer
> > ---
> > libavcodec/libdav1d.c | 21 -
> > 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> > index 8aa248e6cd..87abdb4569 100644
> > --- a/libavcodec/libdav1d.c
> > +++ b/libavcodec/libdav1d.c
> > @@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c,
> > AVFrame *frame)
> >
> > pkt.buf = NULL;
> > av_packet_unref();
> > +
> > +if (c->reordered_opaque != AV_NOPTS_VALUE) {
>
> I'm not sure this comparison is valid. The default value of
> reordered_opaque is 0, and there seems to be no convention whatsoever
> about what this value represents (i.e. its semantics are completely
> user-defined). I think, this check needs to be removed and the code
> below should execute for any reordered_opaque values.
>
AV_NOPTS_VALUE is the default value of that field in AVCodecContext
(see init_context_defaults in avcodec\options.c), so we can easily
avoid an allocation while conveying the same value, which will happen
a lot since the field isn't used everywhere.
I'm not sure if AVFrame also defaults to that value, but even if it
doesn't, further down it could just set the field to that if no value
is provided.
- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 10/17/2019 6:57 PM, Andrey Semashev wrote:
> On 2019-10-18 00:43, Andrey Semashev wrote:
>> On 2019-10-17 23:11, James Almer wrote:
>>> Actually reorder the values.
>>>
>>> Should effectively fix ticket #8300.
>>>
>>> Signed-off-by: James Almer
>>> ---
>>> libavcodec/libdav1d.c | 21 -
>>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>>> index 8aa248e6cd..87abdb4569 100644
>>> --- a/libavcodec/libdav1d.c
>>> +++ b/libavcodec/libdav1d.c
>>> @@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
>>> *c, AVFrame *frame)
>>> pkt.buf = NULL;
>>> av_packet_unref();
>>> +
>>> + if (c->reordered_opaque != AV_NOPTS_VALUE) {
>>> + AVBufferRef *reordered_opaque =
>>> av_buffer_alloc(sizeof(c->reordered_opaque));
>>> +
>>> + if (!reordered_opaque) {
>>> + dav1d_data_unref(data);
>>> + return AVERROR(ENOMEM);
>>> + }
>>> +
>>> + *reordered_opaque->data = c->reordered_opaque;
>>
>> This slices int64_t to uint8_t. Should memcpy instead.
>
> Also, would it be possible to save at least one allocation by saving a
> pointer to int64_t or a struct containing int64_t? AVBufferRef seems
> unnecessary.
The doxy for the free() function called by libdav1d to free user data
states it needs to be thread safe, so i wasn't sure if av_malloc/av_free
on const uint8_t *user_data was a good idea, but on second thought i
guess it's meant to warn about code handling global state.
Will change to a normal alloc/free combination.
>
>>> + res = dav1d_data_wrap_user_data(data,
>>> reordered_opaque->data,
>>> + libdav1d_data_free,
>>> reordered_opaque);
>>> + if (res < 0) {
>>> + av_buffer_unref(_opaque);
>>> + dav1d_data_unref(data);
>>> + return res;
>>> + }
>>> + }
>>> }
>>> }
>>> @@ -260,7 +278,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;
>>> + if (p->m.user_data.data)
>>> + frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
>>> // match timestamps and packet size
>>> frame->pts = frame->best_effort_timestamp = p->m.timestamp;
>>>
>>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 10/17/2019 7:13 PM, Andrey Semashev wrote:
> On 2019-10-17 23:11, James Almer wrote:
>> Actually reorder the values.
>>
>> Should effectively fix ticket #8300.
>>
>> Signed-off-by: James Almer
>> ---
>> libavcodec/libdav1d.c | 21 -
>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>> index 8aa248e6cd..87abdb4569 100644
>> --- a/libavcodec/libdav1d.c
>> +++ b/libavcodec/libdav1d.c
>> @@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
>> *c, AVFrame *frame)
>> pkt.buf = NULL;
>> av_packet_unref();
>> +
>> + if (c->reordered_opaque != AV_NOPTS_VALUE) {
>
> I'm not sure this comparison is valid. The default value of
> reordered_opaque is 0, and there seems to be no convention whatsoever
> about what this value represents (i.e. its semantics are completely
> user-defined). I think, this check needs to be removed and the code
> below should execute for any reordered_opaque values.
AVCodecContext->reordered_opaque is by default AV_NOPTS_VALUE, as set in
avcodec_alloc_context3(). This field is normally used for timestamps
(despite not being the only use), and 0 is a valid timestamp, so that
can't be considered a "not set" value.
And I'd rather not make this code unconditional. It's an allocation per
frame in a zero copy optimized decoder, and i don't want that overhead
when reordered_opaque is rarely going to be used.
Fwiw, timestamps are properly reordered by libdav1d in this same
function and propagated in the output frame. reordered_opaque is not
really needed for them.
>
>> + AVBufferRef *reordered_opaque =
>> av_buffer_alloc(sizeof(c->reordered_opaque));
>> +
>> + if (!reordered_opaque) {
>> + dav1d_data_unref(data);
>> + return AVERROR(ENOMEM);
>> + }
>> +
>> + *reordered_opaque->data = c->reordered_opaque;
>> + res = dav1d_data_wrap_user_data(data,
>> reordered_opaque->data,
>> + libdav1d_data_free,
>> reordered_opaque);
>> + if (res < 0) {
>> + av_buffer_unref(_opaque);
>> + dav1d_data_unref(data);
>> + return res;
>> + }
>> + }
>> }
>> }
>> @@ -260,7 +278,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;
>> + if (p->m.user_data.data)
>> + frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
>> // match timestamps and packet size
>> frame->pts = frame->best_effort_timestamp = p->m.timestamp;
>>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c,
AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+if (c->reordered_opaque != AV_NOPTS_VALUE) {
I'm not sure this comparison is valid. The default value of
reordered_opaque is 0, and there seems to be no convention whatsoever
about what this value represents (i.e. its semantics are completely
user-defined). I think, this check needs to be removed and the code
below should execute for any reordered_opaque values.
+AVBufferRef *reordered_opaque =
av_buffer_alloc(sizeof(c->reordered_opaque));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque->data = c->reordered_opaque;
+res = dav1d_data_wrap_user_data(data, reordered_opaque->data,
+libdav1d_data_free,
reordered_opaque);
+if (res < 0) {
+av_buffer_unref(_opaque);
+dav1d_data_unref(data);
+return res;
+}
+}
}
}
@@ -260,7 +278,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;
+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
// match timestamps and packet size
frame->pts = frame->best_effort_timestamp = p->m.timestamp;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 2019-10-18 00:43, Andrey Semashev wrote:
On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
*c, AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+ if (c->reordered_opaque != AV_NOPTS_VALUE) {
+ AVBufferRef *reordered_opaque =
av_buffer_alloc(sizeof(c->reordered_opaque));
+
+ if (!reordered_opaque) {
+ dav1d_data_unref(data);
+ return AVERROR(ENOMEM);
+ }
+
+ *reordered_opaque->data = c->reordered_opaque;
This slices int64_t to uint8_t. Should memcpy instead.
Also, would it be possible to save at least one allocation by saving a
pointer to int64_t or a struct containing int64_t? AVBufferRef seems
unnecessary.
+ res = dav1d_data_wrap_user_data(data,
reordered_opaque->data,
+ libdav1d_data_free,
reordered_opaque);
+ if (res < 0) {
+ av_buffer_unref(_opaque);
+ dav1d_data_unref(data);
+ return res;
+ }
+ }
}
}
@@ -260,7 +278,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;
+ if (p->m.user_data.data)
+ frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
// match timestamps and packet size
frame->pts = frame->best_effort_timestamp = p->m.timestamp;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 10/17/2019 6:43 PM, Andrey Semashev wrote:
> On 2019-10-17 23:11, James Almer wrote:
>> Actually reorder the values.
>>
>> Should effectively fix ticket #8300.
>>
>> Signed-off-by: James Almer
>> ---
>> libavcodec/libdav1d.c | 21 -
>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
>> index 8aa248e6cd..87abdb4569 100644
>> --- a/libavcodec/libdav1d.c
>> +++ b/libavcodec/libdav1d.c
>> @@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext
>> *c, AVFrame *frame)
>> pkt.buf = NULL;
>> av_packet_unref();
>> +
>> + if (c->reordered_opaque != AV_NOPTS_VALUE) {
>> + AVBufferRef *reordered_opaque =
>> av_buffer_alloc(sizeof(c->reordered_opaque));
>> +
>> + if (!reordered_opaque) {
>> + dav1d_data_unref(data);
>> + return AVERROR(ENOMEM);
>> + }
>> +
>> + *reordered_opaque->data = c->reordered_opaque;
>
> This slices int64_t to uint8_t. Should memcpy instead.
Good catch. Changed locally to
*(int64_t *)reordered_opaque->data = c->reordered_opaque;
>
>> + res = dav1d_data_wrap_user_data(data,
>> reordered_opaque->data,
>> + libdav1d_data_free,
>> reordered_opaque);
>> + if (res < 0) {
>> + av_buffer_unref(_opaque);
>> + dav1d_data_unref(data);
>> + return res;
>> + }
>> + }
>> }
>> }
>> @@ -260,7 +278,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;
>> + if (p->m.user_data.data)
>> + frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
>> // match timestamps and packet size
>> frame->pts = frame->best_effort_timestamp = p->m.timestamp;
>>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
On 2019-10-17 23:11, James Almer wrote:
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c,
AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+if (c->reordered_opaque != AV_NOPTS_VALUE) {
+AVBufferRef *reordered_opaque =
av_buffer_alloc(sizeof(c->reordered_opaque));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque->data = c->reordered_opaque;
This slices int64_t to uint8_t. Should memcpy instead.
+res = dav1d_data_wrap_user_data(data, reordered_opaque->data,
+libdav1d_data_free,
reordered_opaque);
+if (res < 0) {
+av_buffer_unref(_opaque);
+dav1d_data_unref(data);
+return res;
+}
+}
}
}
@@ -260,7 +278,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;
+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
// match timestamps and packet size
frame->pts = frame->best_effort_timestamp = p->m.timestamp;
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque
Actually reorder the values.
Should effectively fix ticket #8300.
Signed-off-by: James Almer
---
libavcodec/libdav1d.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index 8aa248e6cd..87abdb4569 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -191,6 +191,24 @@ static int libdav1d_receive_frame(AVCodecContext *c,
AVFrame *frame)
pkt.buf = NULL;
av_packet_unref();
+
+if (c->reordered_opaque != AV_NOPTS_VALUE) {
+AVBufferRef *reordered_opaque =
av_buffer_alloc(sizeof(c->reordered_opaque));
+
+if (!reordered_opaque) {
+dav1d_data_unref(data);
+return AVERROR(ENOMEM);
+}
+
+*reordered_opaque->data = c->reordered_opaque;
+res = dav1d_data_wrap_user_data(data, reordered_opaque->data,
+libdav1d_data_free,
reordered_opaque);
+if (res < 0) {
+av_buffer_unref(_opaque);
+dav1d_data_unref(data);
+return res;
+}
+}
}
}
@@ -260,7 +278,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;
+if (p->m.user_data.data)
+frame->reordered_opaque = *(int64_t *)p->m.user_data.data;
// match timestamps and packet size
frame->pts = frame->best_effort_timestamp = p->m.timestamp;
--
2.23.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
