[FFmpeg-devel] [PATCH 1/5] avcodec/binkaudio: clear pts when returning more than one frame per input packet

[FFmpeg-devel] [PATCH 1/5] avcodec/binkaudio: clear pts when returning more than one frame per input packet

[James Almer]

This will be needed for the following commit, after which ff_get_buffer() will
stop setting frame->pts to AV_NOPTS_VALUE.

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

diff --git a/libavcodec/binkaudio.c b/libavcodec/binkaudio.c
index 046bf93207..f28ecba760 100644
--- a/libavcodec/binkaudio.c
+++ b/libavcodec/binkaudio.c
@@ -296,9 +296,10 @@ static int binkaudio_receive_frame(AVCodecContext *avctx, AVFrame *frame)
 {
     BinkAudioContext *s = avctx->priv_data;
     GetBitContext *gb = &s->gb;
-    int ret;
+    int new_pkt, ret;
 
 again:
+    new_pkt = !s->pkt->data;
     if (!s->pkt->data) {
         ret = ff_decode_get_packet(avctx, s->pkt);
         if (ret < 0) {
@@ -325,6 +326,8 @@ again:
         frame->nb_samples = s->frame_len;
         if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
             return ret;
+        if (!new_pkt)
+            frame->pts = AV_NOPTS_VALUE;
     }
 
     if (decode_block(s, (float **)frame->extended_data,
-- 
2.38.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/5] Revert "avcodec/decode: use a packet list to store packet properties"

[James Almer]

The idea behind last_pkt_props was to store the properties of the last packet
fed to the decoder. Any sort of queueing required by decoders that consume
several packets before they start outputting frames should be done by the
decoders in question. An example of this is in the libdav1d wrapper.

This is required to maintain its contents during flush, and for the following
commits that will fix last_pkt_props in frame threading scenarios.

This revers commit 022a12b306ab2096e6ac9fc9b149828a849d65b2.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/avcodec.c    | 10 ---------
 libavcodec/decode.c     | 47 ++++++-----------------------------------
 libavcodec/internal.h   |  1 -
 tests/ref/fate/flcl1905 |  2 +-
 4 files changed, 8 insertions(+), 52 deletions(-)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index a85d3c2309..efa76d2740 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -386,9 +386,6 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
             av_frame_unref(avci->recon_frame);
     } else {
         av_packet_unref(avci->last_pkt_props);
-        while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
-            av_packet_unref(avci->last_pkt_props);
-
         av_packet_unref(avci->in_pkt);
 
         avctx->pts_correction_last_pts =
@@ -453,13 +450,6 @@ av_cold int avcodec_close(AVCodecContext *avctx)
         av_freep(&avci->byte_buffer);
         av_frame_free(&avci->buffer_frame);
         av_packet_free(&avci->buffer_pkt);
-        if (avci->pkt_props) {
-            while (av_fifo_can_read(avci->pkt_props)) {
-                av_packet_unref(avci->last_pkt_props);
-                av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1);
-            }
-            av_fifo_freep2(&avci->pkt_props);
-        }
         av_packet_free(&avci->last_pkt_props);
 
         av_packet_free(&avci->in_pkt);
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 6be2d3d6ed..c94d9aa33c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -132,38 +132,16 @@ fail2:
     return 0;
 }
 
-#define IS_EMPTY(pkt) (!(pkt)->data)
-
-static int copy_packet_props(AVPacket *dst, const AVPacket *src)
-{
-    int ret = av_packet_copy_props(dst, src);
-    if (ret < 0)
-        return ret;
-
-    dst->size = src->size; // HACK: Needed for ff_decode_frame_props().
-    dst->data = (void*)1;  // HACK: Needed for IS_EMPTY().
-
-    return 0;
-}
-
 static int extract_packet_props(AVCodecInternal *avci, const AVPacket *pkt)
 {
-    AVPacket tmp = { 0 };
     int ret = 0;
 
-    if (IS_EMPTY(avci->last_pkt_props)) {
-        if (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) < 0)
-            return copy_packet_props(avci->last_pkt_props, pkt);
+    av_packet_unref(avci->last_pkt_props);
+    if (pkt) {
+        ret = av_packet_copy_props(avci->last_pkt_props, pkt);
+        if (!ret)
+            avci->last_pkt_props->size = pkt->size; // HACK: Needed for ff_decode_frame_props().
     }
-
-    ret = copy_packet_props(&tmp, pkt);
-    if (ret < 0)
-        return ret;
-
-    ret = av_fifo_write(avci->pkt_props, &tmp, 1);
-    if (ret < 0)
-        av_packet_unref(&tmp);
-
     return ret;
 }
 
@@ -483,7 +461,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
 
     if (ret >= pkt->size || ret < 0) {
         av_packet_unref(pkt);
-        av_packet_unref(avci->last_pkt_props);
     } else {
         int consumed = ret;
 
@@ -578,8 +555,6 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
 
     if (codec->cb_type == FF_CODEC_CB_TYPE_RECEIVE_FRAME) {
         ret = codec->cb.receive_frame(avctx, frame);
-        if (ret != AVERROR(EAGAIN))
-            av_packet_unref(avci->last_pkt_props);
     } else
         ret = decode_simple_receive_frame(avctx, frame);
 
@@ -593,12 +568,6 @@ static int decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
         return ok;
     }
 
-    if (!(codec->caps_internal & FF_CODEC_CAP_SETS_FRAME_PROPS) &&
-        IS_EMPTY(avci->last_pkt_props)) {
-        // May fail if the FIFO is empty.
-        av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1);
-    }
-
     if (!ret) {
         frame->best_effort_timestamp = guess_correct_pts(avctx,
                                                          frame->pts,
@@ -1293,7 +1262,7 @@ static int add_metadata_from_side_data(const AVPacket *avpkt, AVFrame *frame)
 
 int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
 {
-    AVPacket *pkt = avctx->internal->last_pkt_props;
+    const AVPacket *pkt = avctx->internal->last_pkt_props;
     static const struct {
         enum AVPacketSideDataType packet;
         enum AVFrameSideDataType frame;
@@ -1651,9 +1620,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 
     avci->in_pkt         = av_packet_alloc();
     avci->last_pkt_props = av_packet_alloc();
-    avci->pkt_props      = av_fifo_alloc2(1, sizeof(*avci->last_pkt_props),
-                                          AV_FIFO_FLAG_AUTO_GROW);
-    if (!avci->in_pkt || !avci->last_pkt_props || !avci->pkt_props)
+    if (!avci->in_pkt || !avci->last_pkt_props)
         return AVERROR(ENOMEM);
 
     ret = decode_bsfs_init(avctx);
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 76a6ea6bc6..a283c52e01 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -88,7 +88,6 @@ typedef struct AVCodecInternal {
      * for decoding.
      */
     AVPacket *last_pkt_props;
-    struct AVFifo *pkt_props;
 
     /**
      * temporary buffer used for encoders to store their bitstream
diff --git a/tests/ref/fate/flcl1905 b/tests/ref/fate/flcl1905
index 59cee95459..4500117993 100644
--- a/tests/ref/fate/flcl1905
+++ b/tests/ref/fate/flcl1905
@@ -189,4 +189,4 @@ frame|media_type=audio|stream_index=0|key_frame=1|pts=N/A|pts_time=N/A|pkt_dts=N
 frame|media_type=audio|stream_index=0|key_frame=1|pts=N/A|pts_time=N/A|pkt_dts=N/A|pkt_dts_time=N/A|best_effort_timestamp=N/A|best_effort_timestamp_time=N/A|pkt_duration=22528|pkt_duration_time=0.510839|duration=22528|duration_time=0.510839|pkt_pos=61436|pkt_size=744|sample_fmt=fltp|nb_samples=2048|channels=2|channel_layout=unknown
 frame|media_type=audio|stream_index=0|key_frame=1|pts=N/A|pts_time=N/A|pkt_dts=N/A|pkt_dts_time=N/A|best_effort_timestamp=N/A|best_effort_timestamp_time=N/A|pkt_duration=22528|pkt_duration_time=0.510839|duration=22528|duration_time=0.510839|pkt_pos=61436|pkt_size=372|sample_fmt=fltp|nb_samples=2048|channels=2|channel_layout=unknown
 packet|codec_type=audio|stream_index=0|pts=360448|pts_time=8.173424|dts=360448|dts_time=8.173424|duration=44|duration_time=0.000998|size=8|pos=65528|flags=K_
-frame|media_type=audio|stream_index=0|key_frame=1|pts=N/A|pts_time=N/A|pkt_dts=N/A|pkt_dts_time=N/A|best_effort_timestamp=N/A|best_effort_timestamp_time=N/A|pkt_duration=N/A|pkt_duration_time=N/A|duration=N/A|duration_time=N/A|pkt_pos=N/A|pkt_size=0|sample_fmt=fltp|nb_samples=2048|channels=2|channel_layout=unknown
+frame|media_type=audio|stream_index=0|key_frame=1|pts=360448|pts_time=8.173424|pkt_dts=N/A|pkt_dts_time=N/A|best_effort_timestamp=360448|best_effort_timestamp_time=8.173424|pkt_duration=44|pkt_duration_time=0.000998|duration=44|duration_time=0.000998|pkt_pos=65528|pkt_size=8|sample_fmt=fltp|nb_samples=2048|channels=2|channel_layout=unknown
-- 
2.38.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/5] Revert "avcodec/decode: use a packet list to store packet properties"

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1840 bytes --]

On Sun, Dec 04, 2022 at 06:52:24PM -0300, James Almer wrote:
> The idea behind last_pkt_props was to store the properties of the last packet
> fed to the decoder. Any sort of queueing required by decoders that consume
> several packets before they start outputting frames should be done by the
> decoders in question. An example of this is in the libdav1d wrapper.
> 
> This is required to maintain its contents during flush, and for the following
> commits that will fix last_pkt_props in frame threading scenarios.
> 
> This revers commit 022a12b306ab2096e6ac9fc9b149828a849d65b2.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/avcodec.c    | 10 ---------
>  libavcodec/decode.c     | 47 ++++++-----------------------------------
>  libavcodec/internal.h   |  1 -
>  tests/ref/fate/flcl1905 |  2 +-
>  4 files changed, 8 insertions(+), 52 deletions(-)

This causes a wrong looking final timestamps
for example with V-codecs/WMVP/Arlington.wmv
./ffmpeg -i Arlington.wmv  -f framecrc -

 0,       4410,       4410,        1,   115200, 0x12ea3f01
 0,       4411,       4411,        1,   115200, 0xb376241c
 1,    8105977,    8105977,    10240,    40960, 0xbbacee03
+1,    8105977,    8105977,     2048,     8192, 0x985d6153
 0,       4412,       4412,        1,   115200, 0xc214049c
 0,       4413,       4413,        1,   115200, 0xcdcddf9c
 0,       4414,       4414,        1,   115200, 0xcbd5c21d
 0,       4415,       4415,        1,   115200, 0xed32b414
-1,    8116217,    8116217,     2048,     8192, 0x985d6153


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Homeopathy is like voting while filling the ballot out with transparent ink.
Sometimes the outcome one wanted occurs. Rarely its worse than filling out
a ballot properly.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/6] avcodec/wmadec: clear pts when returning a frame during flush

[James Almer]

This will be needed for the following commit, after which ff_get_buffer() will
stop setting frame->pts to AV_NOPTS_VALUE.

Signed-off-by: James Almer <jamrial@gmail.com>
---
This one goes before '[PATCH 2/5] Revert "avcodec/decode: use a packet list to
store packet properties"'

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

diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c
index 15d6fb42b2..bc18d18222 100644
--- a/libavcodec/wmadec.c
+++ b/libavcodec/wmadec.c
@@ -845,6 +845,7 @@ static int wma_decode_superframe(AVCodecContext *avctx, AVFrame *frame,
         if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
             return ret;
 
+        frame->pts = AV_NOPTS_VALUE;
         for (i = 0; i < s->avctx->ch_layout.nb_channels; i++)
             memcpy(frame->extended_data[i], &s->frame_out[i][0],
                    frame->nb_samples * sizeof(s->frame_out[i][0]));
-- 
2.38.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/wmadec: clear pts when returning a frame during flush

[Paul B Mahol]

On 12/6/22, James Almer <jamrial@gmail.com> wrote:
> This will be needed for the following commit, after which ff_get_buffer()
> will
> stop setting frame->pts to AV_NOPTS_VALUE.

This can not be put into generic code?

>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> This one goes before '[PATCH 2/5] Revert "avcodec/decode: use a packet list
> to
> store packet properties"'
>
>  libavcodec/wmadec.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c
> index 15d6fb42b2..bc18d18222 100644
> --- a/libavcodec/wmadec.c
> +++ b/libavcodec/wmadec.c
> @@ -845,6 +845,7 @@ static int wma_decode_superframe(AVCodecContext *avctx,
> AVFrame *frame,
>          if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
>              return ret;
>
> +        frame->pts = AV_NOPTS_VALUE;
>          for (i = 0; i < s->avctx->ch_layout.nb_channels; i++)
>              memcpy(frame->extended_data[i], &s->frame_out[i][0],
>                     frame->nb_samples * sizeof(s->frame_out[i][0]));
> --
> 2.38.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/wmadec: clear pts when returning a frame during flush

[James Almer]

On 12/6/2022 5:27 AM, Paul B Mahol wrote:
> On 12/6/22, James Almer <jamrial@gmail.com> wrote:
>> This will be needed for the following commit, after which ff_get_buffer()
>> will
>> stop setting frame->pts to AV_NOPTS_VALUE.
> 
> This can not be put into generic code?

Decoders can set pts manually for frames returned after eof (which is 
essentially what I'm doing here), and the generic code can't know the 
decoder was done flushing in order to force something into a frame until 
it effectively gets no frame.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/5] avcodec/decode: don't set last_pkt_props->size

[James Almer]

Use the opaque field instead to store this value.

No functional change, but removes the hack that made the packet technically
invalid, allowing the safe usage of functions like av_packet_ref() on it
if required.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/decode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index c94d9aa33c..b184c3f55b 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -140,7 +140,7 @@ static int extract_packet_props(AVCodecInternal *avci, const AVPacket *pkt)
     if (pkt) {
         ret = av_packet_copy_props(avci->last_pkt_props, pkt);
         if (!ret)
-            avci->last_pkt_props->size = pkt->size; // HACK: Needed for ff_decode_frame_props().
+            avci->last_pkt_props->opaque = (void *)(intptr_t)pkt->size; // Needed for ff_decode_frame_props().
     }
     return ret;
 }
@@ -469,7 +469,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
         pkt->pts                  = AV_NOPTS_VALUE;
         pkt->dts                  = AV_NOPTS_VALUE;
         if (!(codec->caps_internal & FF_CODEC_CAP_SETS_FRAME_PROPS)) {
-            avci->last_pkt_props->size -= consumed; // See extract_packet_props() comment.
+            // See extract_packet_props() comment.
+            avci->last_pkt_props->opaque = (void *)((intptr_t)avci->last_pkt_props->opaque - consumed);
             avci->last_pkt_props->pts = AV_NOPTS_VALUE;
             avci->last_pkt_props->dts = AV_NOPTS_VALUE;
         }
@@ -1284,7 +1285,7 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
         frame->pts = pkt->pts;
         frame->pkt_pos      = pkt->pos;
         frame->duration     = pkt->duration;
-        frame->pkt_size     = pkt->size;
+        frame->pkt_size     = (int)(intptr_t)pkt->opaque;
 
         for (int i = 0; i < FF_ARRAY_ELEMS(sd); i++) {
             size_t size;
-- 
2.38.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/5] avcodec/pthread_frame.c: keep the last_pkt_props from worker threads in sync with the user context

[James Almer]

Making it point to the input packet results in different behavior during flush,
where its contents will be that of an empty packet instead of containing the
props from the last input packet fed to the decoder.
After this change, decoding with more than one thread will shield the same
results as using a single thread.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/pthread_frame.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index df82a4125f..3397e0cc66 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -371,6 +371,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
  */
 static int update_context_from_user(AVCodecContext *dst, AVCodecContext *src)
 {
+    int err;
+
     dst->flags          = src->flags;
 
     dst->draw_horiz_band= src->draw_horiz_band;
@@ -406,6 +408,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
                src->slice_count * sizeof(*dst->slice_offset));
     }
     dst->slice_count = src->slice_count;
+
+    av_packet_unref(dst->internal->last_pkt_props);
+    err = av_packet_copy_props(dst->internal->last_pkt_props, src->internal->last_pkt_props);
+    if (err < 0)
+        return err;
+
     return 0;
 }
 
@@ -775,6 +783,7 @@ void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
             av_freep(&ctx->slice_offset);
 
             av_buffer_unref(&ctx->internal->pool);
+            av_packet_free(&ctx->internal->last_pkt_props);
             av_freep(&ctx->internal);
             av_buffer_unref(&ctx->hw_frames_ctx);
         }
@@ -846,9 +855,9 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
         return err;
 
     if (!(p->frame = av_frame_alloc()) ||
-        !(p->avpkt = av_packet_alloc()))
+        !(p->avpkt = av_packet_alloc()) ||
+        !(copy->internal->last_pkt_props = av_packet_alloc()))
         return AVERROR(ENOMEM);
-    copy->internal->last_pkt_props = p->avpkt;
 
     if (!first)
         copy->internal->is_copy = 1;
-- 
2.38.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/5] avcodec/mpeg4videodec: duplicate the last decoded frame when the last coded frame was skipped

