diff mbox series

[FFmpeg-devel,30/30] lavc/libaomenc: pass through frame durations to encoded packets

Message ID 20221127170351.11477-30-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/30] lavc/libx264: factor out setting up the input frame | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Nov. 27, 2022, 5:03 p.m. UTC 
---
 libavcodec/libaomenc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

James Almer Nov. 27, 2022, 8:19 p.m. UTC | #1 
On 11/27/2022 2:03 PM, Anton Khirnov wrote:
> ---
>   libavcodec/libaomenc.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index bd576fdd3a..c117ca4ee6 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -1094,6 +1094,7 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
>       }
>       memcpy(pkt->data, cx_frame->buf, pkt->size);
>       pkt->pts = pkt->dts = cx_frame->pts;
> +    pkt->duration = cx_frame->duration;
>   
>       if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
>           pkt->flags |= AV_PKT_FLAG_KEY;
> @@ -1275,6 +1276,7 @@ static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
>       AOMContext *ctx = avctx->priv_data;
>       struct aom_image *rawimg = NULL;
>       int64_t timestamp = 0;
> +    unsigned long duration = 0;

Why does aom_codec_encode() take an unsigned long as argument for this 
when timestamp is int64_t...

>       int res, coded_size;
>       aom_enc_frame_flags_t flags = 0;
>   
> @@ -1287,6 +1289,8 @@ static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
>           rawimg->stride[AOM_PLANE_U] = frame->linesize[1];
>           rawimg->stride[AOM_PLANE_V] = frame->linesize[2];
>           timestamp                   = frame->pts;
> +        duration                    = frame->duration ? frame->duration :
> +                                        avctx->ticks_per_frame;

Extremely unlikely, but on 32bit targets and Win64 this could end up in 
a truncated value.

>           switch (frame->color_range) {
>           case AVCOL_RANGE_MPEG:
>               rawimg->range = AOM_CR_STUDIO_RANGE;
> @@ -1300,8 +1304,7 @@ static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
>               flags |= AOM_EFLAG_FORCE_KF;
>       }
>   
> -    res = aom_codec_encode(&ctx->encoder, rawimg, timestamp,
> -                           avctx->ticks_per_frame, flags);
> +    res = aom_codec_encode(&ctx->encoder, rawimg, timestamp, duration, flags);
>       if (res != AOM_CODEC_OK) {
>           log_encoder_error(avctx, "Error encoding frame");
>           return AVERROR_INVALIDDATA;
Anton Khirnov Jan. 4, 2023, 4:15 p.m. UTC | #2 
Quoting James Almer (2022-11-27 21:19:33)
> On 11/27/2022 2:03 PM, Anton Khirnov wrote:
> > ---
> >   libavcodec/libaomenc.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> > index bd576fdd3a..c117ca4ee6 100644
> > --- a/libavcodec/libaomenc.c
> > +++ b/libavcodec/libaomenc.c
> > @@ -1094,6 +1094,7 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
> >       }
> >       memcpy(pkt->data, cx_frame->buf, pkt->size);
> >       pkt->pts = pkt->dts = cx_frame->pts;
> > +    pkt->duration = cx_frame->duration;
> >   
> >       if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
> >           pkt->flags |= AV_PKT_FLAG_KEY;
> > @@ -1275,6 +1276,7 @@ static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
> >       AOMContext *ctx = avctx->priv_data;
> >       struct aom_image *rawimg = NULL;
> >       int64_t timestamp = 0;
> > +    unsigned long duration = 0;
> 
> Why does aom_codec_encode() take an unsigned long as argument for this 
> when timestamp is int64_t...

the ways of AOM are mysterious

> 
> >       int res, coded_size;
> >       aom_enc_frame_flags_t flags = 0;
> >   
> > @@ -1287,6 +1289,8 @@ static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
> >           rawimg->stride[AOM_PLANE_U] = frame->linesize[1];
> >           rawimg->stride[AOM_PLANE_V] = frame->linesize[2];
> >           timestamp                   = frame->pts;
> > +        duration                    = frame->duration ? frame->duration :
> > +                                        avctx->ticks_per_frame;
> 
> Extremely unlikely, but on 32bit targets and Win64 this could end up in 
> a truncated value.

I'll add a check.
diff mbox series

Patch

diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index bd576fdd3a..c117ca4ee6 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -1094,6 +1094,7 @@  static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
     }
     memcpy(pkt->data, cx_frame->buf, pkt->size);
     pkt->pts = pkt->dts = cx_frame->pts;
+    pkt->duration = cx_frame->duration;
 
     if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
         pkt->flags |= AV_PKT_FLAG_KEY;
@@ -1275,6 +1276,7 @@  static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
     AOMContext *ctx = avctx->priv_data;
     struct aom_image *rawimg = NULL;
     int64_t timestamp = 0;
+    unsigned long duration = 0;
     int res, coded_size;
     aom_enc_frame_flags_t flags = 0;
 
@@ -1287,6 +1289,8 @@  static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
         rawimg->stride[AOM_PLANE_U] = frame->linesize[1];
         rawimg->stride[AOM_PLANE_V] = frame->linesize[2];
         timestamp                   = frame->pts;
+        duration                    = frame->duration ? frame->duration :
+                                        avctx->ticks_per_frame;
         switch (frame->color_range) {
         case AVCOL_RANGE_MPEG:
             rawimg->range = AOM_CR_STUDIO_RANGE;
@@ -1300,8 +1304,7 @@  static int aom_encode(AVCodecContext *avctx, AVPacket *pkt,
             flags |= AOM_EFLAG_FORCE_KF;
     }
 
-    res = aom_codec_encode(&ctx->encoder, rawimg, timestamp,
-                           avctx->ticks_per_frame, flags);
+    res = aom_codec_encode(&ctx->encoder, rawimg, timestamp, duration, flags);
     if (res != AOM_CODEC_OK) {
         log_encoder_error(avctx, "Error encoding frame");
         return AVERROR_INVALIDDATA;