[FFmpeg-devel] [PATCH] matroska: Set CodecDelay and SeekPreroll in the AVStream

Vignesh Venkatasubramanian vigneshv at google.com
Tue Sep 24 23:50:16 CEST 2013


On Tue, Sep 24, 2013 at 2:14 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Tue, Sep 24, 2013 at 12:09:31PM -0700, Vignesh Venkatasubramanian wrote:
>> On Tue, Sep 24, 2013 at 11:36 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Tue, Sep 24, 2013 at 10:35:47AM -0700, Vignesh Venkatasubramanian wrote:
>> >> On Tue, Sep 24, 2013 at 3:53 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > On Mon, Sep 23, 2013 at 04:03:25PM -0700, Vignesh Venkatasubramanian wrote:
>> >> >> On Mon, Sep 23, 2013 at 4:02 PM, Vignesh Venkatasubramanian
>> >> >> <vigneshv at google.com> wrote:
>> >> >> > This patch exports the values of Codec Delay and Seek Preroll
>> >> >> > container elements as in the AVStream structure. The seek_preroll
>> >> >> > field has been added to the AVStream struct and the minor version
>> >> >> > of libavformat has been bumped.
>> >> >> >
>> >> >> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv at google.com>
>> >> >> > ---
>> >> >> >  libavformat/avformat.h    | 16 ++++++++++++++++
>> >> >> >  libavformat/matroskadec.c | 14 ++++++++++++++
>> >> >> >  libavformat/version.h     |  2 +-
>> >> >> >  3 files changed, 31 insertions(+), 1 deletion(-)
>> >> >> >
>> >> >> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> >> >> > index b18eb3f..6671a83 100644
>> >> >> > --- a/libavformat/avformat.h
>> >> >> > +++ b/libavformat/avformat.h
>> >> >> > @@ -856,6 +856,9 @@ typedef struct AVStream {
>> >> >> >
>> >> >> >      /**
>> >> >> >       * Number of samples to skip at the start of the frame decoded from the next packet.
>> >> >> > +     *
>> >> >> > +     * Code outside avformat should access this field using:
>> >> >> > +     * av_stream_get_skip_samples/set_skip_samples(stream)
>> >> >> >       */
>> >> >> >      int skip_samples;
>> >> >
>> >> > when does code outside lavf need to access this ?
>> >> >
>> >>
>> >> you are right. it need not be accessed outside lavf.
>> >>
>> >> >
>> >> >> >
>> >> >> > @@ -888,10 +891,23 @@ typedef struct AVStream {
>> >> >> >       */
>> >> >> >      int pts_wrap_behavior;
>> >> >> >
>> >> >> > +    /**
>> >> >> > +     * Number of samples to skip after a discontinuity. For example, when a seek
>> >> >> > +     * happens.
>> >> >> > +     *
>> >> >> > +     * Code outside avformat should access this field using:
>> >> >> > +     * av_stream_get/set_seek_preroll(stream)
>> >> >> > +     */
>> >> >> > +    int seek_preroll;
>> >> >> > +
>> >> >> >  } AVStream;
>> >> >> >
>> >> >> >  AVRational av_stream_get_r_frame_rate(const AVStream *s);
>> >> >> >  void       av_stream_set_r_frame_rate(AVStream *s, AVRational r);
>> >> >> > +int  av_stream_get_skip_samples(const AVStream *s);
>> >> >> > +void av_stream_set_skip_samples(AVStream *s, int skip_samples);
>> >> >> > +int  av_stream_get_seek_preroll(const AVStream *s);
>> >> >> > +void av_stream_set_seek_preroll(AVStream *s, int seek_preroll);
>> >> >> >
>> >> >> >  #define AV_PROGRAM_RUNNING 1
>> >> >> >
>> >> >> > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> >> >> > index a1b7f56..ecbc723 100644
>> >> >> > --- a/libavformat/matroskadec.c
>> >> >> > +++ b/libavformat/matroskadec.c
>> >> >> > @@ -163,6 +163,8 @@ typedef struct {
>> >> >> >      uint64_t default_duration;
>> >> >> >      uint64_t flag_default;
>> >> >> >      uint64_t flag_forced;
>> >> >> > +    uint64_t codec_delay;
>> >> >> > +    uint64_t seek_preroll;
>> >> >> >      MatroskaTrackVideo video;
>> >> >> >      MatroskaTrackAudio audio;
>> >> >> >      MatroskaTrackOperation operation;
>> >> >> > @@ -410,6 +412,8 @@ static EbmlSyntax matroska_track[] = {
>> >> >> >      { MATROSKA_ID_TRACKOPERATION,       EBML_NEST, 0, offsetof(MatroskaTrack,operation), {.n=matroska_track_operation} },
>> >> >> >      { MATROSKA_ID_TRACKCONTENTENCODINGS,EBML_NEST, 0, 0, {.n=matroska_track_encodings} },
>> >> >> >      { MATROSKA_ID_TRACKMAXBLKADDID,     EBML_UINT, 0, offsetof(MatroskaTrack,max_block_additional_id) },
>> >> >> > +    { MATROSKA_ID_CODECDELAY,           EBML_UINT, 0, offsetof(MatroskaTrack,codec_delay) },
>> >> >> > +    { MATROSKA_ID_SEEKPREROLL,          EBML_UINT, 0, offsetof(MatroskaTrack,seek_preroll) },
>> >> >> >      { MATROSKA_ID_TRACKFLAGENABLED,     EBML_NONE },
>> >> >> >      { MATROSKA_ID_TRACKFLAGLACING,      EBML_NONE },
>> >> >> >      { MATROSKA_ID_CODECNAME,            EBML_NONE },
>> >> >
>> >> >> > @@ -1874,6 +1878,16 @@ static int matroska_read_header(AVFormatContext *s)
>> >> >> >              st->codec->bits_per_coded_sample = track->audio.bitdepth;
>> >> >> >              if (st->codec->codec_id != AV_CODEC_ID_AAC)
>> >> >> >              st->need_parsing = AVSTREAM_PARSE_HEADERS;
>> >> >> > +            if (track->codec_delay > 0) {
>> >> >> > +                st->skip_samples = av_rescale_q(track->codec_delay,
>> >> >> > +                                                (AVRational){1, 1000000000},
>> >> >> > +                                                (AVRational){1, st->codec->sample_rate});
>> >> >> > +            }
>> >> >
>> >> > i suspect this alone wont work when the user seeks back to the first
>> >> > packet
>> >>
>> >> i don't understand why. can you please explain?
>> >
>> > because its set when parsing the header but when seeking back the
>> > header is not re parsed so its not set again so the first packet
>> > would then skip a different amount
>> > thats unless iam missing something
>> >
>>
>> yes, you are right. it is up to the decoder implementation to make sure
>> that it uses the correct skip_samples value when the input timestamp is
>> zero (or start of stream) when seeking.
>
> the decoder doesnt know when the timestamp is zero, a file might start
> from a timestamp differnt than zero
>

i think i haven't communicated correctly about what i'm trying to
achieve with this patch.
basically, lavf will put this container value in the stream struct
while parsing the
header. once it populated, this field should not change (on seeking,
etc.). so , it's a place
where i can put the container field in the stream.

it is up to the decoder to handle it's actual behavior correctly. so,
i think the correct thing
is to add another element to the stream called codec_delay whose
description will be something like
"number of samples to discard at the beginning of the stream" (rather
than using skip_samples
which has a slightly different meaning).

please let me know if this sounds reasonable. if not, please let me
know what you think is the
correct way of doing what i want. thanks!

> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> In fact, the RIAA has been known to suggest that students drop out
> of college or go to community college in order to be able to afford
> settlements. -- The RIAA
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list