[FFmpeg-devel] [PATCH] avformat/nut: fix duration estimation by writting EOR packets
Michael Niedermayer
michael at niedermayer.cc
Tue Dec 18 13:22:07 EET 2018
On Mon, Dec 17, 2018 at 08:29:35PM +0100, Paul B Mahol wrote:
> 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?
the interleaving needs to know when EOF is reached otherwise it would
not be able to flush the last packets.
* interleaving before EOF has to expect that at the end of each stream another
packet is added (that is the location of a potential EOR frame)
-> thus before EOF/flush we still should be able to inject all EOR at the
packet interleave level
* once EOF/flush is reached no more packets can be added into the interleaver
thus its state provides us with information about what is the end of each
stream and this is the obvious time/location to add the EOR frames into
the interleaver
copying my reply from IRC for the archieves too:
<michaelni> durandal_1707, the flush parameter would be set during trailer writing (and for one other case that would need to be distinguished). When flush is reached no more packets should get added into the buffers so the buffer content should work to indentify what is last.
<michaelni> maybe theres a easier way, i dont know
<michaelni> maybe st->last_in_packet_buffer can be used to simplify this
> What about buffering of packets? If it is required than this design is flawed.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181218/879847df/attachment.sig>
More information about the ffmpeg-devel
mailing list