diff mbox series

[FFmpeg-devel] ivfenc: write duration for frame_cnt=1.

Message ID 1614698584-36861-1-git-send-email-rsbultje@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel] ivfenc: write duration for frame_cnt=1. | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Ronald S. Bultje March 2, 2021, 3:23 p.m. UTC 
---
 libavformat/ivfenc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Andreas Rheinhardt March 2, 2021, 4:19 p.m. UTC | #1 
Ronald S. Bultje:
> ---
>  libavformat/ivfenc.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c
> index 0951f56..e22625b 100644
> --- a/libavformat/ivfenc.c
> +++ b/libavformat/ivfenc.c
> @@ -23,7 +23,7 @@
>  
>  typedef struct IVFEncContext {
>      unsigned frame_cnt;
> -    uint64_t last_pts, sum_delta_pts;
> +    uint64_t last_pts, sum_delta_pts, first_duration;
>  } IVFEncContext;
>  
>  static int ivf_init(AVFormatContext *s)
> @@ -86,6 +86,8 @@ static int ivf_write_packet(AVFormatContext *s, AVPacket *pkt)
>      avio_write(pb, pkt->data, pkt->size);
>      if (ctx->frame_cnt)
>          ctx->sum_delta_pts += pkt->pts - ctx->last_pts;
> +    else
> +        ctx->first_duration = pkt->duration;
>      ctx->frame_cnt++;
>      ctx->last_pts = pkt->pts;
>  
> @@ -97,12 +99,15 @@ static int ivf_write_trailer(AVFormatContext *s)
>      AVIOContext *pb = s->pb;
>      IVFEncContext *ctx = s->priv_data;
>  
> -    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && ctx->frame_cnt > 1) {
> +    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) &&
> +        (ctx->frame_cnt > 1 || (ctx->frame_cnt == 1 && ctx->first_duration))) {
>          int64_t end = avio_tell(pb);
>  
>          avio_seek(pb, 24, SEEK_SET);
>          // overwrite the "length" field (duration)
> -        avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1));
> +        avio_wl32(pb, ctx->frame_cnt > 1 ?
> +                  ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1) :
> +                  ctx->first_duration);
>          avio_wl32(pb, 0); // zero out unused bytes
>          avio_seek(pb, end, SEEK_SET);
>      }
> 
Shouldn't we not always use the duration of the last packet for the
duration and not only when it is the only packet?

- Andreas
Ronald S. Bultje March 2, 2021, 6:28 p.m. UTC | #2 
Hi,

On Tue, Mar 2, 2021 at 11:20 AM Andreas Rheinhardt <
andreas.rheinhardt@gmail.com> wrote:

> Ronald S. Bultje:
> > ---
> >  libavformat/ivfenc.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c
> > index 0951f56..e22625b 100644
> > --- a/libavformat/ivfenc.c
> > +++ b/libavformat/ivfenc.c
> > @@ -23,7 +23,7 @@
> >
> >  typedef struct IVFEncContext {
> >      unsigned frame_cnt;
> > -    uint64_t last_pts, sum_delta_pts;
> > +    uint64_t last_pts, sum_delta_pts, first_duration;
> >  } IVFEncContext;
> >
> >  static int ivf_init(AVFormatContext *s)
> > @@ -86,6 +86,8 @@ static int ivf_write_packet(AVFormatContext *s,
> AVPacket *pkt)
> >      avio_write(pb, pkt->data, pkt->size);
> >      if (ctx->frame_cnt)
> >          ctx->sum_delta_pts += pkt->pts - ctx->last_pts;
> > +    else
> > +        ctx->first_duration = pkt->duration;
> >      ctx->frame_cnt++;
> >      ctx->last_pts = pkt->pts;
> >
> > @@ -97,12 +99,15 @@ static int ivf_write_trailer(AVFormatContext *s)
> >      AVIOContext *pb = s->pb;
> >      IVFEncContext *ctx = s->priv_data;
> >
> > -    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && ctx->frame_cnt > 1) {
> > +    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) &&
> > +        (ctx->frame_cnt > 1 || (ctx->frame_cnt == 1 &&
> ctx->first_duration))) {
> >          int64_t end = avio_tell(pb);
> >
> >          avio_seek(pb, 24, SEEK_SET);
> >          // overwrite the "length" field (duration)
> > -        avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts /
> (ctx->frame_cnt - 1));
> > +        avio_wl32(pb, ctx->frame_cnt > 1 ?
> > +                  ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt
> - 1) :
> > +                  ctx->first_duration);
> >          avio_wl32(pb, 0); // zero out unused bytes
> >          avio_seek(pb, end, SEEK_SET);
> >      }
> >
> Shouldn't we not always use the duration of the last packet for the
> duration and not only when it is the only packet?


