[FFmpeg-devel] [PATCH 1/4] lavf: deprecate av_stream_get_end_pts()

[FFmpeg-devel] [PATCH 1/4] lavf: deprecate av_stream_get_end_pts()

[Anton Khirnov]

According to its documentation it returns "pts of the last muxed packet
+ its duration", but the value it actually returns right now is
(possibly guessed) dts after muxer-internal bitstream filtering (if
any).

This function was added for ffmpeg.c, but it is not used there anymore.
Since the value it returns is ill-defined and so inappropriate for any
serious use, deprecate it.
---
 doc/APIchanges              | 3 +++
 libavformat/avformat.h      | 3 +++
 libavformat/mux_utils.c     | 2 ++
 libavformat/version_major.h | 1 +
 4 files changed, 9 insertions(+)

diff --git a/doc/APIchanges b/doc/APIchanges
index b3ba07ee7c..bc355b59ed 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@ libavutil:     2021-04-27
 
 API changes, most recent first:
 
+2022-08-xx - xxxxxxxxxx - lavf 59 - avformat.h
+  Deprecate av_stream_get_end_pts() without replacement.
+
 2022-08-07 - e95b08a7dd - lavu 57.33.101 - pixfmt.h
   Add AV_PIX_FMT_RGBAF16{BE,LE} pixel formats.
 
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index f12fa7d904..9d46875cce 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1121,12 +1121,15 @@ typedef struct AVStream {
 
 struct AVCodecParserContext *av_stream_get_parser(const AVStream *s);
 
+#if FF_API_GET_END_PTS
 /**
  * Returns the pts of the last muxed packet + its duration
  *
  * the retuned value is undefined when used with a demuxer.
  */
+attribute_deprecated
 int64_t    av_stream_get_end_pts(const AVStream *st);
+#endif
 
 #define AV_PROGRAM_RUNNING 1
 
diff --git a/libavformat/mux_utils.c b/libavformat/mux_utils.c
index 2fa2ab5b0f..8b95fc5e7e 100644
--- a/libavformat/mux_utils.c
+++ b/libavformat/mux_utils.c
@@ -29,6 +29,7 @@
 #include "internal.h"
 #include "mux.h"
 
+#if FF_API_GET_END_PTS
 int64_t av_stream_get_end_pts(const AVStream *st)
 {
     if (cffstream(st)->priv_pts) {
@@ -36,6 +37,7 @@ int64_t av_stream_get_end_pts(const AVStream *st)
     } else
         return AV_NOPTS_VALUE;
 }
+#endif
 
 int avformat_query_codec(const AVOutputFormat *ofmt, enum AVCodecID codec_id,
                          int std_compliance)
diff --git a/libavformat/version_major.h b/libavformat/version_major.h
index 5f71298bcc..099a17873f 100644
--- a/libavformat/version_major.h
+++ b/libavformat/version_major.h
@@ -46,6 +46,7 @@
 #define FF_API_AVIOCONTEXT_WRITTEN      (LIBAVFORMAT_VERSION_MAJOR < 60)
 #define FF_HLS_TS_OPTIONS               (LIBAVFORMAT_VERSION_MAJOR < 60)
 #define FF_API_AVSTREAM_CLASS           (LIBAVFORMAT_VERSION_MAJOR > 59)
+#define FF_API_GET_END_PTS              (LIBAVFORMAT_VERSION_MAJOR < 60)
 
 
 #define FF_API_R_FRAME_RATE            1
-- 
2.35.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/4] fftools/ffmpeg: stop using av_stream_get_parser()

[Anton Khirnov]

The parser is internal to the demuxer, so its state at any particular
point is not well-defined for the caller. Additionally, it is being
accessed from the main thread, while demuxing runs in a separate thread.

Use a separate parser owned by ffmpeg.c to retrieve the same
information.

Fixes races, e.g. in:
- fate-h264-brokensps-2580
- fate-h264-extradata-reload
- fate-iv8-demux
- fate-m4v-cfr
- fate-m4v
---
 fftools/ffmpeg.c     | 33 +++++++++++++++++++++++++++++++--
 fftools/ffmpeg.h     |  9 +++++++++
 fftools/ffmpeg_opt.c |  9 +++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index ef7177fc33..580df0443a 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -610,6 +610,9 @@ static void ffmpeg_cleanup(int ret)
         avcodec_free_context(&ist->dec_ctx);
         avcodec_parameters_free(&ist->par);
 
+        avcodec_free_context(&ist->parser_dec);
+        av_parser_close(ist->parser);
+
         av_freep(&input_streams[i]);
     }
 
