Re: [libav-devel] [PATCH 05/14] cbs: Minor comment fixes / cosmetics

2017-11-12 Thread Mark Thompson
On 10/11/17 12:27, Diego Biurrun wrote:
> On Thu, Nov 09, 2017 at 01:07:47AM +, Mark Thompson wrote:
>> --- a/libavcodec/cbs.h
>> +++ b/libavcodec/cbs.h
>> @@ -25,6 +25,19 @@
>>  
>> +/*
>> + * 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.
>> + */
>> +
>>  struct CodedBitstreamType;
> 
> This looks like it should be Doxygen in one form or another, probably
> some sort of section.

Would it actually end up somewhere?  This isn't public API.

>> @@ -114,7 +127,7 @@ typedef struct CodedBitstreamFragment {
>>  /**
>>   * Number of units in this fragment.
>>   *
>> - * This may be zero if the fragment only exists in bistream form
>> + * This may be zero if the fragment only exists in bitstream form
> 
> Always a classic :)
> 
>> --- a/libavcodec/cbs_internal.h
>> +++ b/libavcodec/cbs_internal.h
>> @@ -32,6 +32,9 @@ typedef struct CodedBitstreamType {
>>  
>>  // Split frag->data into coded bitstream units, creating the
>>  // frag->units array.  Fill data but not content on each unit.
>> +// header is set if the fragment came from a header block, which
>> +// may require different parsing for some codecs (e.g. the AVCC
>> +// header in H.264).
> 
> I'd capitalize "header" here.

It's referring to the argument "header", capitalising it would be wrong.

Maybe "The header argument should be set if the fragment ..." to move it away 
from the start of the sentence?

- Mark
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 05/14] cbs: Minor comment fixes / cosmetics

2017-11-10 Thread Diego Biurrun
On Thu, Nov 09, 2017 at 01:07:47AM +, Mark Thompson wrote:
> --- a/libavcodec/cbs.h
> +++ b/libavcodec/cbs.h
> @@ -25,6 +25,19 @@
>  
> +/*
> + * 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.
> + */
> +
>  struct CodedBitstreamType;

This looks like it should be Doxygen in one form or another, probably
some sort of section.

> @@ -114,7 +127,7 @@ typedef struct CodedBitstreamFragment {
>  /**
>   * Number of units in this fragment.
>   *
> - * This may be zero if the fragment only exists in bistream form
> + * This may be zero if the fragment only exists in bitstream form

Always a classic :)

> --- a/libavcodec/cbs_internal.h
> +++ b/libavcodec/cbs_internal.h
> @@ -32,6 +32,9 @@ typedef struct CodedBitstreamType {
>  
>  // Split frag->data into coded bitstream units, creating the
>  // frag->units array.  Fill data but not content on each unit.
> +// header is set if the fragment came from a header block, which
> +// may require different parsing for some codecs (e.g. the AVCC
> +// header in H.264).

I'd capitalize "header" here.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH 05/14] cbs: Minor comment fixes / cosmetics

2017-11-08 Thread Mark Thompson
---
 libavcodec/cbs.h  | 35 +++
 libavcodec/cbs_internal.h |  3 +++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 85c7b5557..ffeca057a 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -25,6 +25,19 @@
 #include "avcodec.h"
 
 
+/*
+ * 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.
+ */
+
 struct CodedBitstreamType;
 
 /**
@@ -39,7 +52,7 @@ typedef uint32_t CodedBitstreamUnitType;
 /**
  * Coded bitstream unit structure.
  *
- * A bitstream unit the the smallest element of a bitstream which
+ * 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
@@ -52,7 +65,7 @@ typedef struct CodedBitstreamUnit {
 CodedBitstreamUnitType type;
 
 /**
- * Pointer to the bitstream form of this unit.
+ * Pointer to the directly-parsable bitstream form of this unit.
  *
  * May be NULL if the unit currently only exists in decomposed form.
  */
@@ -114,7 +127,7 @@ typedef struct CodedBitstreamFragment {
 /**
  * Number of units in this fragment.
  *
- * This may be zero if the fragment only exists in bistream form
+ * This may be zero if the fragment only exists in bitstream form
  * and has not been decomposed.
  */
 int  nb_units;
@@ -162,7 +175,7 @@ typedef struct CodedBitstreamContext {
 /**
  * Length of the decompose_unit_types array.
  */
-intnb_decompose_unit_types;
+int nb_decompose_unit_types;
 
 /**
  * Enable trace output during read/write operations.
@@ -204,6 +217,10 @@ int ff_cbs_read_extradata(CodedBitstreamContext *ctx,
 /**
  * Read the data bitstream from a packet into a fragment, then
  * split into units and decompose.
+ *
+ * This also updates the internal state of the coded bitstream context
+ * with any persistent data from the fragment which may be required to
+ * read following fragments (e.g. parameter sets).
  */
 int ff_cbs_read_packet(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag,
@@ -212,6 +229,10 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
 /**
  * Read a bitstream from a memory region into a fragment, then
  * split into units and decompose.
+ *
+ * This also updates the internal state of the coded bitstream context
+ * with any persistent data from the fragment which may be required to
+ * read following fragments (e.g. parameter sets).
  */
 int ff_cbs_read(CodedBitstreamContext *ctx,
 CodedBitstreamFragment *frag,
@@ -225,12 +246,18 @@ int ff_cbs_read(CodedBitstreamContext *ctx,
  * data buffer.  When modifying the content of decomposed units, this
  * can be used to regenerate the bitstream form of units or the whole
  * fragment so that it can be extracted for other use.
+ *
+ * This also updates the internal state of the coded bitstream context
+ * with any persistent data from the fragment which may be required to
+ * write following fragments (e.g. parameter sets).
  */
 int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag);
 
 /**
  * Write the bitstream of a fragment to the extradata in codec parameters.
+ *
+ * This replaces any existing extradata in the structure.
  */
 int ff_cbs_write_extradata(CodedBitstreamContext *ctx,
AVCodecParameters *par,
diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h
index dddeae9d5..fd3848c95 100644
--- a/libavcodec/cbs_internal.h
+++ b/libavcodec/cbs_internal.h
@@ -32,6 +32,9 @@ typedef struct CodedBitstreamType {
 
 // Split frag->data into coded bitstream units, creating the
 // frag->units array.  Fill data but not content on each unit.
+// header is set if the fragment came from a header block, which
+// may require different parsing for some codecs (e.g. the AVCC
+// header in H.264).
 int (*split_fragment)(CodedBitstreamContext *ctx,
   CodedBitstreamFragment *frag,
   int header);
-- 
2.11.0

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel