diff mbox series

[FFmpeg-devel] lavd/lavfi.c: Set time_base for 608 cc to container time_base.

Message ID 25c125e5-5f74-492f-1c8d-ac9035058aac@mail.de
State New
Headers show
Series [FFmpeg-devel] lavd/lavfi.c: Set time_base for 608 cc to container time_base. | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate fail Make fate failed
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate warning Make fate failed

Commit Message

Thilo Borgmann June 20, 2021, 7:08 p.m. UTC 
Hi,

when transcoding 608 cc, the cc stream frame pts is set to the same value as its 
container frame's pts. However, the time_base is always set to 1/90000 (default) 
in the initialization stage. Which causes timing issues when the container 
time_base is actually not 1/90000.

-Thilo
From 98e48c5c34edaefd5a9310dab3803259efdfd502 Mon Sep 17 00:00:00 2001
From: Yun Zhang <yunz@devvm259.frc1.facebook.com>
Date: Sun, 20 Jun 2021 21:08:00 +0200
Subject: [PATCH] lavd/lavfi.c: Set time_base for 608 cc to container
 time_base.

Suggested-By: ffmpeg@fb.com
---
 libavdevice/lavfi.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Thilo Borgmann June 28, 2021, 1:12 p.m. UTC | #1 
Hi,

> when transcoding 608 cc, the cc stream frame pts is set to the same value as its container frame's pts. However, the time_base is always set to 1/90000 (default) in the initialization stage. Which causes timing issues when the container time_base is actually not 1/90000.

identical v2 attached that also includes updates to the FATE references affected by the patch (and failed with patchwork of course).

-Thilo
From 41b619e5d5083ca59a41cca9cb515190939d6573 Mon Sep 17 00:00:00 2001
From: Yun Zhang <yunz@devvm259.frc1.facebook.com>
Date: Mon, 28 Jun 2021 15:09:42 +0200
Subject: [PATCH] lavd/lavfi.c: Set time_base for 608 cc to container
 time_base.

Suggested-By: ffmpeg@fb.com
---
 libavdevice/lavfi.c            |  3 +++
 tests/ref/fate/sub-cc          |  4 ++--
 tests/ref/fate/sub-cc-realtime | 38 +++++++---------------------------
 3 files changed, 13 insertions(+), 32 deletions(-)

diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
index 57d977e7ce..e07f20c872 100644
--- a/libavdevice/lavfi.c
+++ b/libavdevice/lavfi.c
@@ -100,6 +100,7 @@ static int create_subcc_streams(AVFormatContext *avctx)
     LavfiContext *lavfi = avctx->priv_data;
     AVStream *st;
     int stream_idx, sink_idx;
+    AVRational *time_base;
 
     for (stream_idx = 0; stream_idx < lavfi->nb_sinks; stream_idx++) {
         sink_idx = lavfi->stream_sink_map[stream_idx];
@@ -109,6 +110,8 @@ static int create_subcc_streams(AVFormatContext *avctx)
                 return AVERROR(ENOMEM);
             st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
             st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
+            time_base = &avctx->streams[stream_idx]->time_base;
+            avpriv_set_pts_info(st, 64, time_base->num, time_base->den);
         } else {
             lavfi->sink_stream_subcc_map[sink_idx] = -1;
         }
diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index 2b30a35be0..13f393cc86 100644
--- a/tests/ref/fate/sub-cc
+++ b/tests/ref/fate/sub-cc
@@ -11,5 +11,5 @@ Style: Default,Monospace,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,3,1,0,
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
-Dialogue: 0,0:00:12.36,0:00:40.83,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter{\i0} )
-Dialogue: 0,0:00:40.83,0:00:59.07,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number one
+Dialogue: 0,0:00:00.83,0:00:02.97,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter{\i0} )
+Dialogue: 0,0:00:02.97,0:00:04.34,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number one
diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index 5a95ff5cb7..169361f540 100644
--- a/tests/ref/fate/sub-cc-realtime
+++ b/tests/ref/fate/sub-cc-realtime
@@ -11,33 +11,11 @@ Style: Default,Monospace,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,3,1,0,
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
-Dialogue: 0,0:00:14.14,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}(
-Dialogue: 0,0:00:15.47,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} in
-Dialogue: 0,0:00:15.92,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inau
-Dialogue: 0,0:00:16.36,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudi
-Dialogue: 0,0:00:16.81,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudibl
-Dialogue: 0,0:00:17.25,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible 
-Dialogue: 0,0:00:17.70,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible ra
-Dialogue: 0,0:00:18.14,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radi
-Dialogue: 0,0:00:18.59,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio 
-Dialogue: 0,0:00:19.03,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio ch
-Dialogue: 0,0:00:19.48,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chat
-Dialogue: 0,0:00:19.92,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatte
-Dialogue: 0,0:00:20.36,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter
-Dialogue: 0,0:00:21.70,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter{\i0} )
-Dialogue: 0,0:00:42.61,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>>
-Dialogue: 0,0:00:43.05,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> S
-Dialogue: 0,0:00:43.50,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Saf
-Dialogue: 0,0:00:43.94,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safet
-Dialogue: 0,0:00:44.39,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety 
-Dialogue: 0,0:00:44.83,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety re
-Dialogue: 0,0:00:45.28,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety rema
-Dialogue: 0,0:00:45.72,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remain
-Dialogue: 0,0:00:46.17,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains 
-Dialogue: 0,0:00:46.61,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains ou
-Dialogue: 0,0:00:47.06,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our 
-Dialogue: 0,0:00:47.50,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our nu
-Dialogue: 0,0:00:47.95,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our numb
-Dialogue: 0,0:00:48.39,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number
-Dialogue: 0,0:00:48.84,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number o
-Dialogue: 0,0:00:49.28,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number one
+Dialogue: 0,0:00:00.97,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}(
+Dialogue: 0,0:00:01.17,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudibl
+Dialogue: 0,0:00:01.37,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chat
+Dialogue: 0,0:00:01.57,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter{\i0} )
+Dialogue: 0,0:00:03.10,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>>
+Dialogue: 0,0:00:03.30,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety rema
+Dialogue: 0,0:00:03.50,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our numb
+Dialogue: 0,0:00:03.70,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number one
Thilo Borgmann July 4, 2021, 12:23 p.m. UTC | #2 
Am 28.06.21 um 15:12 schrieb Thilo Borgmann:
> Hi,
> 
>> when transcoding 608 cc, the cc stream frame pts is set to the same value as its container frame's pts. However, the time_base is always set to 1/90000 (default) in the initialization stage. Which causes timing issues when the container time_base is actually not 1/90000.
> 
> identical v2 attached that also includes updates to the FATE references affected by the patch (and failed with patchwork of course).

Ping for review.

