[FFmpeg-devel] [PATCH 03/20] lavc: Add coded bitstream read/write API

Michael Niedermayer michael at niedermayer.cc
Fri Oct 13 14:23:34 EEST 2017


On Thu, Oct 12, 2017 at 12:35:35AM +0100, Mark Thompson wrote:
> On 11/10/17 23:34, Michael Niedermayer wrote:
> > Hi
> > 
> > On Mon, Oct 09, 2017 at 08:04:47PM +0100, Mark Thompson wrote:
> > [...]
> > 
> >> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> >> new file mode 100644
> >> index 0000000000..e35175fc74
> >> --- /dev/null
> >> +++ b/libavcodec/cbs.h
> >> @@ -0,0 +1,283 @@
> >> +/*
> >> + * This file is part of FFmpeg.
> >> + *
> >> + * FFmpeg is free software; you can redistribute it and/or
> >> + * modify it under the terms of the GNU Lesser General Public
> >> + * License as published by the Free Software Foundation; either
> >> + * version 2.1 of the License, or (at your option) any later version.
> >> + *
> >> + * FFmpeg is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> + * Lesser General Public License for more details.
> >> + *
> >> + * You should have received a copy of the GNU Lesser General Public
> >> + * License along with FFmpeg; if not, write to the Free Software
> >> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> >> + */
> >> +
> >> +#ifndef AVCODEC_CBS_H
> >> +#define AVCODEC_CBS_H
> >> +
> >> +#include <stddef.h>
> >> +#include <stdint.h>
> >> +
> >> +#include "avcodec.h"
> >> +
> 
> (Here)
> 
> >> +
> >> +struct CodedBitstreamType;
> >> +
> > 
> >> +/**
> >> + * The codec-specific type of a bitstream unit.
> >> + */
> >> +typedef uint32_t CodedBitstreamUnitType;
> > 
> > Whats a bitstream unit ? (no iam not asking you to explain me but it
> > should be documeted)
> 
> See the comment ten lines below this one.
> 
> > Is there some high level overview of this API and its components ?
> > I mean without having to search terms in dispersed documentation.
> > 
> > The high level documentation should also explain what this API does
> > in a terse and clear way.
> > Maybe something like: (if thats correct)
> > Converts between a bit/byte stream and object tree representation of
> > a frame.
> > The (serialized) bitstream representation is what is commonly stored
> > in containers and passed around on networks. The object tree
> > representation allows easier modification of individual elements.
> 
> Inserted at "Here" above:
> 
> /*
>  * This defines a framework for converting between a coded bitstream
>  * and structures defining all individual syntax elements found in
>  * such a stream.
>  *
>  * Conversion in both directions is possible.  Given a coded bitstream
>  * (any meaningful fragment), it can be parsed and decomposed into
>  * syntax elements stored in a set of codec-specific structures.
>  * Similarly, given a set of those same codec-specific structures the
>  * syntax elements can be serialised and combined to create a coded
>  * bitstream.
>  */
> 

> >> +
> >> +/**
> >> + * Coded bitstream unit structure.
> >> + *
> > 
> >> + * A bitstream unit the smallest element of a bitstream which
> >> + * is meaningful on its own.  For example, an H.264 NAL unit.
> > 
> >> + *
> >> + * See the codec-specific header for the meaning of this for any
> >> + * particular codec.
> >> + */
> >> +typedef struct CodedBitstreamUnit {
> >> +    /**
> >> +     * Codec-specific type of this unit.
> >> +     */
> >> +    CodedBitstreamUnitType type;
> >> +
> > 
> >> +    /**
> >> +     * Pointer to the bitstream form of this unit.
> >> +     *
> >> +     * May be NULL if the unit currently only exists in decomposed form.
> >> +     */
> >> +    uint8_t *data;
> > 
> > whats a "bitstream form" ?
> > is that the bitstream itself? if so why is it called "bitstream form"
> 
> It is a bitstream form appropriate for direct parsing (for H.264/H.265, rbsp, so unescaped).

Thats a bit surprising.

Wouldnt a API that uses the unmodified bitstream be more clear and
simpler ?
Or an explicit parameter that specifies if the input has been
unescaped already.


[...]

> >> +#endif /* AVCODEC_CBS_H */
> 
> (I can't help but feel that refining this internal documentation is somewhat futile.  It isn't a public API, and will never be used by anyone who isn't highly familiar with the concepts involved.)

I do disagree here

Everyone is initially unfamiliar with the concepts, API and
implementation.

All the various internal documentation (and code) is all there is
to get one from "unfamiliar" to "familiar" to "highly familiar"

(and the specifications of various standards which often dont shine
 in terms of clarity either)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171013/84fae0a9/attachment.sig>


More information about the ffmpeg-devel mailing list