[FFmpeg-devel,1/2] avcodec/vp9: don't set AVFrame.pkt_dts

Submitted by James Almer on May 1, 2017, 11:52 p.m.

Details

Message ID 20170501235215.348-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer May 1, 2017, 11:52 p.m.
decode.c already sets it to the default value.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/vp9.c | 1 -
 1 file changed, 1 deletion(-)

Comments

wm4 May 2, 2017, 5:19 p.m.
On Mon,  1 May 2017 20:52:14 -0300
James Almer <jamrial@gmail.com> wrote:

> decode.c already sets it to the default value.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/vp9.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> index 4d7310f6d4..3147ffa0db 100644
> --- a/libavcodec/vp9.c
> +++ b/libavcodec/vp9.c
> @@ -1125,7 +1125,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
>          ((AVFrame *)frame)->pkt_pts = pkt->pts;
>  FF_ENABLE_DEPRECATION_WARNINGS
>  #endif
> -        ((AVFrame *)frame)->pkt_dts = pkt->dts;
>          for (i = 0; i < 8; i++) {
>              if (s->next_refs[i].f->buf[0])
>                  ff_thread_release_buffer(avctx, &s->next_refs[i]);

Is there any possibility the "default" value is actually wrong? It
assumes a fixed delay etc.
James Almer May 2, 2017, 5:50 p.m.
On 5/2/2017 2:19 PM, wm4 wrote:
> On Mon,  1 May 2017 20:52:14 -0300
> James Almer <jamrial@gmail.com> wrote:
> 
>> decode.c already sets it to the default value.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavcodec/vp9.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
>> index 4d7310f6d4..3147ffa0db 100644
>> --- a/libavcodec/vp9.c
>> +++ b/libavcodec/vp9.c
>> @@ -1125,7 +1125,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
>>          ((AVFrame *)frame)->pkt_pts = pkt->pts;
>>  FF_ENABLE_DEPRECATION_WARNINGS
>>  #endif
>> -        ((AVFrame *)frame)->pkt_dts = pkt->dts;
>>          for (i = 0; i < 8; i++) {
>>              if (s->next_refs[i].f->buf[0])
>>                  ff_thread_release_buffer(avctx, &s->next_refs[i]);
> 
> Is there any possibility the "default" value is actually wrong? It
> assumes a fixed delay etc.

The default is set after the AVCodec.decode() call (unless the
FF_CODEC_CAP_SETS_PKT_DTS cap is set), so the above is overwritten by
it, and considering it's also pkt->dts, I assume no?
But if the above line is required, then perhaps the
FF_CODEC_CAP_SETS_PKT_DTS cap should be set.

I may be missing something, though.
wm4 May 2, 2017, 6:32 p.m.
On Tue, 2 May 2017 14:50:32 -0300
James Almer <jamrial@gmail.com> wrote:

> On 5/2/2017 2:19 PM, wm4 wrote:
> > On Mon,  1 May 2017 20:52:14 -0300
> > James Almer <jamrial@gmail.com> wrote:
> >   
> >> decode.c already sets it to the default value.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>  libavcodec/vp9.c | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
> >> index 4d7310f6d4..3147ffa0db 100644
> >> --- a/libavcodec/vp9.c
> >> +++ b/libavcodec/vp9.c
> >> @@ -1125,7 +1125,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
> >>          ((AVFrame *)frame)->pkt_pts = pkt->pts;
> >>  FF_ENABLE_DEPRECATION_WARNINGS
> >>  #endif
> >> -        ((AVFrame *)frame)->pkt_dts = pkt->dts;
> >>          for (i = 0; i < 8; i++) {
> >>              if (s->next_refs[i].f->buf[0])
> >>                  ff_thread_release_buffer(avctx, &s->next_refs[i]);  
> > 
> > Is there any possibility the "default" value is actually wrong? It
> > assumes a fixed delay etc.  
> 
> The default is set after the AVCodec.decode() call (unless the
> FF_CODEC_CAP_SETS_PKT_DTS cap is set), so the above is overwritten by
> it, and considering it's also pkt->dts, I assume no?
> But if the above line is required, then perhaps the
> FF_CODEC_CAP_SETS_PKT_DTS cap should be set.
> 
> I may be missing something, though.

Yeah, you're right, it's going to be overwritten, and it always was
this way. So the patch can't change behavior.

Just wondering whether originally there was an intention to set this
directly, i.e. whether we should just set FF_CODEC_CAP_SETS_PKT_DTS.

Patch hide | download patch | download mbox

diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 4d7310f6d4..3147ffa0db 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -1125,7 +1125,6 @@  FF_DISABLE_DEPRECATION_WARNINGS
         ((AVFrame *)frame)->pkt_pts = pkt->pts;
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
-        ((AVFrame *)frame)->pkt_dts = pkt->dts;
         for (i = 0; i < 8; i++) {
             if (s->next_refs[i].f->buf[0])
                 ff_thread_release_buffer(avctx, &s->next_refs[i]);