[FFmpeg-devel] [PATCH v2] avformat/movenc: split empty text sample when duration overflow

[Jun Li]

On Sun, Sep 8, 2019 at 5:59 PM Jun Li <junli1026 at gmail.com> wrote:

>
>
> On Sun, Sep 8, 2019 at 5:51 PM Jun Li <junli1026 at gmail.com> wrote:
>
>> Fix #7637
>> One empty/end sample is created and inserted between two caption lines
>> when there is a gap.
>> This patch is to split the sample into multiple ones when its duration is
>> too long (>= INT_MAX).
>> ---
>>  libavformat/movenc.c                          | 24 ++++++++++++++-----
>>  tests/fate/subtitles.mak                      |  6 +++++
>>  tests/ref/fate/binsub-movtextenc-long-dur     |  1 +
>>  .../fate/binsub-movtextenc-long-dur-timebase  |  1 +
>>  4 files changed, 26 insertions(+), 6 deletions(-)
>>  create mode 100644 tests/ref/fate/binsub-movtextenc-long-dur
>>  create mode 100644 tests/ref/fate/binsub-movtextenc-long-dur-timebase
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index edddfeeb00..aeb7de351f 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -5746,7 +5746,8 @@ static int mov_write_packet(AVFormatContext *s,
>> AVPacket *pkt)
>>           *
>>           * 2) For each subtitle track, check if the current packet's
>>           * dts is past the duration of the last subtitle sample. If
>> -         * so, we now need to write an end sample for that subtitle.
>> +         * so, we now need to write one or multiple end samples for
>> +         * that subtitle.
>>           *
>>           * This must be done conditionally to allow for subtitles that
>>           * immediately replace each other, in which case an end sample
>> @@ -5760,11 +5761,22 @@ static int mov_write_packet(AVFormatContext *s,
>> AVPacket *pkt)
>>              int ret;
>>
>>              if (trk->par->codec_id == AV_CODEC_ID_MOV_TEXT &&
>> -                trk->track_duration < pkt->dts &&
>> -                (trk->entry == 0 || !trk->last_sample_is_subtitle_end)) {
>> -                ret = mov_write_subtitle_end_packet(s, i,
>> trk->track_duration);
>> -                if (ret < 0) return ret;
>> -                trk->last_sample_is_subtitle_end = 1;
>> +                trk->track_duration < pkt->dts) {
>> +                int max_duration = INT_MAX - 1;
>> +                if (trk->entry == 0 ||
>> !trk->last_sample_is_subtitle_end) {
>> +                    ret = mov_write_subtitle_end_packet(s, i,
>> trk->track_duration);
>> +                    if (ret < 0) return ret;
>> +                    trk->last_sample_is_subtitle_end = 1;
>> +                }
>> +                if (trk->last_sample_is_subtitle_end &&
>> +                    pkt->dts - trk->track_duration > max_duration) {
>> +                    int64_t dts = trk->track_duration;
>> +                    while(pkt->dts - dts > max_duration) {
>> +                        dts += max_duration;
>> +                        ret = mov_write_subtitle_end_packet(s, i, dts);
>> +                        if (ret < 0) return ret;
>> +                    }
>> +                }
>>              }
>>          }
>>
>> diff --git a/tests/fate/subtitles.mak b/tests/fate/subtitles.mak
>> index 0042902161..59055e975c 100644
>> --- a/tests/fate/subtitles.mak
>> +++ b/tests/fate/subtitles.mak
>> @@ -34,6 +34,12 @@ fate-sub-movtext: CMD = fmtstdout ass -i
>> $(TARGET_SAMPLES)/sub/MovText_capabilit
>>  FATE_SUBTITLES-$(call ENCDEC, MOVTEXT, MOV) += fate-binsub-movtextenc
>>  fate-binsub-movtextenc: CMD = md5 -i
>> $(TARGET_SAMPLES)/sub/MovText_capability_tester.mp4 -map 0 -scodec mov_text
>> -f mp4 -flags +bitexact -fflags +bitexact -movflags frag_keyframe+empty_moov
>>
>> +FATE_SUBTITLES-$(call ENCDEC, MOVTEXT, MOV) +=
>> fate-binsub-movtextenc-long-dur
>> +fate-binsub-movtextenc-long-dur: CMD = md5 -i
>> $(TARGET_SAMPLES)/sub/WebVTT_movtext_long_dur.vtt -map 0 -scodec mov_text
>> -f mp4 -flags +bitexact -fflags +bitexact -movflags empty_moov
>> +
>> +FATE_SUBTITLES-$(call ENCDEC, MOVTEXT, MOV) +=
>> fate-binsub-movtextenc-long-dur-timebase
>> +fate-binsub-movtextenc-long-dur-timebase: CMD = md5 -i
>> $(TARGET_SAMPLES)/sub/WebVTT_movtext_long_dur.vtt -map 0 -scodec mov_text
>> -time_base 1000 -f mp4 -flags +bitexact -fflags +bitexact -movflags
>> empty_moov
>> +
>>  FATE_SUBTITLES_ASS-$(call DEMDEC, MPL2, MPL2) += fate-sub-mpl2
>>  fate-sub-mpl2: CMD = fmtstdout ass -i
>> $(TARGET_SAMPLES)/sub/MPL2_capability_tester.txt
>>
>> diff --git a/tests/ref/fate/binsub-movtextenc-long-dur
>> b/tests/ref/fate/binsub-movtextenc-long-dur
>> new file mode 100644
>> index 0000000000..4db0be514b
>> --- /dev/null
>> +++ b/tests/ref/fate/binsub-movtextenc-long-dur
>> @@ -0,0 +1 @@
>> +043848a72c3ae75744115fe8d06c8bf1
>>
>
The md5 is not correct here, wil update in next version.
Meanwhile, sometimes the md5 value changes, not consistent. I have no idea
how that happens, I thought the MP4 output's binary should be always the
same. Does anyone know why ? I would really appreciate that.

-Jun


> diff --git a/tests/ref/fate/binsub-movtextenc-long-dur-timebase
>> b/tests/ref/fate/binsub-movtextenc-long-dur-timebase
>> new file mode 100644
>> index 0000000000..14b2f0167b
>> --- /dev/null
>> +++ b/tests/ref/fate/binsub-movtextenc-long-dur-timebase
>> @@ -0,0 +1 @@
>> +043848a72c3ae75744115fe8d06c8bf1
>> \ No newline at end of file
>> --
>> 2.17.1
>>
>>
> Version 2 addressed the feedback from Jan, adding fate test.
> The attachment is the sample file for the tests. I will send it to
> samples-request at ffmpeg.org if this patch is approved.
>
> -Jun
>