diff mbox series

[FFmpeg-devel,2/2] encode: copy the context timebase to the packet timebase

Message ID Mg5rhXT--3-2@lynne.ee
State New
Headers show
Series [FFmpeg-devel,1/2] ffprobe: print packet timebase | 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

Lynne Aug. 2, 2021, 12:37 p.m. UTC 
(One-line) Patch attached:

> --- a/libavcodec/encode.c 
> +++ b/libavcodec/encode.c
> @@ -44,6 +44,7 @@ int ff_alloc_packet(AVCodecContext *avctx, AVPacket *avpkt, int64_t size)
>      av_fast_padded_malloc(&avctx->internal->byte_buffer,
>                            &avctx->internal->byte_buffer_size, size);
>      avpkt->data = avctx->internal->byte_buffer;
> +    avpkt->time_base = avctx->time_base;
>      if (!avpkt->data) {
>          av_log(avctx, AV_LOG_ERROR, "Failed to allocate packet of size %"PRId64"\n", size);
>          return AVERROR(ENOMEM);

I'm not sure if this is the correct place to do this.
Subject: [PATCH 2/2] encode: copy the context timebase to the packet timebase

---
 libavcodec/encode.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andreas Rheinhardt Aug. 2, 2021, 12:42 p.m. UTC | #1 
Lynne:
> (One-line) Patch attached:
> 
>> --- a/libavcodec/encode.c 
>> +++ b/libavcodec/encode.c
>> @@ -44,6 +44,7 @@ int ff_alloc_packet(AVCodecContext *avctx, AVPacket *avpkt, int64_t size)
>>       av_fast_padded_malloc(&avctx->internal->byte_buffer,
>>                             &avctx->internal->byte_buffer_size, size);
>>       avpkt->data = avctx->internal->byte_buffer;
>> +    avpkt->time_base = avctx->time_base;
>>       if (!avpkt->data) {
>>           av_log(avctx, AV_LOG_ERROR, "Failed to allocate packet of size %"PRId64"\n", size);
>>           return AVERROR(ENOMEM);
> 
> I'm not sure if this is the correct place to do this.
> 

Does this not need lots of FATE updates due to 1/2? I am quite surprised.

And yes, this is definitely the wrong place to do this: It should be
done at the end of avcodec_receive_packet() instead; not all encoders
use ff_alloc_packet() at all (only those that don't allow custom buffers
use it).

- Andreas
Andreas Rheinhardt Aug. 2, 2021, 12:55 p.m. UTC | #2 
Andreas Rheinhardt:
> Lynne:
>> (One-line) Patch attached:
>>
>>> --- a/libavcodec/encode.c 
>>> +++ b/libavcodec/encode.c
>>> @@ -44,6 +44,7 @@ int ff_alloc_packet(AVCodecContext *avctx, AVPacket *avpkt, int64_t size)
>>>       av_fast_padded_malloc(&avctx->internal->byte_buffer,
>>>                             &avctx->internal->byte_buffer_size, size);
>>>       avpkt->data = avctx->internal->byte_buffer;
>>> +    avpkt->time_base = avctx->time_base;
>>>       if (!avpkt->data) {
>>>           av_log(avctx, AV_LOG_ERROR, "Failed to allocate packet of size %"PRId64"\n", size);
>>>           return AVERROR(ENOMEM);
>>
>> I'm not sure if this is the correct place to do this.
>>
> 
> Does this not need lots of FATE updates due to 1/2? I am quite surprised.
> 
I am not surprised anymore: We do not run ffprobe on freshly encoded
packets, so it is no wonder FATE doesn't need updates after 1/2.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 98dfbfdff3..d4084b42da 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -44,6 +44,7 @@  int ff_alloc_packet(AVCodecContext *avctx, AVPacket *avpkt, int64_t size)
     av_fast_padded_malloc(&avctx->internal->byte_buffer,
                           &avctx->internal->byte_buffer_size, size);
     avpkt->data = avctx->internal->byte_buffer;
+    avpkt->time_base = avctx->time_base;
     if (!avpkt->data) {
         av_log(avctx, AV_LOG_ERROR, "Failed to allocate packet of size %"PRId64"\n", size);
         return AVERROR(ENOMEM);