-Thilo
Anton Khirnov July 13, 2021, 6:23 p.m. UTC | #3 
Quoting Thilo Borgmann (2021-06-28 15:12:11)
> Hi,
> 
> > when transcoding 608 cc, the cc stream frame pts is set to the same value as its container frame's pts. However, the time_base is always set to 1/90000 (default) in the initialization stage. Which causes timing issues when the container time_base is actually not 1/90000.
> 
> identical v2 attached that also includes updates to the FATE references affected by the patch (and failed with patchwork of course).
> 
> -Thilo
> 
> From 41b619e5d5083ca59a41cca9cb515190939d6573 Mon Sep 17 00:00:00 2001
> From: Yun Zhang <yunz@devvm259.frc1.facebook.com>
> Date: Mon, 28 Jun 2021 15:09:42 +0200
> Subject: [PATCH] lavd/lavfi.c: Set time_base for 608 cc to container
>  time_base.
> 
> Suggested-By: ffmpeg@fb.com
> ---
>  libavdevice/lavfi.c            |  3 +++
>  tests/ref/fate/sub-cc          |  4 ++--
>  tests/ref/fate/sub-cc-realtime | 38 +++++++---------------------------
>  3 files changed, 13 insertions(+), 32 deletions(-)
> 
> diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
> index 57d977e7ce..e07f20c872 100644
> --- a/libavdevice/lavfi.c
> +++ b/libavdevice/lavfi.c
> @@ -100,6 +100,7 @@ static int create_subcc_streams(AVFormatContext *avctx)
>      LavfiContext *lavfi = avctx->priv_data;
>      AVStream *st;
>      int stream_idx, sink_idx;
> +    AVRational *time_base;
>  
>      for (stream_idx = 0; stream_idx < lavfi->nb_sinks; stream_idx++) {
>          sink_idx = lavfi->stream_sink_map[stream_idx];
> @@ -109,6 +110,8 @@ static int create_subcc_streams(AVFormatContext *avctx)
>                  return AVERROR(ENOMEM);
>              st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
>              st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
> +            time_base = &avctx->streams[stream_idx]->time_base;
> +            avpriv_set_pts_info(st, 64, time_base->num, time_base->den);

This look like an unnecessariily complicated way to write
st->time_base.{num,den}
Thilo Borgmann July 16, 2021, 7:45 a.m. UTC | #4 
Hi,

>>> when transcoding 608 cc, the cc stream frame pts is set to the same value as its container frame's pts. However, the time_base is always set to 1/90000 (default) in the initialization stage. Which causes timing issues when the container time_base is actually not 1/90000.
>>
>> identical v2 attached that also includes updates to the FATE references affected by the patch (and failed with patchwork of course).
>>
>> -Thilo
>>
>> From 41b619e5d5083ca59a41cca9cb515190939d6573 Mon Sep 17 00:00:00 2001
>> From: Yun Zhang <yunz@devvm259.frc1.facebook.com>
>> Date: Mon, 28 Jun 2021 15:09:42 +0200
>> Subject: [PATCH] lavd/lavfi.c: Set time_base for 608 cc to container
>>  time_base.
>>
>> Suggested-By: ffmpeg@fb.com
>> ---
>>  libavdevice/lavfi.c            |  3 +++
>>  tests/ref/fate/sub-cc          |  4 ++--
>>  tests/ref/fate/sub-cc-realtime | 38 +++++++---------------------------
>>  3 files changed, 13 insertions(+), 32 deletions(-)
>>
>> diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
>> index 57d977e7ce..e07f20c872 100644
>> --- a/libavdevice/lavfi.c
>> +++ b/libavdevice/lavfi.c
>> @@ -100,6 +100,7 @@ static int create_subcc_streams(AVFormatContext *avctx)
>>      LavfiContext *lavfi = avctx->priv_data;
>>      AVStream *st;
>>      int stream_idx, sink_idx;
>> +    AVRational *time_base;
>>  
>>      for (stream_idx = 0; stream_idx < lavfi->nb_sinks; stream_idx++) {
>>          sink_idx = lavfi->stream_sink_map[stream_idx];
>> @@ -109,6 +110,8 @@ static int create_subcc_streams(AVFormatContext *avctx)
>>                  return AVERROR(ENOMEM);
>>              st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
>>              st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
>> +            time_base = &avctx->streams[stream_idx]->time_base;
>> +            avpriv_set_pts_info(st, 64, time_base->num, time_base->den);
> 
> This look like an unnecessariily complicated way to write
> st->time_base.{num,den}

Changed patch attached.

Thanks,
Thilo
From ca110e104f7fa8b18db92ba9c79a5f0093ba9719 Mon Sep 17 00:00:00 2001
From: Yun Zhang <yunz@devvm259.frc1.facebook.com>
Date: Fri, 16 Jul 2021 09:43:54 +0200
Subject: [PATCH] lavd/lavfi.c: Set time_base for 608 cc to container
 time_base.

Suggested-By: ffmpeg@fb.com
---
 libavdevice/lavfi.c            |  4 ++++
 tests/ref/fate/sub-cc          |  4 ++--
 tests/ref/fate/sub-cc-realtime | 38 +++++++---------------------------
 3 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
index 57d977e7ce..4eb1f56f7d 100644
--- a/libavdevice/lavfi.c
+++ b/libavdevice/lavfi.c
@@ -100,6 +100,7 @@ static int create_subcc_streams(AVFormatContext *avctx)
     LavfiContext *lavfi = avctx->priv_data;
     AVStream *st;
     int stream_idx, sink_idx;
+    AVRational *time_base;
 
     for (stream_idx = 0; stream_idx < lavfi->nb_sinks; stream_idx++) {
         sink_idx = lavfi->stream_sink_map[stream_idx];
@@ -109,6 +110,9 @@ static int create_subcc_streams(AVFormatContext *avctx)
                 return AVERROR(ENOMEM);
             st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
             st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
+            time_base = &avctx->streams[stream_idx]->time_base;
+            st->time_base.num = time_base->num;
+            st->time_base.den = time_base->den;
         } else {
             lavfi->sink_stream_subcc_map[sink_idx] = -1;
         }
diff --git a/tests/ref/fate/sub-cc b/tests/ref/fate/sub-cc
index 2b30a35be0..13f393cc86 100644
--- a/tests/ref/fate/sub-cc
+++ b/tests/ref/fate/sub-cc
@@ -11,5 +11,5 @@ Style: Default,Monospace,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,3,1,0,
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
-Dialogue: 0,0:00:12.36,0:00:40.83,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter{\i0} )
-Dialogue: 0,0:00:40.83,0:00:59.07,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number one
+Dialogue: 0,0:00:00.83,0:00:02.97,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter{\i0} )
+Dialogue: 0,0:00:02.97,0:00:04.34,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number one
diff --git a/tests/ref/fate/sub-cc-realtime b/tests/ref/fate/sub-cc-realtime
index 5a95ff5cb7..169361f540 100644
--- a/tests/ref/fate/sub-cc-realtime
+++ b/tests/ref/fate/sub-cc-realtime
@@ -11,33 +11,11 @@ Style: Default,Monospace,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,3,1,0,
 
 [Events]
 Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
