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 <jamr...@gmail.com>
---
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(&pkt);
+
+ 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".
