[FFmpeg-devel] [PATCH] avformat/nut: fix duration estimation by writting EOR packets

Paul B Mahol onemda at gmail.com
Mon Dec 17 21:29:35 EET 2018


On 12/17/18, Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Mon, Dec 17, 2018 at 07:32:49PM +0100, Paul B Mahol wrote:
>> On 12/17/18, Michael Niedermayer <michael at niedermayer.cc> wrote:
>> > On Mon, Dec 17, 2018 at 06:04:40PM +0100, Paul B Mahol wrote:
>> >> Hi,
>> >>
>> >> patches attached.
>> >
>> >>  nut.h    |    2 +
>> >>  nutdec.c |    3 ++
>> >>  nutenc.c |   87
>> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>> >>  3 files changed, 88 insertions(+), 4 deletions(-)
>> >> 870df19d733f29294a2cb2e3ea5ed1a09745f3a4
>> >> 0001-avformat-nut-fix-duration-estimation-by-writing-EOR-.patch
>> >> From 80d6805fdf386182341fe72035ab88e06a602752 Mon Sep 17 00:00:00 2001
>> >> From: Paul B Mahol <onemda at gmail.com>
>> >> Date: Mon, 17 Dec 2018 16:43:57 +0100
>> >> Subject: [PATCH 1/2] avformat/nut: fix duration estimation by writing
>> >> EOR
>> >>  packets
>> >>
>> >> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> >> ---
>> >>  libavformat/nut.h    |  2 +
>> >>  libavformat/nutdec.c |  3 ++
>> >>  libavformat/nutenc.c | 87
>> >> ++++++++++++++++++++++++++++++++++++++++++--
>> >>  3 files changed, 88 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/libavformat/nut.h b/libavformat/nut.h
>> >> index a4409ee23d..d7ba86b5e5 100644
>> >> --- a/libavformat/nut.h
>> >> +++ b/libavformat/nut.h
>> >> @@ -76,6 +76,8 @@ typedef struct StreamContext {
>> >>      int last_flags;
>> >>      int skip_until_key_frame;
>> >>      int64_t last_pts;
>> >> +    int64_t last_dts;
>> >> +    int64_t last_duration;
>> >>      int time_base_id;
>> >>      AVRational *time_base;
>> >>      int msb_pts_shift;
>> >> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
>> >> index 056ef59d00..f2490f9842 100644
>> >
>> >> --- a/libavformat/nutdec.c
>> >> +++ b/libavformat/nutdec.c
>> >> @@ -1080,6 +1080,9 @@ static int decode_frame(NUTContext *nut,
>> >> AVPacket
>> >> *pkt, int frame_code)
>> >>
>> >>      stc = &nut->stream[stream_id];
>> >>
>> >> +    if (stc->last_flags & FLAG_EOR)
>> >> +        return 1;
>> >
>> > EOR can occur in the middle of subtitle streams. It would be used to
>> > mark
>> > areas with no vissible subtitles.
>> > (The point behind this is that it allows seeking to consider a subtitle
>> > stream
>> >  if and only if there are actually any displayed subtitles at that
>> > time)
>> >
>> > this code would break that
>> >
>> > also demxuer and muxer changes should be in seperate patches
>> >
>> >
>> >> +
>> >>      if (stc->last_flags & FLAG_KEY)
>> >>          stc->skip_until_key_frame = 0;
>> >>
>> >> diff --git a/libavformat/nutenc.c b/libavformat/nutenc.c
>> >> index e9a3bb49db..c12b4cc8cf 100644
>> >> --- a/libavformat/nutenc.c
>> >> +++ b/libavformat/nutenc.c
>> >> @@ -940,6 +940,80 @@ fail:
>> >>      return ret;
>> >>  }
>> >>
>> >> +static void nut_update_max_pts(AVFormatContext *s, int stream_index,
>> >> int64_t pts)
>> >> +{
>> >> +    NUTContext *nut = s->priv_data;
>> >> +    StreamContext *nus = &nut->stream[stream_index];
>> >> +
>> >> +    if (!nut->max_pts_tb || av_compare_ts(nut->max_pts,
>> >> *nut->max_pts_tb,
>> >> pts, *nus->time_base) < 0) {
>> >> +        nut->max_pts = pts;
>> >> +        nut->max_pts_tb = nus->time_base;
>> >> +    }
>> >> +}
>> >> +
>> >> +static int nut_write_eor(AVFormatContext *s, int stream_index)
>> >> +{
>> >> +    AVIOContext *bc = s->pb, *dyn_bc;
>> >> +    NUTContext *nut = s->priv_data;
>> >> +    StreamContext *nus = &nut->stream[stream_index];
>> >> +    int ret, frame_code, flags, needed_flags;
>> >> +    int64_t pts = nus->last_pts + nus->last_duration;
>> >> +    int64_t coded_pts;
>> >> +    FrameCode *fc;
>> >> +
>> >> +    coded_pts = pts & ((1 << nus->msb_pts_shift) - 1);
>> >> +    if (ff_lsb2full(nus, coded_pts) != pts)
>> >> +        coded_pts = pts + (1 << nus->msb_pts_shift);
>> >> +
>> >> +    nut->last_syncpoint_pos = avio_tell(bc);
>> >> +    ret                     = avio_open_dyn_buf(&dyn_bc);
>> >> +    if (ret < 0)
>> >> +        return ret;
>> >> +    put_tt(nut, nus->time_base, dyn_bc, nus->last_dts +
>> >> nus->last_duration);
>> >> +    ff_put_v(dyn_bc, 0);
>> >> +
>> >> +    if (nut->flags & NUT_BROADCAST) {
>> >> +        put_tt(nut, nus->time_base, dyn_bc,
>> >> +               av_rescale_q(av_gettime(), AV_TIME_BASE_Q,
>> >> *nus->time_base));
>> >> +    }
>> >> +    put_packet(nut, bc, dyn_bc, 1, SYNCPOINT_STARTCODE);
>> >> +
>> >> +    frame_code  = -1;
>> >> +    for (int i = 0; i < 256; i++) {
>> >> +        FrameCode *fc = &nut->frame_code[i];
>> >> +        int flags     = fc->flags;
>> >> +
>> >> +        if (flags & FLAG_INVALID)
>> >> +            continue;
>> >> +
>> >> +        if (!(flags & FLAG_CODED)) {
>> >> +            continue;
>> >> +        }
>> >> +
>> >> +        frame_code = i;
>> >> +        break;
>> >> +    }
>> >> +    av_assert0(frame_code != -1);
>> >> +
>> >> +    fc           = &nut->frame_code[frame_code];
>> >> +    flags        = fc->flags;
>> >> +    needed_flags = FLAG_KEY | FLAG_EOR | FLAG_CODED_PTS |
>> >> FLAG_STREAM_ID;
>> >> +
>> >> +    ffio_init_checksum(bc, ff_crc04C11DB7_update, 0);
>> >> +    avio_w8(bc, frame_code);
>> >> +    if (flags & FLAG_CODED) {
>> >> +        ff_put_v(bc, (flags ^ needed_flags) & ~(FLAG_CODED));
>> >> +        flags = needed_flags;
>> >> +    }
>> >> +    if (flags & FLAG_STREAM_ID)  ff_put_v(bc, stream_index);
>> >> +    if (flags & FLAG_CODED_PTS)  ff_put_v(bc, coded_pts);
>> >> +    ffio_get_checksum(bc);
>> >> +
>> >> +    nut_update_max_pts(s, stream_index, nus->last_pts +
>> >> nus->last_duration);
>> >> +
>> >> +    return 0;
>> >> +}
>> >> +
>> >>  static int nut_write_packet(AVFormatContext *s, AVPacket *pkt)
>> >>  {
>> >>      NUTContext *nut    = s->priv_data;
>> >> @@ -1136,6 +1210,8 @@ static int nut_write_packet(AVFormatContext *s,
>> >> AVPacket *pkt)
>> >>
>> >>      nus->last_flags = flags;
>> >>      nus->last_pts   = pkt->pts;
>> >> +    nus->last_dts   = pkt->dts;
>> >> +    nus->last_duration = pkt->duration;
>> >>
>> >>      //FIXME just store one per syncpoint
>> >>      if (flags & FLAG_KEY && !(nut->flags & NUT_PIPE)) {
>> >> @@ -1150,10 +1226,7 @@ static int nut_write_packet(AVFormatContext *s,
>> >> AVPacket *pkt)
>> >>              nus->keyframe_pts[nut->sp_count] = pkt->pts;
>> >>      }
>> >>
>> >> -    if (!nut->max_pts_tb || av_compare_ts(nut->max_pts,
>> >> *nut->max_pts_tb,
>> >> pkt->pts, *nus->time_base) < 0) {
>> >> -        nut->max_pts = pkt->pts;
>> >> -        nut->max_pts_tb = nus->time_base;
>> >> -    }
>> >> +    nut_update_max_pts(s, pkt->stream_index, pkt->pts);
>> >>
>> >>  fail:
>> >>      av_freep(&sm_buf);
>> >> @@ -1167,6 +1240,12 @@ static int nut_write_trailer(AVFormatContext
>> >> *s)
>> >>      AVIOContext *bc = s->pb, *dyn_bc;
>> >>      int ret;
>> >>
>> >> +    for (int i = 0; i < s->nb_streams; i++) {
>> >> +        ret = nut_write_eor(s, i);
>> >> +        if (ret < 0)
>> >> +            return ret;
>> >> +    }
>> >> +
>> >>      while (nut->header_count < 3)
>> >>          write_headers(s, bc);
>> >
>> > This violates the packet interleaving requirements
>> > if one stream ends. lets say at time 5sec and one continues with frames
>> > at
>> > 6,7,8 sec then the EOR frame for the stream ending at 5sec has to be
>> > written
>> > before the later frames.
>>
>> How this violates it? It writes it at end. After end there is nothing.
>
> from nut.txt:
> "pts of all frames in all streams MUST be bigger or equal to dts of all
>  previous frames in all streams, compared in common timebase. (EOR
>  frames are NOT exempt from this rule.)"
>
> so if you have 2 streams (i assume for simplicity they have dts=pts)
> with a pts=dts assumtation the above rule requires all frames to be
> simply ordered by timestamp
>
> A Time: 1 2 4 5
> A len : 1 2 1 1
>
> B Time: 1 2 3 5 6 7 8
> B len : 1 1 2 1 1 1 1
>
> Now the EOR frame for A would have timestamp 6 (5 + length(1)) and
> the last frame for B has timestamp 8, the EOR for it would be 9
> that means the EOR of A would have to be stored around the place where
> the frame for B with timestamp 6 is. If its stored later then there
> would be a frame in stream B stored with a timestamp larger than the
> later EOR for A violating the rule

But how to do that with that interleaving function.
You are basically saying that EOR should be right after last stream
packet, does that
function know anything about stream EOF?
What about buffering of packets? If it is required than this design is flawed.


More information about the ffmpeg-devel mailing list