-Dialogue: 0,0:00:14.14,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}(
-Dialogue: 0,0:00:15.47,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} in
-Dialogue: 0,0:00:15.92,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inau
-Dialogue: 0,0:00:16.36,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudi
-Dialogue: 0,0:00:16.81,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudibl
-Dialogue: 0,0:00:17.25,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible 
-Dialogue: 0,0:00:17.70,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible ra
-Dialogue: 0,0:00:18.14,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radi
-Dialogue: 0,0:00:18.59,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio 
-Dialogue: 0,0:00:19.03,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio ch
-Dialogue: 0,0:00:19.48,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chat
-Dialogue: 0,0:00:19.92,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatte
-Dialogue: 0,0:00:20.36,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter
-Dialogue: 0,0:00:21.70,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter{\i0} )
-Dialogue: 0,0:00:42.61,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>>
-Dialogue: 0,0:00:43.05,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> S
-Dialogue: 0,0:00:43.50,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Saf
-Dialogue: 0,0:00:43.94,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safet
-Dialogue: 0,0:00:44.39,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety 
-Dialogue: 0,0:00:44.83,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety re
-Dialogue: 0,0:00:45.28,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety rema
-Dialogue: 0,0:00:45.72,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remain
-Dialogue: 0,0:00:46.17,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains 
-Dialogue: 0,0:00:46.61,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains ou
-Dialogue: 0,0:00:47.06,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our 
-Dialogue: 0,0:00:47.50,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our nu
-Dialogue: 0,0:00:47.95,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our numb
-Dialogue: 0,0:00:48.39,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number
-Dialogue: 0,0:00:48.84,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number o
-Dialogue: 0,0:00:49.28,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number one
+Dialogue: 0,0:00:00.97,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}(
+Dialogue: 0,0:00:01.17,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudibl
+Dialogue: 0,0:00:01.37,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chat
+Dialogue: 0,0:00:01.57,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,44)}({\i1} inaudible radio chatter{\i0} )
+Dialogue: 0,0:00:03.10,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>>
+Dialogue: 0,0:00:03.30,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety rema
+Dialogue: 0,0:00:03.50,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our numb
+Dialogue: 0,0:00:03.70,9:59:59.99,Default,,0,0,0,,{\an7}{\pos(38,28)}({\i1} inaudible radio chatter{\i0} )\N{\an7}{\pos(38,44)}>> Safety remains our number one
Thilo Borgmann July 25, 2021, 7:47 a.m. UTC | #5 
Am 16.07.21 um 09:45 schrieb Thilo Borgmann:
> Hi,
> 
>>>> when transcoding 608 cc, the cc stream frame pts is set to the same value as its container frame's pts. However, the time_base is always set to 1/90000 (default) in the initialization stage. Which causes timing issues when the container time_base is actually not 1/90000.
>>>
>>> identical v2 attached that also includes updates to the FATE references affected by the patch (and failed with patchwork of course).
>>>
>>> -Thilo
>>>
>>> From 41b619e5d5083ca59a41cca9cb515190939d6573 Mon Sep 17 00:00:00 2001
>>> From: Yun Zhang <yunz@devvm259.frc1.facebook.com>
>>> Date: Mon, 28 Jun 2021 15:09:42 +0200
>>> Subject: [PATCH] lavd/lavfi.c: Set time_base for 608 cc to container
>>>  time_base.
>>>
>>> Suggested-By: ffmpeg@fb.com
>>> ---
>>>  libavdevice/lavfi.c            |  3 +++
>>>  tests/ref/fate/sub-cc          |  4 ++--
>>>  tests/ref/fate/sub-cc-realtime | 38 +++++++---------------------------
>>>  3 files changed, 13 insertions(+), 32 deletions(-)
>>>
>>> diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
>>> index 57d977e7ce..e07f20c872 100644
>>> --- a/libavdevice/lavfi.c
>>> +++ b/libavdevice/lavfi.c
>>> @@ -100,6 +100,7 @@ static int create_subcc_streams(AVFormatContext *avctx)
>>>      LavfiContext *lavfi = avctx->priv_data;
>>>      AVStream *st;
>>>      int stream_idx, sink_idx;
>>> +    AVRational *time_base;
>>>  
>>>      for (stream_idx = 0; stream_idx < lavfi->nb_sinks; stream_idx++) {
>>>          sink_idx = lavfi->stream_sink_map[stream_idx];
>>> @@ -109,6 +110,8 @@ static int create_subcc_streams(AVFormatContext *avctx)
>>>                  return AVERROR(ENOMEM);
>>>              st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
>>>              st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
>>> +            time_base = &avctx->streams[stream_idx]->time_base;
>>> +            avpriv_set_pts_info(st, 64, time_base->num, time_base->den);
>>
>> This look like an unnecessariily complicated way to write
>> st->time_base.{num,den}
> 
> Changed patch attached.

Will push soon (tm) if there are no further comments.

Thanks,
Thilo
Thilo Borgmann Aug. 2, 2021, 12:17 p.m. UTC | #6 
Am 25.07.21 um 09:47 schrieb Thilo Borgmann:
> Am 16.07.21 um 09:45 schrieb Thilo Borgmann:
>> Hi,
>>
>>>>> when transcoding 608 cc, the cc stream frame pts is set to the same value as its container frame's pts. However, the time_base is always set to 1/90000 (default) in the initialization stage. Which causes timing issues when the container time_base is actually not 1/90000.
>>>>
>>>> identical v2 attached that also includes updates to the FATE references affected by the patch (and failed with patchwork of course).
>>>>
>>>> -Thilo
>>>>
>>>> From 41b619e5d5083ca59a41cca9cb515190939d6573 Mon Sep 17 00:00:00 2001
>>>> From: Yun Zhang <yunz@devvm259.frc1.facebook.com>
>>>> Date: Mon, 28 Jun 2021 15:09:42 +0200
>>>> Subject: [PATCH] lavd/lavfi.c: Set time_base for 608 cc to container
>>>>  time_base.
>>>>
>>>> Suggested-By: ffmpeg@fb.com
>>>> ---
>>>>  libavdevice/lavfi.c            |  3 +++
>>>>  tests/ref/fate/sub-cc          |  4 ++--
>>>>  tests/ref/fate/sub-cc-realtime | 38 +++++++---------------------------
>>>>  3 files changed, 13 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
>>>> index 57d977e7ce..e07f20c872 100644
>>>> --- a/libavdevice/lavfi.c
>>>> +++ b/libavdevice/lavfi.c
>>>> @@ -100,6 +100,7 @@ static int create_subcc_streams(AVFormatContext *avctx)
>>>>      LavfiContext *lavfi = avctx->priv_data;
>>>>      AVStream *st;
>>>>      int stream_idx, sink_idx;
>>>> +    AVRational *time_base;
>>>>  
>>>>      for (stream_idx = 0; stream_idx < lavfi->nb_sinks; stream_idx++) {
>>>>          sink_idx = lavfi->stream_sink_map[stream_idx];
>>>> @@ -109,6 +110,8 @@ static int create_subcc_streams(AVFormatContext *avctx)
>>>>                  return AVERROR(ENOMEM);
>>>>              st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
>>>>              st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
>>>> +            time_base = &avctx->streams[stream_idx]->time_base;
>>>> +            avpriv_set_pts_info(st, 64, time_base->num, time_base->den);
>>>
>>> This look like an unnecessariily complicated way to write
>>> st->time_base.{num,den}
>>
>> Changed patch attached.
> 
> Will push soon (tm) if there are no further comments.

Pushed, thanks!

-Thilo
diff mbox series

Patch

diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
index 57d977e..e07f20c 100644
--- a/libavdevice/lavfi.c
+++ b/libavdevice/lavfi.c
@@ -100,6 +100,7 @@  static int create_subcc_streams(AVFormatContext *avctx)
     LavfiContext *lavfi = avctx->priv_data;
     AVStream *st;
     int stream_idx, sink_idx;
+    AVRational *time_base;
 
     for (stream_idx = 0; stream_idx < lavfi->nb_sinks; stream_idx++) {
         sink_idx = lavfi->stream_sink_map[stream_idx];
@@ -109,6 +110,8 @@  static int create_subcc_streams(AVFormatContext *avctx)
                 return AVERROR(ENOMEM);
             st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
             st->codecpar->codec_type = AVMEDIA_TYPE_SUBTITLE;
+            time_base = &avctx->streams[stream_idx]->time_base;
+            avpriv_set_pts_info(st, 64, time_base->num, time_base->den);
         } else {
             lavfi->sink_stream_subcc_map[sink_idx] = -1;
         }