diff mbox

[FFmpeg-devel] Fix segment muxer

Message ID 17749581570450366@myt6-4218ece6190d.qloud-c.yandex.net
State Superseded
Headers show

Commit Message

just.one.man@yandex.ru Oct. 7, 2019, 12:12 p.m. UTC 
Updated patch
---

When incoming media has non-zero start PTS,
segment muxer would fail to correctly calculate
the point where to chunk segments, as it always
assumed that media starts with PTS==0.

This change removes this assumption by remembering
the PTS of the very first frame passed through the muxer.

Also fix starting points of first segment
---
 libavformat/segment.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

--
1.7.9.5


06.10.2019, 14:38, "Vasily" <just.one.man@yandex.ru>:
> I used that value somewhere during patch development, but I eventually
> settled on -1, because that "start_pts" and "end_pts" are using some other
> units, not the unit used by pkt->pts.
>
> So I wanted to make a distinction. Though a possibility of negative
> timestamps didn't come to me, so I probably have to change it back to
> AV_NOPTS_VALUE.
>
> P.S. Thanks for reviewing my patch!
>
> вс, 6 окт. 2019 г., 13:56 Marton Balint <cus@passwd.hu>:
>
>>  On Thu, 3 Oct 2019, just.one.man@yandex.ru wrote:
>>
>>  > It seems that my first attempt to send the patch failed (probably
>>  because my box where I executed "git send-email" failed reverse smtp
>>  check), so I'm going to re-send it to this same thread.
>>  >
>>  > ---
>>  >
>>  > When incoming media has non-zero start PTS,
>>  > segment muxer would fail to correctly calculate
>>  > the point where to chunk segments, as it always
>>  > assumed that media starts with PTS==0.
>>  >
>>  > This change removes this assumption by remembering
>>  > the PTS of the very first frame passed through the muxer.
>>  >
>>  > Also fix starting points of first segment
>>  > ---
>>  > libavformat/segment.c | 12 +++++++++++-
>>  > 1 file changed, 11 insertions(+), 1 deletion(-)
>>  >
>>  > diff --git a/libavformat/segment.c b/libavformat/segment.c
>>  > index e308206..2478d8f 100644
>>  > --- a/libavformat/segment.c
>>  > +++ b/libavformat/segment.c
>>  > @@ -87,6 +87,7 @@ typedef struct SegmentContext {
>>  > int64_t last_val; ///< remember last time for wrap around
>>  detection
>>  > int cut_pending;
>>  > int header_written; ///< whether we've already called
>>  avformat_write_header
>>  > + int64_t start_pts; ///< pts of the very first packet processed,
>>  used to compute correct segment length
>>  >
>>  > char *entry_prefix; ///< prefix to add to list entry filenames
>>  > int list_type; ///< set the list type
>>  > @@ -702,6 +703,7 @@ static int seg_init(AVFormatContext *s)
>>  > if ((ret = parse_frames(s, &seg->frames, &seg->nb_frames,
>>  seg->frames_str)) < 0)
>>  > return ret;
>>  > } else {
>>  > + seg->start_pts = -1;
>>
>>  AV_NOPTS_VALUE would be probably better for this purpose, even if the
>>  muxer won't get negative timestamps unless allowed by the
>>  AVFMT_TS_NEGATIVE flag.
>>
>>  Regards,
>>  Marton
>>
>>  > /* set default value if not specified */
>>  > if (!seg->time_str)
>>  > seg->time_str = av_strdup("2");
>>  > @@ -914,7 +916,15 @@ calc_times:
>>  > seg->cut_pending = 1;
>>  > seg->last_val = wrapped_val;
>>  > } else {
>>  > - end_pts = seg->time * (seg->segment_count + 1);
>>  > + if (seg->start_pts != -1) {
>>  > + end_pts = seg->start_pts + seg->time *
>>  (seg->segment_count + 1);
>>  > + } else if (pkt->stream_index == seg->reference_stream_index
>>  && pkt->pts != AV_NOPTS_VALUE) {
>>  > + // this is the first packet of the reference stream we
>>  see, initialize start point
>>  > + seg->start_pts = av_rescale_q(pkt->pts, st->time_base,
>>  AV_TIME_BASE_Q);
>>  > + seg->cur_entry.start_time = (double)pkt->pts *
>>  av_q2d(st->time_base);
>>  > + seg->cur_entry.start_pts = seg->start_pts;
>>  > + end_pts = seg->start_pts + seg->time *
>>  (seg->segment_count + 1);
>>  > + }
>>  > }
>>  > }
>>  >
>>  > --
>>  > 1.7.9.5
>>  > _______________________________________________
>>  > ffmpeg-devel mailing list
>>  > ffmpeg-devel@ffmpeg.org
>>  > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>  >
>>  > To unsubscribe, visit link above, or email
>>  > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>  _______________________________________________
>>  ffmpeg-devel mailing list
>>  ffmpeg-devel@ffmpeg.org
>>  https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>  To unsubscribe, visit link above, or email
>>  ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Comments

Nicolas George Oct. 7, 2019, 12:14 p.m. UTC | #1 
just.one.man@yandex.ru (12019-10-07):
> Updated patch

This should be after the triple dash, because it does not belong in the
commit message.

> ---
> 
> When incoming media has non-zero start PTS,
> segment muxer would fail to correctly calculate
> the point where to chunk segments, as it always
> assumed that media starts with PTS==0.
> 
> This change removes this assumption by remembering
> the PTS of the very first frame passed through the muxer.
> 
> Also fix starting points of first segment
> ---

This should be before the triple dash, because it does belong in the
commit message.

I did not look at the code.

Regards,
diff mbox

Patch

diff --git a/libavformat/segment.c b/libavformat/segment.c
index e308206..8b985df 100644
--- a/libavformat/segment.c
+++ b/libavformat/segment.c
@@ -87,6 +87,7 @@  typedef struct SegmentContext {
     int64_t last_val;      ///< remember last time for wrap around detection
     int cut_pending;
     int header_written;    ///< whether we've already called avformat_write_header
+    int64_t start_pts;     ///< pts of the very first packet processed, used to compute correct segment length

     char *entry_prefix;    ///< prefix to add to list entry filenames
     int list_type;         ///< set the list type
@@ -702,6 +703,7 @@  static int seg_init(AVFormatContext *s)
         if ((ret = parse_frames(s, &seg->frames, &seg->nb_frames, seg->frames_str)) < 0)
             return ret;
     } else {
+        seg->start_pts = AV_NOPTS_VALUE;
         /* set default value if not specified */
         if (!seg->time_str)
             seg->time_str = av_strdup("2");
@@ -914,7 +916,15 @@  calc_times:
                 seg->cut_pending = 1;
             seg->last_val = wrapped_val;
         } else {
-            end_pts = seg->time * (seg->segment_count + 1);
+            if (seg->start_pts != AV_NOPTS_VALUE) {
+                end_pts = seg->start_pts + seg->time * (seg->segment_count + 1);
+            } else if (pkt->stream_index == seg->reference_stream_index && pkt->pts != AV_NOPTS_VALUE) {
+                // this is the first packet of the reference stream we see, initialize start point
+                seg->start_pts = av_rescale_q(pkt->pts, st->time_base, AV_TIME_BASE_Q);
+                seg->cur_entry.start_time = (double)pkt->pts * av_q2d(st->time_base);
+                seg->cur_entry.start_pts = seg->start_pts;
+                end_pts = seg->start_pts + seg->time * (seg->segment_count + 1);
+            }
         }
     }