@@ -2421,6 +2424,15 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
         ret = av_packet_ref(avpkt, pkt);
         if (ret < 0)
             return ret;
+
+        if (ist->parser) {
+            // we do not use the parsed output, only the
+            // filled codec context fields
+            uint8_t *dummy;
+            int      dummy_size;
+            av_parser_parse2(ist->parser, ist->parser_dec, &dummy, &dummy_size,
+                             pkt->data, pkt->size, pkt->pts, pkt->dts, pkt->pos);
+        }
     }
 
     if (pkt && pkt->dts != AV_NOPTS_VALUE) {
@@ -2452,7 +2464,8 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
                 if (pkt && pkt->duration) {
                     duration_dts = av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q);
                 } else if(ist->dec_ctx->framerate.num != 0 && ist->dec_ctx->framerate.den != 0) {
-                    int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict+1 : ist->dec_ctx->ticks_per_frame;
+                    int ticks = ist->parser ? ist->parser->repeat_pict + 1 :
+                                              ist->dec_ctx->ticks_per_frame;
                     duration_dts = ((int64_t)AV_TIME_BASE *
                                     ist->dec_ctx->framerate.den * ticks) /
                                     ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame;
@@ -2555,7 +2568,8 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
             } else if (pkt->duration) {
                 ist->next_dts += av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q);
             } else if(ist->dec_ctx->framerate.num != 0) {
-                int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict + 1 : ist->dec_ctx->ticks_per_frame;
+                int ticks = ist->parser ? ist->parser->repeat_pict + 1 :
+                                          ist->dec_ctx->ticks_per_frame;
                 ist->next_dts += ((int64_t)AV_TIME_BASE *
                                   ist->dec_ctx->framerate.den * ticks) /
                                   ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame;
@@ -2688,6 +2702,21 @@ static int init_input_stream(int ist_index, char *error, int error_len)
         assert_avoptions(ist->decoder_opts);
     }
 
