[FFmpeg-devel] [PATCH] avcodec/libdav1d: fix setting AVFrame reordered_opaque

[James Almer]

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 <jamrial at 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.
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(&reordered_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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".