diff mbox series

[FFmpeg-devel,19/25] fftools/ffmpeg: report new streams from the input thread

Message ID 20220803135844.16662-19-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/25] fftools/ffmpeg_opt: move adding attachments out of open_output_file() | 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 Aug. 3, 2022, 1:58 p.m. UTC
This avoids a potential race with the demuxer adding new streams. It is
also more efficient, since we no longer do inter-thread transfers of
packets that will be just discarded.
---
 fftools/ffmpeg.c       | 22 ----------------------
 fftools/ffmpeg_demux.c | 23 +++++++++++++++++++++++
 2 files changed, 23 insertions(+), 22 deletions(-)

Comments

Andreas Rheinhardt Aug. 3, 2022, 6:47 p.m. UTC | #1
Anton Khirnov:
> This avoids a potential race with the demuxer adding new streams. It is
> also more efficient, since we no longer do inter-thread transfers of
> packets that will be just discarded.
> ---
>  fftools/ffmpeg.c       | 22 ----------------------
>  fftools/ffmpeg_demux.c | 23 +++++++++++++++++++++++
>  2 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 1393ca9c1e..37f52f0208 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -3277,21 +3277,6 @@ static int init_output_stream(OutputStream *ost, AVFrame *frame,
>      return ret;
>  }
>  
> -static void report_new_stream(int input_index, AVPacket *pkt)
> -{
> -    InputFile *file = input_files[input_index];
> -    AVStream *st = file->ctx->streams[pkt->stream_index];
> -
> -    if (pkt->stream_index < file->nb_streams_warn)
> -        return;
> -    av_log(file->ctx, AV_LOG_WARNING,
> -           "New %s stream %d:%d at pos:%"PRId64" and DTS:%ss\n",
> -           av_get_media_type_string(st->codecpar->codec_type),
> -           input_index, pkt->stream_index,
> -           pkt->pos, av_ts2timestr(pkt->dts, &st->time_base));
> -    file->nb_streams_warn = pkt->stream_index + 1;
> -}
> -
>  static int transcode_init(void)
>  {
>      int ret = 0, i, j, k;
> @@ -3831,13 +3816,6 @@ static int process_input(int file_index)
>  
>      reset_eagain();
>  
> -    /* the following test is needed in case new streams appear
> -       dynamically in stream : we ignore them */
> -    if (pkt->stream_index >= ifile->nb_streams) {
> -        report_new_stream(file_index, pkt);
> -        goto discard_packet;
> -    }
> -
>      ist = input_streams[ifile->ist_index + pkt->stream_index];
>  
>      ist->data_size += pkt->size;
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index f9bd6d47fe..66cb6ebd5f 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -20,6 +20,7 @@
>  
>  #include "libavutil/error.h"
>  #include "libavutil/time.h"
> +#include "libavutil/timestamp.h"
>  #include "libavutil/thread.h"
>  #include "libavutil/threadmessage.h"
>  
> @@ -27,6 +28,20 @@
>  
>  #include "libavformat/avformat.h"
>  
> +static void report_new_stream(InputFile *file, AVPacket *pkt)
> +{
> +    AVStream *st = file->ctx->streams[pkt->stream_index];

Missing const on st and pkt.

> +
> +    if (pkt->stream_index < file->nb_streams_warn)
> +        return;
> +    av_log(file->ctx, AV_LOG_WARNING,
> +           "New %s stream %d:%d at pos:%"PRId64" and DTS:%ss\n",
> +           av_get_media_type_string(st->codecpar->codec_type),
> +           file->index, pkt->stream_index,
> +           pkt->pos, av_ts2timestr(pkt->dts, &st->time_base));
> +    file->nb_streams_warn = pkt->stream_index + 1;
> +}
> +
>  static void *input_thread(void *arg)
>  {
>      InputFile *f = arg;
> @@ -51,6 +66,14 @@ static void *input_thread(void *arg)
>                               f->ctx->streams[pkt->stream_index]);
>          }
>  
> +        /* the following test is needed in case new streams appear
> +           dynamically in stream : we ignore them */
> +        if (pkt->stream_index >= f->nb_streams) {
> +            report_new_stream(f, pkt);
> +            av_packet_unref(pkt);
> +            continue;
> +        }
> +
>          queue_pkt = av_packet_alloc();
>          if (!queue_pkt) {
>              av_packet_unref(pkt);
Anton Khirnov Aug. 4, 2022, 8:20 a.m. UTC | #2
Quoting Andreas Rheinhardt (2022-08-03 20:47:36)
> Missing const on st

src/fftools/ffmpeg_demux.c:41:46: warning: passing argument 3 of ‘av_ts_make_time_string’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
   41 |            pkt->pos, av_ts2timestr(pkt->dts, &st->time_base));