[James Almer]

This ensures the video stream duration is not lost after decoding.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/h263dec.c   | 13 +++++++++++++
 libavcodec/mpegvideo.h |  1 +
 2 files changed, 14 insertions(+)

diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index ac7a8521e5..0a2d7487a8 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -430,6 +430,18 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict,
                 return ret;
             s->next_picture_ptr = NULL;
 
+            *got_frame = 1;
+        } else if (s->skipped_last_frame && s->current_picture_ptr) {
+            /* Output the last picture we decoded again if the stream ended with
+             * an NVOP */
+            if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0)
+                return ret;
+            /* Copy props from the last input packet. Otherwise, props from the last
+             * returned picture would be reused */
+            if ((ret = ff_decode_frame_props(avctx, pict)) < 0)
+                return ret;
+            s->current_picture_ptr = NULL;
+
             *got_frame = 1;
         }
 
@@ -500,6 +512,7 @@ retry:
             s->extradata_parsed = 1;
         }
         ret = ff_mpeg4_decode_picture_header(avctx->priv_data, &s->gb, 0, 0);
+        s->skipped_last_frame = (ret == FRAME_SKIPPED);
     } else if (CONFIG_H263I_DECODER && s->codec_id == AV_CODEC_ID_H263I) {
         ret = ff_intel_h263_decode_picture_header(s);
     } else if (CONFIG_FLV_DECODER && s->h263_flv) {
diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
index 6440b906b1..42275953b9 100644
--- a/libavcodec/mpegvideo.h
+++ b/libavcodec/mpegvideo.h
@@ -175,6 +175,7 @@ typedef struct MpegEncContext {
     Picture *last_picture_ptr;     ///< pointer to the previous picture.
     Picture *next_picture_ptr;     ///< pointer to the next picture (for bidir pred)
     Picture *current_picture_ptr;  ///< pointer to the current picture
+    int skipped_last_frame;
     int last_dc[3];                ///< last DC values for MPEG-1
     int16_t *dc_val_base;
     int16_t *dc_val[3];            ///< used for MPEG-4 DC prediction, all 3 arrays must be continuous
-- 
2.38.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/mpeg4videodec: duplicate the last decoded frame when the last coded frame was skipped

[Andreas Rheinhardt]

James Almer:
> This ensures the video stream duration is not lost after decoding.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/h263dec.c   | 13 +++++++++++++
>  libavcodec/mpegvideo.h |  1 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
> index ac7a8521e5..0a2d7487a8 100644
> --- a/libavcodec/h263dec.c
> +++ b/libavcodec/h263dec.c
> @@ -430,6 +430,18 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict,
>                  return ret;
>              s->next_picture_ptr = NULL;
>  
> +            *got_frame = 1;
> +        } else if (s->skipped_last_frame && s->current_picture_ptr) {
> +            /* Output the last picture we decoded again if the stream ended with
> +             * an NVOP */
> +            if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0)
> +                return ret;
> +            /* Copy props from the last input packet. Otherwise, props from the last
> +             * returned picture would be reused */
> +            if ((ret = ff_decode_frame_props(avctx, pict)) < 0)
> +                return ret;
> +            s->current_picture_ptr = NULL;
> +
>              *got_frame = 1;
>          }
>  
> @@ -500,6 +512,7 @@ retry:
>              s->extradata_parsed = 1;
>          }
>          ret = ff_mpeg4_decode_picture_header(avctx->priv_data, &s->gb, 0, 0);
> +        s->skipped_last_frame = (ret == FRAME_SKIPPED);
>      } else if (CONFIG_H263I_DECODER && s->codec_id == AV_CODEC_ID_H263I) {
>          ret = ff_intel_h263_decode_picture_header(s);
>      } else if (CONFIG_FLV_DECODER && s->h263_flv) {
> diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
> index 6440b906b1..42275953b9 100644
> --- a/libavcodec/mpegvideo.h
> +++ b/libavcodec/mpegvideo.h
> @@ -175,6 +175,7 @@ typedef struct MpegEncContext {
>      Picture *last_picture_ptr;     ///< pointer to the previous picture.
>      Picture *next_picture_ptr;     ///< pointer to the next picture (for bidir pred)
>      Picture *current_picture_ptr;  ///< pointer to the current picture
> +    int skipped_last_frame;
>      int last_dc[3];                ///< last DC values for MPEG-1
>      int16_t *dc_val_base;
>      int16_t *dc_val[3];            ///< used for MPEG-4 DC prediction, all 3 arrays must be continuous

Can you give an example where this matters? And what does the spec say
about this? Is "output the last frame again" really the appropriate
response upon encountering a NVOP?

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/mpeg4videodec: duplicate the last decoded frame when the last coded frame was skipped

[James Almer]

On 12/6/2022 6:17 AM, Andreas Rheinhardt wrote:
> James Almer:
>> This ensures the video stream duration is not lost after decoding.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/h263dec.c   | 13 +++++++++++++
>>   libavcodec/mpegvideo.h |  1 +
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
>> index ac7a8521e5..0a2d7487a8 100644
>> --- a/libavcodec/h263dec.c
>> +++ b/libavcodec/h263dec.c
>> @@ -430,6 +430,18 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict,
>>                   return ret;
>>               s->next_picture_ptr = NULL;
>>   
>> +            *got_frame = 1;
>> +        } else if (s->skipped_last_frame && s->current_picture_ptr) {
>> +            /* Output the last picture we decoded again if the stream ended with
>> +             * an NVOP */
>> +            if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0)
>> +                return ret;
>> +            /* Copy props from the last input packet. Otherwise, props from the last
>> +             * returned picture would be reused */
>> +            if ((ret = ff_decode_frame_props(avctx, pict)) < 0)
>> +                return ret;
>> +            s->current_picture_ptr = NULL;
>> +
>>               *got_frame = 1;
>>           }
>>   
>> @@ -500,6 +512,7 @@ retry:
>>               s->extradata_parsed = 1;
>>           }
>>           ret = ff_mpeg4_decode_picture_header(avctx->priv_data, &s->gb, 0, 0);
>> +        s->skipped_last_frame = (ret == FRAME_SKIPPED);
>>       } else if (CONFIG_H263I_DECODER && s->codec_id == AV_CODEC_ID_H263I) {
>>           ret = ff_intel_h263_decode_picture_header(s);
>>       } else if (CONFIG_FLV_DECODER && s->h263_flv) {
>> diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
>> index 6440b906b1..42275953b9 100644
>> --- a/libavcodec/mpegvideo.h
>> +++ b/libavcodec/mpegvideo.h
>> @@ -175,6 +175,7 @@ typedef struct MpegEncContext {
>>       Picture *last_picture_ptr;     ///< pointer to the previous picture.
>>       Picture *next_picture_ptr;     ///< pointer to the next picture (for bidir pred)
>>       Picture *current_picture_ptr;  ///< pointer to the current picture
>> +    int skipped_last_frame;
>>       int last_dc[3];                ///< last DC values for MPEG-1
>>       int16_t *dc_val_base;
>>       int16_t *dc_val[3];            ///< used for MPEG-4 DC prediction, all 3 arrays must be continuous
> 
> Can you give an example where this matters? And what does the spec say
> about this? Is "output the last frame again" really the appropriate
> response upon encountering a NVOP?
> 

The reference decoder returns a frame for every packet, nvop or 
otherwise. If nvop, it will be the last decoded frame. Doing that here 
was rejected some years ago as it would make the decoder unconditionally 
cfr, and that's undesired.

Say you have a video stream that's 6 minutes and 30 seconds long, where 
the last thirty seconds worth of packets are NVOPs. The decoder will 
consume them and generate nothing, as expected, but then at EOF the last 
returned frame was that with a pts at the 6 minutes mark, resulting in 
the decoded stream being 6 minutes, and as such stream length 
information is lost.
By returning the last frame again at the end with the last input 
packet's pts, you will get the correct stream length with no difference 
in video output while remaining vfr.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/mpeg4videodec: duplicate the last decoded frame when the last coded frame was skipped

[James Almer]

On 12/4/2022 6:52 PM, James Almer wrote:
> This ensures the video stream duration is not lost after decoding.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavcodec/h263dec.c   | 13 +++++++++++++
>   libavcodec/mpegvideo.h |  1 +
>   2 files changed, 14 insertions(+)
> 
> diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
> index ac7a8521e5..0a2d7487a8 100644
> --- a/libavcodec/h263dec.c
> +++ b/libavcodec/h263dec.c
> @@ -430,6 +430,18 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict,
>                   return ret;
>               s->next_picture_ptr = NULL;
>   
> +            *got_frame = 1;
> +        } else if (s->skipped_last_frame && s->current_picture_ptr) {
> +            /* Output the last picture we decoded again if the stream ended with
> +             * an NVOP */
> +            if ((ret = av_frame_ref(pict, s->current_picture_ptr->f)) < 0)
> +                return ret;
> +            /* Copy props from the last input packet. Otherwise, props from the last
> +             * returned picture would be reused */
> +            if ((ret = ff_decode_frame_props(avctx, pict)) < 0)
> +                return ret;
> +            s->current_picture_ptr = NULL;
> +
>               *got_frame = 1;
>           }
>   
> @@ -500,6 +512,7 @@ retry:
>               s->extradata_parsed = 1;
>           }
>           ret = ff_mpeg4_decode_picture_header(avctx->priv_data, &s->gb, 0, 0);
> +        s->skipped_last_frame = (ret == FRAME_SKIPPED);
>       } else if (CONFIG_H263I_DECODER && s->codec_id == AV_CODEC_ID_H263I) {
>           ret = ff_intel_h263_decode_picture_header(s);
>       } else if (CONFIG_FLV_DECODER && s->h263_flv) {
> diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
> index 6440b906b1..42275953b9 100644
> --- a/libavcodec/mpegvideo.h
> +++ b/libavcodec/mpegvideo.h
> @@ -175,6 +175,7 @@ typedef struct MpegEncContext {
>       Picture *last_picture_ptr;     ///< pointer to the previous picture.
>       Picture *next_picture_ptr;     ///< pointer to the next picture (for bidir pred)
>       Picture *current_picture_ptr;  ///< pointer to the current picture
> +    int skipped_last_frame;
>       int last_dc[3];                ///< last DC values for MPEG-1
>       int16_t *dc_val_base;
>       int16_t *dc_val[3];            ///< used for MPEG-4 DC prediction, all 3 arrays must be continuous

Will apply.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".