+    if (ist->parser) {
+        AVCodecParameters *par_tmp;
+
+        par_tmp = avcodec_parameters_alloc();
+        if (!par_tmp)
+            return AVERROR(ENOMEM);
+
+        ret = avcodec_parameters_from_context(par_tmp, ist->dec_ctx);
+        if (ret >= 0)
+            ret = avcodec_parameters_to_context(ist->parser_dec, par_tmp);
+        avcodec_parameters_free(&par_tmp);
+        if (ret < 0)
+            return ret;
+    }
+
     ist->next_pts = AV_NOPTS_VALUE;
     ist->next_dts = AV_NOPTS_VALUE;
 
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 44cc23fa84..f67a2f1d1d 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -334,6 +334,15 @@ typedef struct InputStream {
     AVFrame *decoded_frame;
     AVPacket *pkt;
 
+    /**
+     * Parser for timestamp processing.
+     */
+    AVCodecParserContext *parser;
+    /**
+     * Codec context needed by parsing.
+     */
+    AVCodecContext       *parser_dec;
+
     int64_t       prev_pkt_pts;
     int64_t       start;     /* time when read started */
     /* predicted dts of the next packet read for this stream or (when there are
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 30ca5cd609..a505c7b26f 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1065,6 +1065,15 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
             ist->top_field_first = -1;
             MATCH_PER_STREAM_OPT(top_field_first, i, ist->top_field_first, ic, st);
 
+            ist->parser = av_parser_init(ist->dec->id);
+            if (ist->parser) {
+                ist->parser->flags |= PARSER_FLAG_COMPLETE_FRAMES;
+
+                ist->parser_dec = avcodec_alloc_context3(NULL);
+                if (!ist->parser_dec)
+                    exit_program(1);
+            }
+
             break;
         case AVMEDIA_TYPE_AUDIO:
             ist->guess_layout_max = INT_MAX;
-- 
2.35.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/4] fftools/ffmpeg: stop using av_stream_get_parser()

[Andreas Rheinhardt]

Anton Khirnov:
> The parser is internal to the demuxer, so its state at any particular
> point is not well-defined for the caller. Additionally, it is being
> accessed from the main thread, while demuxing runs in a separate thread.
> 
> Use a separate parser owned by ffmpeg.c to retrieve the same
> information.
> 
> Fixes races, e.g. in:
> - fate-h264-brokensps-2580
> - fate-h264-extradata-reload
> - fate-iv8-demux
> - fate-m4v-cfr
> - fate-m4v
> ---
>  fftools/ffmpeg.c     | 33 +++++++++++++++++++++++++++++++--
>  fftools/ffmpeg.h     |  9 +++++++++
>  fftools/ffmpeg_opt.c |  9 +++++++++
>  3 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index ef7177fc33..580df0443a 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -610,6 +610,9 @@ static void ffmpeg_cleanup(int ret)
>          avcodec_free_context(&ist->dec_ctx);
>          avcodec_parameters_free(&ist->par);
>  
> +        avcodec_free_context(&ist->parser_dec);
> +        av_parser_close(ist->parser);
> +
>          av_freep(&input_streams[i]);
>      }
>  
> @@ -2421,6 +2424,15 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>          ret = av_packet_ref(avpkt, pkt);
>          if (ret < 0)
>              return ret;
> +
> +        if (ist->parser) {
> +            // we do not use the parsed output, only the
> +            // filled codec context fields
> +            uint8_t *dummy;
> +            int      dummy_size;
> +            av_parser_parse2(ist->parser, ist->parser_dec, &dummy, &dummy_size,
> +                             pkt->data, pkt->size, pkt->pts, pkt->dts, pkt->pos);
> +        }
>      }
>  
>      if (pkt && pkt->dts != AV_NOPTS_VALUE) {
> @@ -2452,7 +2464,8 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>                  if (pkt && pkt->duration) {
>                      duration_dts = av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q);
>                  } else if(ist->dec_ctx->framerate.num != 0 && ist->dec_ctx->framerate.den != 0) {
> -                    int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict+1 : ist->dec_ctx->ticks_per_frame;
> +                    int ticks = ist->parser ? ist->parser->repeat_pict + 1 :
> +                                              ist->dec_ctx->ticks_per_frame;
>                      duration_dts = ((int64_t)AV_TIME_BASE *
>                                      ist->dec_ctx->framerate.den * ticks) /
>                                      ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame;
> @@ -2555,7 +2568,8 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>              } else if (pkt->duration) {
>                  ist->next_dts += av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q);
>              } else if(ist->dec_ctx->framerate.num != 0) {
> -                int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict + 1 : ist->dec_ctx->ticks_per_frame;
> +                int ticks = ist->parser ? ist->parser->repeat_pict + 1 :
> +                                          ist->dec_ctx->ticks_per_frame;
>                  ist->next_dts += ((int64_t)AV_TIME_BASE *
>                                    ist->dec_ctx->framerate.den * ticks) /
>                                    ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame;
> @@ -2688,6 +2702,21 @@ static int init_input_stream(int ist_index, char *error, int error_len)
>          assert_avoptions(ist->decoder_opts);
>      }
>  
> +    if (ist->parser) {
> +        AVCodecParameters *par_tmp;
> +
> +        par_tmp = avcodec_parameters_alloc();
> +        if (!par_tmp)
> +            return AVERROR(ENOMEM);
> +
> +        ret = avcodec_parameters_from_context(par_tmp, ist->dec_ctx);
> +        if (ret >= 0)
> +            ret = avcodec_parameters_to_context(ist->parser_dec, par_tmp);
> +        avcodec_parameters_free(&par_tmp);
> +        if (ret < 0)
> +            return ret;
> +    }
> +
>      ist->next_pts = AV_NOPTS_VALUE;
>      ist->next_dts = AV_NOPTS_VALUE;
>  
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index 44cc23fa84..f67a2f1d1d 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -334,6 +334,15 @@ typedef struct InputStream {
>      AVFrame *decoded_frame;
>      AVPacket *pkt;
>  
> +    /**
> +     * Parser for timestamp processing.
> +     */
> +    AVCodecParserContext *parser;
> +    /**
> +     * Codec context needed by parsing.
> +     */
> +    AVCodecContext       *parser_dec;
> +
>      int64_t       prev_pkt_pts;
>      int64_t       start;     /* time when read started */
>      /* predicted dts of the next packet read for this stream or (when there are
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index 30ca5cd609..a505c7b26f 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1065,6 +1065,15 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
>              ist->top_field_first = -1;
>              MATCH_PER_STREAM_OPT(top_field_first, i, ist->top_field_first, ic, st);
>  
> +            ist->parser = av_parser_init(ist->dec->id);
> +            if (ist->parser) {
> +                ist->parser->flags |= PARSER_FLAG_COMPLETE_FRAMES;
> +
> +                ist->parser_dec = avcodec_alloc_context3(NULL);
> +                if (!ist->parser_dec)
> +                    exit_program(1);
> +            }
> +
>              break;
>          case AVMEDIA_TYPE_AUDIO:
>              ist->guess_layout_max = INT_MAX;

Are you aware that some parsers (e.g. the HEVC one) are still slow even
with the PARSER_FLAG_COMPLETE_FRAMES flag and consume non-negligible
amounts of memory?

- 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 2/4] fftools/ffmpeg: stop using av_stream_get_parser()

[James Almer]

On 8/18/2022 10:58 AM, Andreas Rheinhardt wrote:
> Anton Khirnov:
>> The parser is internal to the demuxer, so its state at any particular
>> point is not well-defined for the caller. Additionally, it is being
>> accessed from the main thread, while demuxing runs in a separate thread.
>>
>> Use a separate parser owned by ffmpeg.c to retrieve the same
>> information.
>>
>> Fixes races, e.g. in:
>> - fate-h264-brokensps-2580
>> - fate-h264-extradata-reload
>> - fate-iv8-demux
>> - fate-m4v-cfr
>> - fate-m4v
>> ---
>>   fftools/ffmpeg.c     | 33 +++++++++++++++++++++++++++++++--
>>   fftools/ffmpeg.h     |  9 +++++++++
>>   fftools/ffmpeg_opt.c |  9 +++++++++
>>   3 files changed, 49 insertions(+), 2 deletions(-)
>>
>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>> index ef7177fc33..580df0443a 100644
>> --- a/fftools/ffmpeg.c
>> +++ b/fftools/ffmpeg.c
>> @@ -610,6 +610,9 @@ static void ffmpeg_cleanup(int ret)
>>           avcodec_free_context(&ist->dec_ctx);
>>           avcodec_parameters_free(&ist->par);
>>   
>> +        avcodec_free_context(&ist->parser_dec);
>> +        av_parser_close(ist->parser);
>> +
>>           av_freep(&input_streams[i]);
>>       }
>>   
>> @@ -2421,6 +2424,15 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>>           ret = av_packet_ref(avpkt, pkt);
>>           if (ret < 0)
>>               return ret;
>> +
>> +        if (ist->parser) {
>> +            // we do not use the parsed output, only the
>> +            // filled codec context fields
>> +            uint8_t *dummy;
>> +            int      dummy_size;
>> +            av_parser_parse2(ist->parser, ist->parser_dec, &dummy, &dummy_size,
>> +                             pkt->data, pkt->size, pkt->pts, pkt->dts, pkt->pos);
>> +        }
>>       }
>>   
>>       if (pkt && pkt->dts != AV_NOPTS_VALUE) {
>> @@ -2452,7 +2464,8 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>>                   if (pkt && pkt->duration) {
>>                       duration_dts = av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q);
>>                   } else if(ist->dec_ctx->framerate.num != 0 && ist->dec_ctx->framerate.den != 0) {
>> -                    int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict+1 : ist->dec_ctx->ticks_per_frame;
>> +                    int ticks = ist->parser ? ist->parser->repeat_pict + 1 :
>> +                                              ist->dec_ctx->ticks_per_frame;
>>                       duration_dts = ((int64_t)AV_TIME_BASE *
>>                                       ist->dec_ctx->framerate.den * ticks) /
>>                                       ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame;
>> @@ -2555,7 +2568,8 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>>               } else if (pkt->duration) {
>>                   ist->next_dts += av_rescale_q(pkt->duration, ist->st->time_base, AV_TIME_BASE_Q);
>>               } else if(ist->dec_ctx->framerate.num != 0) {
>> -                int ticks= av_stream_get_parser(ist->st) ? av_stream_get_parser(ist->st)->repeat_pict + 1 : ist->dec_ctx->ticks_per_frame;
>> +                int ticks = ist->parser ? ist->parser->repeat_pict + 1 :
>> +                                          ist->dec_ctx->ticks_per_frame;
>>                   ist->next_dts += ((int64_t)AV_TIME_BASE *
>>                                     ist->dec_ctx->framerate.den * ticks) /
>>                                     ist->dec_ctx->framerate.num / ist->dec_ctx->ticks_per_frame;
>> @@ -2688,6 +2702,21 @@ static int init_input_stream(int ist_index, char *error, int error_len)
>>           assert_avoptions(ist->decoder_opts);
>>       }
>>   
>> +    if (ist->parser) {
>> +        AVCodecParameters *par_tmp;
>> +
>> +        par_tmp = avcodec_parameters_alloc();
>> +        if (!par_tmp)
>> +            return AVERROR(ENOMEM);
>> +
>> +        ret = avcodec_parameters_from_context(par_tmp, ist->dec_ctx);
>> +        if (ret >= 0)
>> +            ret = avcodec_parameters_to_context(ist->parser_dec, par_tmp);
>> +        avcodec_parameters_free(&par_tmp);
>> +        if (ret < 0)
>> +            return ret;
>> +    }
>> +
>>       ist->next_pts = AV_NOPTS_VALUE;
>>       ist->next_dts = AV_NOPTS_VALUE;
>>   
>> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>> index 44cc23fa84..f67a2f1d1d 100644
>> --- a/fftools/ffmpeg.h
>> +++ b/fftools/ffmpeg.h
>> @@ -334,6 +334,15 @@ typedef struct InputStream {
>>       AVFrame *decoded_frame;
>>       AVPacket *pkt;
>>   
>> +    /**
>> +     * Parser for timestamp processing.
>> +     */
>> +    AVCodecParserContext *parser;
>> +    /**
>> +     * Codec context needed by parsing.
>> +     */
>> +    AVCodecContext       *parser_dec;
>> +
>>       int64_t       prev_pkt_pts;
>>       int64_t       start;     /* time when read started */
>>       /* predicted dts of the next packet read for this stream or (when there are
>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>> index 30ca5cd609..a505c7b26f 100644
>> --- a/fftools/ffmpeg_opt.c
>> +++ b/fftools/ffmpeg_opt.c
>> @@ -1065,6 +1065,15 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
>>               ist->top_field_first = -1;
>>               MATCH_PER_STREAM_OPT(top_field_first, i, ist->top_field_first, ic, st);
>>   
>> +            ist->parser = av_parser_init(ist->dec->id);
>> +            if (ist->parser) {
>> +                ist->parser->flags |= PARSER_FLAG_COMPLETE_FRAMES;
>> +
>> +                ist->parser_dec = avcodec_alloc_context3(NULL);
>> +                if (!ist->parser_dec)
>> +                    exit_program(1);
>> +            }
>> +
>>               break;
>>           case AVMEDIA_TYPE_AUDIO:
>>               ist->guess_layout_max = INT_MAX;
> 
> Are you aware that some parsers (e.g. the HEVC one) are still slow even
> with the PARSER_FLAG_COMPLETE_FRAMES flag and consume non-negligible
> amounts of memory?

Is it because it uses ff_h2645_packet_split()? It used to do like 
h264_parser and feature a custom NAL splitting implementation in order 
to not bother unscaping entire slice NALs (For raw annexb) since it only 
cares about their headers.

Maybe adding a max_read_size parameter to it so it stops parsing NALs 
after that point would help. It would also allow us to use it in 
h264_parser, simplifying it considerably.

> 
> - 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".
_______________________________________________
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/4] fftools/ffmpeg: stop using av_stream_get_parser()

[Andreas Rheinhardt]

James Almer:
> On 8/18/2022 10:58 AM, Andreas Rheinhardt wrote:
>> Anton Khirnov:
>>> The parser is internal to the demuxer, so its state at any particular
>>> point is not well-defined for the caller. Additionally, it is being
>>> accessed from the main thread, while demuxing runs in a separate thread.
>>>
>>> Use a separate parser owned by ffmpeg.c to retrieve the same
>>> information.
>>>
>>> Fixes races, e.g. in:
>>> - fate-h264-brokensps-2580
>>> - fate-h264-extradata-reload
>>> - fate-iv8-demux
>>> - fate-m4v-cfr
>>> - fate-m4v
>>> ---
>>>   fftools/ffmpeg.c     | 33 +++++++++++++++++++++++++++++++--
>>>   fftools/ffmpeg.h     |  9 +++++++++
>>>   fftools/ffmpeg_opt.c |  9 +++++++++
>>>   3 files changed, 49 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
>>> index ef7177fc33..580df0443a 100644
>>> --- a/fftools/ffmpeg.c
>>> +++ b/fftools/ffmpeg.c
>>> @@ -610,6 +610,9 @@ static void ffmpeg_cleanup(int ret)
>>>           avcodec_free_context(&ist->dec_ctx);
>>>           avcodec_parameters_free(&ist->par);
>>>   +        avcodec_free_context(&ist->parser_dec);
>>> +        av_parser_close(ist->parser);
>>> +
>>>           av_freep(&input_streams[i]);
>>>       }
>>>   @@ -2421,6 +2424,15 @@ static int process_input_packet(InputStream
>>> *ist, const AVPacket *pkt, int no_eo
>>>           ret = av_packet_ref(avpkt, pkt);
>>>           if (ret < 0)
>>>               return ret;
>>> +
>>> +        if (ist->parser) {
>>> +            // we do not use the parsed output, only the
>>> +            // filled codec context fields
>>> +            uint8_t *dummy;
>>> +            int      dummy_size;
>>> +            av_parser_parse2(ist->parser, ist->parser_dec, &dummy,
>>> &dummy_size,
>>> +                             pkt->data, pkt->size, pkt->pts,
>>> pkt->dts, pkt->pos);
>>> +        }
>>>       }
>>>         if (pkt && pkt->dts != AV_NOPTS_VALUE) {
>>> @@ -2452,7 +2464,8 @@ static int process_input_packet(InputStream
>>> *ist, const AVPacket *pkt, int no_eo
>>>                   if (pkt && pkt->duration) {
>>>                       duration_dts = av_rescale_q(pkt->duration,
>>> ist->st->time_base, AV_TIME_BASE_Q);
>>>                   } else if(ist->dec_ctx->framerate.num != 0 &&
>>> ist->dec_ctx->framerate.den != 0) {
>>> -                    int ticks= av_stream_get_parser(ist->st) ?
>>> av_stream_get_parser(ist->st)->repeat_pict+1 :
>>> ist->dec_ctx->ticks_per_frame;
>>> +                    int ticks = ist->parser ?
>>> ist->parser->repeat_pict + 1 :
>>> +                                             
>>> ist->dec_ctx->ticks_per_frame;
>>>                       duration_dts = ((int64_t)AV_TIME_BASE *
>>>                                       ist->dec_ctx->framerate.den *
>>> ticks) /
>>>                                       ist->dec_ctx->framerate.num /
>>> ist->dec_ctx->ticks_per_frame;
>>> @@ -2555,7 +2568,8 @@ static int process_input_packet(InputStream
>>> *ist, const AVPacket *pkt, int no_eo
>>>               } else if (pkt->duration) {
>>>                   ist->next_dts += av_rescale_q(pkt->duration,
>>> ist->st->time_base, AV_TIME_BASE_Q);
>>>               } else if(ist->dec_ctx->framerate.num != 0) {
>>> -                int ticks= av_stream_get_parser(ist->st) ?
>>> av_stream_get_parser(ist->st)->repeat_pict + 1 :
>>> ist->dec_ctx->ticks_per_frame;
>>> +                int ticks = ist->parser ? ist->parser->repeat_pict +
>>> 1 :
>>> +                                         
>>> ist->dec_ctx->ticks_per_frame;
>>>                   ist->next_dts += ((int64_t)AV_TIME_BASE *
>>>                                     ist->dec_ctx->framerate.den *
>>> ticks) /
>>>                                     ist->dec_ctx->framerate.num /
>>> ist->dec_ctx->ticks_per_frame;
>>> @@ -2688,6 +2702,21 @@ static int init_input_stream(int ist_index,
>>> char *error, int error_len)
>>>           assert_avoptions(ist->decoder_opts);
>>>       }
>>>   +    if (ist->parser) {
>>> +        AVCodecParameters *par_tmp;
>>> +
>>> +        par_tmp = avcodec_parameters_alloc();
>>> +        if (!par_tmp)
>>> +            return AVERROR(ENOMEM);
>>> +
>>> +        ret = avcodec_parameters_from_context(par_tmp, ist->dec_ctx);
>>> +        if (ret >= 0)
>>> +            ret = avcodec_parameters_to_context(ist->parser_dec,
>>> par_tmp);
>>> +        avcodec_parameters_free(&par_tmp);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +    }
>>> +
>>>       ist->next_pts = AV_NOPTS_VALUE;
>>>       ist->next_dts = AV_NOPTS_VALUE;
>>>   diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
>>> index 44cc23fa84..f67a2f1d1d 100644
>>> --- a/fftools/ffmpeg.h
>>> +++ b/fftools/ffmpeg.h
>>> @@ -334,6 +334,15 @@ typedef struct InputStream {
>>>       AVFrame *decoded_frame;
>>>       AVPacket *pkt;
>>>   +    /**
>>> +     * Parser for timestamp processing.
>>> +     */
>>> +    AVCodecParserContext *parser;
>>> +    /**
>>> +     * Codec context needed by parsing.
>>> +     */
>>> +    AVCodecContext       *parser_dec;
>>> +
>>>       int64_t       prev_pkt_pts;
>>>       int64_t       start;     /* time when read started */
>>>       /* predicted dts of the next packet read for this stream or
>>> (when there are
>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>>> index 30ca5cd609..a505c7b26f 100644
>>> --- a/fftools/ffmpeg_opt.c
>>> +++ b/fftools/ffmpeg_opt.c
>>> @@ -1065,6 +1065,15 @@ static void add_input_streams(OptionsContext
>>> *o, AVFormatContext *ic)
>>>               ist->top_field_first = -1;
>>>               MATCH_PER_STREAM_OPT(top_field_first, i,
>>> ist->top_field_first, ic, st);
>>>   +            ist->parser = av_parser_init(ist->dec->id);
>>> +            if (ist->parser) {
>>> +                ist->parser->flags |= PARSER_FLAG_COMPLETE_FRAMES;
>>> +
>>> +                ist->parser_dec = avcodec_alloc_context3(NULL);
>>> +                if (!ist->parser_dec)
>>> +                    exit_program(1);
>>> +            }
>>> +
>>>               break;
>>>           case AVMEDIA_TYPE_AUDIO:
>>>               ist->guess_layout_max = INT_MAX;
>>
>> Are you aware that some parsers (e.g. the HEVC one) are still slow even
>> with the PARSER_FLAG_COMPLETE_FRAMES flag and consume non-negligible
>> amounts of memory?
> 
> Is it because it uses ff_h2645_packet_split()? It used to do like
> h264_parser and feature a custom NAL splitting implementation in order
> to not bother unscaping entire slice NALs (For raw annexb) since it only
> cares about their headers.
> 
> Maybe adding a max_read_size parameter to it so it stops parsing NALs
> after that point would help. It would also allow us to use it in
> h264_parser, simplifying it considerably.
> 

1. It is because ff_h2645_packet_split().
2. And even if unescaping is stopped after the first few bytes (which is
e.g. not possible for SEIs as it might be the last SEI message that
contains important information), one would still need a buffer to hold
the complete input packet (because it might be that it contains a
billion little NALUs).
3. The simplifications due to ceb085906623dcc64b82151d433d1be2b0c71f73
were in part due to the HEVC parser retaining a list of NALUs instead of
only retaining the current NALU (it only looks at the current NALU
anyway). The H.264 parser does not make this mistake, so any
simplifications (if any) will be smaller.

- 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 2/4] fftools/ffmpeg: stop using av_stream_get_parser()

[Anton Khirnov]

Quoting Andreas Rheinhardt (2022-08-18 15:58:37)
> 
> Are you aware that some parsers (e.g. the HEVC one) are still slow even
> with the PARSER_FLAG_COMPLETE_FRAMES flag and consume non-negligible
> amounts of memory?

Alternative suggestions are welcome.

I would also be extremely happy to drop this disgusting hack, but I
expect that would break some sample from bug 913458 when the stars are
not right.

-- 
Anton Khirnov
_______________________________________________
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/4] fftools/ffmpeg: stop using av_stream_get_parser()

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2022-08-18 15:58:37)
>>
>> Are you aware that some parsers (e.g. the HEVC one) are still slow even
>> with the PARSER_FLAG_COMPLETE_FRAMES flag and consume non-negligible
>> amounts of memory?
> 
> Alternative suggestions are welcome.
> 

Can't we pass the relevant information via DemuxMsg to the main thread?

> I would also be extremely happy to drop this disgusting hack, but I
> expect that would break some sample from bug 913458 when the stars are
> not right.
> 

_______________________________________________
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/4] fftools/ffmpeg: stop using av_stream_get_parser()

[Michael Niedermayer]

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

On Thu, Aug 18, 2022 at 03:46:03PM +0200, Anton Khirnov wrote:
> The parser is internal to the demuxer, so its state at any particular
> point is not well-defined for the caller. Additionally, it is being
> accessed from the main thread, while demuxing runs in a separate thread.
> 
> Use a separate parser owned by ffmpeg.c to retrieve the same
> information.
> 
> Fixes races, e.g. in:
> - fate-h264-brokensps-2580
> - fate-h264-extradata-reload
> - fate-iv8-demux
> - fate-m4v-cfr
> - fate-m4v
> ---
>  fftools/ffmpeg.c     | 33 +++++++++++++++++++++++++++++++--
>  fftools/ffmpeg.h     |  9 +++++++++
>  fftools/ffmpeg_opt.c |  9 +++++++++
>  3 files changed, 49 insertions(+), 2 deletions(-)

This segfaults:
./ffmpeg -max_alloc 100000 -i ~/tickets/2950/mpeg2_fuzz.mpg  -max_muxing_queue_size 8000 -f null -

==25621== Invalid read of size 4
==25621==    at 0x2F3018: add_input_streams (in ffmpeg_g)
==25621==    by 0x2F3BB0: open_input_file (in ffmpeg_g)
==25621==    by 0x2FA93B: ffmpeg_parse_options (in ffmpeg_g)
==25621==    by 0x2E74B4: main (in ffmpeg_g)
==25621==  Address 0x14 is not stack'd, malloc'd or (recently) free'd

thx

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

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell

[-- 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 3/4] lavf: deprecate av_stream_get_parser()

[Anton Khirnov]

It retrieves an AVStream's internal parser, whose state is not
well-defined from the caller's point of view. This function was added
for ffmpeg.c, which is no longer using it.

As there is no valid use for this function, deprecate it without
replacement.
---
 doc/APIchanges              | 3 ++-
 libavformat/avformat.h      | 3 +++
 libavformat/demux_utils.c   | 2 ++
 libavformat/version_major.h | 1 +
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index bc355b59ed..078f9de223 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,7 +15,8 @@ libavutil:     2021-04-27
 API changes, most recent first:
 
 2022-08-xx - xxxxxxxxxx - lavf 59 - avformat.h
-  Deprecate av_stream_get_end_pts() without replacement.
+  Deprecate av_stream_get_end_pts() and av_stream_get_parser()
+  without replacement.
 
 2022-08-07 - e95b08a7dd - lavu 57.33.101 - pixfmt.h
   Add AV_PIX_FMT_RGBAF16{BE,LE} pixel formats.
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 9d46875cce..7c36972735 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1119,7 +1119,10 @@ typedef struct AVStream {
     int pts_wrap_bits;
 } AVStream;
 
+#if FF_API_STREAM_GET_PARSER
+attribute_deprecated
 struct AVCodecParserContext *av_stream_get_parser(const AVStream *s);
+#endif
 
 #if FF_API_GET_END_PTS
 /**
diff --git a/libavformat/demux_utils.c b/libavformat/demux_utils.c
index 56cc6e15d8..d8cbcb630c 100644
--- a/libavformat/demux_utils.c
+++ b/libavformat/demux_utils.c
@@ -29,10 +29,12 @@
 #include "demux.h"
 #include "internal.h"
 
+#if FF_API_STREAM_GET_PARSER
 struct AVCodecParserContext *av_stream_get_parser(const AVStream *st)
 {
     return cffstream(st)->parser;
 }
+#endif
 
 void avpriv_stream_set_need_parsing(AVStream *st, enum AVStreamParseType type)
 {
diff --git a/libavformat/version_major.h b/libavformat/version_major.h
index 099a17873f..2747767768 100644
--- a/libavformat/version_major.h
+++ b/libavformat/version_major.h
@@ -47,6 +47,7 @@
 #define FF_HLS_TS_OPTIONS               (LIBAVFORMAT_VERSION_MAJOR < 60)
 #define FF_API_AVSTREAM_CLASS           (LIBAVFORMAT_VERSION_MAJOR > 59)
 #define FF_API_GET_END_PTS              (LIBAVFORMAT_VERSION_MAJOR < 60)
+#define FF_API_STREAM_GET_PARSER        (LIBAVFORMAT_VERSION_MAJOR < 60)
 
 
 #define FF_API_R_FRAME_RATE            1
-- 
2.35.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/4] fftools/ffmpeg: call av_guess_frame_rate() when opening the file

[Anton Khirnov]

It is currently called when configuring the filter, which races with the
demuxer thread.
---
 fftools/ffmpeg.h        | 2 ++
 fftools/ffmpeg_filter.c | 2 +-
 fftools/ffmpeg_opt.c    | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index f67a2f1d1d..ee3ae8120e 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -343,6 +343,8 @@ typedef struct InputStream {
      */
     AVCodecContext       *parser_dec;
 
+    AVRational framerate_guessed;
+
     int64_t       prev_pkt_pts;
     int64_t       start;     /* time when read started */
     /* predicted dts of the next packet read for this stream or (when there are
diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index f9ae76f76d..16622e49c1 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -738,7 +738,7 @@ static int configure_input_video_filter(FilterGraph *fg, InputFilter *ifilter,
     }
 
     if (!fr.num)
-        fr = av_guess_frame_rate(input_files[ist->file_index]->ctx, ist->st, NULL);
+        fr = ist->framerate_guessed;
 
     if (ist->dec_ctx->codec_type == AVMEDIA_TYPE_SUBTITLE) {
         ret = sub2video_prepare(ist, ifilter);
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index a505c7b26f..0810bd0b9e 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -1074,6 +1074,8 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
                     exit_program(1);
             }
 
+            ist->framerate_guessed = av_guess_frame_rate(ic, st, NULL);
+
             break;
         case AVMEDIA_TYPE_AUDIO:
             ist->guess_layout_max = INT_MAX;
-- 
2.35.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".