> and pkt.

can do
Nicolas George Aug. 4, 2022, 8:23 a.m. UTC | #3
Anton Khirnov (12022-08-04):
> src/fftools/ffmpeg_demux.c:41:46: warning: passing argument 3 of ‘av_ts_make_time_string’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>    41 |            pkt->pos, av_ts2timestr(pkt->dts, &st->time_base));

av_ts_make_time_string() should have const on its *tb argument. Good
catch.

Regards,
Andreas Rheinhardt Aug. 4, 2022, 8:25 a.m. UTC | #4
Nicolas George:
> Anton Khirnov (12022-08-04):
>> src/fftools/ffmpeg_demux.c:41:46: warning: passing argument 3 of ‘av_ts_make_time_string’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>>    41 |            pkt->pos, av_ts2timestr(pkt->dts, &st->time_base));
> 
> av_ts_make_time_string() should have const on its *tb argument. Good
> catch.
> 

Or the timebase should be passed by value (as is commonly done elsewhere).

- Andreas
Nicolas George Aug. 4, 2022, 8:29 a.m. UTC | #5
Andreas Rheinhardt (12022-08-04):
> Or the timebase should be passed by value (as is commonly done elsewhere).

That would have been the best initially. But it is a public function,
changing it would be an API break, probably not worth the deprecation
dance.

Regards,
diff mbox series

Patch

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 1393ca9c1e..37f52f0208 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -3277,21 +3277,6 @@  static int init_output_stream(OutputStream *ost, AVFrame *frame,
     return ret;
 }
 
-static void report_new_stream(int input_index, AVPacket *pkt)
-{
-    InputFile *file = input_files[input_index];
-    AVStream *st = file->ctx->streams[pkt->stream_index];
-
-    if (pkt->stream_index < file->nb_streams_warn)
-        return;
-    av_log(file->ctx, AV_LOG_WARNING,
-           "New %s stream %d:%d at pos:%"PRId64" and DTS:%ss\n",
-           av_get_media_type_string(st->codecpar->codec_type),
-           input_index, pkt->stream_index,
-           pkt->pos, av_ts2timestr(pkt->dts, &st->time_base));
-    file->nb_streams_warn = pkt->stream_index + 1;
-}
-
 static int transcode_init(void)
 {
     int ret = 0, i, j, k;
@@ -3831,13 +3816,6 @@  static int process_input(int file_index)
 
     reset_eagain();
 
-    /* the following test is needed in case new streams appear
-       dynamically in stream : we ignore them */
-    if (pkt->stream_index >= ifile->nb_streams) {
-        report_new_stream(file_index, pkt);
-        goto discard_packet;
-    }
-
     ist = input_streams[ifile->ist_index + pkt->stream_index];
 
     ist->data_size += pkt->size;
diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index f9bd6d47fe..66cb6ebd5f 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -20,6 +20,7 @@ 
 
 #include "libavutil/error.h"
 #include "libavutil/time.h"
+#include "libavutil/timestamp.h"
 #include "libavutil/thread.h"
 #include "libavutil/threadmessage.h"
 
@@ -27,6 +28,20 @@ 
 
 #include "libavformat/avformat.h"
 
+static void report_new_stream(InputFile *file, AVPacket *pkt)
+{
+    AVStream *st = file->ctx->streams[pkt->stream_index];
+
+    if (pkt->stream_index < file->nb_streams_warn)
+        return;
+    av_log(file->ctx, AV_LOG_WARNING,
+           "New %s stream %d:%d at pos:%"PRId64" and DTS:%ss\n",
+           av_get_media_type_string(st->codecpar->codec_type),
+           file->index, pkt->stream_index,
+           pkt->pos, av_ts2timestr(pkt->dts, &st->time_base));
+    file->nb_streams_warn = pkt->stream_index + 1;
+}
+
 static void *input_thread(void *arg)
 {
     InputFile *f = arg;
@@ -51,6 +66,14 @@  static void *input_thread(void *arg)
                              f->ctx->streams[pkt->stream_index]);
         }
 
+        /* the following test is needed in case new streams appear
+           dynamically in stream : we ignore them */
+        if (pkt->stream_index >= f->nb_streams) {
+            report_new_stream(f, pkt);
+            av_packet_unref(pkt);
+            continue;
+        }
+
         queue_pkt = av_packet_alloc();
         if (!queue_pkt) {
             av_packet_unref(pkt);