[FFmpeg-devel] [libav-devel] mpegts encoder not interleaving audio/video packets well

[Tony Strauss]

Any ayes or nays?

On Thu, Jun 16, 2011 at 5:47 PM, Tony Strauss <tony at animoto.com> wrote:

> On Thu, Jun 16, 2011 at 12:09 PM, Michael Niedermayer <michaelni at gmx.at>wrote:
>
>> On Thu, Jun 16, 2011 at 12:05:34PM -0400, Tony Strauss wrote:
>> >
>> > I took a look through the mpeg ps encoder (libavformat/mpegenc.c).  It
>> seems
>> > to require interleaving within .7 sec by default (max_delay).  I think
>> that
>> > it's reasonable to implement this for the mpegts muxer as well.
>>  max_delay
>> > also is configurable via ffmpeg's command-line, so people can set it to
>> be
>> > whatever they want.  I can create a patch for this, if it sounds
>> reasonable
>> > to everyone.
>>
>> sounds reasonable to me
>
> I've attached a patch (reproduced below as well).  The tests continue to
> pass with it.
>
> While testing, I noticed a block of code in the mpegts encoder that struck
> me as strange (mpegtsenc.c:885):
>     const uint64_t delay = av_rescale(s->max_delay, 90000, AV_TIME_BASE)*2;
>     int64_t dts = AV_NOPTS_VALUE, pts = AV_NOPTS_VALUE;
>
>     if (pkt->pts != AV_NOPTS_VALUE)
>         pts = pkt->pts + delay;
>     if (pkt->dts != AV_NOPTS_VALUE)
>         dts = pkt->dts + delay;
>
> The pts and dts of all of the packets gets incremented by twice max_delay,
> which seems contrary to its use in the mpeg ps muxer (although I may well be
> missing something).
>
> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index 7e96472..cbc194c 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -878,6 +878,7 @@ static int mpegts_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>      uint8_t *buf= pkt->data;
>      uint8_t *data= NULL;
>      MpegTSWriteStream *ts_st = st->priv_data;
> +    const uint64_t max_delay = av_rescale(s->max_delay, 90000,
> AV_TIME_BASE);
>      const uint64_t delay = av_rescale(s->max_delay, 90000,
> AV_TIME_BASE)*2;
>      int64_t dts = AV_NOPTS_VALUE, pts = AV_NOPTS_VALUE;
>
> @@ -947,6 +948,18 @@ static int mpegts_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>          }
>      }
>
> +    /*
> +     * Flush the audio packets once we've amassed a full PES payload or
> +     * once the stream has moved a certain amount of time past the first
> audio
> +     * packet in the buffer.
> +     */
> +    if ((ts_st->payload_index + size > DEFAULT_PES_PAYLOAD_SIZE) ||
> +        ((ts_st->payload_index > 0) && (dts - ts_st->payload_dts >
> max_delay))) {
> +        mpegts_write_pes(s, st, ts_st->payload, ts_st->payload_index,
> +                         ts_st->payload_pts, ts_st->payload_dts);
> +        ts_st->payload_index = 0;
> +    }
> +
>      if (st->codec->codec_type != AVMEDIA_TYPE_AUDIO) {
>          // for video and subtitle, write a single pes packet
>          mpegts_write_pes(s, st, buf, size, pts, dts);
> @@ -954,12 +967,6 @@ static int mpegts_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>          return 0;
>      }
>
> -    if (ts_st->payload_index + size > DEFAULT_PES_PAYLOAD_SIZE) {
> -        mpegts_write_pes(s, st, ts_st->payload, ts_st->payload_index,
> -                         ts_st->payload_pts, ts_st->payload_dts);
> -        ts_st->payload_index = 0;
> -    }
> -
>      if (!ts_st->payload_index) {
>          ts_st->payload_pts = pts;
>          ts_st->payload_dts = dts;
>
>
> Tony
>