That assumes the duration is always set, which according to the
documentation is not guaranteed.

Ronald
Andreas Rheinhardt March 2, 2021, 6:33 p.m. UTC | #3 
Ronald S. Bultje:
> Hi,
> 
> On Tue, Mar 2, 2021 at 11:20 AM Andreas Rheinhardt <
> andreas.rheinhardt@gmail.com> wrote:
> 
>> Ronald S. Bultje:
>>> ---
>>>  libavformat/ivfenc.c | 11 ++++++++---
>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c
>>> index 0951f56..e22625b 100644
>>> --- a/libavformat/ivfenc.c
>>> +++ b/libavformat/ivfenc.c
>>> @@ -23,7 +23,7 @@
>>>
>>>  typedef struct IVFEncContext {
>>>      unsigned frame_cnt;
>>> -    uint64_t last_pts, sum_delta_pts;
>>> +    uint64_t last_pts, sum_delta_pts, first_duration;
>>>  } IVFEncContext;
>>>
>>>  static int ivf_init(AVFormatContext *s)
>>> @@ -86,6 +86,8 @@ static int ivf_write_packet(AVFormatContext *s,
>> AVPacket *pkt)
>>>      avio_write(pb, pkt->data, pkt->size);
>>>      if (ctx->frame_cnt)
>>>          ctx->sum_delta_pts += pkt->pts - ctx->last_pts;
>>> +    else
>>> +        ctx->first_duration = pkt->duration;
>>>      ctx->frame_cnt++;
>>>      ctx->last_pts = pkt->pts;
>>>
>>> @@ -97,12 +99,15 @@ static int ivf_write_trailer(AVFormatContext *s)
>>>      AVIOContext *pb = s->pb;
>>>      IVFEncContext *ctx = s->priv_data;
>>>
>>> -    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && ctx->frame_cnt > 1) {
>>> +    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) &&
>>> +        (ctx->frame_cnt > 1 || (ctx->frame_cnt == 1 &&
>> ctx->first_duration))) {
>>>          int64_t end = avio_tell(pb);
>>>
>>>          avio_seek(pb, 24, SEEK_SET);
>>>          // overwrite the "length" field (duration)
>>> -        avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts /
>> (ctx->frame_cnt - 1));
>>> +        avio_wl32(pb, ctx->frame_cnt > 1 ?
>>> +                  ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt
>> - 1) :
>>> +                  ctx->first_duration);
>>>          avio_wl32(pb, 0); // zero out unused bytes
>>>          avio_seek(pb, end, SEEK_SET);
>>>      }
>>>
>> Shouldn't we not always use the duration of the last packet for the
>> duration and not only when it is the only packet?
> 
> 
> That assumes the duration is always set, which according to the
> documentation is not guaranteed.
> 
Then why don't you use it if it is set?

- Andreas
diff mbox series

Patch

diff --git a/libavformat/ivfenc.c b/libavformat/ivfenc.c
index 0951f56..e22625b 100644
--- a/libavformat/ivfenc.c
+++ b/libavformat/ivfenc.c
@@ -23,7 +23,7 @@ 
 
 typedef struct IVFEncContext {
     unsigned frame_cnt;
-    uint64_t last_pts, sum_delta_pts;
+    uint64_t last_pts, sum_delta_pts, first_duration;
 } IVFEncContext;
 
 static int ivf_init(AVFormatContext *s)
@@ -86,6 +86,8 @@  static int ivf_write_packet(AVFormatContext *s, AVPacket *pkt)
     avio_write(pb, pkt->data, pkt->size);
     if (ctx->frame_cnt)
         ctx->sum_delta_pts += pkt->pts - ctx->last_pts;
+    else
+        ctx->first_duration = pkt->duration;
     ctx->frame_cnt++;
     ctx->last_pts = pkt->pts;
 
@@ -97,12 +99,15 @@  static int ivf_write_trailer(AVFormatContext *s)
     AVIOContext *pb = s->pb;
     IVFEncContext *ctx = s->priv_data;
 
-    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && ctx->frame_cnt > 1) {
+    if ((pb->seekable & AVIO_SEEKABLE_NORMAL) &&
+        (ctx->frame_cnt > 1 || (ctx->frame_cnt == 1 && ctx->first_duration))) {
         int64_t end = avio_tell(pb);
 
         avio_seek(pb, 24, SEEK_SET);
         // overwrite the "length" field (duration)
-        avio_wl32(pb, ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1));
+        avio_wl32(pb, ctx->frame_cnt > 1 ?
+                  ctx->frame_cnt * ctx->sum_delta_pts / (ctx->frame_cnt - 1) :
+                  ctx->first_duration);
         avio_wl32(pb, 0); // zero out unused bytes
         avio_seek(pb, end, SEEK_SET);
     }